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

People
Owner: Nobody in particular
Requestors: user42 [...] zip.com.au
Cc:
AdminCc:

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

Attachments
ProhibitUselessNoCritic.pm.moreutils-023.diff
ProhibitUselessNoCritic.pm.not-any.diff



Subject: ProhibitUselessNoCritic no report if nothing suppressed (patch)
Date: Sun, 05 Jul 2009 10:07:18 +1000
To: bug-Perl-Critic [...] rt.cpan.org
From: Kevin Ryde <user42 [...] zip.com.au>
Download (untitled) / with headers
text/plain 640b
With the current perlcritic cvs and recent debian i386 perl 5.10.0, a file foo.pl containing just ## no critic (BlahBlah) processed with perlcritic -s ProhibitUselessNoCritic foo.pl gives no violations, where I hoped it would report on the useless BlahBlah suppression. In ProhibitUselessNoCritic.pm I think the none() means the policy doesn't trigger if there's no suppressed violations. Ie. there has to be something validly suppressed before useless suppressions are detected. It seems dubious that List::MoreUtils none() isn't the same as !any(), but at any rate a change to the latter as below seems to do the trick.
Index: ProhibitUselessNoCritic.pm =================================================================== --- ProhibitUselessNoCritic.pm (revision 3371) +++ ProhibitUselessNoCritic.pm (working copy) @@ -13,7 +13,7 @@ use Readonly; -use List::MoreUtils qw< none >; +use List::MoreUtils qw< any >; use Perl::Critic::Utils qw{ :severities :classification hashify }; use base 'Perl::Critic::Policy'; @@ -44,7 +44,9 @@ my @suppressed_viols = $doc->suppressed_violations(); for my $ann ( $doc->annotations() ) { - if ( none { _annotation_suppresses_violation($ann, $_) } @suppressed_viols ) { + # Nb. List::MoreUtils::none() returns undef, ie. false, if + # @suppressed_violations is empty, hence !any() instead + unless (any { _annotation_suppresses_violation($ann, $_) } @suppressed_viols ) { push @violations, $self->violation($DESC, $EXPL, $ann->element()); } }
Download (untitled) / with headers
text/plain 1.5k
Show quoted text
> > It seems dubious that List::MoreUtils none() isn't the same as !any(), > but at any rate a change to the latter as below seems to do the trick. > >
What a mess. It appears that as of right now, the current production release of List::MoreUtils is 0.22. In this version, both any() and none() simply return if @_ is empty. Somewhere, I got a copy of List::MoreUtils 0.23, installed July 12 2009. In this version, none() returns 1 if @_ is empty. And with this version of List::MoreUtils, Miscellanea::ProhibitUselessNoCritic performs as you request. I do not know where my 0.23 came from, since there is currently no copy of this on either CPAN or BackPAN, though there's an RT bug against it for multiple test failures. There is a version 0.24 on BackPAN but not CPAN, also with an RT bug filed against it for multiple test failures. I have not downloaded 0.24 to investigate. But there are versions 0.25_01 (July 31 2009) and 0.25_02 (August 1 2009) on CPAN, and in both none() returns 1 if @_ is empty. My read on this is that the author of List::MoreUtils considers the current (meaning version 0.22) behavior of none() when @_ is empty to be a bug, and intends to correct it. All things being equal I personally believe that Perl::Critic should use the subroutine that behaves consistently, i.e. any(). But all things are never equal, and it looks to me like List::MoreUtils will be fixed before Perl::Critic can, since Perl::Critic is in the throes of being updated to work with the next release of PPI. So at this point I recommend just using List::MoreUtils 0.25_02. Tom Wyant
Subject: Re: [rt.cpan.org #47572] ProhibitUselessNoCritic no report if nothing suppressed (patch)
Date: Sun, 02 Aug 2009 08:25:23 +1000
To: bug-Perl-Critic [...] rt.cpan.org
From: Kevin Ryde <user42 [...] zip.com.au>
Download (untitled) / with headers
text/plain 313b
"Tom Wyant via RT" <bug-Perl-Critic@rt.cpan.org> writes: Show quoted text
> > List::MoreUtils 0.23, installed July 12 2009. > In this version, none() returns 1 if @_ is empty.
Yep, that can do it too, per below. There's some scary xsub bug fixes in 0.23 and 0.24 too, so demanding a version around there would be no bad thing.
Index: ProhibitUselessNoCritic.pm =================================================================== --- ProhibitUselessNoCritic.pm (revision 3459) +++ ProhibitUselessNoCritic.pm (working copy) @@ -13,7 +13,8 @@ use Readonly; -use List::MoreUtils qw< none >; +# 0.23 for none() to return true on empty list +use List::MoreUtils 0.23 qw< none >; use Perl::Critic::Utils qw{ :severities :classification hashify }; use base 'Perl::Critic::Policy';
Subject: Re: [rt.cpan.org #47572] ProhibitUselessNoCritic no report if nothing suppressed (patch)
Date: Sun, 02 Aug 2009 11:35:04 -0500
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Download (untitled) / with headers
text/plain 363b
Kevin Ryde via RT wrote: Show quoted text
> Yep, that can do it too, per below. There's some scary xsub bug fixes > in 0.23 and 0.24 too, so demanding a version around there would be no > bad thing.
At this point, I want to wait for whatever's going on with him sorting out 5.10.1, stack, etc. issues is done. Once that has happened, I say we bump the minimum version to that.
Subject: Re: [rt.cpan.org #47572] ProhibitUselessNoCritic no report if nothing suppressed (patch)
Date: Sat, 08 Aug 2009 07:31:01 +1000
To: bug-Perl-Critic [...] rt.cpan.org
From: Kevin Ryde <user42 [...] zip.com.au>
Download (untitled) / with headers
text/plain 576b
"Elliot Shank via RT" <bug-Perl-Critic@rt.cpan.org> writes: Show quoted text
> > At this point, I want to wait for whatever's going on with him sorting > out 5.10.1, stack, etc. issues is done. Once that has happened, I say > we bump the minimum version to that.
Make use of any() instead of none() until then in that case, so that ProhibitUselessNoCritic works. There's definite xs bugs in the older, and the newer seems far better, so I wouldn't wait. The alternative though is to force the pure-perl code. I did that for my program when I seemed to be striking too much strange stuff.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.