NAME

TODO - Things for Perl::Critic developers to do

SOURCE

    #######################################################################
    #      $URL: http://perlcritic.tigris.org/svn/perlcritic/trunk/Perl-Critic/TODO.pod $
    #     $Date: 2007-01-19 20:58:44 -0800 (Fri, 19 Jan 2007) $
    #   $Author: thaljef $
    # $Revision: 1158 $
    #######################################################################

SEE ALSO

Perl-Critic-More is a separate distribution for less-widely-accepted policies. It contains its own TODO.pod.

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.

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

BUGS/LIMITATIONS

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

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

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

• Documentation::RequirePodSections

  Need to make this configurable to conform to either the book or his Module::Starter::PBP, since Damian's apparently changed his mind.

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

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;

  OTOH, this assumes that the common use of scalar grep is to check for existence. I'm not sure that's the case.

• Variables::RequireLocalizedPunctuationVars (p81)

• 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. Allow an exception for close and print.

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

  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.

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

NON-PBP POLICIES WANTED

• 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://home.earthlink.net/~josh.jore/new-warnings/slides/slide1.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.

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

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

• 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

• Variables::ProhibitPerl4PackageNames

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

• 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

• CodeLayout::ProhibitTrailingSpaces

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

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

• BuiltinFunctions::ProhibitExtraneousScalarCall

  Recommend that if (scalar @array) be rewritten as if (@array).

REFACTORINGS and ENHANCEMENTS

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

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

• Enhance P::C::critique() to accept files, directories, or code strings

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

  This cachedir should perhaps include the PPI version number! At least until PPI incorporates it's 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.

  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] }; }

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

Hash constructors with a parenthesis directly to the left.

The PPI::Statement surrounding the PPI::Constructor returns undef for location() for the following:

  ({})

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.

Operators

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.