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