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

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

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

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

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

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

10 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').

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

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

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

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

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

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

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

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

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

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

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

10 years agoFix missing import of PPIx::Regexp in Perl::Critic.
Elliot Shank [Mon, 23 Aug 2010 01:53:01 +0000 (01:53 +0000)] 
Fix missing import of PPIx::Regexp in Perl::Critic.

10 years agoMake xt/author/94_includes.t be more descriptive of what it's testing.
Elliot Shank [Mon, 23 Aug 2010 01:51:23 +0000 (01:51 +0000)] 
Make xt/author/94_includes.t be more descriptive of what it's testing.

10 years agoNote ValuesAndExpressions::RequireInterpolationOfMetachars fix in
Elliot Shank [Mon, 23 Aug 2010 01:27:54 +0000 (01:27 +0000)] 
Note ValuesAndExpressions::RequireInterpolationOfMetachars fix in
Changes.

10 years agoMake PPIx::Regexp, Perl::Tidy, Pod::Spell, and Text::ParseWords
Elliot Shank [Mon, 23 Aug 2010 01:25:02 +0000 (01:25 +0000)] 
Make PPIx::Regexp, Perl::Tidy, Pod::Spell, and Text::ParseWords
non-optional.

10 years agoFix ValuesAndExpressions::RequireInterpolationOfMetachars with respect
Elliot Shank [Mon, 23 Aug 2010 01:04:56 +0000 (01:04 +0000)] 
Fix ValuesAndExpressions::RequireInterpolationOfMetachars with respect
to Email::Address 1.890.  This consists of checking for the presence
of an "@" anywhere in the value and removal of a bad test: "@{}" and
"${}" are syntax errors.

10 years agoMove IPC::Open2 from the recommended to required dependencies since
Elliot Shank [Sat, 21 Aug 2010 02:58:38 +0000 (02:58 +0000)] 
Move IPC::Open2 from the recommended to required dependencies since
it's core.

10 years agoRealized I could simplify the code in commits 3886 and 3888 (both to do
Tom Wyant [Fri, 20 Aug 2010 04:33:39 +0000 (04:33 +0000)] 
Realized I could simplify the code in commits 3886 and 3888 (both to do
with having P::C::P::RegularExpressions::ProhibitUnusedCapture look
inside regular expressions) because PPIx::Regexp::Token::Interpolation
is a subclass of PPIx::Regexp::Token::Code, and so I could just look for
the latter, which may appear anywhere in the regexp.

Yes, this proves I don't know my way around my own code. Sigh.

10 years agoVariables::ProhibitUnusedVariables now looks inside regular expressions
Tom Wyant [Fri, 20 Aug 2010 02:47:25 +0000 (02:47 +0000)] 
Variables::ProhibitUnusedVariables now looks inside regular expressions
if PPIx::Regexp can be loaded.

10 years agoHave Subroutines::ProhibitUnusedPrivateSubroutines look inside regular
Tom Wyant [Wed, 18 Aug 2010 18:48:32 +0000 (18:48 +0000)] 
Have Subroutines::ProhibitUnusedPrivateSubroutines look inside regular
expressions if PPIx::Regexp is available.

10 years agoAdded the line number of the offending '=head1 NAME' to the
Tom Wyant [Wed, 18 Aug 2010 05:21:43 +0000 (05:21 +0000)] 
Added the line number of the offending '=head1 NAME' to the
P::C::P::Documentation::RequirePackageMatchesPodName violation
description, using the same dodge Elliot used for
Documentation::RequirePodLinksIncludeText. This is kind of for RT
#59176, though that ticket asked for a way to jigger the line number in
the exception.

10 years ago#59065 Suggested policy: prohibit conditional use statements
Tom Wyant [Fri, 13 Aug 2010 20:34:38 +0000 (20:34 +0000)] 
#59065 Suggested policy: prohibit conditional use statements

Modules::ProhibitContitionalUseStatements and the associated .run file
were attached to the RT ticket.  These are committed with very minor
changes:
* Added =for stopwords evals;
* Dropped trailing space in a few places.

These changes were made to satisfy authortest.

10 years agoUpdate inc/Perl/Critic/BuildUtilities.pm to require version 0.010 of
Tom Wyant [Mon, 9 Aug 2010 03:39:17 +0000 (03:39 +0000)] 
Update inc/Perl/Critic/BuildUtilities.pm to require version 0.010 of
PPIx::Regexp, since that is needed for the
P::C::P::RegularExpressions::ProhibitUnusedCapture ability to find
captures in the replacement portion of s/.../.../e.

This really should have been in commit 3888, but I forgot. Sorry.

10 years agoHave RegularExpressions::ProhibitUnusedCapture account for capture
Tom Wyant [Mon, 9 Aug 2010 00:34:55 +0000 (00:34 +0000)] 
Have RegularExpressions::ProhibitUnusedCapture account for capture
variables used in the replacement portion of s/.../.../e.

10 years ago#60179: BuiltinFunctions::ProhibitStringyEval dies on eval "#...";
Tom Wyant [Sun, 8 Aug 2010 19:42:39 +0000 (19:42 +0000)] 
#60179: BuiltinFunctions::ProhibitStringyEval dies on eval "#...";

The code assumed that a PPI::Document manufactured from the argument to
a stringy eval would have significant children. But Sullivan Beck, in
Date::Manip 5.54, found a reason to do a stringy eval on a comment. This
resulted in a fatal error in the policy.

The fix was simply to check to see if the first significant child was
actually defined, and report a violation if not. The .run file was
updated as well. If the patch is not in place, the .run file experiences
a fatal error, not a test failure. Caveat tester.

10 years agoAdded support for @-, @+, and their English equivalents. This only works
Tom Wyant [Wed, 4 Aug 2010 22:59:05 +0000 (22:59 +0000)] 
Added support for @-, @+, and their English equivalents. This only works
for literal integer subscripts, though. There appears to be no way for
static analysis to take account of $^N and its English equivalent.

10 years agoForgot to put the RT number on the ProhibitUnusedCapture fix committed
Tom Wyant [Wed, 4 Aug 2010 21:03:01 +0000 (21:03 +0000)] 
Forgot to put the RT number on the ProhibitUnusedCapture fix committed
as svn revision 3884. It's RT #60002.

10 years agoRT #60002 - RegularExpressions::ProhibitUnusedCapture does not
Tom Wyant [Wed, 4 Aug 2010 00:36:29 +0000 (00:36 +0000)] 
RT #60002 - RegularExpressions::ProhibitUnusedCapture does not
understand English equivalents of capture variables.

The variable of immediate concern is $LAST_PAREN_MATCH. This is now
recognized provide the document includes 'use English;'.

10 years agoAdd new policy InputOutput::RequireEncodingWithUTF8Layer. This complains
Tom Wyant [Sat, 31 Jul 2010 03:45:01 +0000 (03:45 +0000)] 
Add new policy InputOutput::RequireEncodingWithUTF8Layer. This complains
if it sees ':utf8' rather than ':encoding(utf8)' in open() or binmode().
It is severity 5.

The issue is that ':utf8' flags the string as UTF8 without validating
it. Invalid UTF8 can have bad effects in your code. Exhibit "A" is an
exploit reported on PerlMonks where a script running with -T read input
and sanitized it with m/^(\w+)$/, after which $1 contained shell
meta-characters. This leads to the same kind of problems that the
two-argument open() has, and that policy is severity 5.

10 years agoOn the mailing list, Andy Lester suggested having the "policy warning
Tom Wyant [Sat, 31 Jul 2010 03:09:03 +0000 (03:09 +0000)] 
On the mailing list, Andy Lester suggested having the "policy warning
against checking $@" recommend Try::Tiny. In the belief that this means
P::C::P::ErrorHandling::RequireCheckingReturnValueOfEval, I have done my
best (or worst) to implement the suggestion there.

10 years agoRT #59268: Documentation::RequirePodSections should not complain until the first...
Tom Wyant [Mon, 12 Jul 2010 23:10:51 +0000 (23:10 +0000)] 
RT #59268: Documentation::RequirePodSections should not complain until the first line of POD

Saved the first PPI::Token::Pod containing a '=head1', for use as the
pod-of-record. Reported the error relative to this pod-of-record, so
that the disabling annotation can come before the __END__, the __DATA__,
or the pod-of-record, whichever comes first.

Updated the .run file to test for disablement. Yes, it failed before
applying the patch to the policy.

And holy smokes, Sandy, he remembered to update the Changes file!

10 years agoRT 59176: "no critic" for pod after __END__
Tom Wyant [Fri, 9 Jul 2010 00:14:43 +0000 (00:14 +0000)] 
RT 59176: "no critic" for pod after __END__

The problem turned out to be that the determination of the end of the
scope of the Pod::Critic::Annotation was short-stopped because PPI
parses the __END__ as a PPI::Statement::End, and everything after that
is descended from the PPI::Statement::End. The solution was to descend
into the PPI::Statement::End (if it was the last thing found) and
continue the analysis. I suppose it's still possible to short-stop the
annotation, but I can't think of a way to do it.

t/03_annotations.t was updated to check for the ability to extend
annotations into the __END__ of the document.

Astoundingly, the Changes file was also updated.

10 years agoSpecify a minimum version of version.pm in BuildUtilities. RT #58952.
Elliot Shank [Tue, 29 Jun 2010 21:05:29 +0000 (21:05 +0000)] 
Specify a minimum version of version.pm in BuildUtilities.  RT #58952.

10 years agoNit-pick the docs in P::C::P::RegularExpressions::RequireDotMatchAnything
Tom Wyant [Mon, 28 Jun 2010 00:17:14 +0000 (00:17 +0000)] 
Nit-pick the docs in P::C::P::RegularExpressions::RequireDotMatchAnything

10 years agoHandle the absence of include statements in
Elliot Shank [Sun, 27 Jun 2010 23:30:04 +0000 (23:30 +0000)] 
Handle the absence of include statements in
P::C::Document::uses_module().

10 years agoRemove debugging code from P::C::Document.
Elliot Shank [Sun, 27 Jun 2010 23:08:48 +0000 (23:08 +0000)] 
Remove debugging code from P::C::Document.

10 years agoFix stupid bug in P::C::Document::_nodes_by_namespace().
Elliot Shank [Sun, 27 Jun 2010 23:07:22 +0000 (23:07 +0000)] 
Fix stupid bug in P::C::Document::_nodes_by_namespace().

10 years agoStupid just-prior-to-commit change to Test::P::C::Policy broke
Elliot Shank [Sun, 27 Jun 2010 22:47:42 +0000 (22:47 +0000)] 
Stupid just-prior-to-commit change to Test::P::C::Policy broke
everything.  :]

10 years agoMake Test::P::C::Policy well behaved and not export anything by
Elliot Shank [Sun, 27 Jun 2010 22:43:39 +0000 (22:43 +0000)] 
Make Test::P::C::Policy well behaved and not export anything by
default.

10 years agoCode cleanup of ProhibitMismatchedOperators after applying patch from
Elliot Shank [Sun, 27 Jun 2010 19:12:24 +0000 (19:12 +0000)] 
Code cleanup of ProhibitMismatchedOperators after applying patch from
RT #58751.

10 years agoApply the patch from RT #58751 to ProhibitMismatchedOperators
Elliot Shank [Sun, 27 Jun 2010 18:23:36 +0000 (18:23 +0000)] 
Apply the patch from RT #58751 to ProhibitMismatchedOperators
unmodified (modulo trailing whitespace).  Will need cleanup.

10 years agoCreate P::C::Document::uses_module().
Elliot Shank [Sun, 27 Jun 2010 18:14:32 +0000 (18:14 +0000)] 
Create P::C::Document::uses_module().