Skip Menu |

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

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

People
Owner: Nobody in particular
Requestors: PETDANCE [...] cpan.org
Cc:
AdminCc:

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



Subject: False positive in CodeLayout::RequireFinalSemicolon
The following code confuses CodeLayout::RequireFinalSemicolon. It seems to boil down to: an arrayref that contains a hashref that contains a variable. $ cat -n foo.pl 1 my %foo; 2 my @foo; 3 my $foo; 4 my %bongo = ( 5 key => { 6 hrows => [ 7 { 8 %foo, other => 'field' 9 } 10 ], 11 hcols => [ 12 { 13 %foo 14 } 15 ], 16 arows => [ 17 { 18 @foo, other => 'field' 19 } 20 ], 21 acols => [ 22 { 23 @foo 24 } 25 ], 26 arows => [ 27 { 28 $foo, $foo, other => 'field' 29 } 30 ], 31 acols => [ 32 { 33 $foo, $foo 34 } 35 ], 36 }, 37 ); $ perlcritic -s CodeLayout::RequireFinalSemicolon foo.pl foo.pl: Put semicolon ; on last statement in a block. 8: %foo, other => 'field' foo.pl: Put semicolon ; on last statement in a block. 13: %foo foo.pl: Put semicolon ; on last statement in a block. 18: @foo, other => 'field' foo.pl: Put semicolon ; on last statement in a block. 23: @foo foo.pl: Put semicolon ; on last statement in a block. 28: $foo, $foo, other => 'field' foo.pl: Put semicolon ; on last statement in a block. 33: $foo, $foo
Subject: Re: [rt.cpan.org #130725] False positive in CodeLayout::RequireFinalSemicolon
Date: Sat, 19 Oct 2019 12:25:21 +1100
To: "Andy Lester via RT" <bug-Perl-Critic-Pulp [...] rt.cpan.org>
From: Kevin Ryde <user42_kevin [...] yahoo.com.au>
"Andy Lester via RT" <bug-Perl-Critic-Pulp@rt.cpan.org> writes: Show quoted text
> > RequireFinalSemicolon ... an arrayref that contains a hashref that > contains a variable.
Thanks, I uploaded a new version. The problem may belong at the feet of PPI actually, eg. ppidump '[{%foo,x=>1}]' gives PPI::Structure::Block which is code block, whereas I believe a hashref ought to be PPI::Structure::Constructor like you get from ppidump '[{x=>1,%foo}]' I had some code recognising Block which is actually hashref when in PPI::Structure::List, but hadn't encountered it in an arrayref. The List one seems to have been fixed since I did that. Is that right? Eg. ppidump 'func({%args})' is now good PPI::Structure::Constructor. Maybe whatever happened for it wouldn't be too hard for arrayrefs too. Unless there's some ambiguity I don't know.
I'm still doing running this new ::Pulp against our codebase, but Pulp 97 seems to have also turned up a new failure that it didn't catch before: The line below with the duplicate custnum keys in the hashref did not get caught by ValuesAndExpressions::ProhibitDuplicateHashKeys under 96 but does now under 97. my %cases = ( ..... 'All required fields except address_type' => { filename => "$testdir/20080101000003.ADDRESS", rows => [ { %all_nulls, custnum => '7654321', custnum => '7654321', }, ], expected => { 'read' => 1, rejected => 1 }, }, That hashref looks like it's pretty much the same sort of thing that I posted this ticket about: A single hashref in an arrayref, and the hashrefs values start with a variable being expanded. Mind you, I'm not complaining that we found another bit of bad code. Just an FYI for you.
I ran against my entire codebase and the false positives are no longer coming up. Thanks for the fix.
(For some reason the system didn't send me your followup at the time. But beaut that it's right now.)