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