9 years agoRT #79289: False Postive in Perl::Critic::Utils::is_in_void_context() master
Tom Wyant [Sun, 2 Sep 2012 21:17:16 +0000 (21:17 +0000)] 
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.

9 years agoRT #79138: RequireArgUnpacking confused by @_ in finally{}
Tom Wyant [Fri, 24 Aug 2012 03:22:08 +0000 (03:22 +0000)] 
RT #79138: RequireArgUnpacking confused by @_ in finally{}

This policy contains some logic for testing the size of the argument
list -- basically allowing the combinations '$something == @_', '@_ ==
$something', '$something != @_, and '@_ != $something'.

My read on the ticket is that finally{} is not involved, but that the
requestor is asking for the size test logic to be more comprehensive.

This I have done, adding the other comparison operators, plus boolean
operators, plus logic to handle suffix conditionals.

The .run file has an added test, though the test does not cover the
Cartesian product of all the possibilities.

9 years agoThe Perl::Critic::Document method
Tom Wyant [Tue, 7 Aug 2012 22:36:00 +0000 (22:36 +0000)] 
The Perl::Critic::Document method
element_is_in_lexical_scope_after_statement_containing() goes into an
infinite loop if the parent of the second element does not define a

The only core policies that use this (so far!) are RegularExpressions
policies that are trying to determine if there are any default
modifiers, so this has not bitten us yet.

I discovered this when trying to revive the
Variables::ProhibitUnusedVariables work that has been languishing in
http://perlcritic.tigris.org/svn/perlcritic/branches/rt64929. The
original work used a Perl::Critic::Scope object to represent the scope,
but this turned out to be overkill, so it was not used in the regexp
default modifier work. The discovery was made because,
    ( use re '/smx'; )
is invalid Perl,
    open ( my $handle, $mode, $name );
is perfectly valid -- and more to the point it appears in
ProhibitUnusedVariables.run in the branch.

I have NOT put this into the Changes file, because I could not figure
out what to write.

9 years agoUpdated changelog w/r/t RT #75300.
Jeffrey Ryan Thalhammer [Thu, 26 Jul 2012 19:08:49 +0000 (19:08 +0000)] 
Updated changelog w/r/t RT #75300.

9 years agoApplied patch from RT #75300.
Jeffrey Ryan Thalhammer [Thu, 26 Jul 2012 19:03:43 +0000 (19:03 +0000)] 
Applied patch from RT #75300.

This is the more modern way to use Exporter.

9 years agoCorrect release date (I got distracted). v1.118
Jeffrey Ryan Thalhammer [Wed, 11 Jul 2012 06:48:36 +0000 (06:48 +0000)] 
Correct release date (I got distracted).

9 years agoRegnerated MANIFEST.
Jeffrey Ryan Thalhammer [Wed, 11 Jul 2012 06:47:25 +0000 (06:47 +0000)] 
Regnerated MANIFEST.

9 years agoNeed to load extra RequireRcsKeywords policy for self-compliance tests.
Jeffrey Ryan Thalhammer [Wed, 11 Jul 2012 06:29:58 +0000 (06:29 +0000)] 
Need to load extra RequireRcsKeywords policy for self-compliance tests.

9 years agoNote to self about the purpose of the 41_criticize-policies.t test.
Jeffrey Ryan Thalhammer [Wed, 11 Jul 2012 06:17:12 +0000 (06:17 +0000)] 
Note to self about the purpose of the 41_criticize-policies.t test.

9 years agoEdit comments and white space. Also workaround RT #78182.
Jeffrey Ryan Thalhammer [Wed, 11 Jul 2012 05:41:50 +0000 (05:41 +0000)] 
Edit comments and white space.  Also workaround RT #78182.

Perl::Tidy does nefarious things with STDERR.

9 years agoAdjust expected stats, now that RequireVcsKeywords is gone.
Jeffrey Ryan Thalhammer [Wed, 11 Jul 2012 04:33:28 +0000 (04:33 +0000)] 
Adjust expected stats, now that RequireVcsKeywords is gone.

9 years agoARRGH! RestrictLongStrings is not a core Policy, so I can't
Jeffrey Ryan Thalhammer [Tue, 3 Jul 2012 20:16:50 +0000 (20:16 +0000)] 
ARRGH!  RestrictLongStrings is not a core Policy, so I can't
use it for testing.  So I switched to ProhibitQuotedWordLists.
I just needed any core Policy that accepts some config params.

9 years agoA little commentary to remind me how the -test option works.
Jeffrey Ryan Thalhammer [Tue, 3 Jul 2012 19:26:05 +0000 (19:26 +0000)] 
A little commentary to remind me how the -test option works.

9 years agoPrevent warning about RequireRcsKeyword policy being missing.
Jeffrey Ryan Thalhammer [Tue, 3 Jul 2012 19:21:34 +0000 (19:21 +0000)] 
Prevent warning about RequireRcsKeyword policy being missing.

9 years agoFix some failing tests, following removal of RequireRcsKeywords.
Jeffrey Ryan Thalhammer [Tue, 3 Jul 2012 19:12:42 +0000 (19:12 +0000)] 
Fix some failing tests, following removal of RequireRcsKeywords.

9 years agoMoving RequireRcsKeywords into the Perl-Critic-More distribution.
Jeffrey Ryan Thalhammer [Tue, 3 Jul 2012 18:20:50 +0000 (18:20 +0000)] 
Moving RequireRcsKeywords into the Perl-Critic-More distribution.

See RT #69546 for the lengthy discussion behind this decision.

9 years agoUnmark test as TODO.
Jeffrey Ryan Thalhammer [Tue, 3 Jul 2012 06:28:32 +0000 (06:28 +0000)] 
Unmark test as TODO.

PPI-1.215 correct parses that as a hash constructor, rather than a block.

But there are some cases where it still fails, so we can't close the bug entirely.

9 years agoActually, it seems like we don't need the newer Perl::Tidy.
Jeffrey Ryan Thalhammer [Tue, 3 Jul 2012 05:28:04 +0000 (05:28 +0000)] 
Actually, it seems like we don't need the newer Perl::Tidy.
It still works with older versions.

9 years agoRT #77977 requires us to upgrade to Perl::Tidy version 20120619.
Jeffrey Ryan Thalhammer [Tue, 3 Jul 2012 05:23:29 +0000 (05:23 +0000)] 
RT #77977 requires us to upgrade to Perl::Tidy version 20120619.

9 years agoNote VERSION and release date in change log.
Jeffrey Ryan Thalhammer [Tue, 3 Jul 2012 05:22:36 +0000 (05:22 +0000)] 
Note VERSION and release date in change log.

9 years agoBump version number
Jeffrey Ryan Thalhammer [Tue, 3 Jul 2012 05:16:39 +0000 (05:16 +0000)] 
Bump version number

9 years agoRT #77977: Perl::Tidy 20120619 breaks CodeLayout::RequireTidyCode
Tom Wyant [Sat, 23 Jun 2012 17:33:39 +0000 (17:33 +0000)] 
RT #77977: Perl::Tidy 20120619 breaks CodeLayout::RequireTidyCode

Perl::Tidy version 20120619 breaks CodeLayout::RequireTidyCode. There
appear actually to be three breakages:

1) The stderr argument to perltidy() no longer takes a scalar reference.
This is a documented change. Fortunately Perl::Critic already has
IO::String as a dependency, so the solution is to instantiate an
IO::String object from the $stderr variable, and pass that.

2) Perl::Tidy now chomps the perltidyrc string.  This fails if it is
passed a reference to a Readonly::Scalar, and throws an exception. The
solution is to assign $EMPTY to a scalar variable, and pass that.

3) If you pass a scalar reference as the source argument to perltidy(),
it is modified after the call -- the trailing newline is removed. The
only solution I can come up with is more ad-hocery.

9 years agoAdd ActivePerl to the stop words list.
Tom Wyant [Sat, 23 Jun 2012 16:58:50 +0000 (16:58 +0000)] 
Add ActivePerl to the stop words list.

9 years agoperlcritic-gui now ships with ActivePerl, not PDK.
Jeffrey Ryan Thalhammer [Fri, 8 Jun 2012 21:04:49 +0000 (21:04 +0000)] 
perlcritic-gui now ships with ActivePerl, not PDK.

Thanks to Thomas J. Dillman for letting me know.

9 years agoRT #77510: Left curlys as literals in regexps are deprecated.
Tom Wyant [Mon, 28 May 2012 23:53:27 +0000 (23:53 +0000)] 
RT #77510: Left curlys as literals in regexps are deprecated.

The use of left curly brackets ("{") as literals in regular expressions
is deprecated, and as of Perl 5.17.0 produces a compiler warning. Given
the code policies required for self-compliance, the solution is to make
them into a singleton character class (i.e. "[{]").

A change in the version of PPIx::Regexp was also done, because that also
turned out to have literal curlies (blush!).

9 years agoRT #75397: ProhibitBuiltinHomonyms should include the sub name in the
Tom Wyant [Thu, 1 Mar 2012 00:12:49 +0000 (00:12 +0000)] 
RT #75397: ProhibitBuiltinHomonyms should include the sub name in the
error message

I took a swing at this. Just a matter of adding $elem->name() on the end
of the message.

9 years agoRT #74429: 'Negative array index should be used' a little misleading
Tom Wyant [Tue, 28 Feb 2012 02:53:09 +0000 (02:53 +0000)] 
RT #74429: 'Negative array index should be used' a little misleading

The problem was that the documentation of
Variables::RequireNegativeIndices did not mention at all the fact
the semantics of $#foo-$n change when $n becomes greater than $#foo,
whereas the semantics of -$n do not. The author of the ticket seems to
have been burned when trying to straighten out non-compliances.

The long explanation of a non-compliance has been revised to feature the
semantics change prominently.

The short explanation has not been changed, because I confess my
inability to do significantly better without taking significantly more

9 years agoMANIFEST is now stashed in the repository.
Jeffrey Ryan Thalhammer [Sun, 26 Feb 2012 06:25:46 +0000 (06:25 +0000)] 
MANIFEST is now stashed in the repository.

Even though it is a generated file,  M::B and EU::MM
both expect it to be there before making a build.  We
believe one should be able to checkout from the repository
and run the usual build/test/install cycle without
any surprises.

This resolves RT #43908.

9 years agoRT #74647: False positive in TestingAndDebugging::ProhibitNoWarnings
Tom Wyant [Fri, 3 Feb 2012 20:11:27 +0000 (20:11 +0000)] 
RT #74647: False positive in TestingAndDebugging::ProhibitNoWarnings
(bad parsing)

The problem here was that the original code just stringified the
'no' statement, used a regular expression to extract all lowercase
strings, and then grep'ed out 'no', 'warnings', and 'qw'. Unfortunately,
'qw' is an actual warning category (believe it or not!), so the original
code saw

 no warnings 'qw';

as an unqualified 'no warnings'.

The patch recurses into the statement being analyzed, and finds all:
* PPI::Token::Word (because of 'foo => "bar"'),
* PPI::Token::Quote (because of "'foo'"), and
* PPI::Token::QuoteLike::Words (obviously).
The first two words (which are 'no' and 'warnings') are discarded, and
the rest are subjected to analysis. Note that, though the original code
rejects everything that is not entirely lower case alphabetic, the patch
accepts everything. My reason is that the difference was a "don't care",
becaue unknown warning categories don't compile, and thus are not
Perl::Critic's problem.

9 years agoFix typo (thanks, petdance)
Jeffrey Ryan Thalhammer [Wed, 4 Jan 2012 18:24:55 +0000 (18:24 +0000)] 
Fix typo (thanks, petdance)

9 years agoAdd OpenSUSE to links page.
Elliot Shank [Sun, 25 Dec 2011 17:42:19 +0000 (17:42 +0000)] 
Add OpenSUSE to links page.

9 years agoPrepare change log for release. v1.117
Jeffrey Ryan Thalhammer [Wed, 21 Dec 2011 22:41:54 +0000 (22:41 +0000)] 
Prepare change log for release.

9 years agoBumped version number
Jeffrey Ryan Thalhammer [Wed, 21 Dec 2011 22:40:10 +0000 (22:40 +0000)] 
Bumped version number

9 years agoTurn on svn:keywords
Jeffrey Ryan Thalhammer [Wed, 21 Dec 2011 22:30:04 +0000 (22:30 +0000)] 
Turn on svn:keywords

9 years agoFix self-compliance problems in new Policy.
Jeffrey Ryan Thalhammer [Wed, 21 Dec 2011 22:29:00 +0000 (22:29 +0000)] 
Fix self-compliance problems in new Policy.

9 years agoSince we imported the firstval function, we might as well use the imported symbol.
Jeffrey Ryan Thalhammer [Wed, 21 Dec 2011 22:27:09 +0000 (22:27 +0000)] 
Since we imported the firstval function, we might as well use the imported symbol.

9 years agoNew Policy: ProhibitAugmentedAssignmentInDeclaration
Jeffrey Ryan Thalhammer [Wed, 21 Dec 2011 22:22:58 +0000 (22:22 +0000)] 
New Policy: ProhibitAugmentedAssignmentInDeclaration

This was created from a patch provided by Mike O'Regan.
See http://perlcritic.tigris.org/ds/viewMessage.do?dsForumId=4230&dsMessageId=2735595

9 years agoRT #72660: Exclude pragmas from Modules::RequireExplicitPackage
Tom Wyant [Sat, 17 Dec 2011 23:24:12 +0000 (23:24 +0000)] 
RT #72660: Exclude pragmas from Modules::RequireExplicitPackage

Remove the allow_perl_version configuration option that was added in
commit 4100, there having been no dissent that I am aware of since I
proposed this on the RT ticket a week ago.

9 years agoAdd a separate configuration item, 'allow_perl_version', to
Tom Wyant [Fri, 9 Dec 2011 00:14:08 +0000 (00:14 +0000)] 
Add a separate configuration item, 'allow_perl_version', to
Modules::RequireExplicitPackage to allow things like 'use 5.010;', since
this is the one use of 'use' not covered in commits 4098 and 4099 (RT

9 years agoCorrect the code commited in revision 4098 (allowing loads of specific
Tom Wyant [Thu, 8 Dec 2011 23:56:11 +0000 (23:56 +0000)] 
Correct the code commited in revision 4098 (allowing loads of specific
modules before the package statement in
Modules::RequireExplicitPackage). The original code tested for a defined
return from PPI::Statement::Include->module(), but that method returns
'' for 'use 5.010;'.

9 years agoRT #72660: Exclude pragmas from Modules::RequireExplicitPackage
Tom Wyant [Mon, 5 Dec 2011 20:03:31 +0000 (20:03 +0000)] 
RT #72660: Exclude pragmas from Modules::RequireExplicitPackage

This commit does not do what the original request asked for, but does
allow the user to specify modules which are allowed to be imported
before the 'package' statement. The configuration option is
'allow_import_of' (for the moment -- I'm sure someone can think of a
better name), and by default it allows nothing.

9 years agoRT #72910: False-positive of Variables::ProhibitPunctuationVars when {}
Tom Wyant [Sat, 3 Dec 2011 20:48:10 +0000 (20:48 +0000)] 
RT #72910: False-positive of Variables::ProhibitPunctuationVars when {}
used in interpolated string

Since the original interpolation code in this policy recognized
punctuation variables using a big honkin' regexp, the fix was by
computing and adding to the regexp the bracketed forms of the forbidden
variables. The brackets (if any) are stripped before comparison to the
{_allowed} configuration.

It also seemed a good idea to add the name of the variable to the error
description, since the position of the violation may not tell you enough
when the element containing the violation is a string.

9 years agoRT #70901: Policies missing operators
Tom Wyant [Sun, 27 Nov 2011 00:35:48 +0000 (00:35 +0000)] 
RT #70901: Policies missing operators

The issue was missing augmented assignment operators. Patches were
included (and accepted). The only policy patched was
ControlStructures::ProhibitMutatingListFunctions, but '//=' was also
missing from Perl::Critic::Utils::precedence_of().

I'm not sure how serious this was, since PPI parses most of these as two
operators (that is, '+=' gets parsed as '+', '='). But '//=' appears to
be an exception, and is therefore missed both in the policy and anything
that uses precedence_of().

9 years agoRT #72151: 5.14 re pragma and
Tom Wyant [Sat, 26 Nov 2011 23:54:50 +0000 (23:54 +0000)] 
RT #72151: 5.14 re pragma and
::RequireExtendedFormatting, ::RequireLineBoundaryMatching


The requester asked that Perl::Critic honor the Perl 5.14 'use re
/modifiers' pragma. The implementation is divided between Perl::Critic
(which figures out which default modifiers are in-scope) and
PPIx::Regexp (which figures out what modifiers are actually in effect
based on the modifiers actually asserted, and the in-scope default
modifiers if any).

The Perl::Critic portion involved:
* Adding method element_is_in_lexical_scope_after_statement_containing()
  to Perl::Critic::Document. There is no current reason for this code to
  be here rather than in one of the utility packages, but this way
  caching of scope objects can be done without changing the interface.
* Modified Perl::Critic::Document method ppix_regexp_from_element() to
  make use of the above to find all the default modifier pragmas
  in-scope and pass them to PPIx::Regexp->new(). The PPIx::Regexp
  objects are already cached, the overhead of finding the pragmas should
  only be incurred once for a given regexp.
* Converting the relevant policies to use the new PPIx::Regexp
  modifier_asserted() method (which takes the defaults into account)
  rather than using the PPIx::Regexp modifiers() method (to return the
  object representing the modifiers present on the regexp) or the
  PPI::Token::Regexp get_modifiers method (ditto). This included:
  - ControlStructures::ProhibitMutatingListFunctions
  - RegularExpressions::RequireExtendedFormatting *
  - RegularExpressions::ProhibitUnusedCapture
  - RegularExpressions::ProhinitFixedStringMatches
  - RegularExpressions::RequireDotMatchAnything *
  = RegularExpressions::RequireLineBoundaryMatching *
  The starred policies had their t/*.run files updated as well.
* Boosting the version of PPIx::Regexp required to 0.022.

The so-called 'extra credit' portion of the ticket (handling
Regexp::DefaultFlags as well) is not in this commit. Or anywhere else at
the moment.

9 years agoRT #71093: CodeLayout::ProhibitHardTabs comes from PBP, but is not used
Tom Wyant [Thu, 17 Nov 2011 18:14:13 +0000 (18:14 +0000)] 
RT #71093: CodeLayout::ProhibitHardTabs comes from PBP, but is not used
when theme = pbp.

Added 'pbp' to default_themes().

9 years agoRT #72086 - ProhibitUnusedCapture false positive with /e and parens
Tom Wyant [Tue, 1 Nov 2011 15:48:42 +0000 (15:48 +0000)] 
RT #72086 - ProhibitUnusedCapture false positive with /e and parens

The problem was that the policy was simply looking at the top level of
the replacement expression. Fortunately this was just a matter of
calling the right subroutine. This should cover (?{}) and (??{}) as
well, though I was not ambitious enough to add them to the tests.

10 years agoRT #69867: RegularExpressions::ProhibitUnusedCapture false positive when
Tom Wyant [Fri, 12 Aug 2011 00:33:48 +0000 (00:33 +0000)] 
RT #69867: RegularExpressions::ProhibitUnusedCapture false positive when
capture used in an else branch

The problem was that the complainant used '!~' to bind the regexp to its
operand. The policy checks all parts of an 'if', and fell over a regular
expression in the 'if' block.

The fix was to skip the first block of an 'if' or 'elsif' if the regular
expression under analysis used '!~' to bind to the operand. Yes, this is
bogus code, and there probably should be a policy against it. But it is
not in violation of _this_ policy.

Strictly speaking the code should also skip 'if' or 'elsif' blocks other
than the first if the regexp is not bound with a '!~', but that is an
optimization (and possible false negative) that I did not tackle at this

10 years agoThe Perl::Critic::Violation source() method (and therefore the %r
Tom Wyant [Mon, 25 Jul 2011 00:16:10 +0000 (00:16 +0000)] 
The Perl::Critic::Violation source() method (and therefore the %r
format) now handles multi-line statements by returning the line of the
statement that contains the violation. Or at least, tries very hard to.
The previous version returned the first line, whether or not it
contained the violation.

10 years agoI switched aspell versions, and evidently got a less fulsome dictionary.
Tom Wyant [Sun, 24 Jul 2011 23:56:40 +0000 (23:56 +0000)] 
I switched aspell versions, and evidently got a less fulsome dictionary.
So I added the missing words to xt/author/40_stop_words.

10 years agoRT #69489 - ErrorHandling::RequireCheckingReturnValueOfEval false
Tom Wyant [Fri, 15 Jul 2011 21:09:27 +0000 (21:09 +0000)] 
RT #69489 - ErrorHandling::RequireCheckingReturnValueOfEval false

The issue is that the subject policy does not accept grep { eval ... }.
The failing code provided in the ticket was
    grep { ! eval "require $_" } @packages;
After a bit of thought (enough, I hope!) I decided that this was the
moral equivalent of
    foreach ( @packages ) {
        eval "require $_" and next;
which is acceptable to this policy. So ...

This modification looks for both
    grep { eval ... } ...
    grep eval ... , ...
though it does not look quite as hard for the latter.

10 years agoRT #69322 - RegularExpressions::ProhibitEnumeratedClasses false positive
Tom Wyant [Thu, 7 Jul 2011 00:15:39 +0000 (00:15 +0000)] 
RT #69322 - RegularExpressions::ProhibitEnumeratedClasses false positive
on [A-Za-z_]

Added '0-9' to the recognition code.

10 years agoRT #68898: POD markup bug in Perl::Critic::PPI::Utils
Tom Wyant [Sat, 18 Jun 2011 19:38:59 +0000 (19:38 +0000)] 
RT #68898: POD markup bug in Perl::Critic::PPI::Utils

s/ C (?= <PPI::Element ) /L/smx at or about line 364.

10 years agoTeach ControlStructures::ProhibitMutatingListFunctions that tr///r and
Tom Wyant [Mon, 30 May 2011 16:21:47 +0000 (16:21 +0000)] 
Teach ControlStructures::ProhibitMutatingListFunctions that tr///r and
y///r do not modify their operands. The s///r case was taken care of

10 years agoMake Subroutines::ProhibitManyArgs aware of '+' as a prototype
Tom Wyant [Sun, 29 May 2011 17:46:11 +0000 (17:46 +0000)] 
Make Subroutines::ProhibitManyArgs aware of '+' as a prototype
character. This was introduced with Perl 5.14.

10 years ago#68498 for Perl-Critic: Missing 'use version' prevents Red Hat install
Tom Wyant [Fri, 27 May 2011 18:41:02 +0000 (18:41 +0000)] 
#68498 for Perl-Critic: Missing 'use version' prevents Red Hat install

Added the 'use version'. Obviously any module that calls version->new()
needs 'version' loaded. It was just the luck (or unluck) of load order
that made this work in so many places.

10 years agoWe need to use Test::POD version 1.41 or newer, because older
Jeffrey Ryan Thalhammer [Thu, 19 May 2011 03:15:29 +0000 (03:15 +0000)] 
We need to use Test::POD version 1.41 or newer, because older
versions don't permit L<text|scheme:...> links.  This actually
was verbotten in older perls.  But since about 5.11, it is

I suppose we could write a Policy that would forbid the
L<text|scheme:...> style of links if the target perl is not
5.11 or higher.  But this feels like re-inventing the wheel,
since Test::POD is already out there.

10 years agoPodSpelling problem in BuiltinFunctions::ProhibitLvalueSubstr. v1.116
Elliot Shank [Sun, 15 May 2011 21:43:26 +0000 (21:43 +0000)] 
PodSpelling problem in BuiltinFunctions::ProhibitLvalueSubstr.

10 years agoBump P::C to v1.116.
Elliot Shank [Sun, 15 May 2011 21:34:46 +0000 (21:34 +0000)] 
Bump P::C to v1.116.

10 years agoAlphabetize Policy entries in Changes.
Elliot Shank [Sun, 15 May 2011 21:28:51 +0000 (21:28 +0000)] 
Alphabetize Policy entries in Changes.

10 years agoRT #67760: RequireLexicalLoopIterators vs perl version
Tom Wyant [Thu, 28 Apr 2011 00:45:39 +0000 (00:45 +0000)] 
RT #67760: RequireLexicalLoopIterators vs perl version

Tweak docs and test, since the requester points out that 'use 5.003;'
does not parse.

This is on top of SVN revision 4074.

10 years agoRT #59112: ProhibitLvalueSubstr vs highest_explicit_perl_version
Tom Wyant [Wed, 27 Apr 2011 14:59:08 +0000 (14:59 +0000)] 
RT #59112: ProhibitLvalueSubstr vs highest_explicit_perl_version

The implementation adds a prepare_to_scan_document() which returns false
if the Perl version was specified as less than 5.005. The ticket talks
about an option to control whether documents are skipped based on Perl
version, but the two other policies (prior to today's work) which are
sensitive to Perl version do this.

Test provided and Changes updated.

10 years agoRT #67760: RequireLexicalLoopIterators vs perl version
Tom Wyant [Wed, 27 Apr 2011 13:45:33 +0000 (13:45 +0000)] 
RT #67760: RequireLexicalLoopIterators vs perl version

The idea of the patch was accepted, but the implementation simply has
prepare_to_scan_document() return false if the highest explicit Perl
version is less than 5.004.

The associated documentation tweaks were also accepted in spirit but

A test was provided, and yes the Changes file was updated.

10 years agoBump required version of PPIx::Regexp to 0.019, since that is the
Tom Wyant [Wed, 13 Apr 2011 00:31:48 +0000 (00:31 +0000)] 
Bump required version of PPIx::Regexp to 0.019, since that is the
version that understands the most about things like ${foo}. Version
0.020 is out, but that was a tweak in the minimum Perl version
functionality, which Perl::Critic does not use.

10 years agoUpdate instructions on generating docs for perlcritic.com site.
Jeffrey Ryan Thalhammer [Wed, 13 Apr 2011 00:11:26 +0000 (00:11 +0000)] 
Update instructions on generating docs for perlcritic.com site.
The old tools (Pod::Tree::HTML) generated stuff with lots of
broken links.  Pod::Simple::HTMLBatch seems to work better.

10 years agoMake note of command used to generate POD for perlcritic.com site.
Jeffrey Ryan Thalhammer [Tue, 12 Apr 2011 23:43:23 +0000 (23:43 +0000)] 
Make note of command used to generate POD for perlcritic.com site.

10 years agoRT #67231: Perlcritic crashes when evaluating a file
Tom Wyant [Tue, 5 Apr 2011 00:22:51 +0000 (00:22 +0000)] 
RT #67231: Perlcritic crashes when evaluating a file

Sorry, guys. This is another one of mine.

The problem was introduced in the code that implemented RT #59268. The
problem is that the enhancement code assumes that if there is POD at
all, there will be a '=head1' to report the problem against. Which there
isn't if the only reason this policy is looking at the code is because
someone put in a =pod/=cut pair to comment out some code.

In an attempt to preserve the spirit of the enhancement, the failure is
reported against the first POD found if no =head1 was found.

The commit includes a revised t/Documentation/RequirePodSections.run,
and an entry to Changes.

10 years agoRT # 67159: perl 5.12 package NAME VERSION not understood
Tom Wyant [Fri, 1 Apr 2011 22:24:20 +0000 (22:24 +0000)] 
RT # 67159: perl 5.12 package NAME VERSION not understood

P::C::P::Modules::RequireVersionVar now recognizes a version passed as
the second argument of a package statement.

P::C::P::ValuesAndExpressions::ProhibitMagicNumbers now allows any
number when passed as the second argument of a package statement. This
is consistent (I believe) with its handling of numbers assigned to

Tests for this have been added to the relevant .run files, and (glory
be!) the Changes file has been updated.

10 years agoAdd META.json to Perl-Critic/Build.PL. v1.115
Elliot Shank [Fri, 1 Apr 2011 00:00:29 +0000 (00:00 +0000)] 
Add META.json to Perl-Critic/Build.PL.

10 years agoBump P::C version to 1.115.
Elliot Shank [Thu, 31 Mar 2011 23:57:08 +0000 (23:57 +0000)] 
Bump P::C version to 1.115.

10 years agoRT #67116 - Crash in RegularExpressions::ProhibitUnusedCapture
Tom Wyant [Thu, 31 Mar 2011 15:31:33 +0000 (15:31 +0000)] 
RT #67116 - Crash in RegularExpressions::ProhibitUnusedCapture

This turned out to be a bug in the work that taught the policy to look
inside here documents -- the code assumed that the PPI::Token::HereDoc
heredoc() method returned the here document as a string, when it
actually returned an array. The self-test only passed because it used
one-line here documents.

Mea culpa, mea maxima culpa.

I'm afraid this is going to need another release. My apologies.

10 years agoIgnoring META.json in main Perl-Critic distribution. Will need to be
Elliot Shank [Mon, 28 Mar 2011 11:30:23 +0000 (11:30 +0000)] 
Ignoring META.json in main Perl-Critic distribution.  Will need to be
repeated for all of the others.

10 years agoSmall POD fix in Documentation::RequirePodLinksIncludeText.
Elliot Shank [Mon, 28 Mar 2011 11:24:23 +0000 (11:24 +0000)] 
Small POD fix in Documentation::RequirePodLinksIncludeText.

10 years agoBump P::C version to 1.114. v1.114
Elliot Shank [Sat, 26 Mar 2011 21:16:47 +0000 (21:16 +0000)] 
Bump P::C version to 1.114.

10 years agoAdd suggestion of using lc() instead of case-insensitive match to
Elliot Shank [Mon, 14 Mar 2011 20:24:18 +0000 (20:24 +0000)] 
Add suggestion of using lc() instead of case-insensitive match to

10 years agoRT #38942: RegularExpressions::ProhibitUnusedCapture false positive
Tom Wyant [Sun, 6 Mar 2011 04:11:17 +0000 (04:11 +0000)] 
RT #38942: RegularExpressions::ProhibitUnusedCapture false positive

This commit addresses the

    if ( /(a)/ || /(b)/ ) {
        say $1;

case by (basically) not terminating the scan for the next regexp if an
alternation operator is encountered first. It is more complicated than
that, of course, because if the alternation was one of the
tightly-binding ones you have to start looking again after an 'and', and
you have to handle subexpressions appropriately.

There was some collateral damage because passing around the closure used
to scan for the next regexp put _check_node_children() over the limit
for number of arguments, so it was rewritten to put all capture-tracking
stuff in a hash which gets passed around. This hash could usefully be
used in other places (e.g. _mark_magic()), but I didn't want to get too
far beyond addressing the RT ticket.

As a sanity check the policy both before and after was run against the
beginning of CPAN (through authors/id/A/AM/*). Before (i.e. rev 4048)
produced 701 violations. With the changes in this commit, there were
698, with all three non-violations being of the form /(a)/ || /(b)/. Not
a big payoff for the work that went into this, but having done it I hate
to throw it away.

10 years agoRT #64247: bless {} probably contains a hash constructor, not a block
Tom Wyant [Tue, 1 Mar 2011 21:09:32 +0000 (21:09 +0000)] 
RT #64247: bless {} probably contains a hash constructor, not a block

This is fixed by PPI 1.215.

This commit adds a test to
t/ValuesAndExpressions/ProhibitCommaSeparatedStatements.run, to be sure
this stays fixed. Or at least to let us know if it breaks. The
dependency on PPI was bumped to 1.215 in commit 4047.

10 years agoRT #61301: Bug in policy
Tom Wyant [Tue, 1 Mar 2011 20:38:18 +0000 (20:38 +0000)] 
RT #61301: Bug in policy

This was really a PPI bug (the Perl docs say return {...} is a hash
constructor, but PPI was making it a block), fixed in PPI 1.215.

This commit bumps the PPI dependency to 1.215, and adds a test to
t/ValuesAndExpressions/ProhibitCommaSeparatedStatements.run to try to
make sure it stays fixed.

10 years agoRT #38942: RegularExpressions::ProhibitUnusedCapture false positive
Tom Wyant [Sat, 26 Feb 2011 23:17:50 +0000 (23:17 +0000)] 
RT #38942: RegularExpressions::ProhibitUnusedCapture false positive

This has become a bit of a catch-all ticket.

This commit deals with the case where the capture variables are hiding
in an interpolation.

The ticket needs to remain open because of the '/(a)/ or /(b)/' case.

10 years agoRT #64437: InputOutput::RequireBriefOpen false-positive
Tom Wyant [Fri, 25 Feb 2011 21:58:41 +0000 (21:58 +0000)] 
RT #64437: InputOutput::RequireBriefOpen false-positive

The problem was that the policy assumed the close() was in the same
lexical scope as the open() under analysis. This is not necessarily true
unless the open() is of the form 'open my $fh, ...'.

The fix applied was, if we did _not_ have 'open my $fh, ...', to select
as the scope for analysis the most-local scope that ends outside the
allowed line range. This may not technically be the right scope for the
file handle, but if the close() falls outside this scope, we know we
have a violation.

The main change involved jiggering _get_scope() to be sensitive to
which kind of file handle the open() used.

10 years agoRT #65514: RequireLocalizedPunctuationVars caveats no longer apply
Tom Wyant [Thu, 24 Feb 2011 18:17:18 +0000 (18:17 +0000)] 
RT #65514: RequireLocalizedPunctuationVars caveats no longer apply

Caveats no longer apply with PPI 1.208, which is required by P::C 1.113.

10 years agoRT #66017 (severity description confusing)
Tom Wyant [Wed, 23 Feb 2011 18:01:52 +0000 (18:01 +0000)] 
RT #66017 (severity description confusing)

Inserted a paragraph just below both severity name-to-number tables
attempting to clarify the fact the (e.g.) severity 5 (the worst) is
called 'gentle'.

10 years ago#65569: Documentation::RequirePodLinksIncludeText gives false-positive
Tom Wyant [Wed, 16 Feb 2011 00:51:40 +0000 (00:51 +0000)] 
#65569: Documentation::RequirePodLinksIncludeText gives false-positive
on nested POD formatting

Converted the module to use Pod::Parser to do the heavy lifting. This
introduces a nominal new dependency on Pod::Parser, but since we already
have a dependency on Pod::PlainText, and Pod::PlainText needs
Pod::Parser, I hope this is a non-issue.

10 years agoUpdate Changes and bump version for P::C v1.113. v1.113
Elliot Shank [Tue, 15 Feb 2011 01:31:57 +0000 (01:31 +0000)] 
Update Changes and bump version for P::C v1.113.

10 years agoRT #65663 RequireConsistentNewlines five-part PPI location
Tom Wyant [Sun, 13 Feb 2011 00:10:17 +0000 (00:10 +0000)] 
RT #65663 RequireConsistentNewlines five-part PPI location

This policy is a problem because it needs to bypass PPI to get at the
actual line endings. But since Perl::Critic needs a PPI::Element to
report a violation, it makes one up out of whole cloth, then reaches
into its innards to slam-dunk the information it wants to put in the
violation message.

Unfortunately, when it was coded it only slam-dunked three pieces of
information, not five. This may not have mattered before #file and #line
support, but it does now even if you are not using %f in the output
format as noted in the ticket -- at least, if you object to lots of
undefined value messages it matters.

The work in branches/violation_named_args/ was done to try to make it
possible to specify a violation position relative to the start of the
element, but until that makes it into the trunk I see no way to address
this other than to compound the felony.

This probably should be released fairly shortly. The symptoms are not
fatal, but are extremely annoying.

10 years agoBump P::C version to v1.112_002. v1.112_002
Elliot Shank [Thu, 10 Feb 2011 02:31:08 +0000 (02:31 +0000)] 
Bump P::C version to v1.112_002.

10 years agoCommit some of the self-compliance failures found with a more aggressive
Tom Wyant [Tue, 8 Feb 2011 23:56:51 +0000 (23:56 +0000)] 
Commit some of the self-compliance failures found with a more aggressive
Variables::ProhibitUnusedVariables. This does _not_ include the revised
Variables::ProhibitUnusedVariables itself, nor any collateral changes.
Nor does it include anything where '## no critic' seemed appropriate,
since without the revised ProhibitUnusedVariables these will trigger

10 years agoRT #65322: PPI 1.214_0? breaks Perl::Critic
Tom Wyant [Thu, 3 Feb 2011 04:26:57 +0000 (04:26 +0000)] 
RT #65322: PPI 1.214_0? breaks Perl::Critic

Remove bad test from t/Subroutines/RequireArgUnpacking.t. This was
exposed by PPI RT#65199
(https://rt.cpan.org/Ticket/Display.html?id=65199), which is in PPI

10 years agoMore trailing whitespace.
Elliot Shank [Mon, 31 Jan 2011 12:30:57 +0000 (12:30 +0000)] 
More trailing whitespace.

10 years agoCopyright year update in PPIx-Utilities and Perl-Critic.
Elliot Shank [Mon, 31 Jan 2011 12:09:54 +0000 (12:09 +0000)] 
Copyright year update in PPIx-Utilities and Perl-Critic.

10 years agoMake P::C::Module::Build a subclass of P::C::Module::Build::Standard.
Elliot Shank [Sun, 30 Jan 2011 22:47:52 +0000 (22:47 +0000)] 
Make P::C::Module::Build a subclass of P::C::Module::Build::Standard.

10 years agoSet P::C version to 1.112_001. v1.112_001
Elliot Shank [Wed, 15 Dec 2010 02:31:40 +0000 (02:31 +0000)] 
Set P::C version to 1.112_001.

10 years agoUpdate Changes for 1.112_001.
Elliot Shank [Wed, 15 Dec 2010 02:27:34 +0000 (02:27 +0000)] 
Update Changes for 1.112_001.

10 years agoRT #63816: installation fails when using lastest version of List::MoreUtils
Tom Wyant [Sat, 11 Dec 2010 08:19:59 +0000 (08:19 +0000)] 
RT #63816: installation fails when using lastest version of List::MoreUtils

The proximate problem is that the behavior of all() when the list is
empty has changed. Previous behavior (as of List::MoreUtils 0.26,
anyway) was that all() with an empty list returned false. With
List::MoreUtils it returns true. But the code relies on the old

Unfortunately, what's really needed is first a review of the
List::MoreUtils functions used by Perl::Critic, then protection of all
affected functions. Or maybe just protect them all, to be truly

10 years agoFix ProhibitPostfixControls' handling of normal when statements.
Elliot Shank [Fri, 3 Dec 2010 02:23:59 +0000 (02:23 +0000)] 
Fix ProhibitPostfixControls' handling of normal when statements.

10 years agoSet the P::C version to 1.110_001. I still want to look through the v1.110_001
Elliot Shank [Wed, 1 Dec 2010 03:05:15 +0000 (03:05 +0000)] 
Set the P::C version to 1.110_001.  I still want to look through the
diff since the last release before calling it ready, but we can't wait
any longer to at least get this out there.

10 years agoNotes for 5.14 in TODO.pod
Tom Wyant [Tue, 23 Nov 2010 16:51:04 +0000 (16:51 +0000)] 
Notes for 5.14 in TODO.pod

10 years agoOfficially bless Const::Fast.
Elliot Shank [Sat, 13 Nov 2010 21:11:06 +0000 (21:11 +0000)] 
Officially bless Const::Fast.

10 years agoChange ValuesAndExpressions::ProhibitMagicNumbers test to not use
Elliot Shank [Sat, 13 Nov 2010 20:39:24 +0000 (20:39 +0000)] 
Change ValuesAndExpressions::ProhibitMagicNumbers test to not use
Const::Fast as its example of a custom constant-declaring subroutine.

10 years agoTest Const::Fast support in NamingConventions::Capitalization.
Elliot Shank [Sat, 13 Nov 2010 20:36:40 +0000 (20:36 +0000)] 
Test Const::Fast support in NamingConventions::Capitalization.

10 years agoTrailing whitespace in t/NamingConventions/Capitalization.run.PL.
Elliot Shank [Sat, 13 Nov 2010 20:29:46 +0000 (20:29 +0000)] 
Trailing whitespace in t/NamingConventions/Capitalization.run.PL.