perl critic in depth

174
Jeffrey Thalhammer S o f t w a r e S y s t e m s [email protected] Thursday, September 30, 2010

Upload: jeffrey-ryan-thalhammer

Post on 31-Oct-2014

3.533 views

Category:

Documents


3 download

DESCRIPTION

 

TRANSCRIPT

Page 1: Perl Critic In Depth

Jeffrey Thalhammer

S o f t w a r e S y s t e m s

[email protected]

Thursday, September 30, 2010

Page 2: Perl Critic In Depth

San Francisco Perl MongersJuly 27, 2010

Some Ways Are Better Than Others

Thursday, September 30, 2010

Page 3: Perl Critic In Depth

HaveYou Ever?...

Thursday, September 30, 2010

Page 4: Perl Critic In Depth

print ‘Hello, my name is $name\n’;

Thursday, September 30, 2010

Page 5: Perl Critic In Depth

eval ‘reqire Some::Module’;

Thursday, September 30, 2010

Page 6: Perl Critic In Depth

if(not $foo && $bar or $baz){...}

Thursday, September 30, 2010

Page 7: Perl Critic In Depth

if($this == ‘that’){...}

Thursday, September 30, 2010

Page 8: Perl Critic In Depth

What Is Perl::Critic?

Thursday, September 30, 2010

Page 9: Perl Critic In Depth

What Is Perl::Critic?

• Static Source Code Analyzer

Thursday, September 30, 2010

Page 10: Perl Critic In Depth

What Is Perl::Critic?

• Static Source Code Analyzer

• Like “lint” but for Perl code

Thursday, September 30, 2010

Page 11: Perl Critic In Depth

What Is Perl::Critic?

• Static Source Code Analyzer

• Like “lint” but for Perl code

• Finds bugs & enforces style

Thursday, September 30, 2010

Page 12: Perl Critic In Depth

What Is Perl::Critic?

• Static Source Code Analyzer

• Like “lint” but for Perl code

• Finds bugs & enforces style

• Mostly based on Perl Best Practices

Thursday, September 30, 2010

Page 13: Perl Critic In Depth

Genesis

Thursday, September 30, 2010

Page 14: Perl Critic In Depth

Genesis

• Large legacy code base (~500k lines)

Thursday, September 30, 2010

Page 15: Perl Critic In Depth

Genesis

• Large legacy code base (~500k lines)

• Evolved organically over 10 years

Thursday, September 30, 2010

Page 16: Perl Critic In Depth

Genesis

• Large legacy code base (~500k lines)

• Evolved organically over 10 years

• Many developers with various skill levels

Thursday, September 30, 2010

Page 17: Perl Critic In Depth

Genesis

• Large legacy code base (~500k lines)

• Evolved organically over 10 years

• Many developers with various skill levels

• “Worst code I’ve ever seen.”

Thursday, September 30, 2010

Page 18: Perl Critic In Depth

Genesis

• Large legacy code base (~500k lines)

• Evolved organically over 10 years

• Many developers with various skill levels

• “Worst code I’ve ever seen.”

• “Most profitable code I’ve ever seen.”

Thursday, September 30, 2010

Page 19: Perl Critic In Depth

Genesis

Thursday, September 30, 2010

Page 20: Perl Critic In Depth

Genesis

• Highly inconsistent (anti-patterns)

Thursday, September 30, 2010

Page 21: Perl Critic In Depth

Genesis

• Highly inconsistent (anti-patterns)

• Extremely complicated code

Thursday, September 30, 2010

Page 22: Perl Critic In Depth

Genesis

• Highly inconsistent (anti-patterns)

• Extremely complicated code

• Much copy & pasted code

Thursday, September 30, 2010

Page 23: Perl Critic In Depth

Genesis

• Highly inconsistent (anti-patterns)

• Extremely complicated code

• Much copy & pasted code

• Cargo cult

Thursday, September 30, 2010

Page 24: Perl Critic In Depth

Behold!

Thursday, September 30, 2010

Page 25: Perl Critic In Depth

But How Can I Get Others To Learn Perl

Best Practices?

Thursday, September 30, 2010

Page 26: Perl Critic In Depth

Introducing PPI

Thursday, September 30, 2010

Page 27: Perl Critic In Depth

Introducing PPI

• By Adam Kennedy (a.k.a. alias)

Thursday, September 30, 2010

Page 28: Perl Critic In Depth

Introducing PPI

• By Adam Kennedy (a.k.a. alias)

• “Parse Perl Independently”

Thursday, September 30, 2010

Page 29: Perl Critic In Depth

Introducing PPI

• By Adam Kennedy (a.k.a. alias)

• “Parse Perl Independently”

• Parse and lex Perl code into DOM

Thursday, September 30, 2010

Page 30: Perl Critic In Depth

Introducing PPI

• By Adam Kennedy (a.k.a. alias)

• “Parse Perl Independently”

• Parse and lex Perl code into DOM

• Only perl can parse Perl

Thursday, September 30, 2010

Page 31: Perl Critic In Depth

Introducing PPI

• By Adam Kennedy (a.k.a. alias)

• “Parse Perl Independently”

• Parse and lex Perl code into DOM

• Only perl can parse Perl

• PPI comes pretty darn close

Thursday, September 30, 2010

Page 32: Perl Critic In Depth

And Perl::Critic Was Born

Thursday, September 30, 2010

Page 33: Perl Critic In Depth

Getting Feet Wet

package Foo;

sub showGreeting ($) {my $name = shift;return undef if not $name;open FH, “>out.txt”;print FH ‘Hello $name\n’;

}

File: Bar.pm

12345678

Thursday, September 30, 2010

Page 34: Perl Critic In Depth

Getting Feet Wet

[jeff@callahan:/Users/jeff]$ perlcritic Bar.pm Package declaration must match filename at line 1, column 1. Correct the filename or package statement. (Severity: 5)Subroutine prototypes used at line 3, column 1. See page 194 of PBP. (Severity: 5)Code before strictures are enabled at line 3, column 1. See page 429 of PBP. (Severity: 5)"return" statement with explicit "undef" at line 5, column 3. See page 199 of PBP. (Severity: 5)Bareword file handle opened at line 6, column 3. See pages 202,204 of PBP. (Severity: 5)Two-argument "open" used at line 6, column 3. See page 207 of PBP. (Severity: 5)

USAGE: perlcritic FILENAME

Thursday, September 30, 2010

Page 35: Perl Critic In Depth

Two-argument "open" used at line 6, column 3. See page 207 of PBP. (Severity: 5)

A Closer Look

Thursday, September 30, 2010

Page 36: Perl Critic In Depth

Two-argument "open" used at line 6, column 3. See page 207 of PBP. (Severity: 5)

Description

A Closer Look

Thursday, September 30, 2010

Page 37: Perl Critic In Depth

Two-argument "open" used at line 6, column 3. See page 207 of PBP. (Severity: 5)

Description Location

A Closer Look

Thursday, September 30, 2010

Page 38: Perl Critic In Depth

Two-argument "open" used at line 6, column 3. See page 207 of PBP. (Severity: 5)

Description Location

PBP Reference

A Closer Look

Thursday, September 30, 2010

Page 39: Perl Critic In Depth

Two-argument "open" used at line 6, column 3. See page 207 of PBP. (Severity: 5)

Description Location

PBP Reference Severity

A Closer Look

Thursday, September 30, 2010

Page 40: Perl Critic In Depth

package Bar;

use strict;use warnings;

sub showGreeting {my $name = shift;return if not $name;open my $fh, ‘>’, “out.txt”;print $fh ‘Hello $name’;

}

File: Bar.pm

1234567891011

Fixing Violations

Thursday, September 30, 2010

Page 41: Perl Critic In Depth

[jeff@callahan:/Users/jeff]$ perlcritic Bar.pm Bar.pm source OK

USAGE: perlcritic FILENAME

Try Again

Thursday, September 30, 2010

Page 42: Perl Critic In Depth

[jeff@callahan:/Users/jeff]$ perlcritic --severity 4 Bar.pm Module does not end with "1;" at line 6, column 1. Must end with a recognizable true value. (Severity: 4)Subroutine "showGreeting" does not end with "return" at line 6, column 1. See page 197 of PBP. (Severity: 4)Close filehandles as soon as possible after opening them at line 9, column 1. See page 209 of PBP. (Severity: 4)

USAGE: perlcritic --serverity N FILENAME

Tightening The Screws

Thursday, September 30, 2010

Page 43: Perl Critic In Depth

package Bar;

use strict;use warnings;

sub showGreeting {my $name = shift;return if not $name;open my $fh, ‘>’, $name;print $fh ‘Hello $name’;close $fh;return 1;}

1;

123456789101112131415

File: Bar.pm

Thursday, September 30, 2010

Page 44: Perl Critic In Depth

One More Time

[jeff@callahan:/Users/jeff]$ perlcritic --severity 4 Bar.pm Bar.pm source OK

USAGE: perlcritic --serverity N FILENAME

Thursday, September 30, 2010

Page 45: Perl Critic In Depth

USAGE: perlcritic --serverity N FILENAME

Thursday, September 30, 2010

Page 46: Perl Critic In Depth

USAGE: perlcritic --serverity N FILENAME[jeff@callahan:/Users/jeff]$ perlcritic --severity 2 Bar.pm RCS keywords $Revision$, $HeadURL$, $Date$ not found at line 1, column 1. See page 441 of PBP. (Severity: 2)No "$VERSION" variable found at line 1, column 1. See page 404 of PBP. (Severity: 2)Return value of "close" ignored at line 11, column 1. Check the return value of "close" for success. (Severity: 2)

Thursday, September 30, 2010

Page 47: Perl Critic In Depth

USAGE: perlcritic --serverity N FILENAME[jeff@callahan:/Users/jeff]$ perlcritic --severity 2 Bar.pm RCS keywords $Revision$, $HeadURL$, $Date$ not found at line 1, column 1. See page 441 of PBP. (Severity: 2)No "$VERSION" variable found at line 1, column 1. See page 404 of PBP. (Severity: 2)Return value of "close" ignored at line 11, column 1. Check the return value of "close" for success. (Severity: 2)

[jeff@callahan:/Users/jeff]$ perlcritic --severity 1 Bar.pm RCS keywords $Id$ not found at line 1, column 1. See page 441 of PBP. (Severity: 2)RCS keywords $Revision$, $HeadURL$, $Date$ not found at line 1, column 1. See page 441 of PBP. (Severity: 2)RCS keywords $Revision$, $Source$, $Date$ not found at line 1, column 1. See page 441 of PBP. (Severity: 2)No "$VERSION" variable found at line 1, column 1. See page 404 of PBP. (Severity: 2)Subroutine "showGreeting" is not all lower case or all upper case at line 6, column 1. See pages 45,46 of PBP. (Severity: 1)Return value of "open" ignored at line 9, column 1. Check the return value of "open" for success. (Severity: 3)Return value of flagged function ignored - open at line 9, column 1. See pages 208,278 of PBP. (Severity: 1)File handle for "print" or "printf" is not braced at line 10, column 1. See page 217 of PBP. (Severity: 1)Return value of flagged function ignored - print at line 10, column 1. See pages 208,278 of PBP. (Severity: 1)String *may* require interpolation at line 10, column 11. See page 51 of PBP. (Severity: 1)Return value of "close" ignored at line 11, column 1. Check the return value of "close" for success. (Severity: 2)Return value of flagged function ignored - close at line 11, column 1. See pages 208,278 of PBP. (Severity: 1)

Thursday, September 30, 2010

Page 48: Perl Critic In Depth

Five Levels Of Severity

Thursday, September 30, 2010

Page 49: Perl Critic In Depth

Five Levels Of Severity

• 5 (Highest): Likely bug or widely accepted practice

Thursday, September 30, 2010

Page 50: Perl Critic In Depth

Five Levels Of Severity

• 5 (Highest): Likely bug or widely accepted practice

• 1 (Lowest): Purely cosmetic or highly controversial

Thursday, September 30, 2010

Page 51: Perl Critic In Depth

Five Levels Of Severity

• 5 (Highest): Likely bug or widely accepted practice

• 1 (Lowest): Purely cosmetic or highly controversial

• Severities are determined by Policy authors

Thursday, September 30, 2010

Page 52: Perl Critic In Depth

Five Levels Of Severity

• 5 (Highest): Likely bug or widely accepted practice

• 1 (Lowest): Purely cosmetic or highly controversial

• Severities are determined by Policy authors

• Also have names (brutal, cruel, harsh, stern, gentle)

Thursday, September 30, 2010

Page 53: Perl Critic In Depth

Five Levels Of Severity

• 5 (Highest): Likely bug or widely accepted practice

• 1 (Lowest): Purely cosmetic or highly controversial

• Severities are determined by Policy authors

• Also have names (brutal, cruel, harsh, stern, gentle)

• Can be configured

Thursday, September 30, 2010

Page 54: Perl Critic In Depth

Configuration perlcritic [-12345 | --brutal | --cruel | --harsh | --stern | --gentle] [--severity number | name] [{-p | --profile} file | --noprofile] [--top [ number ]] [--theme expression] [--include pattern] [--exclude pattern] [{-s | --single-policy} pattern] [--only | --noonly] [--profile-strictness {warn|fatal|quiet}] [--force | --noforce] [--statistics] [--statistics-only] [--count | -C] [--verbose {number | format}] [--color | --nocolor] [--pager pager] [--quiet] [--color-severity-highest color_specification] [--color-severity-high color_specification] [--color-severity-medium color_specification] [--color-severity-low color_specification] [--color-severity-lowest color_specification] [--files-with-violations | -l] [--files-without-violations | -L] {FILE | DIRECTORY | STDIN}

perlcritic --profile-proto

perlcritic { --list | --list-enabled | --list-themes | --doc pattern [...] }

perlcritic { --help | --options | --man | --version }

Thursday, September 30, 2010

Page 55: Perl Critic In Depth

Configuration

Thursday, September 30, 2010

Page 56: Perl Critic In Depth

Configuration

• .perlcriticrc file

Thursday, September 30, 2010

Page 57: Perl Critic In Depth

Configuration

• .perlcriticrc file

• In home directory or project directory

Thursday, September 30, 2010

Page 58: Perl Critic In Depth

Configuration

• .perlcriticrc file

• In home directory or project directory

• INI-style syntax

Thursday, September 30, 2010

Page 59: Perl Critic In Depth

# Global options hereseverity = 2pager = /usr/bin/lesscolor = 1

File: ~/.perlcriticrc

These will be the defaults, unless

overridden at the command-line

Configuration

Thursday, September 30, 2010

Page 60: Perl Critic In Depth

USAGE: perlcritic FILENAME[jeff@callahan:/Users/jeff]$ perlcritic Bar.pm RCS keywords $Revision$, $HeadURL$, $Date$ not found at line 1, column 1. See page 441 of PBP. (Severity: 2)No "$VERSION" variable found at line 1, column 1. See page 404 of PBP. (Severity: 2)Return value of "close" ignored at line 11, column 1. Check the return value of "close" for success. (Severity: 2)

Default severity is now 2, as defined in .perlcriticrc file

Thursday, September 30, 2010

Page 61: Perl Critic In Depth

USAGE: perlcritic --verbose 8 FILENAME

[jeff@callahan:/Users/jeff]$ perlcritic --verbose 8 Bar.pm [Miscellanea::RequireRcsKeywords] RCS keywords $Revision$, $Source$, $Date$ not found at line 1, column 1. (Severity: 2)[Modules::RequireVersionVar] No "$VERSION" variable found at line 1, column 1. (Severity: 2)[InputOutput::RequireCheckedClose] Return value of "close" ignored at line 11, column 1. (Severity: 2)

Now showing name of the Policy

Getting More Information

Thursday, September 30, 2010

Page 62: Perl Critic In Depth

# Global options hereseverity = 2verbose = 8

# Policy options here[Modules::RequireVersionVar]severity = 1

# These policies are disabled[-Miscellanea::RequireRcsKeywords][-InputOutput::RequireCheckedClose]

File: ~/.perlcriticrc

Configuration

Thursday, September 30, 2010

Page 63: Perl Critic In Depth

[jeff@callahan:/Users/jeff]$ perlcritic Bar.pm Bar.pm source OK

Now running with severity = 2, and

verbose = 8

Thursday, September 30, 2010

Page 64: Perl Critic In Depth

Eleven Levels Of Verbosity

Thursday, September 30, 2010

Page 65: Perl Critic In Depth

Eleven Levels Of Verbosity

• Increasing levels of detail

Thursday, September 30, 2010

Page 66: Perl Critic In Depth

Eleven Levels Of Verbosity

• Increasing levels of detail

• Can be customized with printf-style formats:“%m near %s at line %l in file %f”

Thursday, September 30, 2010

Page 67: Perl Critic In Depth

Eleven Levels Of Verbosity

• Increasing levels of detail

• Can be customized with printf-style formats:“%m near %s at line %l in file %f”

• Some of the available details:

Thursday, September 30, 2010

Page 68: Perl Critic In Depth

Eleven Levels Of Verbosity

• Increasing levels of detail

• Can be customized with printf-style formats:“%m near %s at line %l in file %f”

• Some of the available details:

• Message, diagnostic (PBP pages), POD extract

Thursday, September 30, 2010

Page 69: Perl Critic In Depth

Eleven Levels Of Verbosity

• Increasing levels of detail

• Can be customized with printf-style formats:“%m near %s at line %l in file %f”

• Some of the available details:

• Message, diagnostic (PBP pages), POD extract

• Line, column, file name, file path, severity

Thursday, September 30, 2010

Page 70: Perl Critic In Depth

Eleven Levels Of Verbosity

• Increasing levels of detail

• Can be customized with printf-style formats:“%m near %s at line %l in file %f”

• Some of the available details:

• Message, diagnostic (PBP pages), POD extract

• Line, column, file name, file path, severity

• Source code, PPI element, policy name

Thursday, September 30, 2010

Page 71: Perl Critic In Depth

Eleven Levels Of Verbosity

• Increasing levels of detail

• Can be customized with printf-style formats:“%m near %s at line %l in file %f”

• Some of the available details:

• Message, diagnostic (PBP pages), POD extract

• Line, column, file name, file path, severity

• Source code, PPI element, policy name

• Integrates with editor’s compile-mode (vim, emacs)Thursday, September 30, 2010

Page 72: Perl Critic In Depth

Lots of Policies

Thursday, September 30, 2010

Page 73: Perl Critic In Depth

Lots of Policies

• 128 Policies in Perl::Critic core

Thursday, September 30, 2010

Page 74: Perl Critic In Depth

Lots of Policies

• 128 Policies in Perl::Critic core

• 85 Policies taken directly from PBP

Thursday, September 30, 2010

Page 75: Perl Critic In Depth

Lots of Policies

• 128 Policies in Perl::Critic core

• 85 Policies taken directly from PBP

• Dozens of add-on Policies in CPAN

Thursday, September 30, 2010

Page 76: Perl Critic In Depth

Lots of Policies

• 128 Policies in Perl::Critic core

• 85 Policies taken directly from PBP

• Dozens of add-on Policies in CPAN

• Covering all aspects of programming

Thursday, September 30, 2010

Page 77: Perl Critic In Depth

Just A Taste

Thursday, September 30, 2010

Page 78: Perl Critic In Depth

Just A Taste• BuiltinFunctions::ProhibitVoidMap

Thursday, September 30, 2010

Page 79: Perl Critic In Depth

Just A Taste• BuiltinFunctions::ProhibitVoidMap

• ControlStructures::ProhibitUnreachableCode

Thursday, September 30, 2010

Page 80: Perl Critic In Depth

Just A Taste• BuiltinFunctions::ProhibitVoidMap

• ControlStructures::ProhibitUnreachableCode

• NamingConventions::ProhibitAmbiguousNames

Thursday, September 30, 2010

Page 81: Perl Critic In Depth

Just A Taste• BuiltinFunctions::ProhibitVoidMap

• ControlStructures::ProhibitUnreachableCode

• NamingConventions::ProhibitAmbiguousNames

• Subroutines::ProhibitExcessComplexity

Thursday, September 30, 2010

Page 82: Perl Critic In Depth

Just A Taste• BuiltinFunctions::ProhibitVoidMap

• ControlStructures::ProhibitUnreachableCode

• NamingConventions::ProhibitAmbiguousNames

• Subroutines::ProhibitExcessComplexity

• CodeLayout::RequireTidyCode

Thursday, September 30, 2010

Page 83: Perl Critic In Depth

Just A Taste• BuiltinFunctions::ProhibitVoidMap

• ControlStructures::ProhibitUnreachableCode

• NamingConventions::ProhibitAmbiguousNames

• Subroutines::ProhibitExcessComplexity

• CodeLayout::RequireTidyCode

• RegularExpressions::ProhibitFixedStringMatches

Thursday, September 30, 2010

Page 84: Perl Critic In Depth

Just A Taste• BuiltinFunctions::ProhibitVoidMap

• ControlStructures::ProhibitUnreachableCode

• NamingConventions::ProhibitAmbiguousNames

• Subroutines::ProhibitExcessComplexity

• CodeLayout::RequireTidyCode

• RegularExpressions::ProhibitFixedStringMatches

• Documentation::RequirePodSections

Thursday, September 30, 2010

Page 85: Perl Critic In Depth

Tips For Choosing Policies

Thursday, September 30, 2010

Page 86: Perl Critic In Depth

Tips For Choosing Policies• Use --profile-proto option to generate your

first .perlcriticrc file.

Thursday, September 30, 2010

Page 87: Perl Critic In Depth

Tips For Choosing Policies• Use --profile-proto option to generate your

first .perlcriticrc file.

• Start with --severity 5 and work your way down.

Thursday, September 30, 2010

Page 88: Perl Critic In Depth

Tips For Choosing Policies• Use --profile-proto option to generate your

first .perlcriticrc file.

• Start with --severity 5 and work your way down.

• Use the --doc option to view Policy documentation.

Thursday, September 30, 2010

Page 89: Perl Critic In Depth

Tips For Choosing Policies• Use --profile-proto option to generate your

first .perlcriticrc file.

• Start with --severity 5 and work your way down.

• Use the --doc option to view Policy documentation.

• Disable or reconfigure Policies that you don’t agree with.

Thursday, September 30, 2010

Page 90: Perl Critic In Depth

Tips For Choosing Policies• Use --profile-proto option to generate your

first .perlcriticrc file.

• Start with --severity 5 and work your way down.

• Use the --doc option to view Policy documentation.

• Disable or reconfigure Policies that you don’t agree with.

• Don’t try to do it all at once.

Thursday, September 30, 2010

Page 91: Perl Critic In Depth

#!/usr/bin/perl

print “Hi\n” if $^O eq ‘darwin’;print “Hello\n” if $^O =~ /win/i;

Bending The Rules

Thursday, September 30, 2010

Page 92: Perl Critic In Depth

#!/usr/bin/perl

print “Hi\n” if $^O eq ‘darwin’;print “Hello\n” if $^O =~ /win/i;

Bending The Rules

• This would violate ProhibitPostfixControls

Thursday, September 30, 2010

Page 93: Perl Critic In Depth

#!/usr/bin/perl

print “Hi\n” if $^O eq ‘darwin’;print “Hello\n” if $^O =~ /win/i;

Bending The Rules

• This would violate ProhibitPostfixControls

• But we really like the way it reads

Thursday, September 30, 2010

Page 94: Perl Critic In Depth

#!/usr/bin/perl

## no critic

print “Hi\n” if $^O eq ‘darwin’;print “Hello\n” if $^O =~ /win/i;

## use critic

Bending The Rules

• ## no critic disables Perl::Critic until end of block or until ## use critic

Thursday, September 30, 2010

Page 95: Perl Critic In Depth

#!/usr/bin/perl

frobulate($this) if $that; ## no critic

Bending The Rules

• When attached to a line of code ## no critic disables Perl::Critic for that line only

Thursday, September 30, 2010

Page 96: Perl Critic In Depth

#!/usr/bin/perl

frobulate($this) if $that; ## no critic (ProhibitPostfixControls)

Bending The Rules

You can also specify exactly which Policies to disable

Thursday, September 30, 2010

Page 97: Perl Critic In Depth

Perl::Critic Applied

$> perlcritic --verbose 8 MyModule.pm

#!/usr/bin/perl

use strict;use warnings;use criticism ‘brutal’; ...

As command-line tool...

As pragma...

Thursday, September 30, 2010

Page 98: Perl Critic In Depth

Perl::Critic Applied

#!/usr/bin/perl

use Test::Perl::Critic;all_critic_ok();

As test library...

Thursday, September 30, 2010

Page 99: Perl Critic In Depth

Tips For Working With Legacy Code

Thursday, September 30, 2010

Page 100: Perl Critic In Depth

Tips For Working With Legacy Code

• Use --statistics option to identify hotspots.

Thursday, September 30, 2010

Page 101: Perl Critic In Depth

Tips For Working With Legacy Code

• Use --statistics option to identify hotspots.

• Segregate new code from old code.

Thursday, September 30, 2010

Page 102: Perl Critic In Depth

Tips For Working With Legacy Code

• Use --statistics option to identify hotspots.

• Segregate new code from old code.

• Use “no critic” annotations around blocks of nasty legacy code.

Thursday, September 30, 2010

Page 103: Perl Critic In Depth

Tips For Working With Legacy Code

• Use --statistics option to identify hotspots.

• Segregate new code from old code.

• Use “no critic” annotations around blocks of nasty legacy code.

• Don’t mix clean-up commits with regular work.

Thursday, September 30, 2010

Page 104: Perl Critic In Depth

Tips For Working With Legacy Code

• Use --statistics option to identify hotspots.

• Segregate new code from old code.

• Use “no critic” annotations around blocks of nasty legacy code.

• Don’t mix clean-up commits with regular work.

• Use Test::Perl::Critic::Progressive to chip away at the problem.

Thursday, September 30, 2010

Page 105: Perl Critic In Depth

Test::Perl::Critic::Progressive

Thursday, September 30, 2010

Page 106: Perl Critic In Depth

Test::Perl::Critic::Progressive

• TAP-compatible test module.

Thursday, September 30, 2010

Page 107: Perl Critic In Depth

Test::Perl::Critic::Progressive

• TAP-compatible test module.

• Always passes the first time.

Thursday, September 30, 2010

Page 108: Perl Critic In Depth

Test::Perl::Critic::Progressive

• TAP-compatible test module.

• Always passes the first time.

• Makes a record of violations (in a local file).

Thursday, September 30, 2010

Page 109: Perl Critic In Depth

Test::Perl::Critic::Progressive

• TAP-compatible test module.

• Always passes the first time.

• Makes a record of violations (in a local file).

• Subsequent runs will fail if new violations are added.

Thursday, September 30, 2010

Page 110: Perl Critic In Depth

Test::Perl::Critic::Progressive

• TAP-compatible test module.

• Always passes the first time.

• Makes a record of violations (in a local file).

• Subsequent runs will fail if new violations are added.

• Works best as part of continuous integration system.

Thursday, September 30, 2010

Page 111: Perl Critic In Depth

Extending Perl::Critic

Thursday, September 30, 2010

Page 112: Perl Critic In Depth

Extending Perl::Critic

• Each Policy is a module in the Perl::Critic::Policy namespace.

Thursday, September 30, 2010

Page 113: Perl Critic In Depth

Extending Perl::Critic

• Each Policy is a module in the Perl::Critic::Policy namespace.

• Each Policy extends the Perl::Critic::Policy abstract base class.

Thursday, September 30, 2010

Page 114: Perl Critic In Depth

Extending Perl::Critic

• Each Policy is a module in the Perl::Critic::Policy namespace.

• Each Policy extends the Perl::Critic::Policy abstract base class.

• Automatically loaded via Module::Pluggable.

Thursday, September 30, 2010

Page 115: Perl Critic In Depth

Extending Perl::Critic

• Each Policy is a module in the Perl::Critic::Policy namespace.

• Each Policy extends the Perl::Critic::Policy abstract base class.

• Automatically loaded via Module::Pluggable.

• Sometimes as little as 20 lines of code.

Thursday, September 30, 2010

Page 116: Perl Critic In Depth

Extending Perl::Critic

• Each Policy is a module in the Perl::Critic::Policy namespace.

• Each Policy extends the Perl::Critic::Policy abstract base class.

• Automatically loaded via Module::Pluggable.

• Sometimes as little as 20 lines of code.

• See POD for step-by-step tutorial.

Thursday, September 30, 2010

Page 117: Perl Critic In Depth

Extending Perl::Critic

Thursday, September 30, 2010

Page 118: Perl Critic In Depth

Extending Perl::Critic

• Policies do not have to be from PBP.

Thursday, September 30, 2010

Page 119: Perl Critic In Depth

Extending Perl::Critic

• Policies do not have to be from PBP.

• Your Policies can conflict with existing ones.

Thursday, September 30, 2010

Page 120: Perl Critic In Depth

Extending Perl::Critic

• Policies do not have to be from PBP.

• Your Policies can conflict with existing ones.

• Policies can be general or domain-specific.

Thursday, September 30, 2010

Page 121: Perl Critic In Depth

if ($foo && ($bar || $baz) || !($bar && $baz) || $qux) {...}

Suppose We Want To Ban Really Complex

Boolean Expressions...

Thursday, September 30, 2010

Page 122: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Thursday, September 30, 2010

Page 123: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Put Policy in Perl::Critic::Policy namespace

Thursday, September 30, 2010

Page 124: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Thursday, September 30, 2010

Page 125: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Policies organized by PBP table of contents

Thursday, September 30, 2010

Page 126: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Thursday, September 30, 2010

Page 127: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Some boilerplate

Thursday, September 30, 2010

Page 128: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Thursday, September 30, 2010

Page 129: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Extend the Policy baseclass

Thursday, September 30, 2010

Page 130: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Thursday, September 30, 2010

Page 131: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Define default attributes

Thursday, September 30, 2010

Page 132: Perl Critic In Depth

package Perl::Critic::Policy::ValuesAndExpressions::ProhibitComplexBoolean;

use strict;use warnings;use Readonly;

use Perl::Critic::Utils ‘:severities’;use base ‘Perl::Critic::Policy’;

sub default_severity{ return $SEVERITY_HIGH }sub default_theme{ return qw(complexity) }sub supported_params{ return () }

Thursday, September 30, 2010

Page 133: Perl Critic In Depth

sub default_severity{ return $SEVERITY_HIGH };sub default_theme{ return qw(complexity) };sub supported_params{ return () }

Readonly::Scalar my $DESC => 'Too many boolean operators';

Readonly::Scalar my $EXPL => 'Complex expressions are hard to read';

Thursday, September 30, 2010

Page 134: Perl Critic In Depth

sub default_severity{ return $SEVERITY_HIGH };sub default_theme{ return qw(complexity) };sub supported_params{ return () }

Readonly::Scalar my $DESC => 'Too many boolean operators';

Readonly::Scalar my $EXPL => 'Complex expressions are hard to read';

Just declaring some variables to store the Description and

Explanation

Thursday, September 30, 2010

Page 135: Perl Critic In Depth

sub default_severity{ return $SEVERITY_HIGH };sub default_theme{ return qw(complexity) };sub supported_params{ return () }

Readonly::Scalar my $DESC => 'Too many boolean operators';

Readonly::Scalar my $EXPL => 'Complex expressions are hard to read';

Thursday, September 30, 2010

Page 136: Perl Critic In Depth

USAGE: ppidump ‘some source code’USAGE: ppidump STDIN

$> ./tools/ppidump 'if($foo && $bar && $baz){}'PPI::Document PPI::Statement::Compound PPI::Token::Word 'if' PPI::Structure::Condition ( ... ) PPI::Statement::Expression PPI::Token::Symbol '$foo' PPI::Token::Operator '&&' PPI::Token::Symbol '$bar' PPI::Token::Operator '&&' PPI::Token::Symbol '$baz' PPI::Structure::Block { ... }

How Does PPI See Your Code?

Thursday, September 30, 2010

Page 137: Perl Critic In Depth

USAGE: ppidump ‘some source code’USAGE: ppidump STDIN

$> ./tools/ppidump 'if($foo && $bar && $baz){}'PPI::Document PPI::Statement::Compound PPI::Token::Word 'if' PPI::Structure::Condition ( ... ) PPI::Statement::Expression PPI::Token::Symbol '$foo' PPI::Token::Operator '&&' PPI::Token::Symbol '$bar' PPI::Token::Operator '&&' PPI::Token::Symbol '$baz' PPI::Structure::Block { ... }

How Does PPI See Your Code?

This is where we want to look for

violations

Thursday, September 30, 2010

Page 138: Perl Critic In Depth

USAGE: ppidump ‘some source code’USAGE: ppidump STDIN

$> ./tools/ppidump 'if($foo && $bar && $baz){}'PPI::Document PPI::Statement::Compound PPI::Token::Word 'if' PPI::Structure::Condition ( ... ) PPI::Statement::Expression PPI::Token::Symbol '$foo' PPI::Token::Operator '&&' PPI::Token::Symbol '$bar' PPI::Token::Operator '&&' PPI::Token::Symbol '$baz' PPI::Structure::Block { ... }

How Does PPI See Your Code?

Thursday, September 30, 2010

Page 139: Perl Critic In Depth

USAGE: ppidump ‘some source code’USAGE: ppidump STDIN

$> ./tools/ppidump 'if($foo && $bar && $baz){}'PPI::Document PPI::Statement::Compound PPI::Token::Word 'if' PPI::Structure::Condition ( ... ) PPI::Statement::Expression PPI::Token::Symbol '$foo' PPI::Token::Operator '&&' PPI::Token::Symbol '$bar' PPI::Token::Operator '&&' PPI::Token::Symbol '$baz' PPI::Structure::Block { ... }

How Does PPI See Your Code?

The offensive code is in a child node

Thursday, September 30, 2010

Page 140: Perl Critic In Depth

USAGE: ppidump ‘some source code’USAGE: ppidump STDIN

$> ./tools/ppidump 'if($foo && $bar && $baz){}'PPI::Document PPI::Statement::Compound PPI::Token::Word 'if' PPI::Structure::Condition ( ... ) PPI::Statement::Expression PPI::Token::Symbol '$foo' PPI::Token::Operator '&&' PPI::Token::Symbol '$bar' PPI::Token::Operator '&&' PPI::Token::Symbol '$baz' PPI::Structure::Block { ... }

How Does PPI See Your Code?

Thursday, September 30, 2010

Page 141: Perl Critic In Depth

sub applies_to { return ‘PPI::Structure::Condition’ }

sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!

}

Thursday, September 30, 2010

Page 142: Perl Critic In Depth

sub applies_to { return ‘PPI::Structure::Condition’ }

sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!

}

Declare which types of PPI elements we are interested in

Thursday, September 30, 2010

Page 143: Perl Critic In Depth

sub applies_to { return ‘PPI::Structure::Condition’ }

sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!

}

Thursday, September 30, 2010

Page 144: Perl Critic In Depth

sub applies_to { return ‘PPI::Structure::Condition’ }

sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!

} As document is traversed, we get called each time a

PPI::Structure::Conditional is encountered

Thursday, September 30, 2010

Page 145: Perl Critic In Depth

sub applies_to { return ‘PPI::Structure::Condition’ }

sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!

}

Thursday, September 30, 2010

Page 146: Perl Critic In Depth

sub applies_to { return ‘PPI::Structure::Condition’ }

sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!

} We also get a reference to the entire document

Thursday, September 30, 2010

Page 147: Perl Critic In Depth

sub applies_to { return ‘PPI::Structure::Condition’ }

sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!

}

Thursday, September 30, 2010

Page 148: Perl Critic In Depth

sub applies_to { return ‘PPI::Structure::Condition’ }

sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!

}

Your job is to figure out if this $elem is violating the Policy

Thursday, September 30, 2010

Page 149: Perl Critic In Depth

sub applies_to { return ‘PPI::Structure::Condition’ }

sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!

}

Thursday, September 30, 2010

Page 150: Perl Critic In Depth

sub violates {my ( $self, $elem, $doc ) = @_;my $ops = $elem->find(‘PPI::Token::Operator’);my $count = grep { _is_boolean($_) } @{$ops}if ($count > 3) {return $self->violation($DESC, $EXPL, $elem )

}}

sub _is_boolean {my ($elem) = @_;return $elem->content()=~ m/^ and | or | not | && | \|\| $/x;

}Thursday, September 30, 2010

Page 151: Perl Critic In Depth

sub violates {my ( $self, $elem, $doc ) = @_;my $ops = $elem->find(‘PPI::Token::Operator’);my $count = grep { _is_boolean($_) } @{$ops}if ($count > 3) {return $self->violation($DESC, $EXPL, $elem )

}}

sub _is_boolean {my ($elem) = @_;return $elem->content()=~ m/^ and | or | not | && | \|\| $/x;

}

Search recursively for all Operators

Thursday, September 30, 2010

Page 152: Perl Critic In Depth

sub violates {my ( $self, $elem, $doc ) = @_;my $ops = $elem->find(‘PPI::Token::Operator’);my $count = grep { _is_boolean($_) } @{$ops}if ($count > 3) {return $self->violation($DESC, $EXPL, $elem )

}}

sub _is_boolean {my ($elem) = @_;return $elem->content()=~ m/^ and | or | not | && | \|\| $/x;

}Thursday, September 30, 2010

Page 153: Perl Critic In Depth

sub violates {my ( $self, $elem, $doc ) = @_;my $ops = $elem->find(‘PPI::Token::Operator’);my $count = grep { _is_boolean($_) } @{$ops}if ($count > 3) {return $self->violation($DESC, $EXPL, $elem )

}}

sub _is_boolean {my ($elem) = @_;return $elem->content()=~ m/^ and | or | not | && | \|\| $/x;

}

Count those that are boolean

Thursday, September 30, 2010

Page 154: Perl Critic In Depth

sub violates {my ( $self, $elem, $doc ) = @_;my $ops = $elem->find(‘PPI::Token::Operator’);my $count = grep { _is_boolean($_) } @{$ops}if ($count > 3) {return $self->violation($DESC, $EXPL, $elem )

}}

sub _is_boolean {my ($elem) = @_;return $elem->content()=~ m/^ and | or | not | && | \|\| $/x;

}Thursday, September 30, 2010

Page 155: Perl Critic In Depth

sub violates {my ( $self, $elem, $doc ) = @_;my $ops = $elem->find(‘PPI::Token::Operator’);my $count = grep { _is_boolean($_) } @{$ops}if ($count > 3) {return $self->violation($DESC, $EXPL, $elem )

}}

sub _is_boolean {my ($elem) = @_;return $elem->content()=~ m/^ and | or | not | && | \|\| $/x;

}

Using regex to determine type of operator

Thursday, September 30, 2010

Page 156: Perl Critic In Depth

sub violates {my ( $self, $elem, $doc ) = @_;my $ops = $elem->find(‘PPI::Token::Operator’);my $count = grep { _is_boolean($_) } @{$ops}if ($count > 3) {return $self->violation($DESC, $EXPL, $elem )

}}

sub _is_boolean {my ($elem) = @_;return $elem->content()=~ m/^ and | or | not | && | \|\| $/x;

}Thursday, September 30, 2010

Page 157: Perl Critic In Depth

sub violates {my ( $self, $elem, $doc ) = @_;my $ops = $elem->find(‘PPI::Token::Operator’);my $count = grep { _is_boolean($_) } @{$ops}if ($count > 3) {return $self->violation($DESC, $EXPL, $elem )

}}

sub _is_boolean {my ($elem) = @_;return $elem->content()=~ m/^ and | or | not | && | \|\| $/x;

}

Return a violation if count exceeds maximum

Thursday, September 30, 2010

Page 158: Perl Critic In Depth

sub violates {my ( $self, $elem, $doc ) = @_;my $ops = $elem->find(‘PPI::Token::Operator’);my $count = grep { _is_boolean($_) } @{$ops}if ($count > 3) {return $self->violation($DESC, $EXPL, $elem )

}}

sub _is_boolean {my ($elem) = @_;return $elem->content()=~ m/^ and | or | not | && | \|\| $/x;

}Thursday, September 30, 2010

Page 159: Perl Critic In Depth

Testing Your Policy

Thursday, September 30, 2010

Page 160: Perl Critic In Depth

Testing Your Policy

• Use Test::Perl::Critic::Policy.

Thursday, September 30, 2010

Page 161: Perl Critic In Depth

Testing Your Policy

• Use Test::Perl::Critic::Policy.

• Specify test cases using POD-like notation.

Thursday, September 30, 2010

Page 162: Perl Critic In Depth

Testing Your Policy

• Use Test::Perl::Critic::Policy.

• Specify test cases using POD-like notation.

• Write Perl as Perl, not strings.

Thursday, September 30, 2010

Page 163: Perl Critic In Depth

#----------------------------------------## name Within limits## failures 0## cut

if ($foo && $bar) {}while ($foo or $bar || $baz) {}

#----------------------------------------## name Too many operators## failures 2## cut

if ($foo && $bar or not $baz and $quxx) {}while ($foo or $bar || $baz && quxx and not $quip) {}

File: t/ValuesAndExpressions/ProhbitComplexBoolean.run

Thursday, September 30, 2010

Page 164: Perl Critic In Depth

#----------------------------------------## name Within limits## failures 0## cut

if ($foo && $bar) {}while ($foo or $bar || $baz) {}

#----------------------------------------## name Too many operators## failures 2## cut

if ($foo && $bar or not $baz and $quxx) {}while ($foo or $bar || $baz && quxx and not $quip) {}

File: t/ValuesAndExpressions/ProhbitComplexBoolean.run

Give test a name

Thursday, September 30, 2010

Page 165: Perl Critic In Depth

#----------------------------------------## name Within limits## failures 0## cut

if ($foo && $bar) {}while ($foo or $bar || $baz) {}

#----------------------------------------## name Too many operators## failures 2## cut

if ($foo && $bar or not $baz and $quxx) {}while ($foo or $bar || $baz && quxx and not $quip) {}

File: t/ValuesAndExpressions/ProhbitComplexBoolean.run

Thursday, September 30, 2010

Page 166: Perl Critic In Depth

#----------------------------------------## name Within limits## failures 0## cut

if ($foo && $bar) {}while ($foo or $bar || $baz) {}

#----------------------------------------## name Too many operators## failures 2## cut

if ($foo && $bar or not $baz and $quxx) {}while ($foo or $bar || $baz && quxx and not $quip) {}

File: t/ValuesAndExpressions/ProhbitComplexBoolean.run

How many violations to expect

Thursday, September 30, 2010

Page 167: Perl Critic In Depth

#----------------------------------------## name Within limits## failures 0## cut

if ($foo && $bar) {}while ($foo or $bar || $baz) {}

#----------------------------------------## name Too many operators## failures 2## cut

if ($foo && $bar or not $baz and $quxx) {}while ($foo or $bar || $baz && quxx and not $quip) {}

File: t/ValuesAndExpressions/ProhbitComplexBoolean.run

Thursday, September 30, 2010

Page 168: Perl Critic In Depth

#----------------------------------------## name Within limits## failures 0## cut

if ($foo && $bar) {}while ($foo or $bar || $baz) {}

#----------------------------------------## name Too many operators## failures 2## cut

if ($foo && $bar or not $baz and $quxx) {}while ($foo or $bar || $baz && quxx and not $quip) {}

File: t/ValuesAndExpressions/ProhbitComplexBoolean.run

Write offending code

Thursday, September 30, 2010

Page 169: Perl Critic In Depth

#----------------------------------------## name Within limits## failures 0## cut

if ($foo && $bar) {}while ($foo or $bar || $baz) {}

#----------------------------------------## name Too many operators## failures 2## cut

if ($foo && $bar or not $baz and $quxx) {}while ($foo or $bar || $baz && quxx and not $quip) {}

File: t/ValuesAndExpressions/ProhbitComplexBoolean.run

Thursday, September 30, 2010

Page 170: Perl Critic In Depth

#----------------------------------------## name Within limits## failures 0## cut

if ($foo && $bar) {}while ($foo or $bar || $baz) {}

#----------------------------------------## name Too many operators## failures 2## cut

if ($foo && $bar or not $baz and $quxx) {}while ($foo or $bar || $baz && quxx and not $quip) {}

File: t/ValuesAndExpressions/ProhbitComplexBoolean.run

Code region ends when next “## name” is found

Thursday, September 30, 2010

Page 171: Perl Critic In Depth

#----------------------------------------## name Within limits## failures 0## cut

if ($foo && $bar) {}while ($foo or $bar || $baz) {}

#----------------------------------------## name Too many operators## failures 2## cut

if ($foo && $bar or not $baz and $quxx) {}while ($foo or $bar || $baz && quxx and not $quip) {}

File: t/ValuesAndExpressions/ProhbitComplexBoolean.run

Thursday, September 30, 2010

Page 172: Perl Critic In Depth

Questions?

Thursday, September 30, 2010

Page 173: Perl Critic In Depth

Thank You!

Thursday, September 30, 2010

Page 174: Perl Critic In Depth

Jeffrey Thalhammer

S o f t w a r e S y s t e m s

[email protected]

Thursday, September 30, 2010