Perl::Critic::TODO - Things for Perl::Critic developers to do
####################################################################### # $URL: http://perlcritic.tigris.org/svn/perlcritic/branches/Perl-Critic-1.073/TODO.pod $ # $Date: 2007-08-09 00:25:39 -0500 (Thu, 09 Aug 2007) $ # $Author: chrisdolan $ # $Revision: 1826 $ #######################################################################
Perl-Critic-More is a separate distribution for less-widely-accepted policies. It contains its own TODO.pod.
Report Safari sections in addition to book page numbers.
Allow policies to say that they've had enough and to not use them for the rest of the current document.
Primarily for things like RequireUseStrict and ProhibitMagicNumbers. Replace current workaround for RequireUseStrict.
RequireUseStrict
ProhibitMagicNumbers
Modules::RequireVersionVar
Doesn't enforce three-part versions
NamingConventions::ProhibitAmbiguousNames
Don't allow compound names with forbidden words, like "last_record". Allow forbidden words in RHS of variable declarations
Also, we should make it easeir to add (or delete) words from the forbbiden list.
Subroutines::ProtectPrivateSubs
Doesn't forbid $pkg->_foo() because it can't tell the difference between that and $self->_foo()
$pkg->_foo()
$self->_foo()
ErrorHandling::RequireCarping
This should not complain about using warn or die if it's not in a function, or if it's not in a non-main:: package.
warn
die
Also, should allow die when it is obvious that the "message" is a reference.
RegularExpressions::ProhibitCaptureWithoutTest
Allow this construct:
for ( ... ) { next unless /(....)/; if ( $1 ) { .... } }
Right now, P::C thinks that the $1 isn't legal to use because it's "outside" of the match. The thing is, we can only get to the if if the regex matched. while ( $str =~ /(expression)/ )
$1
if
CodeLayout::ProhibitParensWithBuiltins
Some builtin functions (particularly those that take a variable number of scalar arguments) should probably get parens. This policy should be enhanced to allow the user to specify a list of builtins that are expempt from the policy.
InputOutput::RequireCheckedOpen and RequireCheckedClose
These policies should not report violations if 'use Fatal' is in effect.
Chris Dolan is in the process of implementing the ideas below (but don't let that stop you from contributing!!!). That work is supported by a Perl Foundation grant. More details about the grant: http://www.perlfoundation.org/april_1_2007_new_grant_awards
InputOutput::RequireChecked* for system calls (p208)
Add policies to ensure checking the return values of system calls. See InputOutput::RequireCheckedOpen and InputOutput::RequireCheckedClose. Needs to take into account the Fatal and Fatal::Exception modules.
InputOutput::RequireBriefOpen (p209)
Make sure there's a close within N statements of an open, both with same lexical FH
InputOutput::ProhibitExplicitStdin (p216)
If you're reading from STDIN, chances are you're really wanting to read from the magic filehandle.
RegularExpressions::RequireBracesForMultiline (p242)
RegularExpressions::ProhibitUnusualDelimiters (p246)
RegularExpressions::ProhibitEscapedMetacharacters (p247)
RegularExpressions::ProhibitEnumeratedClasses (p248)
This will be avoided for ASCII-only code.
RegularExpressions::ProhibitUnusedCapture (p252)
Look for LHS of regexp or use of $1, $2, ... before next regexp.
$2
RegularExpressions::ProhibitComplexRegexes (p261)
If regexp is longer than N characters/lines, require it be split into qr// pieces.
qr//
Even if the x modifier is used and the regex is split across multiple lines?
RegularExpressions::ProhibitSingleCharAlternation (p265)
Not sure if this is easy or hard. Need to look at what PPI emits for regexps. Make an exception for qr/ [ ] /x.
RegularExpressions::ProhibitFixedStringMatches (p271)
Can't be qr/\s*\\A\s*\((?:\?:)?(?:\s*\w+\s*\|)*\s*\w+\s*\)\s*\\z/ or qr/\s*\\A\s*\w+\s*\\z/
qr/\s*\\A\s*\((?:\?:)?(?:\s*\w+\s*\|)*\s*\w+\s*\)\s*\\z/
qr/\s*\\A\s*\w+\s*\\z/
TBD::VariableNotUsed
Detect a variable that has a value assigned to it, but never used.
TBD::AllProgramsNeedShebangs
Anything that is a program should have a shebang line. This includes .t files.
BuiltInFunctions::RequireConstantSprintfFormat
BuiltInFunctions::RequireConstantUnpackFormat
http://diotalevi.isa-geek.net/~josh/yapc-lint/slides/slide5.html
Miscellanea::ProhibitObnoxiousComments
Forbid excessive hash marks e.g. "#### This is a loud comment ####". Make the obnoxious pattern configurable
ValuesAndExpressions::RequireNotOperator
Require the use of "not" instead of "!", except when this would contradict ProhibitMixedBooleanOperators. This may be better suited for Perl::Critic::More.
Modules::RequireExplicitImporting
Require every use statement to have an explicit import list. You could still get around this by calling import directly.
use
import
Modules::ForbidImporting
Require every use to have an explicitly empty import list. This is for folks who like to see fully-qualified function names. Should probably provide a list of exempt modules (like FindBin);
ControlStructures::ProhibitIncludeViaDo
Forbid do "foo.pl". Not sure about this policy name.
do "foo.pl"
Variables::ProhibitUseVars
Disallow use vars qw(...) and require our $foo instead. This contradicts Miscellanea::Prohibit5006isms. Maybe verify use 5.6 before applying this policy. Low severity.
use vars qw(...)
our $foo
use 5.6
VariablesAndExpressions::ProhibitQuotedHashKeys
Forbid quotes around hash keys, unless they are really needed. This is against what Damian says. Suggested by Adam Kennedy. Low severity.
CodeLayout::ProhibitFunctionalNew
Good: Foo::Bar->new, Bad: new Foo::Bar
Foo::Bar->new
new Foo::Bar
VariablesAndExpressions::RequireConstantVersion (low severity)
VariablesAndExpressions::ProhibitComplexVersion (medium severity)
http://rt.cpan.org/Ticket/Display.html?id=20439
Documentation::RequireSynopsis
Documentation::RequireLicense
These are simplified versions of Documentation::RequirePodSections.
Miscellaneous::ProhibitBoilerplate
Complain about copy-and-paste code or docs from h2xs, Module::Starter::*, etc.
Here's a non-PPI implementation: http://search.cpan.org/src/JJORE/Carp-Clan-5.8/t/04boilerplate.t
BuiltinFunctions::ProhibitExtraneousScalarCall
Recommend that if (scalar @array) be rewritten as if (@array).
if (scalar @array)
if (@array)
Create constants for the PPI location array elements.
MOVE THE LINE-DISABLING INTO P::C::Document
All the code that deals with finding all the '##no critic' comments and noting which policies are disabled at each line seems like it would be better placed in Perl::Critic::Document. P::C::Document could then provide methods to indicate if a policy is disabled at a particular line. So the basic algorithm in Perl::Critic might look something like this:
foreach $element (@PPI_ELEMENTS) { foreach $policy (@POLICIES) { $line = $element->location->[0]; next if $doc->policy_is_disabled_at_line( $policy, $line ); push @violations, $policy->violates( $elem, $doc ); } }
Some means of detecting "runnaway" ##no critic
##no critic
Elliot was talking to a couple of users at ETech and one of their major concerns was that they were using ##no critic and forgetting to do a ##use critic after the problematic section. Perhaps an option to perlcritic to scan for such things is in order.
##use critic
Change API to use named parameters
Most of the methods on the public classes use named parameters for passing arguments. I'd like to extend that pattern to include all object-methods. Static methods can still use positional parameters.
Allow more flexible Policy parameter parsing
Several policies use words_from_string() to split their parameters into words. This function is currently limited to splitting on whitespace. It would be nice to allow some lattitude for users who might try and use commas or some other kind of delimiter.
words_from_string()
Enhance P::C::critique() to accept files, directories, or code strings
Just like bin/perlcritic does now.
Add -cache flag to bin/perlcritic
-cache
If enabled, this turns on PPI::Cache:
require PPI::Cache; my $cache_path = "/tmp/test-perl-critic-cache-$ENV{USER}"; mkdir $cache_path, oct 700 if (! -d $cache_path); PPI::Cache->import(path => $cache_path);
This cachedir should perhaps include the PPI version number! At least until PPI incorporates its own version number in the cache.
(see t/40_criticize.t for a more robust implementation)
Use hash-lookup instead of List::MoreUtils::any function.
List::MoreUtils::any
In several places, Perl::Critic uses List::MoreUtils::any to see if a string is a member of a list. Instead, I suggest using a named subroutine that does a hash-lookup like this:
my %logical_ops = hashify( qw( ! || && ||= &&= and or not ) ); sub is_logical_op { return exists $logical_ops{ $_[0] }; }
We're waiting on the following bugs to get fixed in a CPAN release of PPI:
ValuesAndExpressions::RequireNumberSeparators uses a stringy eval to numify. Current PPI SVN has code for the PPI::Token::Number->literal() method which numifies from source. When we depend on a PPI version higher than 1.118, the _to_number() function in that policy can be removed in favor of literal().
PPI does not preserve newlines. That makes CodeLayout::RequireConsistentNewlines impossible to implement under PPI. For now, it's implemented by pulling the source out of the file and skipping PPI.
It's unlikely that PPI will support mixde newlines anytime soon.
The following parses wrong in PPI v1.118. A PPI fix is in progress.
bless( {} );
When this is fixed, uncomment a few tests in t/20_policies_classhierarchies.t
The PPI::Statement surrounding the PPI::Constructor returns undef for location() for the following:
location()
({})
The same problem exists for
({} )
but not for
( {})
Logged as RT #23788.
Remove trinary operator usage in RequireUseStrict, RequireUseWarnings, and RequireExplicitPackage once this is fixed.
For complicated data structures, { blah => blah } will result in a PPI::Structure::Block being created, instead of a PPI::Structure::Constructor.
{ blah => blah }
ValuesAndExpressions::ProhibitCommaSeparatedStatements and other policies have workarounds for this.
ValuesAndExpressions::ProhibitMismatchedOperators has two workarounds for PPI bugs with parsing operators. Many of these bugs have been fixed in PPI, so it would be good to check if those workarounds are still needed.
To install Perl::Critic, copy and paste the appropriate command in to your terminal.
cpanm
cpanm Perl::Critic
CPAN shell
perl -MCPAN -e shell install Perl::Critic
For more information on module installation, please visit the detailed CPAN module installation guide.