Login
Subroutines::ProhibitBuiltinHomonyms now covers Perl keywords
authorJeffrey Ryan Thalhammer <jeff@imaginative-software.com>
Mon, 14 Jul 2008 04:55:38 +0000 (04:55 +0000)
committerJeffrey Ryan Thalhammer <jeff@imaginative-software.com>
Mon, 14 Jul 2008 04:55:38 +0000 (04:55 +0000)
as well.  Inspired by RT #37632

Changes
lib/Perl/Critic/Policy/Subroutines/ProhibitBuiltinHomonyms.pm
t/Subroutines/ProhibitBuiltinHomonyms.run

diff --git a/Changes b/Changes
index a6077ac..d89be45 100644 (file)
--- a/Changes
+++ b/Changes
@@ -3,10 +3,13 @@
     Minor Enhancements:
     * -s is now a synonym for --single-policy.
 
-    Policy Changes:
+    Policy Changes: 
     * ValuesAndExpressions::ProhibitInterpolationOfLiterals now takes a
-      allow_if_string_contains_single_quote option.  Contributed by Ed Avis
-      <ed@membled.com>.  RT #36125.
+      allow_if_string_contains_single_quote option.  Contributed by Ed
+      Avis <ed@membled.com>.  RT #36125.
+    * Subroutines::ProhibitBuiltinHomonyms now also prohibits subroutines 
+      with the same name as a Perl keyword (e.g. if, foreach, while).  
+      Inspired by RT #37632.
 
     Bug fixes:
     * BuiltinFunctions::ProhibitSleepViaSelect would complain if there were
index 59184ee..15727de 100644 (file)
@@ -12,7 +12,9 @@ use strict;
 use warnings;
 use Readonly;
 
-use Perl::Critic::Utils qw{ :severities :data_conversion :classification };
+use Perl::Critic::Utils qw{ :severities :data_conversion
+                            :classification :characters };
+
 use base 'Perl::Critic::Policy';
 
 our $VERSION = '1.088';
@@ -21,7 +23,7 @@ our $VERSION = '1.088';
 
 Readonly::Array my @ALLOW => qw( import AUTOLOAD DESTROY );
 Readonly::Hash my %ALLOW => hashify( @ALLOW );
-Readonly::Scalar my $DESC  => q{Subroutine name is a homonym for builtin function};
+Readonly::Scalar my $DESC  => q{Subroutine name is a homonym for builtin %s};
 Readonly::Scalar my $EXPL  => [177];
 
 #-----------------------------------------------------------------------------
@@ -37,10 +39,20 @@ sub violates {
     my ( $self, $elem, undef ) = @_;
     return if $elem->isa('PPI::Statement::Scheduled'); #e.g. BEGIN, INIT, END
     return if exists $ALLOW{ $elem->name() };
+
+    my $homonym_type = $EMPTY;
     if ( is_perl_builtin( $elem ) ) {
-        return $self->violation( $DESC, $EXPL, $elem );
+        $homonym_type = 'function';
+    }
+    elsif ( is_perl_bareword( $elem ) ) {
+        $homonym_type = 'keyword';
     }
-    return;    #ok!
+    else {
+        return;    #ok!
+    }
+
+    my $desc = sprintf $DESC, $homonym_type;
+    return $self->violation($desc, $EXPL, $elem);
 }
 
 1;
@@ -51,6 +63,8 @@ __END__
 
 =pod
 
+=for stopwords perlfunc perlsyn
+
 =head1 NAME
 
 Perl::Critic::Policy::Subroutines::ProhibitBuiltinHomonyms - Don't declare your own C<open> function.
@@ -63,13 +77,16 @@ distribution.
 
 =head1 DESCRIPTION
 
-Common sense dictates that you shouldn't declare subroutines with the
-same name as one of Perl's built-in functions. See C<`perldoc
-perlfunc`> for a list of built-ins.
+Common sense dictates that you shouldn't declare subroutines with the same
+name as one of Perl's built-in functions or keywords.  See
+L<perlfunc|perlfunc> for a list of built-in functions; see L<perlsyn|perlsyn>
+for keywords.
 
-  sub open {}  #not ok
-  sub exit {}  #not ok
-  sub print {} #not ok
+  sub open {}    #not ok
+  sub exit {}    #not ok
+  sub print {}   #not ok
+  sub foreach {} #not ok
+  sub if {}      #not ok
 
   #You get the idea...
 
index da4f0da..e00fa2f 100644 (file)
@@ -1,10 +1,13 @@
 ## name Basic failures
-## failures 3
+## failures 7
 ## cut
 sub open {}
 sub map {}
 sub eval {}
-
+sub if {}
+sub sub {}
+sub foreach {}
+sub while {}
 #-----------------------------------------------------------------------------
 
 ## name Basic passing