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