Login
RT #79289: False Postive in Perl::Critic::Utils::is_in_void_context() master
authorTom Wyant <harryfmudd@comcast.net>
Sun, 2 Sep 2012 21:17:16 +0000 (21:17 +0000)
committerTom Wyant <harryfmudd@comcast.net>
Sun, 2 Sep 2012 21:17:16 +0000 (21:17 +0000)
commitb946a486708dc31a0be0d4c488beb180974c13a4
treefda3e15729792293203f5c768d1fb18ba156f73a
parentb27093ede5b9b427ed8f1513f7ad6f74c3979f82
RT #79289: False Postive in Perl::Critic::Utils::is_in_void_context()

The ticket is really about BuiltinFunctions::ProhibitVoidGrep, but the
requester's diagnosis of the point of failure and the nature of the
correction was correct. This revision applies the requester's
correction, and adds tests to ProhibitVoidGrep.run and
ProhibitVoidMap.run, since the latter policy had the same problem.

The thing is, it appears to me that most uses of grep and map in slice
subscripts were passing for the wrong reason. Both policies, after
ensuring they have the PPI::Token::Word that they want, call
is_function_call() on the element in question, and accept the element if
this returns false. The first thing that is_function_call() does is to
call is_hash_key(), returning false if is_hash_key() returns true.

Well, is_hash_key() is documented as detecting barewords used as hash
keys. But it returns true when handed the 'grep' token in the following:
    @hash{ grep { /foo/ } keys %hash }
    @hash{ grep /foo/, keys %hash }
This causes the policy to declare a true negative in the above cases,
but to declare it for the wrong reason. The requester, on the other
hand, was doing
    @hash{ grep ( /foo/, keys %hash ) )
which is a violation of a couple other policies, but should not be a
violation of this one. But the parens after the 'grep' were causing
is_hash_key() to fail; consequently is_function_call() succeeded, so the
failure of is_in_void_context() to return false when called on a token
in a subscript was exposed.
Changes
MANIFEST
lib/Perl/Critic/Utils.pm
t/BuiltinFunctions/ProhibitVoidGrep.run
t/BuiltinFunctions/ProhibitVoidMap.run