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: 38942
Status: resolved
Priority: 0/
Queue: Perl-Critic

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

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



Subject: RegularExpressions::ProhibitUnusedCapture false positive
This test program: #!/usr/bin/perl # $Id $ use warnings; use strict; use 5.010; my $VERSION = 1; $_ = 'abcabcabc'; pos = 0; while (pos() < length) { m{\G(a)(b)(c)}gcxs or die; my ($a, $b, $c) = ($1, $2, $3); say "$a $b $c"; } demonstrably does use the values captured by the regular expression, as you can see by running it. But perlcritic doesn't see that: % perlcritic -1 --verbose '%f:%l:%c:%m [%p] (%e)\n' test test:10:5:Only use a capturing group if you plan to use the captured value [RegularExpressions::ProhibitUnusedCapture] (See page 252 of PBP)
I committed a TODO test that demonstrates the bug described here. Grep for the ticket number to find the test.
Proposed fix committed as SVN revision 3287. The commit 3267 logic that checks for the regex being in the condition of a while() loop, and if so prevents the extension of the capture variable list, was extended to include the while() loop's block as well. The TODO test was made no longer TODO.
Has this fix been included in the 1.103 release? The test program still gives the same result.
Subject: Re: [rt.cpan.org #38942] RegularExpressions::ProhibitUnusedCapture false positive
Date: Wed, 19 Aug 2009 22:37:37 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
EDAVIS via RT wrote: Show quoted text
> Has this fix been included in the 1.103 release? The test program still > gives the same result.
No. 1.103 is basically the same as 1.098 with fixes for Test::Pod 1.40 and PPI 1.206. There's a bit of a backlog. We're working on it!
Subject: Re: [rt.cpan.org #38942] RegularExpressions::ProhibitUnusedCapture false positive
Date: Tue, 25 Aug 2009 22:20:47 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Fix released in v1.104.
I confirm that the original example for this bug is fixed in 1.104, thanks. However there are other cases which still generate false positives, for example #!/usr/bin/perl # $Id $ use warnings; use strict; use 5.010; my $VERSION = 1; $_ = 'abcabcabc'; my $allow_a = 1; if (($allow_a and /(a)/) or /(b)/) { say $1; } Again, the $1 is definitely being used if the regexp matches, but the policy thinks it isn't.
Here's a simpler example of a false positive: #!/usr/bin/perl # $Id: $ use warnings; use strict; use Carp qw(croak); our $VERSION = 1; my @a = qw(abc def); foreach (@a) { /(a)/sxm or next; print "$1\n" or croak; }
On Wed Feb 09 12:36:24 2011, EDAVIS wrote: Show quoted text
> Here's a simpler example of a false positive: > > #!/usr/bin/perl > # $Id: $ > use warnings; > use strict; > use Carp qw(croak); > our $VERSION = 1; > my @a = qw(abc def); > foreach (@a) { > /(a)/sxm or next; > print "$1\n" or croak; > }
SVN commit 4046 gives this policy the ability to look inside interpolations, which should address this issue. The ticket needs to remain open because of the '/(a)/ or /(b)/' case, which is not yet addressed.
SVN commit 4049 addresses the if ( /(a)/ || /(b)/ ) { say $1; } case. Since I believe this leaves no unaddressed cases on this ticket, I'm marking it patched.