To use under emacs, simply add the following line to your .emacs, then when you visit a perl file, you can use Ctrl-Return to run perl_checker on this file
(global-set-key [(control return)] (lambda () (interactive) (save-some-buffers 1) (compile (concat "perl_checker --restrict-to-files " (buffer-file-name (current-buffer))))))
To use with vim, use something like:
perl_checker --restrict-to-files scanner.pm > errors.err ; vim -c ':copen 4' -c ':so /usr/share/vim/ftplugin/perl_checker.vim' -qwhere /usr/share/vim/ftplugin/perl_checker.vim is
" Error formats setlocal efm= \%EFile\ \"%f\"\\,\ line\ %l\\,\ characters\ %c-%*\\d:, \%EFile\ \"%f\"\\,\ line\ %l\\,\ character\ %c:%m, \%+EReference\ to\ unbound\ regexp\ name\ %m, \%Eocamlyacc:\ e\ -\ line\ %l\ of\ \"%f\"\\,\ %m, \%Wocamlyacc:\ w\ -\ %m, \%-Zmake%.%#, \%C%m
perl_checker is a checker: it is not a big deal to die horribly on a weird perl expression, it tells the programmer what to write instead. The issue is that perl_checker includes inter-modules analysis, and it implies being able to parse non-perl_checker compliant modules. A solution for this is perl_checker fake modules. No perfect solution though.
| local $xxx ||= $yyy | applying ||= on a new initialized variable is wrong |
| xxx(!my $xxx) | applying not on a new initialized variable is wrong |
| $1 =~ s/xxx/yyy/ | do not modify the result of a match (eg: $1) |
| $xxx[1, 2] | you must give only one argument |
| $xxx[] | you must give one argument |
| my $_x = 'xxx' if $xxx; | replace "my $foo = ... if <cond>" with "my $foo = <cond> && ..." |
| $xxx or my $_x = 'xxx'; | replace "<cond> or my $foo = ..." with "my $foo = !<cond> && ..." |
| '' || 'xxx' | <constant> || ... is the same as ... |
| if ($xxx = '') {} | are you sure you did not mean "==" instead of "="? |
| N("xxx$yyy") | don't use interpolated translated string, use %s or %d instead |
| if ($xxx && $yyy = xxx()) {} | invalid lvalue |
| 1 + 2 >> 3 | missing parentheses (needed for clarity) |
| $xxx ? $yyy = 1 : $zzz = 2; |
missing parentheses (needed for clarity) invalid lvalue |
| N_("xxx") . 'yyy' | N_("xxx") . "yyy" is dumb since the string "xxx" will never get translated |
| join(@l) | first argument of join() must be a scalar |
| join(',', 'foo') | join('...', $foo) is the same as $foo |
| if_($xxx) | not enough parameters |
| push @l | you must give some arguments to push |
| push $xxx, 1 | push is expecting an array |
| pop $xxx | pop is expecting an array and nothing else |
| my (@l2, $xxx) = @l; | @l2 takes all the arguments, $xxx is undef in any case |
| $bad | undeclared variable $bad |
| { my $a } | unused variable $a |
| my $xxx; yyy($xxx); my $xxx; | redeclared variable $xxx |
| { my $xxx; $xxx = 1 } | variable $xxx assigned, but not read |
| $a | undeclared variable $a |
| use bad; | can't find package bad |
| use pkg3 ':bad'; bad(); |
package pkg3 doesn't export tag :bad unknown function bad |
| use pkg3 ':missing_fs'; f(); |
name &f is not defined in package pkg3 name &f0 is not defined in package pkg3 |
| use pkg3 'f'; f(); |
name &f is not defined in package pkg3 |
| foreach (%h) {} |
context hash is not compatible with context list foreach with a hash is usually an error |
| map { 'xxx' } %h | a hash is not a valid parameter to function map |
| $xxx = ('yyy', 'zzz') | context tuple(string, string) is not compatible with context scalar |
| @l ||= 'xxx' | "||=" is only useful with a scalar |
| length @l | never use "length @l", it returns the length of the string int(@l) |
| %h . 'yyy' | context hash is not compatible with context string |
| 'xxx' > 'yyy' |
context string is not compatible with context float context string is not compatible with context float |
| 1 cmp 2 | you should use a number operator, not the string operator "cmp" (or replace the number with a string) |
| $xxx == undef | context undef is not compatible with context float |
| my ($xxx) = 1 | context int is not compatible with context tuple(scalar) |
| ($xxx, $yyy) = 1 | context int is not compatible with context tuple(scalar, scalar) |
| ($xxx, $yyy) = (1, 2, 3) | context tuple(int, int, int) is not compatible with context tuple(scalar, scalar) |
| @l eq '3' | context array is not compatible with context string |
| qw(a b) > 2 | context tuple(string, string) is not compatible with context float |
| %h > 0 | context hash is not compatible with context float |
| %h eq 0 |
context hash is not compatible with context string you should use a number operator, not the string operator "eq" (or replace the number with a string) |
| @{$xxx} | @{$xxx} can be written @$xxx |
| $h{"yyy"} | {"yyy"} can be written {yyy} |
| "$xxx" | $xxx is better written without the double quotes |
| $xxx->{yyy}->{zzz} | the arrow "->" is unneeded |
| "xxx\$xxx" | you can replace "xxx\$xxx" with 'xxx$xxx', that way you don't need to escape <$> |
| "xxx\$xxx" | you can replace "xxx\$xxx" with 'xxx$xxx', that way you don't need to escape <$> |
| "xxx\"$xxx" | you can replace "xxx\"xxx" with qq(xxx"xxx), that way you don't need to escape <"> |
| "xxx\"xxx'" | you can replace "xxx\"xxx" with qq(xxx"xxx), that way you don't need to escape <"> |
| /xxx\'xxx/ | you can replace \' with ' |
| /xxx\;xxx/ | you can replace \; with ; |
| /\// | change the delimit character / to get rid of this escape |
| { nop(); } | spurious ";" before closing block |
| +1 | don't use unary + |
| return ($xxx) | unneeded parentheses |
| if (($xxx eq $yyy) || $zzz) {} | unneeded parentheses |
| if (($xxx =~ /yyy/) || $zzz) {} | unneeded parentheses |
| nop() foreach ($xxx, $yyy); | unneeded parentheses |
| ($xxx) ||= 'xxx' | remove the parentheses |
| $o->m0() | remove these unneeded parentheses |
| $o = xxx() if !$o; | "$foo = ... if !$foo" can be written "$foo ||= ..." |
| $o = xxx() unless $o; | "$foo = ... unless $foo" can be written "$foo ||= ..." |
| $o or $o = xxx(); | "$foo or $foo = ..." can be written "$foo ||= ..." |
| $_ =~ s/xxx/yyy/ | "$_ =~ s/regexp/.../" can be written "s/regexp/.../" |
| $xxx =~ /^yyy$/ | "... =~ /^yyy$/" is better written "... eq 'yyy'" |
| /xxx.*/ | you can remove ".*" at the end of your regexp |
| /xxx.*$/ | you can remove ".*$" at the end of your regexp |
| /[^\s]/ | you can replace [^\s] with \S |
| /[^\w]/ | you can replace [^\w] with \W |
| $xxx ? $xxx : $yyy | you can replace "$foo ? $foo : $bar" with "$foo || $bar" |
| my @l = (); | no need to initialize variables, it's done by default |
| $l[$#l] | you can replace $#l with -1 |
| $#l == 0 | $#x == 0 is better written @x == 1 |
| $#l == -1 | $#x == -1 is better written @x == 0 |
| $#l < 0 | change your expression to use @xxx instead of $#xxx |
| $l[@l] = 1 | "$a[@a] = ..." is better written "push @a, ..." |
| xxx(@_) | replace xxx(@_) with &xxx |
| member($xxx, keys %h) | you can replace "member($xxx, keys %yyy)" with "exists $yyy{$xxx}" |
| !($xxx =~ /.../) | !($var =~ /.../) is better written $var !~ /.../ |
| !($xxx == 1) | !($foo == $bar) is better written $foo != $bar |
| !($xxx eq 'foo') | !($foo eq $bar) is better written $foo ne $bar |
| grep { !member($_, qw(a b c)) } @l | you can replace "grep { !member($_, ...) } @l" with "difference2([ @l ], [ ... ])" |
| any { $_ eq 'foo' } @l | you can replace "any { $_ eq ... } @l" with "member(..., @l)" |
| foreach (@l) { push @l2, $_ if yyy($_); } |
use "push @l2, grep { ... } ..." instead of "foreach (...) { push @l2, $_ if ... }" or sometimes "@l2 = grep { ... } ..." |
| foreach (@l) { push @l2, yyy($_); } |
use "push @l2, map { ... } ..." instead of "foreach (...) { push @l2, ... }" or sometimes "@l2 = map { ... } ..." |
| foreach (@l) { push @l2, yyy($_) if zzz($_); } |
use "push @l2, map { ... ? ... : () } ..." instead of "foreach (...) { push @l2, ... if ... }" or sometimes "@l2 = map { ... ? ... : () } ..." or sometimes "@l2 = map { if_(..., ...) } ..." |
| foreach (@l) { if (xxx($_)) { $xxx = $_; last; } } |
use "$xxx = find { ... } ..." |
| if (grep { xxx() } @l) {} | in boolean context, use "any" instead of "grep" |
| $xxx = grep { xxx() } @l; | you may use "find" instead of "grep" |
| $xxx ? $yyy : () |
you may use if_() here beware that the short-circuit semantic of ?: is not kept if you want to keep the short-circuit behaviour, replace () with @{[]} and there will be no warning anymore |
| system(qq(foo "$xxx")) | instead of quoting parameters you should give a list of arguments |
| system("mkdir", $xxx) | you can replace system("mkdir ...") with mkdir(...) |
| sub xxx { 'yyy' } |
if the function doesn't take any parameters, please use the empty prototype. example "sub foo() { ... }" |
| sub xxx { my ($o_xxx, $yyy) = @_; ($o_xxx, $yyy); } |
an non-optional argument must not follow an optional argument |
| sub xxx { my (@xxx, $yyy) = @_; @xxx, $yyy; } |
an array must be the last variable in a prototype |
| bad() | unknown function bad |
| sub f0() {} f0('yyy') |
too many parameters |
| sub f2 { my ($x, $_y) = @_; $x } f2('yyy') |
not enough parameters |
| N("xxx %s yyy") | not enough parameters |
| bad->yyy | unknown package bad |
| pkg->bad | unknown method bad starting in package pkg |
| $xxx->bad | unknown method bad |
| $xxx->m1 | not enough parameters |
| $xxx->m0('zzz') | too many parameters |
| $xxx->m0_or_2('zzz') | not enough or too many parameters |
| die; xxx(); | unreachable code |
| exit 1; xxx(); | unreachable code |
| if ($xxx or $yyy) {} |
value should be dropped context () is not compatible with context bool |
| if ($xxx and $yyy) {} |
value should be dropped context () is not compatible with context bool |
| $xxx && yyy(); | value is dropped |
| `xxx`; | value is dropped |
| /(.*)/; | value is dropped |
| 'xxx'; | value is dropped |
| 'xxx' if $xxx; | value is dropped |
| map { xxx($_) } @l; | if you don't use the return value, use "foreach" instead of "map" |
| $xxx = chomp; |
() context not accepted here context () is not compatible with context scalar |
| $xxx = push @l, 1 |
() context not accepted here context () is not compatible with context scalar |
| sub xxx {} |
you should not have a carriage-return (\n) here |
| xxx ($xxx); |
you should not have a carriage-return (\n) here |
| xxx( $xxx) | you should not have a space here |
| $xxx ++ | you should not have a space here |
| my($_xxx, $_yyy) | you should have a space here |
| xxx ($xxx) | you should not have a space here |
| 'foo'.'bar' | you should have a space here |
| if ($xxx) { xxx() } |
missing ";" |
| if ($xxx) { xxx(); }; |
unneeded ";" |
| $xxx <<= 2 | don't use "<<=", use the expanded version instead |
| m@xxx@ | don't use m@...@, replace @ with / ! , or | |
| s:xxx:yyy: | don't use s:...:, replace : with / ! , or | |
| qw/a b c/ | don't use qw/.../, use qw(...) instead |
| qw{a b c} | don't use qw{...}, use qw(...) instead |
| q{xxx} | don't use q{...}, use q(...) instead |
| qq{xxx} | don't use qq{...}, use qq(...) instead |
| qx(xxx) | don't use qx(...), use `...` instead |
| -xxx | don't use -xxx, use '-xxx' instead |
| not $xxx | don't use "not", use "!" instead |
| $1 =~ /xxx/ | do not use the result of a match (eg: $1) to match another pattern |
| $xxx =~ "yyy" | use a regexp, not a string |
| xxx() =~ s/xxx/yyy/ | you can only use s/// on a variable |
| $1 =~ /xxx/ | do not use the result of a match (eg: $1) to match another pattern |
| grep /xxx/, @l | always use "grep" with a block (eg: grep { ... } @list) |
| for (@l) {} | write "foreach" instead of "for" |
| foreach ($xxx = 0; $xxx < 9; $xxx++) {} | write "for" instead of "foreach" |
| foreach $xxx (@l) {} | don't use for without "my"ing the iteration variable |
| foreach ($xxx) {} | you are using the special trick to locally set $_ with a value, for this please use "for" instead of "foreach" |
| unless ($xxx) {} else {} | don't use "else" with "unless" (replace "unless" with "if") |
| unless ($xxx) {} elsif ($yyy) {} | don't use "elsif" with "unless" (replace "unless" with "if") |
| zzz() unless $xxx || $yyy; | don't use "unless" when the condition is complex, use "if" instead |
| $$xxx{yyy} | for complex dereferencing, use "->" |
| wantarray | please use wantarray() instead of wantarray |
| eval | please use "eval $_" instead of "eval" |
| local *F; open F, "foo"; | use a scalar instead of a bareword (eg: occurrences of F with $F) |
| $xxx !~ s/xxx/yyy/ | use =~ instead of !~ and negate the return value |
| pkg::nop $xxx; | use parentheses around argument (otherwise it might cause syntax errors if the package is "require"d and not "use"d |
| new foo $xxx | you must parenthesize parameters: "new Class(...)" instead of "new Class ..." |
| *xxx = *yyy | "*xxx = *yyy" is better written "*xxx = \&yyy" |
| $_xxx = 1 |
variable $_xxx must not be used (variable with name _XXX are reserved for unused variables) |
| sub f2 { my ($x, $_y) = @_; $x } f2(@l); # ok f2(xxx()); # bad |
not enough parameters |
| $xxx = <<"EOF"; foo EOF |
Don't use <<"MARK", use <<MARK instead |