NAME

TODO - Things for Perl::Critic developers to do

SOURCE

#######################################################################
#      $URL: http://perlcritic.tigris.org/svn/perlcritic/tags/Perl-Critic-0.20/TODO.pod $
#     $Date: 2006-09-10 21:18:18 -0700 (Sun, 10 Sep 2006) $
#   $Author: thaljef $
# $Revision: 663 $
#######################################################################

NEW FEATURES

• Report Safari sections in addition to book page numbers.

• Report some statistics on the violations and the source code.

  e.g. Number of violations of each policy/severity. Total lines of code, number of subroutines, average number of statements/operators per sub.

BUGS/LIMITATIONS

• Errors in .perlcriticrc are silent

• 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

• Subroutines::ProtectPrivateSubs

  Doesn't forbid $pkg->_foo() because it can't tell the difference between that and $self->_foo()

• ErrorHandling::RequireCarping

  This should not complain about using warn or die if it's not in a function.

  Also, if the error string ends with a "\n", then the line number and package information is not shown, so it doesn't matter if you use carp/confess or not. For example:

  my $fh;
  if ( !open( $fh, '<', $filename ) ) {
      warn "ack: $filename: $!\n";
      return;
  }

  I just want to print a warning and move on. The location is irrelevant.

OTHER PBP POLICIES THAT SEEM FEASIBLE TO IMPLEMENT

• ValuesAndExpressions::ProhibitCommaSeparatedStatements (p68)

• ValuesAndExpressions::RequireListParens (p71)

• ValuesAndExpressions::ProhibitScalarGrep (p71)

  Look for grep in a scalar context and recommend any() instead. Perhaps we need to distinguish cases like: $count += grep {qr/foo/} @list;

• Variables::RequireLocalizedPunctuationVars (p81)

• Variables::ProhibitTopicChangeInListFunction (p114)

• Documentation::PodSpelling (p148)

  Base it on Pod::Spell or Test::Spelling. Add a "=for stopwords" section for words to skip, as per Pod::Spell.

• BuiltinFunctions::ProhibitReverseSortBlock (p152)

  Don't allow first instance of $b to be before first instance of $a.

  [Note: already implemented by Chris Dolan, but not committed yet due to PPI bugs. Ask Chris for the patch.]

• Subroutines::RequireArgUnpacking (p178)

  Ensure that the first child of a sub is PPI::Statement::Variable (unless the sub has N or fewer statements, where N defaults to 1.

• Subroutines::ProhibitManyArgs (p182)

  If first PPI::Statement::Variable is a list my, and @_ is used, make sure it's fewer than N elements. Otherwise make sure there are less than N PPI::Statement::Variables in a row at begin which shift.

• InputOutput::RequireErrorChecking (p208)

  Forbid open, print, close in void context, unless "use Fatal" is in effect.

• InputOutput::RequireBriefOpen (p209)

  Make sure there's a close within N statements of an open, both with same lexical FH

• InputOutput::ProhibitJoinedReadline (p213)

• InputOutput::ProhibitExplicitStdin (p216)

• Miscellanea::ProhibitObnoxiousComments

  Forbid excessive hash marks e.g. "#### This is a loud comment ####". Make the obnoxious pattern configurable

• 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

• RegularExpressions::ProhibitComplexRegexps (p261)

  If regexp is longer than N characters/lines, require it be split into qr// pieces.

• 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/

• TestingAndDebugging::ProhibitProlongedStrictureOverride (p443)

  This conflicts with TestingAndDebugging::ProhibitNoStrict

NON-PBP POLICIES WANTED

• BuiltInFunctions::RequireConstantSprintfFormat

• BuiltInFunctions::RequireConstantUnpackFormat

  http://home.earthlink.net/~josh.jore/new-warnings/slides/slide1.html

• ControlStructures::ProhibitIncludeViaDo

  Forbid do "foo.pl". Not sure about this policy name.

• CodingStyle::ProhibitNonASCII

  Definitely low severity! Only looks at code, not comments or POD

• CodingStyle::RequireUTF8

  All characters must be valid UTF-8. Note that typical ASCII Perl code is a valid UTF8 subset.

• Miscellanea::RequireMinimumPerlVersion

  Every module should have something like use 5.6.0

• Miscellanea::Prohibit5006isms

  Keep the code 5.005 compatible Low severity

• Variables::ProhibitUseVars

  Require our $foo instead. This contradicts Miscellanea::Prohibit5006isms. Maybe verify use 5.6 before applying this policy. Low severity.

• VariablesAndExpressions::ProhibitQuotedHashKeys

  Forbid quotes around hash keys, unless they are really needed. This is against what Damian says. Suggested by Adam Kennedy. Low severity.

• Miscellanea::B::Lint

  Create a compatibility layer for the B::Lint code analyzer. Make it very clear that this runs code and thus is a security hole.

• CodingStyle::ProhibitFunctionalNew

  Good: Foo::Bar->new, Bad: new Foo::Bar

• VariablesAndExpressions::RequireConstantVersion (low severity)

  VariablesAndExpressions::ProhibitComplexVersion (medium severity) http://rt.cpan.org/Ticket/Display.html?id=20439

• Tests::RequireTestDescriptions

  If Test::More is detected:

  ok $foo;                        # not ok
  ok $foo, 'foo got populated';   # ok
  is $foo, 'bar';                 # not ok
  is $foo, 'bar', 'foo value';    # ok

• ValuesAndExpressions::ProhibitUselessQuotingOfVariables

  I've seen a lot of beginners do things like this:

  open( my $fh, "$filename" );

  There's no reason for $filename to be quoted. (except stringifying overloaded instances)

• Variables::ProhibitPerl4PackageNames

  Forbid old-schoole package names like Foo'Bar'Baz. This should also apply to any variables or subroutines that get declared/called.

• CodingStyle::RequireEditorSettings

  Files must have something like the following in them for Emacs and Vi:

  # Local Variables:
  #   mode: cperl
  #   cperl-indent-level: 4
  #   fill-column: 100
  # End:
  # vim: expandtab shiftwidth=4:

• Documentation::RequireSynopsis

• Documentation::RequireLicense

  These are simplified versions of Documentation::RequirePodSections.

• CodingStyle::RequireConsistentNewlines

  Mixing any two of \r\n, \r and \n in the same file causes problems with GnuPG (and thus Module::Signature). This applies to the whole file, including strings, heredocs, POD, __DATA__, etc.

• Miscellaneous::ProhibitBoilerplate

  Complain about copy-and-paste code or docs from h2xs, Module::Starter, etc.

REFACTORINGS and ENHANCEMENTS

• Alias -verbose to -format option in Test::P::C.

  Give it same functionality as the -verbose option in `perlcritic`

• Enhance P::C::critique() to accept file names, directories, or code strings (as refs)

  Just like bin/perlcritic does now.

• Add -cache flag to bin/perlcritic

  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);

  (see t/40_criticize.t for a more robust implementation)

• Use the PPI::Document::File features

  This gives us access to the filename. New possible policies:

  • File::RequirePortableName

    No spaces, punctuation, etc.

  • File::RequireNameMatchesPackage

    If the file has package Foo::Bar; then the file path must end with File::Spec-catfile('Foo', 'Bar.$ext');> where $ext is one of pm, pl, PL, pod, etc.

PPI BUGS

We're waiting on the following bugs to get fixed in a CPAN release of PPI:

• goto as a PPI::Statement::Break instance

  Broken through PPI v1.115, fixed in PPI 1.117, needed to remove workaround from Subroutines::RequireFinalReturn

• RT #19629: End of list mistakenly seen as end of statement

  Broken through PPI v1.115, fixed in PPI 1.117, needed for BuiltinFunctions::ProhibitReverseSortBlock