Login
f57d8d996c54fa8bc530a20f0b6893ea492d4d2d
[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 * Refactor guts of F<perlcritic> into L<Perl::Critic::CLI>
445
446 So the F<perlcritic> script would basically just say...
447
448   use Perl::Critic::CLI;
449   exit Perl::Critic::CLI->run(@ARGV);
450
451 This would make the stuff that is currently inside F<perlcritic>
452 easier to test.  Also would open the door for developers to
453 subclass and extend F<perlcritic>, if they had some reason
454 to do so.  Also, just cuz it feels like the right thing todo.
455
456 =item * Create constants for the PPI location array elements.
457
458 =item * Some means of detecting "runnaway" C<##no critic>
459
460 Elliot was talking to a couple of users at ETech and one of their major
461 concerns was that they were using C<##no critic> and forgetting to do a
462 C<##use critic> after the problematic section.  Perhaps an option to
463 F<perlcritic> to scan for such things is in order.
464
465
466 =item * Change API to use named parameters
467
468 Most of the methods on the public classes use named parameters for passing
469 arguments.  I'd like to extend that pattern to include all object-methods.
470 Static methods can still use positional parameters.
471
472
473 =item * Enhance P::C::critique() to accept files, directories, or code strings
474
475 Just like F<bin/perlcritic> does now.
476
477
478 =item * Add C<-cache> flag to F<bin/perlcritic>
479
480 If enabled, this turns on L<PPI::Cache|PPI::Cache>:
481
482     require PPI::Cache;
483     my $cache_path = "/tmp/test-perl-critic-cache-$ENV{USER}";
484     mkdir $cache_path, oct 700 if (! -d $cache_path);
485     PPI::Cache->import(path => $cache_path);
486
487 This cachedir should perhaps include the PPI version number!  At least until
488 PPI incorporates its own version number in the cache.
489
490 (see F<t/40_criticize.t> for a more robust implementation)
491
492
493 =item * Use hash-lookup instead of C<List::MoreUtils::any> function.
494
495 In several places, Perl::Critic uses C<List::MoreUtils::any> to see if a
496 string is a member of a list.  Instead, I suggest using a named subroutine
497 that does a hash-lookup like this:
498
499     my %logical_ops = hashify( qw( ! || && ||= &&= and or not ) );
500     sub is_logical_op { return exists $logical_ops{ $_[0] }; }
501
502 Question: Why?
503
504 Answer: Readability, mostly.  Performance, maybe.
505
506 =item * Refactor guts of perlcritic into a module (Perl::Critic::App ??)
507
508 Because it is getting unwieldy in there.
509
510 =back
511
512 =head1 PPI BUGS
513
514 We're waiting on the following bugs to get fixed in a CPAN release of PPI:
515
516
517 =over
518
519 =item PPI::Token::descendant_of()
520
521 Exists in svn.  Replace _descendant_of() in RequireCheckingReturnValueOfEval
522 with that, once it is released, because it's faster and native.
523
524 =item Newlines
525
526 PPI does not preserve newlines.  That makes
527 CodeLayout::RequireConsistentNewlines impossible to implement under PPI.  For
528 now, it's implemented by pulling the source out of the file and skipping PPI.
529
530 It's unlikely that PPI will support mixde newlines anytime soon.
531
532
533 =item Operators
534
535 ValuesAndExpressions::ProhibitMismatchedOperators has two workarounds for PPI
536 bugs with parsing operators.  Many of these bugs have been fixed in PPI, so it
537 would be good to check if those workarounds are still needed.
538
539
540 =item Regexp methods
541
542 Not strictly a bug -- the PPI Regexp classes have a dearth of accessor methods
543 as of v1.118, meaning that we have to do messy digging into internals.  I
544 wrote Perl::Critic:Utils::PPIRegexp to encapsulate this messiness, but it
545 would be nicer to have an official interface in PPI.
546
547
548 =item QuoteLike::Words in the place of a ForLoop
549
550 PPI mis-parses C<<for qw<blah> {}>>.
551
552
553 =back
554
555 =cut
556
557 ##############################################################################
558 # Local Variables:
559 #   mode: cperl
560 #   cperl-indent-level: 4
561 #   fill-column: 78
562 #   indent-tabs-mode: nil
563 #   c-indentation-style: bsd
564 # End:
565 # ex: set ts=8 sts=4 sw=4 tw=78 ft=pod expandtab shiftround :