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