NAME

TODO - Things for Perl::Critic developers to do

SOURCE

#######################################################################
#      $URL: http://perlcritic.tigris.org/svn/perlcritic/tags/Perl-Critic-0.21/TODO.pod $
#     $Date: 2006-11-05 18:01:38 -0800 (Sun, 05 Nov 2006) $
#   $Author: thaljef $
# $Revision: 809 $
#######################################################################

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

• Invalid policy names in .perlcriticrc do not cause any warnings.

• Invalid parameters in .perlcriticrc do not cause any warnings.

• 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

• ControlStructures::ProhibitComplexMappings (p113)

  Base this on RequireSimpleSortBlock. Make number of statements configurable

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

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

• CodeLayout::ProhibitNonASCII

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

• CodeLayout::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

  Disallow use vars qw(...) and 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.

• CodeLayout::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

• 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-school package names like Foo'Bar'Baz. This should also apply to any variables or subroutines that get declared/called.

• CodeLayout::RequireEditorSettings

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

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

  Emacs file variables

  http://www.gnu.org/software/emacs/manual/html_node/File-Variables.html

  In emacs, this is called "File Variables". There are two syntaxes: -*- ... -*- (single-line) and Local Variables:\n...\nEnd: (multi-line). Both syntaxes allow leading and trailing text on the line.

  The single-line syntax must be used on the first line of the file to be recognized.

  The multi-line syntax must be used "in the last page" at the end of the file. As of Emacs21, this is hard-coded to be the last 3000 bytes of the file (in the hack-local-variables function in files.el). In this syntax, each line must begin and end with the same prefix/suffix pair. That pair is defined by the text before and after the "Local Variables:" string.

  Vim modelines

  In vim, this is called "modelines" and should match the following pattern:

  [text]{white}{vi:|vim:|ex:}[white]se[t] {options}:[text]

  The vim modeline must be within N lines of the top or bottom of the file. That N is user-settable, but defaults to 5. To learn more type ":help modelines" in vim.

  Kate modelines

  I also discovered that Kate supports per-file modelines:

  http://kate-editor.org/article/katepart_modelines

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

• ValuesAndExpressions::ProhibitHereDocs

• ValuesAndExpressions::ProhibitLongStrings

  Low severity.

  Both of these attempt to address problems with code layout and appearance. Large blocks of inline text can disrupt the readability of code. Instead, the text should be external, in __DATA__, or simply declared in separate functions at the end of the module.

  Exceptions: if the only code in a sub is a return of a long string, allow it. If there is a use Inline:: at the top of the module, allow HereDocs.

  http://rt.cpan.org/Ticket/Display.html?id=20714

• CodeLayout::ProhibitTrailingSpaces

  Forbid [ \t] before \n. This is a subset of RequireTidyCode.

  http://rt.cpan.org/Ticket/Display.html?id=20714

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)

• File::RequirePortableName

  No spaces, punctuation, etc.

PPI BUGS

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

literal()

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

Newlines

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.

Anonymous constructors in lists

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