Login
Tweak is_hash_key to return false when the key will be
authorJeffrey Ryan Thalhammer <jeff@imaginative-software.com>
Mon, 21 May 2007 10:19:20 +0000 (10:19 +0000)
committerJeffrey Ryan Thalhammer <jeff@imaginative-software.com>
Mon, 21 May 2007 10:19:20 +0000 (10:19 +0000)
interpreted as a function call rather than as a literal
string.

lib/Perl/Critic/Utils.pm
t/05_utils.t

index abbef08..a087690 100644 (file)
@@ -587,6 +587,9 @@ sub is_hash_key {
     my $elem = shift;
     return if !$elem;
 
+    #If followed by an argument list, then its a function call, not a literal
+    return if _is_followed_by_parens($elem);
+
     #Check curly-brace style: $hash{foo} = bar;
     my $parent = $elem->parent();
     return if !$parent;
@@ -605,6 +608,16 @@ sub is_hash_key {
 
 #-----------------------------------------------------------------------------
 
+sub _is_followed_by_parens {
+    my $elem = shift;
+    return if !$elem;
+
+    my $sibling = $elem->snext_sibling() || return;
+    return $sibling->isa('PPI::Structure::List');
+}
+
+#-----------------------------------------------------------------------------
+
 sub is_included_module_name {
     my $elem  = shift;
     return if !$elem;
@@ -685,7 +698,7 @@ sub is_subroutine_name {
 
 sub is_function_call {
     my $elem  = shift;
-    return if ! $elem;
+    return if !$elem;
 
     return if is_hash_key($elem);
     return if is_method_call($elem);
@@ -1137,8 +1150,8 @@ can't be determined (which is usually because it is not an operator).
 
 =item C<is_hash_key( $element )>
 
-Given a L<PPI::Element>, returns true if the element is a hash key.  PPI
-doesn't distinguish between regular barewords (like keywords or subroutine
+Given a L<PPI::Element>, returns true if the element is a literal hash key.
+PPI doesn't distinguish between regular barewords (like keywords or subroutine
 calls) and barewords in hash subscripts (which are considered literal).  So
 this subroutine is useful if your Policy is searching for L<PPI::Token::Word>
 elements and you want to filter out the hash subscript variety.  In both of
@@ -1147,6 +1160,12 @@ the following examples, "foo" is considered a hash key:
   $hash1{foo} = 1;
   %hash2 = (foo => 1);
 
+But if the bareword is followed by an argument list, then perl treats it as a
+function call.  So in these examples, "foo" is B<not> considered a hash key:
+
+  $hash1{ foo() } = 1;
+  &hash2 = (foo() => 1);
+
 =item C<is_included_module_name( $element )>
 
 Given a L<PPI::Token::Word>, returns true if the element is the name of a
index 2698086..09f6923 100644 (file)
@@ -10,7 +10,7 @@
 use strict;
 use warnings;
 use PPI::Document;
-use Test::More tests => 89;
+use Test::More tests => 91;
 
 #-----------------------------------------------------------------------------
 
@@ -76,7 +76,7 @@ sub make_doc { my $code = shift; return PPI::Document->new( ref $code ? $code :
 #  is_hash_key tests
 
 {
-   my $code = 'sub foo { return $hash1{bar}, $hash2->{baz}; }';
+   my $code = 'sub foo { return $h1{bar}, $h2->{baz}, $h3->{ nuts() } }';
    my $doc = PPI::Document->new(\$code);
    my @words = @{$doc->find('PPI::Token::Word')};
    my @expect = (
@@ -85,6 +85,7 @@ sub make_doc { my $code = shift; return PPI::Document->new( ref $code ? $code :
       ['return', undef],
       ['bar', 1],
       ['baz', 1],
+      ['nuts', undef],
    );
    is(scalar @words, scalar @expect, 'is_hash_key count');
    for my $i (0 .. $#expect)