vcs.maemo.org Git - dh-make-perl/blob - dev/i386/libperl-critic-perl/libperl-critic-perl-1.088/TODO.pod

   1 # best viewed via "perldoc TODO.pod"
   2
   3 =pod
   4
   5 =for stopwords LHS RHS REFACTORINGS FH SVN stopwords
   6
   7 =head1 NAME
   8
   9 Perl::Critic::TODO - Things for Perl::Critic developers to do
  10
  11
  12 =head1 SOURCE
  13
  14     #######################################################################
  15     #      $URL: http://perlcritic.tigris.org/svn/perlcritic/trunk/Perl-Critic/TODO.pod $
  16     #     $Date: 2008-07-03 09:36:05 -0500 (Thu, 03 Jul 2008) $
  17     #   $Author: clonezone $
  18     # $Revision: 2487 $
  19     #######################################################################
  20
  21
  22 =head1 SEE ALSO
  23
  24 Perl-Critic-More is a separate distribution for less-widely-accepted
  25 policies.  It contains its own TODO.pod.
  26
  27
  28 =head1 NEW FEATURES
  29
  30 =over
  31
  32 =item * Report Safari sections in addition to book page numbers.
  33
  34
  35 =item * Add --files-with-violations/-l and --files-without-violations/-L options to F<perlcritic>.
  36
  37 Just print out file names.  I could have used this at work when combined with
  38 C<--single-policy>.
  39
  40     gvim `perlcritic --single-policy QuotedWordLists -l`
  41
  42
  43 =item * Add a file Behavior.
  44
  45
  46 =item * Allow values of (at least) string-list Parameters to be specified in a file.
  47
  48 For the benefit of PodSpelling, etc.
  49
  50
  51 =item * Enhance string-list Behavior to allow specification of delimiters.
  52
  53 For things like RequirePodSections.
  54
  55
  56 =item * Add queries to --list option to F<perlcritic>.
  57
  58 List Policies based upon severity, theme, and (what I want this second)
  59 applies_to.
  60
  61 =item * Add --prohibit-unrestricted-no-critic option to F<perlcritic>.
  62
  63 Requires C<## no critic> to take an argument:
  64
  65   ## no critic (SomePolicyPattern)     # ok
  66   ## no critic                         # not ok
  67
  68 Can't be done as a regular Policy because any line that violated it would disable it.
  69
  70
  71 =item * Support for C<#line 123 "filename"> directives.
  72
  73 For code generators and template languages that allow inline Perl code.
  74
  75 Yes, somebody has an in-house templating system where they've written a custom
  76 test module that extracts the perl code from a template and critiques it.
  77
  78 Actually, this would be useful for programs: Module::Build "fixes" shebang
  79 lines so that there's the bit about invoking perl if the program is attempted
  80 to be run by a Bourne shell, which throws the line numbers off when using
  81 Test::P::C on the contents of a C<blib> directory.
  82
  83
  84 =item * Enhance statistics.
  85
  86 - Blank line count
  87
  88 - POD line count
  89
  90 - Comment line count
  91
  92 - Data section count
  93
  94
  95 =item * Detect 5.10 source and enable stuff for that.
  96
  97 For example, treat C<say> as equivalent to C<print>.
  98
  99
 100 =item * Support a means of failing if a Policy isn't installed.
 101
 102 For example, the self compliance test now depends upon a Policy in the More
 103 distribution.
 104
 105 Something like using a "+" sign in front of the Policy name in its
 106 configuration block, analogous to the "-" sign used for disabling a policy,
 107 e.g. "C<[+Example::Policy]>".
 108
 109
 110 =back
 111
 112
 113 =head1 BUGS/LIMITATIONS
 114
 115 =over
 116
 117 =item * NamingConventions::ProhibitAmbiguousNames
 118
 119 Don't allow compound names with forbidden words, like "last_record".
 120 Allow forbidden words in RHS of variable declarations
 121
 122 Also, we should make it easeir to add (or delete) words from the
 123 forbbiden list.
 124
 125
 126 =item * Subroutines::ProtectPrivateSubs
 127
 128 Doesn't forbid C<< $pkg->_foo() >> because it can't tell the
 129 difference between that and C<< $self->_foo() >>
 130
 131
 132 =item * ErrorHandling::RequireCarping
 133
 134 This should not complain about using C<warn> or C<die> if it's not in a
 135 function, or if it's in main::.
 136
 137 Also, should allow C<die> when it is obvious that the "message" is a reference.
 138
 139
 140 =item * RegularExpressions::ProhibitCaptureWithoutTest
 141
 142 Allow this construct:
 143
 144     for ( ... ) {
 145         next unless /(....)/;
 146         if ( $1 ) {
 147             ....
 148         }
 149     }
 150
 151 Right now, P::C thinks that the C<$1> isn't legal to use because it's
 152 "outside" of the match.  The thing is, we can only get to the C<if>
 153 if the regex matched.
 154     while ( $str =~ /(expression)/ )
 155
 156
 157 =item * CodeLayout::ProhibitParensWithBuiltins
 158
 159 Some builtin functions (particularly those that take a variable number of
 160 scalar arguments) should probably get parentheses.  This policy should be
 161 enhanced to allow the user to specify a list of builtins that are expempt
 162 from the policy.
 163
 164
 165 =item * ValuesAndExpressions::ProhibitCommaSeparatedStatements
 166
 167 Needs to check for C<scalar( something, something )>.
 168
 169
 170 =item * Variables::ProhibitPunctuationVars
 171
 172 Needs to look inside strings.  RT #35970.
 173
 174
 175 =item * TestingAndDebugging::RequireUseWarnings
 176
 177 Check for -w on the shbang line.
 178
 179
 180 =item * Change formatting in Violation to eliminate double periods.
 181
 182
 183 =back
 184
 185
 186 =head1 OTHER PBP POLICIES THAT SEEM FEASIBLE TO IMPLEMENT
 187
 188 =over
 189
 190 =item * Modules::RequireUseVersion [405-406]
 191
 192 =item * Modules::RequireThreePartVersion [405-406]
 193
 194 =item * RegularExpressions::RequireDotMatchAnything [240-241]
 195
 196 =back
 197
 198
 199 =head1 NON-PBP POLICIES WANTED
 200
 201 =over
 202
 203 =item * Documentation::RequireModuleAbstract
 204
 205 Require a C<=head1 NAME> POD section with content that matches
 206 C<\A \s* [\w:]+ \s+ - \s+ \S>.  The single hyphen is the important bit.  Also,
 207 must be a single line.
 208
 209 =item * Expressions::RequireFatCommasInHashConstructors
 210
 211 =item * ErrorHandling::RequireLocalizingEvalErrorInDESTROY
 212
 213 Prevent C<$@> from being cleared unexpectedly by DESTROY methods.
 214
 215     package Foo;
 216
 217     sub DESTROY {
 218         die "Died in Foo::DESTROY()";
 219     }
 220
 221     package main;
 222
 223     eval {
 224         my $foo = Foo->new();
 225
 226         die "Died in eval."
 227     }
 228     print $@;   # "Died in Foo::DESTROY()", not "Died in eval.".
 229
 230 See L<http://use.perl.org/~Ovid/journal/36767>.
 231
 232 =item * Expressions::ProhibitDecimalWithBitwiseOperator
 233
 234 =item * Expressions::ProhibitStringsWithBitwiseOperator
 235
 236
 237 =item * InputOutput::ProhibitMagicDiamond
 238
 239 Steal the idea from L<B::Lint>.
 240
 241
 242 =item * TBD::AllProgramsNeedShebangs
 243
 244 Anything that is a program should have a shebang line.  This includes .t files.
 245
 246
 247 =item * BuiltInFunctions::RequireConstantSprintfFormat
 248
 249
 250 =item * BuiltInFunctions::RequireConstantUnpackFormat
 251
 252 L<http://diotalevi.isa-geek.net/~josh/yapc-lint/slides/slide5.html>
 253
 254
 255 =item * Miscellanea::ProhibitObnoxiousComments
 256
 257 Forbid excessive hash marks e.g. "#### This is a loud comment ####".
 258 Make the obnoxious pattern configurable
 259
 260
 261 =item * ValuesAndExpressions::RequireNotOperator
 262
 263 Require the use of "not" instead of "!", except when this would contradict
 264 ProhibitMixedBooleanOperators.  This may be better suited for
 265 Perl::Critic::More.
 266
 267
 268 =item * Modules::RequireExplicitImporting
 269
 270 Require every C<use> statement to have an explicit import list.  You could
 271 still get around this by calling C<import> directly.
 272
 273
 274 =item * Modules::ForbidImporting
 275
 276 Require every C<use> to have an explicitly empty import list.  This is for
 277 folks who like to see fully-qualified function names.  Should probably provide
 278 a list of exempt modules (like FindBin);
 279
 280
 281 =item * ControlStructures::ProhibitIncludeViaDo
 282
 283 Forbid C<do "foo.pl">.  Not sure about this policy name.
 284
 285
 286 =item * Variables::ProhibitUseVars
 287
 288 Disallow C<use vars qw(...)> and require C<our $foo> instead.  This
 289 contradicts Miscellanea::Prohibit5006isms.  Maybe verify C<use 5.6>
 290 before applying this policy.  Low severity.
 291
 292
 293 =item * VariablesAndExpressions::ProhibitQuotedHashKeys
 294
 295 Forbid quotes around hash keys, unless they are really needed.  This
 296 is against what Damian says.  Suggested by Adam Kennedy.  Low
 297 severity.
 298
 299
 300 =item * CodeLayout::ProhibitFunctionalNew
 301
 302 Good: C<< Foo::Bar->new >>, Bad: C<< new Foo::Bar >>
 303
 304
 305 =item * RegularExpressions::ProhibitSWSWSW
 306
 307 Require C<split> instead of C<m/\s*\w*\s*\w*\s*/>.  From MJD's Red Flags.
 308
 309
 310 =item * VariablesAndExpressions::RequireConstantVersion (low severity)
 311
 312
 313 =item * VariablesAndExpressions::ProhibitComplexVersion (medium severity)
 314
 315 L<http://rt.cpan.org/Ticket/Display.html?id=20439>
 316
 317
 318 =item * Documentation::RequireSynopsis
 319
 320
 321 =item * Documentation::RequireLicense
 322
 323 These are simplified versions of Documentation::RequirePodSections.
 324
 325
 326 =item * Documentation::RequireValidSynopsis
 327
 328 The Synopsis section must be all indented and must be syntactically valid Perl
 329 (as validated by PPI).
 330
 331
 332 =item * Documentation::ProhibitEmptySections
 333
 334 Any C<=headN> and C<=over> sections must not be empty.  This helps catch
 335 boilerplate (althought Test::Pod should catch empty C<=over> blocks).
 336
 337 On the other hand, C<=item ...> sections can be empty, since the item label is
 338 content.
 339
 340
 341 =item * Miscellaneous::ProhibitBoilerplate
 342
 343 Complain about copy-and-paste code or docs from h2xs, Module::Starter::*,
 344 etc.
 345
 346 Here's a non-PPI implementation:
 347 L<http://search.cpan.org/src/JJORE/Carp-Clan-5.8/t/04boilerplate.t>
 348
 349
 350 =item * BuiltinFunctions::ProhibitExtraneousScalarCall
 351
 352 Recommend that C<if (scalar @array)> be rewritten as C<if (@array)>.
 353
 354
 355 =item * RegularExpressions::ProhibitMixedDelimiters
 356
 357 Ban s{foo}(bar)
 358
 359
 360 =item * RegularExpressions::ProhibitScalarAsRegexp
 361
 362 Ban naked srtings as regexps, like:
 363
 364     print 1 if $str =~ $regexp;
 365
 366 Instead, it should be:
 367
 368     print 1 if $str =~ m/$regexp/;
 369
 370 or
 371
 372     print 1 if $str =~ m/$regexp/xms;
 373
 374
 375 =item * ValuesAndExpressions::RequireInterpolatedStringyEval
 376
 377 Ensure that the argument to a stringy eval is not a constant string.  That's
 378 just wasteful.  Real world examples include:
 379
 380   eval 'use Optional::Module';
 381
 382 which is better written as
 383
 384   eval { require Optional::Module; Optional::Module->import };
 385
 386 for performance gains and compile-time syntax checking.
 387
 388
 389 =item * RegularExpressions::ProhibitUnnecessaryEscapes
 390
 391 Complain if user puts a backslash escape in front of non-special characters.  For example:
 392
 393    m/\!/;
 394
 395 Make exceptions for C<\">, C<\'> and C<\`> since those are often inserted to
 396 workaround bugs in syntax highlighting.
 397
 398 Note that this is different inside character classes, where only C<^>, C<]>
 399 and C<-> need to be escaped, I think.  Caret only needs to be escaped at the
 400 beginning, and dash does NOT need to be escaped at the beginning and end.  See
 401 L<perlreref>.
 402
 403
 404 =item * Steal ideas from L<Dunce::Files>.
 405
 406 Can someone expand this entry, please?
 407
 408 =item * ControlStructures::ProhibitAssigmentInConditional
 409
 410 =item * ValuesAndExpressions::RequireConstantBeforeEquals
 411
 412 =item * ValuesAndExpressions::RequireConstantBeforeOperator
 413
 414 L<http://use.perl.org/~stu42j/journal/36412>
 415
 416 Just about everyone has been bitten by C<if ($x = 10) { ... }> when they meant
 417 to use C<==>.  A safer style is C<10 == $x> because omitting the second C<=>
 418 yields a noisy compile-time failure instead of silent runtime error.
 419
 420 ProhibitAssigmentInConditional complains if the condition of a while, until,
 421 if or unless is solely an assignment.  If it's anything more complex (like
 422 C<if (($x=10)){}> or C<while ($x=$y=$z){}>), there is no warning.
 423
 424 RequireConstantBeforeEquals complains if the left side of an C<==> is a
 425 variable while the right side is a constant.
 426
 427 RequireConstantBeforeOperator complains if the left side of any comparison
 428 operator (C<==>, C<eq>, C<&lt;>, etc) is a variable while the right side is a
 429 constant.
 430
 431
 432 =item * InputOutput::ProhibitUTF8IOLayer
 433
 434 http://www.perlfoundation.org/perl5/index.cgi?the_utf8_perlio_layer
 435
 436 =item * BuiltinFunctions::ProhibitExit(?:InModules)?
 437
 438 Forbid C<exit()> in files that lack a shebang.  Inspired by
 439 L<http://use.perl.org/~Ovid/journal/36746> and an analgous checker in
 440 FindBugs.
 441
 442 =back
 443
 444
 445 =head1 REFACTORINGS and ENHANCEMENTS
 446
 447 =over
 448
 449 =item * Create constants for the PPI location array elements.
 450
 451
 452 =item * MOVE THE LINE-DISABLING INTO P::C::Document
 453
 454 All the code that deals with finding all the '##no critic' comments and noting
 455 which policies are disabled at each line seems like it would be better placed
 456 in Perl::Critic::Document.  P::C::Document could then provide methods to
 457 indicate if a policy is disabled at a particular line.  So the basic algorithm
 458 in Perl::Critic might look something like this:
 459
 460   foreach $element (@PPI_ELEMENTS) {
 461      foreach $policy (@POLICIES) {
 462         $line = $element->location->[0];
 463         next if $doc->policy_is_disabled_at_line( $policy, $line );
 464         push @violations, $policy->violates( $elem, $doc );
 465      }
 466   }
 467
 468
 469 =item * Some means of detecting "runnaway" C<##no critic>
 470
 471 Elliot was talking to a couple of users at ETech and one of their major
 472 concerns was that they were using C<##no critic> and forgetting to do a
 473 C<##use critic> after the problematic section.  Perhaps an option to
 474 F<perlcritic> to scan for such things is in order.
 475
 476
 477 =item * Change API to use named parameters
 478
 479 Most of the methods on the public classes use named parameters for passing
 480 arguments.  I'd like to extend that pattern to include all object-methods.
 481 Static methods can still use positional parameters.
 482
 483
 484 =item * Enhance P::C::critique() to accept files, directories, or code strings
 485
 486 Just like F<bin/perlcritic> does now.
 487
 488
 489 =item * Add C<-cache> flag to F<bin/perlcritic>
 490
 491 If enabled, this turns on L<PPI::Cache>:
 492
 493     require PPI::Cache;
 494     my $cache_path = "/tmp/test-perl-critic-cache-$ENV{USER}";
 495     mkdir $cache_path, oct 700 if (! -d $cache_path);
 496     PPI::Cache->import(path => $cache_path);
 497
 498 This cachedir should perhaps include the PPI version number!  At least
 499 until PPI incorporates its own version number in the cache.
 500
 501 (see F<t/40_criticize.t> for a more robust implementation)
 502
 503
 504 =item * Use hash-lookup instead of C<List::MoreUtils::any> function.
 505
 506 In several places, Perl::Critic uses C<List::MoreUtils::any> to see if
 507 a string is a member of a list.  Instead, I suggest using a named
 508 subroutine that does a hash-lookup like this:
 509
 510     my %logical_ops = hashify( qw( ! || && ||= &&= and or not ) );
 511     sub is_logical_op { return exists $logical_ops{ $_[0] }; }
 512
 513 Why?
 514
 515
 516 =item * Allow color output to work through a pipe.
 517
 518 http://rt.cpan.org/Ticket/Display.html?id=30140
 519
 520 F<ack> now supports this.
 521
 522
 523 =back
 524
 525 =head1 PPI BUGS
 526
 527 We're waiting on the following bugs to get fixed in a CPAN release of PPI:
 528
 529
 530 =over
 531
 532 =item PPI::Token::descendant_of()
 533
 534 Exists in svn.  Replace _descendant_of() in RequireCheckingReturnValueOfEval
 535 with that, once it is released, because it's faster and native.
 536
 537 =item Newlines
 538
 539 PPI does not preserve newlines.  That makes
 540 CodeLayout::RequireConsistentNewlines impossible to implement under PPI.  For
 541 now, it's implemented by pulling the source out of the file and skipping PPI.
 542
 543 It's unlikely that PPI will support mixde newlines anytime soon.
 544
 545
 546 =item Operators
 547
 548 ValuesAndExpressions::ProhibitMismatchedOperators has two workarounds
 549 for PPI bugs with parsing operators.  Many of these bugs have been
 550 fixed in PPI, so it would be good to check if those workarounds are
 551 still needed.
 552
 553
 554 =item Regexp methods
 555
 556 Not strictly a bug -- the PPI Regexp classes have a dearth of accessor methods
 557 as of v1.118, meaning that we have to do messy digging into internals.  I
 558 wrote Perl::Critic:Utils::PPIRegexp to encapsulate this messiness, but it
 559 would be nicer to have an official interface in PPI.
 560
 561
 562 =back
 563
 564 =cut
 565
 566 ##############################################################################
 567 # Local Variables:
 568 #   mode: cperl
 569 #   cperl-indent-level: 4
 570 #   fill-column: 78
 571 #   indent-tabs-mode: nil
 572 #   c-indentation-style: bsd
 573 # End:
 574 # ex: set ts=8 sts=4 sw=4 tw=78 ft=pod expandtab shiftround :