Login
gknop/Perl-Critic.git
8 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.

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

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

8 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.

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

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

8 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.

8 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.

8 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

8 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.

8 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
#72660).

8 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;'.

8 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.

8 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.

8 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().

8 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
PCP::RegularExpressions::RequireDotMatchAnything,
::RequireExtendedFormatting, ::RequireLineBoundaryMatching

THIS COMMIT REQUIRES PPix::Regexp 0.022

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.

8 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().

8 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.

9 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
time.

9 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.

9 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.

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

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 ... } ...
and
    grep eval ... , ...
though it does not look quite as hard for the latter.

9 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.

9 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.

9 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
earlier.

9 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.

9 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.

9 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
supported.

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.

9 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.

9 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.

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

9 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.

9 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.

9 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
re-tweaked.

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

9 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.

9 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.

9 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.

9 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.

9 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
$VERSION.

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

9 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.

9 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.

9 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.

9 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.

9 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.

9 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.

9 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
RegularExpressions::ProhibitFixedStringMatches.

9 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.

9 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.

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

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.

9 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.

9 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.

9 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.
Removed.

9 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'.

9 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.

9 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.

9 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.

9 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.

9 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
ProhibitUselessNoCritic.

9 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
1.214_02.

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

9 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.

9 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.

9 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.

9 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.

9 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
behavior.

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
paranoid.

9 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.

9 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.

9 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

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

9 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.

9 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.

9 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.

9 years agoBump PPIx::Utilities version requirement.
Elliot Shank [Sat, 13 Nov 2010 20:28:01 +0000 (20:28 +0000)] 
Bump PPIx::Utilities version requirement.

9 years agoMake Policy tests that now pass as no-longer-TODO.
Elliot Shank [Mon, 1 Nov 2010 01:07:54 +0000 (01:07 +0000)] 
Make Policy tests that now pass as no-longer-TODO.

9 years agoRenamed "forced filename" to "filename override".
Elliot Shank [Mon, 1 Nov 2010 00:42:51 +0000 (00:42 +0000)] 
Renamed "forced filename" to "filename override".

9 years agoRT #62562 - ValuesAndExpressions::ProhibitMagicNumbers should be able to
Tom Wyant [Sun, 31 Oct 2010 03:26:22 +0000 (03:26 +0000)] 
RT #62562 - ValuesAndExpressions::ProhibitMagicNumbers should be able to
give a Readonly equivalent

The request was to be able to configure the policy to recognize
subroutines that manfacture manifest constants other than the Readonly
brothers, specifically the const() subroutine exported by Const::Fast.

The implementation added a constant_creator_subroutines configuration
parameter to the policy, in lieu of the hard-wired values that were in
%READONLY_SUBROUTINES. The values from the hard-wired hash became
list_always_present_values values in the parameter.

9 years agoRemoved extra bullet in Changes.
Elliot Shank [Sun, 24 Oct 2010 22:46:45 +0000 (22:46 +0000)] 
Removed extra bullet in Changes.

9 years agoRemove superfluous argument from violation() call in
Tom Wyant [Wed, 20 Oct 2010 14:00:09 +0000 (14:00 +0000)] 
Remove superfluous argument from violation() call in
P::C::P::BuiltinFunctions::ProhibitReverseSortBlock.

9 years agoFix self-compliance with prospective policy
Tom Wyant [Mon, 18 Oct 2010 23:30:35 +0000 (23:30 +0000)] 
Fix self-compliance with prospective policy
Subroutines::ProhibitPassingCaptureVariable.

9 years agoRT #49609: Subroutines::ProhibitAmpersandSigils: improperly interprets
Tom Wyant [Mon, 11 Oct 2010 06:32:23 +0000 (06:32 +0000)] 
RT #49609: Subroutines::ProhibitAmpersandSigils: improperly interprets
list of sub refs

This turned out not to be as fixed as I thought it was when I committed
SVN revision 3648. With this update, it recognizes as subroutine
references (and therefore allows) not only \( &sub1 ) but also \( &sub1,
&sub2). Ooops.

Paraphrase and Bowdlerization from Cornelius Ryan's novel "The Longest
Day", as a script:

GI: C'mon up! The so-and-sos are cleaned out!

FX: Rifle and automatic weapons fire, followed by a grenade explosion.

Same GI: C'mon up! The so-and-sos are cleaned out NOW!

9 years agoRT #61970 - ValuesAndExpressions::RequireInterpolationOfMetachars
Tom Wyant [Fri, 8 Oct 2010 21:19:24 +0000 (21:19 +0000)] 
RT #61970 - ValuesAndExpressions::RequireInterpolationOfMetachars
false-positives on non-special \-characters

This commit is work touched off by the subject ticket, but does the
exact opposite of what the requester wanted, which was for the policy
not to complain about "\Q" and "\U".

A review of the code versus the perlop documentation showed that it was
not detecting "\b" (backspace) or "\l" (lowercase next character). I
promised in the ticket to investigate "\Q", since

$ perl -E 'say "\Q\t"'|hexdump -C

does not print

00000000  5c 74 0a      |\t.|

as I expected, but instead prints

00000000  5c 09 0a     |\..|

That is, the back slash before the 't' appears _not_ to be quoted.

While reading toke.c (whose Tolkien quote, and name come to think of it,
is wholly appropriate) in a vain attempt to try to figure out what was
going on, I discovered that \1 through \7 do the same thing as \0. I
can't find this documented in perlop, though I seem to remember perlre
alluding to something like this. So those were added.

In summary, this commit makes RequireInterpolationOfMetachars complain
about "\b", "\l", and "\1" through "\7", in addition to all the ones it
complained about before.

Test t/ValuesAndExpressions/RequireInterpolationOfMetachars.run now
checks for everything documented (I cut-and-pasted from perlop), plus
the ones I found in toke.c, and additionally tests that the policy does
not complain about "\.", where '.' is any \w character other than the
ones in perop and toke.c.

And a couple self-compliance issues were fixed by adding "I meant to do
that!" (spelled "## no critic (RequireInterpolationOfMetachars)").

Lastly, bug 78292 ("What should 'say "\Q\t"' print?") has been filed
against Perl 5.12.2.

9 years agoMention the -forced-filename option in Changes.
Jeffrey Ryan Thalhammer [Thu, 7 Oct 2010 17:53:25 +0000 (17:53 +0000)] 
Mention the -forced-filename option in Changes.

9 years agoAdded -forced-filename option to the P::C::Document contstructor.
Jeffrey Ryan Thalhammer [Wed, 6 Oct 2010 18:38:11 +0000 (18:38 +0000)] 
Added -forced-filename option to the P::C::Document contstructor.
This allows you to give the Document a filename in situations where
the Document is being constructed from something that doesn't already
have a filename (like a reference to a scalar containing actual source
code).

9 years agoRT #61311: Subroutines::ProhibitUnusedPrivateSubroutines dies on
Tom Wyant [Tue, 14 Sep 2010 05:25:14 +0000 (05:25 +0000)] 
RT #61311: Subroutines::ProhibitUnusedPrivateSubroutines dies on
"&_name" call

The problem was that the return value of sprevious_sibling() was being
incorrectly tested.

About 12 lines into sub _find_sub_reference_in_document(), and again
about 18 lines in, the code 'defined $prior' occurs. The 'defined'
should be removed; that is, the code should just be '$prior'.

The erroneous code assumed that the PPI method sprevious_sibling()
returned undef if there was no previous sibling in the parse tree. In
fact, it is documented as returning a false value.

9 years agoAdd stop words to P::C::P::ControlStructures::ProhibitPostfixControls.
Tom Wyant [Sat, 11 Sep 2010 16:08:01 +0000 (16:08 +0000)] 
Add stop words to P::C::P::ControlStructures::ProhibitPostfixControls.

9 years agoMove the Changes comment on commit 3919 to the 'Bugs' section (what was
Tom Wyant [Thu, 9 Sep 2010 23:58:10 +0000 (23:58 +0000)] 
Move the Changes comment on commit 3919 to the 'Bugs' section (what was
I thinking?) and added the RT number.

9 years agoRT#61071: OptionsProcessor vs perl 5.8.3 Exporter
Tom Wyant [Thu, 9 Sep 2010 23:51:37 +0000 (23:51 +0000)] 
RT#61071: OptionsProcessor vs perl 5.8.3 Exporter

Updated inc/Perl/Critic/BuildUtilities.pm to require Exporter version
5.63, to get a version that understands export tags in positions other
than leading ones. Another solution was to move all the tags to the
leading position, but I thought it would be difficult to ensure that the
code stayed that way. I could also have gone with version 5.59 (the
first dual-lifed version), but the CPAN testers say 5.63 (the current
version) works back to perl 5.6.1.

9 years agoAdd "when" support to ControlStructures::ProhibitPostfixControls.
Elliot Shank [Sat, 4 Sep 2010 20:38:08 +0000 (20:38 +0000)] 
Add "when" support to ControlStructures::ProhibitPostfixControls.

9 years agoAdd '_' to the tr/// that counts arguments in the subroutine prototype,
Tom Wyant [Wed, 1 Sep 2010 18:42:04 +0000 (18:42 +0000)] 
Add '_' to the tr/// that counts arguments in the subroutine prototype,
since this is now valid (meaning, '$ but use $_ if no argument
specified').

9 years agoMake Subroutines::ProhibitManyArgs understand prototype grouping. That
Tom Wyant [Wed, 1 Sep 2010 15:14:24 +0000 (15:14 +0000)] 
Make Subroutines::ProhibitManyArgs understand prototype grouping. That
is, prototype (\[$@%]) specifies one argument, not three. Tests for this
added to .run file.

9 years agoReduce use of PPI overloads i
Elliot Shank [Mon, 30 Aug 2010 10:53:10 +0000 (10:53 +0000)] 
Reduce use of PPI overloads i
ControlStructures::ProhibitPostfixControls.

9 years agoBump trunk version of Perl-Critic to 1.110.
Elliot Shank [Mon, 30 Aug 2010 02:18:05 +0000 (02:18 +0000)] 
Bump trunk version of Perl-Critic to 1.110.

9 years agoUpdate Changes on trunk to match what was released in 1.109.
Elliot Shank [Mon, 30 Aug 2010 02:14:24 +0000 (02:14 +0000)] 
Update Changes on trunk to match what was released in 1.109.

9 years agoIt turns out that the original complaint in RT #3077 for
Elliot Shank [Mon, 30 Aug 2010 01:27:11 +0000 (01:27 +0000)] 
It turns out that the original complaint in RT #3077 for
RequireInterpolationOfMetachars dinging use of overload.pm really was
a general issue that the policy shouldn't complain about '${}' or
'@{}' because they're invalid syntax.

9 years agoFix detection of q<'@whatever'> in
Elliot Shank [Mon, 30 Aug 2010 01:08:33 +0000 (01:08 +0000)] 
Fix detection of q<'@whatever'> in
ValuesAndExpressions::RequireInterpolationOfMetachars found due to the
Email::Address change.

9 years agoAdded method Perl::Critic::Document->ppix_regexp_from_element(), as a
Tom Wyant [Tue, 24 Aug 2010 17:41:11 +0000 (17:41 +0000)] 
Added method Perl::Critic::Document->ppix_regexp_from_element(), as a
caching wrapper for PPIx::Regexp->new(), inspired by find() and (more
recently) _nodes_by_namespace().

Rewrote all uses of PPIx::Regexp->new_from_cache() and
PPIx::Regexp->new() (found in
RegularExpressions::ProhibitComplexRegexes) in terms of
ppix_regexp_from_element().

Got rid of PPIx::Regexp->flush_cache(), which is no longer needed, and
which I was never really happy with. With the cache on the document, it
Just Works, stays as long as it is needed, and automatically goes away
when it is no longer needed.

This means 'use PPIx::Regexp' only appears in Perl::Critic::Document.

9 years agoChange the test in t/08_document.t which tests
Tom Wyant [Tue, 24 Aug 2010 04:47:24 +0000 (04:47 +0000)] 
Change the test in t/08_document.t which tests
Perl::Critic::Document->uses_module() when there are no include
statements. The original (with an empty document) tickles what just
became RT #60671 in the PPI queue. The minimal way to avoid this
appeared to be using a document consisting of a single space.

It turns out that if you call PPI::Document->index_locations() on a
document with no tokens at all, it gets confused, and that's all RT
#60671 is about. Adam hadn't revoked my commit bit yet, so I committed
a proposed fix.

9 years agoSince PPIx::Regexp is no longer optional, some of the tests that use it
Tom Wyant [Mon, 23 Aug 2010 18:08:06 +0000 (18:08 +0000)] 
Since PPIx::Regexp is no longer optional, some of the tests that use it
no longer need separate .t files. These are moved into the .run files.
Unfortunately it looks like I forgot to add
t/20_policy_prohibit_unused_variables.t to subversion in the first
place, but that's where the tests in the .run file came from.

Added version to 'use PPIx::Regexp' in lib/Perl/Critic.pm.

9 years agoAdd failing test for
Elliot Shank [Mon, 23 Aug 2010 02:15:17 +0000 (02:15 +0000)] 
Add failing test for
ValuesAndExpressions::RequireInterpolationOfMetachars for punctuation
variables (it fails to complain about q<'@_'>) and fix a small
self-compliance issue in the Policy itself.

9 years agoAdd "subscripted" to xt/author/40_stop_words.
Elliot Shank [Mon, 23 Aug 2010 02:09:56 +0000 (02:09 +0000)] 
Add "subscripted" to xt/author/40_stop_words.