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