perl critic in depth
DESCRIPTION
TRANSCRIPT
Jeffrey Thalhammer
S o f t w a r e S y s t e m s
Thursday, September 30, 2010
San Francisco Perl MongersJuly 27, 2010
Some Ways Are Better Than Others
Thursday, September 30, 2010
HaveYou Ever?...
Thursday, September 30, 2010
print ‘Hello, my name is $name\n’;
Thursday, September 30, 2010
eval ‘reqire Some::Module’;
Thursday, September 30, 2010
if(not $foo && $bar or $baz){...}
Thursday, September 30, 2010
if($this == ‘that’){...}
Thursday, September 30, 2010
What Is Perl::Critic?
Thursday, September 30, 2010
What Is Perl::Critic?
• Static Source Code Analyzer
Thursday, September 30, 2010
What Is Perl::Critic?
• Static Source Code Analyzer
• Like “lint” but for Perl code
Thursday, September 30, 2010
What Is Perl::Critic?
• Static Source Code Analyzer
• Like “lint” but for Perl code
• Finds bugs & enforces style
Thursday, September 30, 2010
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
Genesis
Thursday, September 30, 2010
Genesis
• Large legacy code base (~500k lines)
Thursday, September 30, 2010
Genesis
• Large legacy code base (~500k lines)
• Evolved organically over 10 years
Thursday, September 30, 2010
Genesis
• Large legacy code base (~500k lines)
• Evolved organically over 10 years
• Many developers with various skill levels
Thursday, September 30, 2010
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
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
Genesis
Thursday, September 30, 2010
Genesis
• Highly inconsistent (anti-patterns)
Thursday, September 30, 2010
Genesis
• Highly inconsistent (anti-patterns)
• Extremely complicated code
Thursday, September 30, 2010
Genesis
• Highly inconsistent (anti-patterns)
• Extremely complicated code
• Much copy & pasted code
Thursday, September 30, 2010
Genesis
• Highly inconsistent (anti-patterns)
• Extremely complicated code
• Much copy & pasted code
• Cargo cult
Thursday, September 30, 2010
Behold!
Thursday, September 30, 2010
But How Can I Get Others To Learn Perl
Best Practices?
Thursday, September 30, 2010
Introducing PPI
Thursday, September 30, 2010
Introducing PPI
• By Adam Kennedy (a.k.a. alias)
Thursday, September 30, 2010
Introducing PPI
• By Adam Kennedy (a.k.a. alias)
• “Parse Perl Independently”
Thursday, September 30, 2010
Introducing PPI
• By Adam Kennedy (a.k.a. alias)
• “Parse Perl Independently”
• Parse and lex Perl code into DOM
Thursday, September 30, 2010
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
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
And Perl::Critic Was Born
Thursday, September 30, 2010
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
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
Two-argument "open" used at line 6, column 3. See page 207 of PBP. (Severity: 5)
A Closer Look
Thursday, September 30, 2010
Two-argument "open" used at line 6, column 3. See page 207 of PBP. (Severity: 5)
Description
A Closer Look
Thursday, September 30, 2010
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
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
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
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
[jeff@callahan:/Users/jeff]$ perlcritic Bar.pm Bar.pm source OK
USAGE: perlcritic FILENAME
Try Again
Thursday, September 30, 2010
[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
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
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
USAGE: perlcritic --serverity N FILENAME
Thursday, September 30, 2010
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
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
Five Levels Of Severity
Thursday, September 30, 2010
Five Levels Of Severity
• 5 (Highest): Likely bug or widely accepted practice
Thursday, September 30, 2010
Five Levels Of Severity
• 5 (Highest): Likely bug or widely accepted practice
• 1 (Lowest): Purely cosmetic or highly controversial
Thursday, September 30, 2010
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
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
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
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
Configuration
Thursday, September 30, 2010
Configuration
• .perlcriticrc file
Thursday, September 30, 2010
Configuration
• .perlcriticrc file
• In home directory or project directory
Thursday, September 30, 2010
Configuration
• .perlcriticrc file
• In home directory or project directory
• INI-style syntax
Thursday, September 30, 2010
# 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
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
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
# 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
[jeff@callahan:/Users/jeff]$ perlcritic Bar.pm Bar.pm source OK
Now running with severity = 2, and
verbose = 8
Thursday, September 30, 2010
Eleven Levels Of Verbosity
Thursday, September 30, 2010
Eleven Levels Of Verbosity
• Increasing levels of detail
Thursday, September 30, 2010
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
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
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
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
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
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
Lots of Policies
Thursday, September 30, 2010
Lots of Policies
• 128 Policies in Perl::Critic core
Thursday, September 30, 2010
Lots of Policies
• 128 Policies in Perl::Critic core
• 85 Policies taken directly from PBP
Thursday, September 30, 2010
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
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
Just A Taste
Thursday, September 30, 2010
Just A Taste• BuiltinFunctions::ProhibitVoidMap
Thursday, September 30, 2010
Just A Taste• BuiltinFunctions::ProhibitVoidMap
• ControlStructures::ProhibitUnreachableCode
Thursday, September 30, 2010
Just A Taste• BuiltinFunctions::ProhibitVoidMap
• ControlStructures::ProhibitUnreachableCode
• NamingConventions::ProhibitAmbiguousNames
Thursday, September 30, 2010
Just A Taste• BuiltinFunctions::ProhibitVoidMap
• ControlStructures::ProhibitUnreachableCode
• NamingConventions::ProhibitAmbiguousNames
• Subroutines::ProhibitExcessComplexity
Thursday, September 30, 2010
Just A Taste• BuiltinFunctions::ProhibitVoidMap
• ControlStructures::ProhibitUnreachableCode
• NamingConventions::ProhibitAmbiguousNames
• Subroutines::ProhibitExcessComplexity
• CodeLayout::RequireTidyCode
Thursday, September 30, 2010
Just A Taste• BuiltinFunctions::ProhibitVoidMap
• ControlStructures::ProhibitUnreachableCode
• NamingConventions::ProhibitAmbiguousNames
• Subroutines::ProhibitExcessComplexity
• CodeLayout::RequireTidyCode
• RegularExpressions::ProhibitFixedStringMatches
Thursday, September 30, 2010
Just A Taste• BuiltinFunctions::ProhibitVoidMap
• ControlStructures::ProhibitUnreachableCode
• NamingConventions::ProhibitAmbiguousNames
• Subroutines::ProhibitExcessComplexity
• CodeLayout::RequireTidyCode
• RegularExpressions::ProhibitFixedStringMatches
• Documentation::RequirePodSections
Thursday, September 30, 2010
Tips For Choosing Policies
Thursday, September 30, 2010
Tips For Choosing Policies• Use --profile-proto option to generate your
first .perlcriticrc file.
Thursday, September 30, 2010
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
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
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
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
#!/usr/bin/perl
print “Hi\n” if $^O eq ‘darwin’;print “Hello\n” if $^O =~ /win/i;
Bending The Rules
Thursday, September 30, 2010
#!/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
#!/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
#!/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
#!/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
#!/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
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
Perl::Critic Applied
#!/usr/bin/perl
use Test::Perl::Critic;all_critic_ok();
As test library...
Thursday, September 30, 2010
Tips For Working With Legacy Code
Thursday, September 30, 2010
Tips For Working With Legacy Code
• Use --statistics option to identify hotspots.
Thursday, September 30, 2010
Tips For Working With Legacy Code
• Use --statistics option to identify hotspots.
• Segregate new code from old code.
Thursday, September 30, 2010
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
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
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
Test::Perl::Critic::Progressive
Thursday, September 30, 2010
Test::Perl::Critic::Progressive
• TAP-compatible test module.
Thursday, September 30, 2010
Test::Perl::Critic::Progressive
• TAP-compatible test module.
• Always passes the first time.
Thursday, September 30, 2010
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
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
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
Extending Perl::Critic
Thursday, September 30, 2010
Extending Perl::Critic
• Each Policy is a module in the Perl::Critic::Policy namespace.
Thursday, September 30, 2010
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
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
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
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
Extending Perl::Critic
Thursday, September 30, 2010
Extending Perl::Critic
• Policies do not have to be from PBP.
Thursday, September 30, 2010
Extending Perl::Critic
• Policies do not have to be from PBP.
• Your Policies can conflict with existing ones.
Thursday, September 30, 2010
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
if ($foo && ($bar || $baz) || !($bar && $baz) || $qux) {...}
Suppose We Want To Ban Really Complex
Boolean Expressions...
Thursday, September 30, 2010
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
sub applies_to { return ‘PPI::Structure::Condition’ }
sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!
}
Thursday, September 30, 2010
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
sub applies_to { return ‘PPI::Structure::Condition’ }
sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!
}
Thursday, September 30, 2010
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
sub applies_to { return ‘PPI::Structure::Condition’ }
sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!
}
Thursday, September 30, 2010
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
sub applies_to { return ‘PPI::Structure::Condition’ }
sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!
}
Thursday, September 30, 2010
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
sub applies_to { return ‘PPI::Structure::Condition’ }
sub violates {my ( $self, $elem, $doc ) = @_;# meat goes here!
}
Thursday, September 30, 2010
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
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
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
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
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
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
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
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
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
Testing Your Policy
Thursday, September 30, 2010
Testing Your Policy
• Use Test::Perl::Critic::Policy.
Thursday, September 30, 2010
Testing Your Policy
• Use Test::Perl::Critic::Policy.
• Specify test cases using POD-like notation.
Thursday, September 30, 2010
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
#----------------------------------------## 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
#----------------------------------------## 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
#----------------------------------------## 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
#----------------------------------------## 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
#----------------------------------------## 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
#----------------------------------------## 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
#----------------------------------------## 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
#----------------------------------------## 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
#----------------------------------------## 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
Questions?
Thursday, September 30, 2010
Thank You!
Thursday, September 30, 2010
Jeffrey Thalhammer
S o f t w a r e S y s t e m s
Thursday, September 30, 2010