Skip Menu |

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Perl-Critic CPAN distribution.

Report information
The Basics
Id: 64132
Status: resolved
Priority: 0/
Queue: Perl-Critic

People
Owner: Nobody in particular
Requestors: rjray [...] blackperl.com
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 1.111
Fixed in: (no value)



Subject: ValuesAndExpressions::ProhibitCommaSeparatedStatements false-positive in anon hash reference
The attachment exhibits this behavior. Two ways of fixing it are also listed, currently commented-out in the code. Basically, this form: $self = bless { %something, something_extra => 1 }, $class; triggers the ProhibitCommaSeparatedStatements message, even though Perl properly treats it as a hash reference, not a code block. While this can be fixed by slight changes to the syntax (see the commented-out lines in the attachment), it seems that the above should be considered valid code by Critic. -- """"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" Randy J. Ray Silicon Valley Scale Modelers: http://www.svsm.org rjray@blackperl.com randy.j.ray@gmail.com
Subject: Foo.pm
package Foo; use strict; use warnings; sub new { my ($class, %args) = @_; my $self = bless { %args, extra => 1 }, $class; #my $self = bless +{ %args, extra => 1 }, $class; #my $self = { %args, extra => 1 }; #bless $self, $class; return $self; } 1;
On Tue Dec 21 20:46:59 2010, RJRAY wrote: Show quoted text
> The attachment exhibits this behavior. Two ways of fixing it are also > listed, currently commented-out in the code. > > Basically, this form: > > $self = bless { %something, something_extra => 1 }, $class; > > triggers the ProhibitCommaSeparatedStatements message, even though Perl > properly treats it as a hash reference, not a code block. While this can > be fixed by slight changes to the syntax (see the commented-out lines in > the attachment), it seems that the above should be considered valid code > by Critic.
Perl::Critic relies on PPI to produce a Perl parse tree for it to analyze. Unfortunately, curly brackets are overloaded, and can represent either a hash constructor or a block. When PPI interprets something as a block that Perl interprets as a hash constructor, this is the false positive that results. Equally unfortunately, Perl's heuristics to disambiguate blocks and hash constructors are undocumented (or, actually, the docs say something like "Perl guesses, and usually gets it right"). This specification is a bit hard to implement, so it's hard to see how most of the half-dozen or so similar bugs in the queue are going to get fixed. Even finding and reading the Perl source (and understanding it -- my brain generally just bounces off) might not help, since undocumented behavior can change. There are documented (in perlref) ways to force Perl to choose to interpret a set of curlys as either a block or a hash constructor, and PPI supports these. The leading plus forces a hash constructor, as you have noted in your attached file, and is my personal favorite, but you may prefer one of the others. All I can think of to do at the moment is to thank you for your report, and hope someone has a brain wave.
The bless() documentation says that the first argument is a reference. In theory this doesn't help, since a block can yield a reference. But in practice, 'bless {; +{}}' (in which the outer curlys are forced to be a block, and the inner to be an anonymous hash ref, as documented in perlref) fails to parse under selected Perls from 5.6.2 through 5.13.8. Based on this, I have submitted #64247: bless {} probably contains a hash constructor, not a block to the PPI RT queue, and committed a fix to Adam's repository. Whether he accepts it or not is another story. For the PPI ticket, see https://rt.cpan.org/Ticket/Display.html?id=64247
This appears to be fixed by upgrading to PPI 1.215. SVN commit ???? adds a test for it. The PPI dependency was bumped to version 1.215 in commit 4047 for RT 61301).
That was SVN commit 4048.