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