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