Perl::Critic

Chris Dolan

MediaLandscape Software

cdolan@cpan.org

Andy Lester

Perl Foundation Public Relations

petdance@cpan.org

Perl Best Practices

• Damian Conway's book has fantastic recommendations for maintainable code
• TIMTOWTDI, but many ways are suboptimal
  • Bugs
  • Performance
  • Readability
  • Cargo cult
• But optimizing while coding is hard

→ Use code to optimize for you!

What is Perl::Critic?

Static source code analyzer

• Like an extendable lint for Perl
• As opposed to strict.pm which is a runtime live code analyzer
• Helps find potential bugs, bad practices and style non-compliance
• Easily automated via Test::Perl::Critic

by Jeffrey Ryan Thalhammer and crew

Bad code

open F, "$ARGV[0]" or die;
$_ = join("",map chomp, <F>);
s/<.*?>/ /g;
print

What it does:

• open a file
• strip out the line breaks
• strip out the HTML
• print the remaining text

it has a serious bug

Better code

use 5.006;
use warnings;
use strict;
use File::Slurp;
our $VERSION = 0.10;

my $htmlfile = shift
    or die 'Please specify an HTML file';

my $html = read_file($htmlfile);
$html =~ s/ \n //gxms;
$html =~ s/ <.*?>  # Match any html tag
          / /gxms; # replace with a space
print $html;

Policies

• 93 policy modules implemented, 70 of them from PBP
• Adding a new one is not difficult
• Loaded via Module::Pluggable
  — 16 more on CPAN already!

Installation

% cpan install Perl::Critic

Relies on PPI, the pure-perl Perl parser.

Usage

Usage

Configuration

Create a ~/.perlcriticrc file

[-CodeLayout::RequireTidyCode]
[-ControlStructures::ProhibitCStyleForLoops]
[-Documentation::RequirePodAtEnd]
[-Documentation::RequirePodSections]
[-Miscellanea::RequireRcsKeywords]
[-NamingConventions::ProhibitMixedCaseSubs]

[ControlStructures::ProhibitCascadingIfElse]
max_elsif = 3

Making Exceptions

Perl::Critic has flags to ignore special cases

• ## no critic at the end of a line
• Lexical block:
  ## no critic
... code ...
## use critic
• Accepts an optional substring to filter

  $email = 'foo@bar.com'; ## no critic(RequireInterpolation)

  sub _calc_xml_paths {   ## no critic(ExcessComplexity)
     ... lots of code ...
  }

Learn more

• http://search.cpan.org/dist/Perl-Critic
• http://perlcritic.tigris.org
• dev-subscribe@perlcritic.tigris.org

Policy Creation: Choose Goal

Pick a useful goal: detect calls to undeclared functions.

sub main {
   output_strnig('Hello, World!');
}

sub output_string {
   my ($msg) = @_;
   print $msg, "\n";
}

Caveats...

Policy Creation: Write test

## name sub exists
## failures 0
foo();
sub foo { }

## name sub missing
## failures 1
bar();

Policy Creation: Look at PPI DOM

PPI::Document
  PPI::Statement
    PPI::Token::Word    'bar'
    PPI::Structure::List        ( ... )
    PPI::Token::Structure       ';'

Policy Creation: Look at PPI DOM

PPI::Document
  PPI::Statement::Sub
    PPI::Token::Word    'sub'
    PPI::Token::Word    'foo'
    PPI::Structure::Block       { ... }
      PPI::Statement
        PPI::Token::Word        'print'
        PPI::Token::Structure   ';'

Policy Creation: Start the policy

package Perl::Critic::Policy::
  Subroutines::ProhibitUndeclaredSub;

use warnings;
use strict;
use Perl::Critic::Utils;
use base 'Perl::Critic::Policy';

sub applies_to {
   return 'PPI::Token::Word';
}

Policy Creation: Heart of the policy

sub violates {
   my ( $self, $elem, $doc ) = @_;

   return if !is_function_call($elem);

   my $subs = $doc->find('PPI::Statement::Sub');
   $subs ||= [];
   for my $sub ( @{ $subs } ) {
      return if $sub->name eq $elem;
   }

   return $self->violation($desc, $expl, $elem);
}

Policy Creation: Run tests

% t/20_policies.t Subroutines::ProhibitUndeclaredSub
1..3
ok 1 - Perl::Critic::Policy::Subroutines::
       ProhibitUndeclaredSub->can('violates')
ok 2 - Subroutines::ProhibitUndeclaredSub - 
       line 1 - sub exists
ok 3 - Subroutines::ProhibitUndeclaredSub - 
       line 7 - sub missing

Success!

Conclusion

Perl::Critic is:

• a framework for quality metrics
• very extensible
• easy to hack