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