| Subject: | Universally don't apply module policies to scripts (with patch) |
There's a number of policies which should only apply to modules but
don't. The only that always gets me is Modules::RequireEndWithOne.
Only Modules::RequireExplicitPackage has an exception for scripts.
The attached patch creates a superclass (which really should be a role)
for policies which should only apply to modules (PolicyModules.pm) so
they all have the exempt_script option on by default.
I applied it to:
ProhibitExcessMainComplexity
RequireEndWithOne
RequireExplicitPackage
RequireFilenameMatchesPackage
The rest of the Modules policies apply to Perl code universally and were
left alone.
| Subject: | PolicyModules.patch |
--- lib/Perl/Critic/Policy/Modules/ProhibitExcessMainComplexity.pm (revision 60251)
+++ lib/Perl/Critic/Policy/Modules/ProhibitExcessMainComplexity.pm (local)
@@ -15,7 +15,7 @@
use Perl::Critic::Utils qw{ :severities };
use Perl::Critic::Utils::McCabe qw{ calculate_mccabe_of_main };
-use base 'Perl::Critic::Policy';
+use base 'Perl::Critic::PolicyModules';
#-----------------------------------------------------------------------------
@@ -28,7 +28,10 @@
#-----------------------------------------------------------------------------
sub supported_parameters {
+ my $self = shift;
+
return (
+ $self->SUPER::supported_parameters,
{
name => 'max_mccabe',
description => 'The maximum complexity score allowed.',
@@ -48,6 +51,8 @@
sub violates {
my ( $self, $doc, undef ) = @_;
+ return if $self->is_exempt($doc);
+
my $score = calculate_mccabe_of_main( $doc );
# Is it too complex?
@@ -108,7 +113,6 @@
=head1 CONFIGURATION
The maximum acceptable McCabe score can be set with the C<max_mccabe>
-
configuration item. If the sum of all code B<outside> any subroutine has a
McCabe score higher than this number, it will generate a Policy violation.
The default is 20. An example section for a F<.perlcriticrc>:
@@ -116,6 +120,13 @@
[Modules::ProhibitExcessMainComplexity]
max_mccabe = 30
+As for programs, they tend to put their "main" code outside a subroutine, so
+this Policy doesn't apply to files that begin with a perl shebang. If you can
+make it do so by adding the following to your F<.perlcriticrc> file
+
+ [Modules::ProhibitExcessMainComplexity]
+ exempt_scripts = 0
+
=head1 NOTES
--- lib/Perl/Critic/Policy/Modules/RequireEndWithOne.pm (revision 60251)
+++ lib/Perl/Critic/Policy/Modules/RequireEndWithOne.pm (local)
@@ -13,7 +13,7 @@
use Readonly;
use Perl::Critic::Utils qw{ :severities :classification };
-use base 'Perl::Critic::Policy';
+use base 'Perl::Critic::PolicyModules';
our $VERSION = '1.092';
@@ -24,7 +24,6 @@
#-----------------------------------------------------------------------------
-sub supported_parameters { return () }
sub default_severity { return $SEVERITY_HIGH }
sub default_themes { return qw( core bugs pbp ) }
sub applies_to { return 'PPI::Document' }
@@ -33,7 +32,7 @@
sub violates {
my ( $self, $elem, $doc ) = @_;
- return if is_script($doc); #Must be a library or module.
+ return if $self->is_exempt($doc); #Must be a library or module.
# Last statement should be just "1;"
my @significant = grep { _is_code($_) } $doc->schildren();
@@ -80,10 +79,13 @@
C<return "Club sandwich";>. We cannot tolerate such frivolity! OK,
we can, but we don't recommend it since it confuses the newcomers.
+Programs are exempt from this policy.
+
=head1 CONFIGURATION
-This Policy is not configurable except for the standard options.
+You can make this policy apply to scripts by setting C<exempt_scripts = 0>,
+but we don't know why you would.
=head1 AUTHOR
--- lib/Perl/Critic/Policy/Modules/RequireExplicitPackage.pm (revision 60251)
+++ lib/Perl/Critic/Policy/Modules/RequireExplicitPackage.pm (local)
@@ -13,7 +13,7 @@
use Readonly;
use Perl::Critic::Utils qw{ :severities :classification };
-use base 'Perl::Critic::Policy';
+use base 'Perl::Critic::PolicyModules';
our $VERSION = '1.092';
@@ -24,17 +24,6 @@
#-----------------------------------------------------------------------------
-sub supported_parameters {
- return (
- {
- name => 'exempt_scripts',
- description => q{Don't require programs to contain a package statement.},
- default_string => '1',
- behavior => 'boolean',
- },
- );
-}
-
sub default_severity { return $SEVERITY_HIGH }
sub default_themes { return qw( core bugs ) }
sub applies_to { return 'PPI::Document' }
@@ -47,8 +36,7 @@
sub violates {
my ( $self, $elem, $doc ) = @_;
- # You can configure this policy to exclude scripts
- return if $self->{_exempt_scripts} && is_script($doc);
+ return if $self->is_exempt($doc);
# Find the first 'package' statement
my $package_stmnt = $doc->find_first( 'PPI::Statement::Package' );
--- lib/Perl/Critic/Policy/Modules/RequireFilenameMatchesPackage.pm (revision 60251)
+++ lib/Perl/Critic/Policy/Modules/RequireFilenameMatchesPackage.pm (local)
@@ -15,7 +15,7 @@
use File::Spec;
use Perl::Critic::Utils qw{ :severities };
-use base 'Perl::Critic::Policy';
+use base 'Perl::Critic::PolicyModules';
our $VERSION = '1.092';
@@ -26,7 +26,6 @@
#-----------------------------------------------------------------------------
-sub supported_parameters { return () }
sub default_severity { return $SEVERITY_HIGHEST }
sub default_themes { return qw(core bugs) }
sub applies_to { return 'PPI::Document' }
@@ -36,6 +35,8 @@
sub violates {
my ($self, $elem, $doc) = @_;
+ return if $self->is_exempt($doc);
+
my $filename = $doc->filename;
return if !$filename;
@@ -99,10 +100,13 @@
contains it. For example, C<package Foo::Bar;> should be in a file
called C<Bar.pm>.
+This policy does not apply to programs.
+
=head1 CONFIGURATION
-This Policy is not configurable except for the standard options.
+You can make this policy apply to scripts by setting C<exempt_scripts = 0>,
+but we don't know why you would.
=head1 AUTHOR
--- lib/Perl/Critic/PolicyModules.pm (revision 60251)
+++ lib/Perl/Critic/PolicyModules.pm (local)
@@ -0,0 +1,118 @@
+##############################################################################
+# $URL: /local/Perl-Critic/lib/Perl/Critic/Policy/Modules/RequireExplicitPackage.pm $
+# $Date: 2008-09-05T21:34:02.831820Z $
+# $Author: schwern $
+# $Revision: 60251 $
+##############################################################################
+
+package Perl::Critic::PolicyModules;
+
+use 5.006001;
+use strict;
+use warnings;
+use Readonly;
+
+use Perl::Critic::Utils 'is_script';
+
+use base 'Perl::Critic::Policy';
+
+our $VERSION = '1.092';
+
+sub supported_parameters {
+ return (
+ {
+ name => 'exempt_scripts',
+ description => q{Don't require programs to contain a package statement.},
+ default_string => '1',
+ behavior => 'boolean',
+ },
+ );
+}
+
+
+sub is_exempt {
+ my($self, $doc) = @_;
+
+ # You can configure this policy to exclude scripts
+ return 1 if $self->{_exempt_scripts} && is_script($doc);
+}
+
+1;
+
+
+__END__
+
+=head1 NAME
+
+Perl::Critic::PolicyModules - Superclass for policies which only apply to modules
+
+
+=head1 SYNOPSIS
+
+ pacakge Perl::Critic::Policy::Modules::Foo;
+
+ use base qw(Perl::Critic::PolicyModules);
+
+ sub violations {
+ my($self, $elem, $doc) = @_;
+
+ return if $self->is_exempt($doc);
+
+ ...reset of your violation code here...
+ }
+
+
+=head1 DESCRIPTION
+
+Policies which apply only to modules should subclass from this instead
+of Perl::Critic::Policy to provide uniform behavior.
+
+
+=head1 METHODS
+
+=over
+
+=item C<is_exempt>
+
+ $self->is_exempt($document);
+
+Returns true if the C<$document> is exempt from the policy. This
+happens when C<exempt_scripts> is set to true (which it is by default)
+and the C<$document> is a script.
+
+
+=item C<supported_parameters>
+
+The C<exempt_scripts> parameter is supported by default.
+
+If you want to add your own, write something like this:
+
+ sub supported_parameters {
+ my $self = shift;
+
+ return (
+ $self->SUPER::supported_parameters,
+ ...your parameters...
+ );
+ }
+
+=back
+
+=head1 COPYRIGHT
+
+This program is free software; you can redistribute it and/or modify
+it under the same terms as Perl itself. The full text of this license
+can be found in the LICENSE file included with this module.
+
+
+=cut
+
+# Local Variables:
+# mode: cperl
+# cperl-indent-level: 4
+# fill-column: 78
+# indent-tabs-mode: nil
+# c-indentation-style: bsd
+# End:
+# ex: set ts=8 sts=4 sw=4 tw=78 ft=perl expandtab shiftround :
+
--- t/Modules/ProhibitExcessMainComplexity.run (revision 60251)
+++ t/Modules/ProhibitExcessMainComplexity.run (local)
@@ -90,6 +90,47 @@
## failures 0
## cut
+#-----------------------------------------------------------------------------
+
+## name exclude scripts
+## failures 0
+## parms { max_mccabe => 1 }
+## cut
+#!/usr/bin/perl
+
+if ( $foo && $bar || $baz ) {
+ open my $fh, '<', $file or die $!;
+}
+elsif ( $blah >>= some_function() ) {
+ return if $barf;
+}
+else {
+ $results = $condition ? 1 : 0;
+}
+
+croak unless $result;
+
+#-----------------------------------------------------------------------------
+
+## name don't exclude scripts
+## failures 1
+## parms { max_mccabe => 1, exempt_scripts => 0 }
+## cut
+#!/usr/bin/perl
+
+if ( $foo && $bar || $baz ) {
+ open my $fh, '<', $file or die $!;
+}
+elsif ( $blah >>= some_function() ) {
+ return if $barf;
+}
+else {
+ $results = $condition ? 1 : 0;
+}
+
+croak unless $result;
+
+
##############################################################################
# Local Variables:
# mode: cperl
--- t/Modules/RequireEndWithOne.run (revision 60251)
+++ t/Modules/RequireEndWithOne.run (local)
@@ -107,3 +107,23 @@
1;
+#-----------------------------------------------------------------------------
+
+## name programs are exempt
+## failures 0
+## parms
+## cut
+#!/usr/bin/perl
+my $foo = 42;
+
+#-----------------------------------------------------------------------------
+
+## name programs not exempted
+## failures 1
+## parms {exempt_scripts => 0}
+## cut
+#!/usr/bin/perl
+my $foo = 42;
+
+#-----------------------------------------------------------------------------
+
--- t/Modules/RequireExplicitPackage.run (revision 60251)
+++ t/Modules/RequireExplicitPackage.run (local)
@@ -70,9 +70,8 @@
#-----------------------------------------------------------------------------
-## name programs can be exempt
+## name programs are exempt
## failures 0
-## parms {exempt_scripts => 1}
## cut
#!/usr/bin/perl
$foo = $bar;
--- t/Modules/RequireFilenameMatchesPackage.run (revision 60251)
+++ t/Modules/RequireFilenameMatchesPackage.run (local)
@@ -192,6 +192,26 @@
package D'Oh;
1;
+#-----------------------------------------------------------------------------
+
+## name programs are exempt
+## failures 0
+## filename foo.plx
+## cut
+#!/usr/bin/perl
+package Wibble;
+
+#-----------------------------------------------------------------------------
+
+## name programs not exempted
+## failures 1
+## filename foo.plx
+## parms {exempt_scripts => 0}
+## cut
+#!/usr/bin/perl
+package Wibble;
+
+
##############################################################################
# Local Variables:
# mode: cperl