Regular expression without '/m' flag
Regular expression without '/x' flag
[download]

next if /^;/; #skip comments
[download]

The warning about /m simply looks weird for me though I can understand why do you use it. But it must not appear to the lines like

$string =~ /$re/;
[download]

local $" = '|';
[download]

Variable declared as 'local'
Magic punctuation variable used
[download]

while it must cause only the second warning! To use or not to use the punctuation variables is some kind of personal style, but not local'izing them is a severe error!

A little tip about the web-interface: You could add a possibility for the user to re-sort your warnings list by severity or by categories. The list would also look much better if it was represented by a table rather than by a simple linklist :)

Sorry for my bad English—it's not my native language :)

next if /^;/; #skip comments
[download]

It is complaining the xm switches are not included Change it to this,

Update: Also wrong per Jeronim

next if /^;/xms;
[download]

local $" = '|';
[download]

Don't use local variables, use my. Don't the special variables like $", use the English module.

Updated: Suggested but not working code

use English qw( -no_match_vars );

my $LIST_SEPERATOR = q{|};
[download]

Wrong spelling and yes will break your Code, Both pointed out by Jeronim and Chromatic...

use English qw( -no_match_vars );

{
local $LIST_SEPARATOR = q{|}; 
}
[download]

Would love to delete the post for just being wrong... Oh well :)

This will get you through the critic...

... and break your code.

use Test::More tests => 1;

use English '-no_match_vars';

my $LIST_SEPARATOR = '|';
my @items          = qw( foo bar baz );

my $interpolated   = "@items";
is( $interpolated, 'foo|bar|baz', 'somehow package globals and lexical
+s merged' );
[download]

You decided that my English is so poor that i can't read the docs of Perl::Critic and understand why these warnings appeared? ;)

Your comment contains many errors :( Yes, if i modify my code according to your recommendations, Perl::Critic will stop complaining—but my code will stop working :)

Your recommendation to use

next if /^;/xms;
[download]

is very weird. The using of /s and /m switches together causes a very unexpected interpretation of the RegEx:

perlretut says:
both s and m modifiers (//sm): Treat string as a single long line, but detect multiple lines. '.' matches any character, even "\n". ^ and $, however, are able to match at the start or end of any line within the string.

Do you still think that it's a good idea to add /ms to the end of each regex?

Don't use local variables, use my. Don't the special variables like $", use the English module.

use English qw( -no_match_vars ); 
my $LIST_SEPERATOR = q{|};
[download]

1. The Perl built-in variables lose their magic capabilities when declared as my. Didn't you know?
2. There is no $LIST_SEPERATOR. Use $LIST_SEPARATOR instead.
3. If you don't believe me, try the following code:

   #!/usr/bin/perl
   
   use warnings;
   use strict;
   
   use English qw( -no_match_vars );
   {
   local $LIST_SEPARATOR = q{!};
   
   my @list = qw/this is my list/;
   
   print "@list\n";
   }
   [download]

   and then try to replace 'local' with 'my'.
   BTW, Perl::Critic does not complain about local'ising in the code above ;)
      s;;Just-me-not-h-Ni-m-P-Ni-lm-I-ar-O-Ni;;tr?IerONim-?HAcker ?d;print

Jeff,

Howdy, I have been using your module for all my code. Currently I turn off two things for Perl::Critic,

1. RCS Control: am not using Subversion or CVS so I don't need that information in my modules.
2. Tidy: I like tidy, however, I only recently got an email that PerlTidy had fixed a bug with its handling of attributes (it was breaking my Class::Std code).

Otherwise I will accept 95% of the suggestions submitted by Perl::Critic and tell it to ignore the other 5% (generally this has to do with concating single and double quoted strings.

Some people complain about the rules used by Perl::Critic, most of the complaints are about personal preference rather than the rule itself. I am willing to give up some of my preferences, which I am not particularly passionate about, to have very consistent code. I have found it easier to read and maintain code that I had write months ago when I did this.

Overall, I have been very happy with the code I have written and validated by Perl::Critic and will continue to use it in the future.

The following:

gets me this (i believe incorrect) warning:
Integer with leading zeros at line 33, column 16. See page 55 of PBP. Severity: 5

Even just declaring @EXPORT results in Symbols are exported by default even when none are actually exported. Just rewording the warning for this case is probably enough.

When using @ISA, I get a warning both on the declaration and on usage. For @EXPORT I only get a warning on the declaration. Harmless but inconsistent.

Other than that, I think it appears to be pretty useful judging by the couple of checks I ran through the website.

I also wanted to say this about Jeff. Damian Conway wrote the book (Perl Best Practices) but Jeff implemented his ideas with Perl::Critic. I don't think anyone should underestimate his contributions for improving Perl coding standards...

As I'm on Windows and using ActiveState Perl (5.8.7) I used ppm to install Perl::Critic and that install seemed to succeed.

Unfortunately whenever I try to "critique" a file, it always crashes with a message such as "Not a CODE reference at C:/Data/webserver/Perl/site/lib/PPI/Statement/Sub.pm line 86.".

I have the latest version of PPI installed (version 1.115) and that install (through CPAN) and all tests went without a problem.

Anyone knows a solution?

I am using Activestate Perl 5.8.8 with PPI 1.115, and Perl::Critic 0.17. I have no problems.

The error does seem familiar however, I just cannot place my finger on it. Have you tried checking some other files or does it always return this error regardless of the file?

As far as I could see, the error is fairly general and seems perhaps related to the checking of "pragma". At least, when I tried to re-install Perl::Critic with CPAN, the install gave an error when doing the "pragma" test.

May be I should just switch to AS Perl 5.8.8 ... but I have tons of modules installed (some through ppm, some through CPAN) and it takes me days to re-install them all.

CountZero

"If you have four groups working on a compiler, you'll get a 4-pass compiler." - Conway's Law

Ever since starting to clean up some of my ~/bin to be Perl::Critic compliant I've found that I make more syntax errors and my code is harder to read. I suppose those two go together. There seem to be a small number of rules like ' ' -> q that seem designed just to make things difficult to parse. My tool progression went from having automatic syntax checks to automatic lint checks to automatic Perl::Critic tests. The transition away from things that actually compile the source started letting me leave dumb syntax bugs in my code. Bummer.

I was already in a sweet spot where I only ever saved syntactically correct things. I might have other bugs but at least I'd always pass perl's compilation.

I figure I'm going to have to either make P::C do Lint as well (which happens to include a syntax check) or uh... I dunno. Something. I figure having P::C + Lint is the best of both worlds because that's really P::C + Lint + Syntax. The only problem I have with Lint right now is that I haven't written in pragmas to disable it's checks in contexts where they're inappropriate.