Login
Fix date prototype at the top of Changes. Yeah, it annoyed me. :]
[gknop/Perl-Critic.git] / TODO.pod
CommitLineData
9b8f64cf
CD
1# best viewed via "perldoc TODO.pod"
2
501ee198
JRT
3=pod
4
a73f4a71 5=for stopwords LHS RHS REFACTORINGS FH SVN stopwords
501ee198 6
dae851e8
JRT
7=head1 NAME
8
369abea9 9Perl::Critic::TODO - Things for Perl::Critic developers to do
dae851e8
JRT
10
11=head1 SOURCE
daad783f
AL
12
13 #######################################################################
14 # $URL$
15 # $Date$
16 # $Author$
17 # $Revision$
18 #######################################################################
19
2b7b1533 20
de8cacdb
CD
21=head1 SEE ALSO
22
23Perl-Critic-More is a separate distribution for less-widely-accepted
ab1cd5d7 24policies. It contains its own TODO.pod.
de8cacdb 25
2b7b1533 26
daad783f
AL
27=head1 NEW FEATURES
28
2b7b1533 29=over
daad783f 30
8b5892fc 31=item * Report Safari sections in addition to book page numbers.
9bc546d7 32
2b7b1533 33
515a4fff
ES
34=item * Add --files-with-violations/-l and --files-without-violations/-L options to F<perlcritic>.
35
36Just print out file names. I could have used this at work when combined with
37C<--single-policy>.
38
39 gvim `perlcritic --single-policy QuotedWordLists -l`
40
41
0fe898d5
ES
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
47For the benefit of PodSpelling, etc.
48
49
50=item * Enhance string-list Behavior to allow specification of delimiters.
51
52For things like RequirePodSections.
53
54
afceaa1d
ES
55=item * Add queries to --list option to F<perlcritic>.
56
57List Policies based upon severity, theme, and (what I want this second)
58applies_to.
59
a9243c21
ES
60=item * Add formatting of --list output.
61
62Support Jeff Bisbee's use case (he dumps all the policies in severity order
63with full descriptions and other metadata).
64
d0e776fc
ES
65=item * Add --prohibit-unrestricted-no-critic option to F<perlcritic>.
66
67Requires C<## no critic> to take an argument:
68
69 ## no critic (SomePolicyPattern) # ok
70 ## no critic # not ok
71
11f53956
ES
72Can't be done as a regular Policy because any line that violated it would
73disable it.
d0e776fc 74
2b7b1533 75
ad5f2c12
ES
76=item * Support for C<#line 123 "filename"> directives.
77
78For code generators and template languages that allow inline Perl code.
79
96fa2db5
ES
80Yes, somebody has an in-house templating system where they've written a custom
81test module that extracts the perl code from a template and critiques it.
82
1d077d29
ES
83Actually, this would be useful for programs: Module::Build "fixes" shebang
84lines so that there's the bit about invoking perl if the program is attempted
85to be run by a Bourne shell, which throws the line numbers off when using
86Test::P::C on the contents of a C<blib> directory.
87
a6539177
ES
88This actually requires support from PPI.
89
2b7b1533 90
a889d362
ES
91=item * Enhance statistics.
92
93- Blank line count
94
95- POD line count
96
97- Comment line count
98
99- Data section count
100
a889d362 101
afb00bb2
ES
102=item * Detect 5.10 source and enable stuff for that.
103
104For example, treat C<say> as equivalent to C<print>.
105
106
1d077d29
ES
107=item * Support a means of failing if a Policy isn't installed.
108
109For example, the self compliance test now depends upon a Policy in the More
110distribution.
111
112Something like using a "+" sign in front of the Policy name in its
113configuration block, analogous to the "-" sign used for disabling a policy,
114e.g. "C<[+Example::Policy]>".
115
116
a6539177
ES
117=item * Add support in .run files for regexes for violation descriptions.
118
119
d1d6429f
AL
120=back
121
2b7b1533 122
daad783f
AL
123=head1 BUGS/LIMITATIONS
124
2b7b1533 125=over
daad783f 126
bd1718ea
ES
127=item * ControlStructures::ProhibitPostfixControls
128
129Look for the do {} while case and change the explanation to point to page 123
2e69f9b8 130when it is found. RT #37905.
bd1718ea
ES
131
132
daad783f
AL
133=item * NamingConventions::ProhibitAmbiguousNames
134
135Don't allow compound names with forbidden words, like "last_record".
136Allow forbidden words in RHS of variable declarations
137
fd5bd7b5
JRT
138Also, we should make it easeir to add (or delete) words from the
139forbbiden list.
140
2b7b1533 141
daad783f
AL
142=item * Subroutines::ProtectPrivateSubs
143
144Doesn't forbid C<< $pkg->_foo() >> because it can't tell the
145difference between that and C<< $self->_foo() >>
146
2b7b1533 147
d1d6429f
AL
148=item * ErrorHandling::RequireCarping
149
63124782 150This should not complain about using C<warn> or C<die> if it's not in a
dd813c73 151function, or if it's in main::.
d1d6429f 152
11f53956
ES
153Also, should allow C<die> when it is obvious that the "message" is a
154reference.
5c77583a 155
2b7b1533 156
4c542233
AL
157=item * RegularExpressions::ProhibitCaptureWithoutTest
158
159Allow this construct:
160
161 for ( ... ) {
162 next unless /(....)/;
163 if ( $1 ) {
164 ....
165 }
166 }
167
168Right now, P::C thinks that the C<$1> isn't legal to use because it's
169"outside" of the match. The thing is, we can only get to the C<if>
170if the regex matched.
11f53956 171
dfe2eb3f 172 while ( $str =~ /(expression)/ )
4c542233 173
2b7b1533 174
d9abc689
JRT
175=item * CodeLayout::ProhibitParensWithBuiltins
176
177Some builtin functions (particularly those that take a variable number of
c296c678
ES
178scalar arguments) should probably get parentheses. This policy should be
179enhanced to allow the user to specify a list of builtins that are expempt
180from the policy.
d9abc689 181
2b7b1533 182
c59ac28c
ES
183=item * ValuesAndExpressions::ProhibitCommaSeparatedStatements
184
185Needs to check for C<scalar( something, something )>.
186
187
326d1973
ES
188=item * Variables::ProhibitPunctuationVars
189
190Needs to look inside strings. RT #35970.
191
192
96280e8c
ES
193=item * TestingAndDebugging::RequireUseWarnings
194
195Check for -w on the shbang line.
196
2b7b1533 197
21bee11d
ES
198=item * Change formatting in Violation to eliminate double periods.
199
200
365e3d9d
AL
201=back
202
2b7b1533 203
daad783f
AL
204=head1 OTHER PBP POLICIES THAT SEEM FEASIBLE TO IMPLEMENT
205
5331273a
ES
206=over
207
208=item * Modules::RequireUseVersion [405-406]
209
210=item * Modules::RequireThreePartVersion [405-406]
211
212=back
daad783f 213
2b7b1533 214
daad783f
AL
215=head1 NON-PBP POLICIES WANTED
216
2b7b1533 217=over
daad783f 218
0d71785c
JRT
219=item * NamingConventions::ProhibitMisspelledSymbolNames
220
221The idea behind this policy is to encourage better names for variables
222and subroutines by enforcing correct spelling and prohibiting the use of
223home-grown abbreviations. Assumng that the author uses underscores or
224camel-case, it should be possible to split symbols into words, and then look
225them up in a dictionary (see PodSpelling). This policy should probably have
226a similar stopwords feature as well.
227
45136ef4
ES
228=item * Documentation::RequireModuleAbstract
229
230Require a C<=head1 NAME> POD section with content that matches
231C<\A \s* [\w:]+ \s+ - \s+ \S>. The single hyphen is the important bit. Also,
232must be a single line.
233
be48b0c6
ES
234=item * Expressions::RequireFatCommasInHashConstructors
235
f3bcb2ca
ES
236=item * ErrorHandling::RequireLocalizingEvalErrorInDESTROY
237
238Prevent C<$@> from being cleared unexpectedly by DESTROY methods.
239
240 package Foo;
241
242 sub DESTROY {
243 die "Died in Foo::DESTROY()";
244 }
245
246 package main;
247
248 eval {
249 my $foo = Foo->new();
250
251 die "Died in eval."
252 }
253 print $@; # "Died in Foo::DESTROY()", not "Died in eval.".
254
effbbbb3
ES
255See L<http://use.perl.org/~Ovid/journal/36767>.
256
77fa21bd
ES
257=item * Expressions::ProhibitDecimalWithBitwiseOperator
258
259=item * Expressions::ProhibitStringsWithBitwiseOperator
260
2b7b1533 261
3264a614
ES
262=item * InputOutput::ProhibitMagicDiamond
263
11f53956 264Steal the idea from L<B::Lint|B::Lint>.
3264a614
ES
265
266
df88f751 267=item * TBD::AllProgramsNeedShebangs
d92d2828 268
11f53956
ES
269Anything that is a program should have a shebang line. This includes .t
270files.
d92d2828 271
2b7b1533 272
daad783f
AL
273=item * BuiltInFunctions::RequireConstantSprintfFormat
274
2b7b1533 275
daad783f
AL
276=item * BuiltInFunctions::RequireConstantUnpackFormat
277
1cc04072 278L<http://diotalevi.isa-geek.net/~josh/yapc-lint/slides/slide5.html>
daad783f 279
2b7b1533 280
dfe2eb3f
JRT
281=item * Miscellanea::ProhibitObnoxiousComments
282
283Forbid excessive hash marks e.g. "#### This is a loud comment ####".
284Make the obnoxious pattern configurable
285
2b7b1533 286
dfe2eb3f
JRT
287=item * ValuesAndExpressions::RequireNotOperator
288
289Require the use of "not" instead of "!", except when this would contradict
290ProhibitMixedBooleanOperators. This may be better suited for
291Perl::Critic::More.
292
2b7b1533 293
dfe2eb3f
JRT
294=item * Modules::RequireExplicitImporting
295
296Require every C<use> statement to have an explicit import list. You could
297still get around this by calling C<import> directly.
298
2b7b1533 299
dfe2eb3f
JRT
300=item * Modules::ForbidImporting
301
302Require every C<use> to have an explicitly empty import list. This is for
303folks who like to see fully-qualified function names. Should probably provide
304a list of exempt modules (like FindBin);
305
2b7b1533 306
daad783f
AL
307=item * ControlStructures::ProhibitIncludeViaDo
308
309Forbid C<do "foo.pl">. Not sure about this policy name.
310
2b7b1533 311
daad783f
AL
312=item * Variables::ProhibitUseVars
313
f707b14a 314Disallow C<use vars qw(...)> and require C<our $foo> instead. This
11f53956
ES
315contradicts Miscellanea::Prohibit5006isms. Maybe verify C<use 5.6> before
316applying this policy. Low severity.
daad783f 317
2b7b1533 318
daad783f
AL
319=item * VariablesAndExpressions::ProhibitQuotedHashKeys
320
11f53956
ES
321Forbid quotes around hash keys, unless they are really needed. This is
322against what Damian says. Suggested by Adam Kennedy. Low severity.
daad783f 323
2b7b1533 324
b589a229 325=item * CodeLayout::ProhibitFunctionalNew
daad783f
AL
326
327Good: C<< Foo::Bar->new >>, Bad: C<< new Foo::Bar >>
328
2b7b1533 329
7eec5b31
ES
330=item * RegularExpressions::ProhibitSWSWSW
331
332Require C<split> instead of C<m/\s*\w*\s*\w*\s*/>. From MJD's Red Flags.
333
334
daad783f
AL
335=item * VariablesAndExpressions::RequireConstantVersion (low severity)
336
2b7b1533 337
f707b14a
CD
338=item * VariablesAndExpressions::ProhibitComplexVersion (medium severity)
339
daad783f
AL
340L<http://rt.cpan.org/Ticket/Display.html?id=20439>
341
2b7b1533 342
79b72614
CD
343=item * Documentation::RequireSynopsis
344
2b7b1533 345
79b72614
CD
346=item * Documentation::RequireLicense
347
348These are simplified versions of Documentation::RequirePodSections.
349
2b7b1533 350
6a4d1045
CD
351=item * Documentation::RequireValidSynopsis
352
353The Synopsis section must be all indented and must be syntactically valid Perl
354(as validated by PPI).
355
2b7b1533 356
90c0067c
CD
357=item * Documentation::ProhibitEmptySections
358
359Any C<=headN> and C<=over> sections must not be empty. This helps catch
360boilerplate (althought Test::Pod should catch empty C<=over> blocks).
361
362On the other hand, C<=item ...> sections can be empty, since the item label is
363content.
364
2b7b1533 365
79b72614
CD
366=item * Miscellaneous::ProhibitBoilerplate
367
11f53956 368Complain about copy-and-paste code or docs from h2xs, Module::Starter::*, etc.
c6e7b236 369
ef919624
CD
370Here's a non-PPI implementation:
371L<http://search.cpan.org/src/JJORE/Carp-Clan-5.8/t/04boilerplate.t>
372
2b7b1533 373
956825ec
CD
374=item * BuiltinFunctions::ProhibitExtraneousScalarCall
375
376Recommend that C<if (scalar @array)> be rewritten as C<if (@array)>.
377
2b7b1533 378
6a4d1045
CD
379=item * RegularExpressions::ProhibitMixedDelimiters
380
381Ban s{foo}(bar)
382
2b7b1533 383
c79c8b5b
CD
384=item * RegularExpressions::ProhibitScalarAsRegexp
385
386Ban naked srtings as regexps, like:
387
388 print 1 if $str =~ $regexp;
389
390Instead, it should be:
391
392 print 1 if $str =~ m/$regexp/;
393
394or
395
396 print 1 if $str =~ m/$regexp/xms;
397
398
6a4d1045
CD
399=item * ValuesAndExpressions::RequireInterpolatedStringyEval
400
401Ensure that the argument to a stringy eval is not a constant string. That's
402just wasteful. Real world examples include:
403
404 eval 'use Optional::Module';
405
406which is better written as
407
408 eval { require Optional::Module; Optional::Module->import };
409
410for performance gains and compile-time syntax checking.
411
2b7b1533 412
76a987f3
CD
413=item * RegularExpressions::ProhibitUnnecessaryEscapes
414
11f53956
ES
415Complain if user puts a backslash escape in front of non-special characters.
416For example:
76a987f3
CD
417
418 m/\!/;
419
420Make exceptions for C<\">, C<\'> and C<\`> since those are often inserted to
421workaround bugs in syntax highlighting.
422
423Note that this is different inside character classes, where only C<^>, C<]>
424and C<-> need to be escaped, I think. Caret only needs to be escaped at the
425beginning, and dash does NOT need to be escaped at the beginning and end. See
11f53956 426L<perlreref|perlreref>.
76a987f3 427
2b7b1533 428
11f53956 429=item * Steal ideas from L<Dunce::Files|Dunce::Files>.
fef95802 430
18658b1a
CD
431Can someone expand this entry, please?
432
433=item * ControlStructures::ProhibitAssigmentInConditional
434
435=item * ValuesAndExpressions::RequireConstantBeforeEquals
436
437=item * ValuesAndExpressions::RequireConstantBeforeOperator
438
439L<http://use.perl.org/~stu42j/journal/36412>
440
441Just about everyone has been bitten by C<if ($x = 10) { ... }> when they meant
442to use C<==>. A safer style is C<10 == $x> because omitting the second C<=>
443yields a noisy compile-time failure instead of silent runtime error.
444
445ProhibitAssigmentInConditional complains if the condition of a while, until,
446if or unless is solely an assignment. If it's anything more complex (like
447C<if (($x=10)){}> or C<while ($x=$y=$z){}>), there is no warning.
448
449RequireConstantBeforeEquals complains if the left side of an C<==> is a
450variable while the right side is a constant.
451
452RequireConstantBeforeOperator complains if the left side of any comparison
453operator (C<==>, C<eq>, C<&lt;>, etc) is a variable while the right side is a
454constant.
fef95802 455
3f64b6c8
ES
456
457=item * InputOutput::ProhibitUTF8IOLayer
458
459http://www.perlfoundation.org/perl5/index.cgi?the_utf8_perlio_layer
460
b61a141c 461=item * BuiltinFunctions::ProhibitExit(?:InModules)?
f0a7988d
CD
462
463Forbid C<exit()> in files that lack a shebang. Inspired by
464L<http://use.perl.org/~Ovid/journal/36746> and an analgous checker in
465FindBugs.
3f64b6c8 466
c7830a4f
JRT
467=item * Modules::ProhibitRedundantLoading
468
469Don't allow a package to "use" the same module more than once, unless
470there is a "no <module>" between them.
471
472See https://rt.cpan.org/Ticket/Display.html?id=38074.
473
daad783f
AL
474=back
475
2b7b1533 476
daad783f
AL
477=head1 REFACTORINGS and ENHANCEMENTS
478
2b7b1533 479=over
daad783f 480
628facda
ES
481=item * Create constants for the PPI location array elements.
482
2b7b1533 483
992600be
JRT
484=item * MOVE THE LINE-DISABLING INTO P::C::Document
485
486All the code that deals with finding all the '##no critic' comments and noting
487which policies are disabled at each line seems like it would be better placed
488in Perl::Critic::Document. P::C::Document could then provide methods to
489indicate if a policy is disabled at a particular line. So the basic algorithm
490in Perl::Critic might look something like this:
491
492 foreach $element (@PPI_ELEMENTS) {
493 foreach $policy (@POLICIES) {
494 $line = $element->location->[0];
495 next if $doc->policy_is_disabled_at_line( $policy, $line );
496 push @violations, $policy->violates( $elem, $doc );
497 }
498 }
499
2b7b1533 500
aae75e29
ES
501=item * Some means of detecting "runnaway" C<##no critic>
502
503Elliot was talking to a couple of users at ETech and one of their major
504concerns was that they were using C<##no critic> and forgetting to do a
505C<##use critic> after the problematic section. Perhaps an option to
506F<perlcritic> to scan for such things is in order.
507
2b7b1533 508
dfe2eb3f
JRT
509=item * Change API to use named parameters
510
511Most of the methods on the public classes use named parameters for passing
512arguments. I'd like to extend that pattern to include all object-methods.
513Static methods can still use positional parameters.
514
2b7b1533 515
dfe2eb3f 516=item * Enhance P::C::critique() to accept files, directories, or code strings
501ee198 517
9b8f64cf 518Just like F<bin/perlcritic> does now.
daad783f 519
2b7b1533 520
9b8f64cf
CD
521=item * Add C<-cache> flag to F<bin/perlcritic>
522
11f53956 523If enabled, this turns on L<PPI::Cache|PPI::Cache>:
9b8f64cf
CD
524
525 require PPI::Cache;
526 my $cache_path = "/tmp/test-perl-critic-cache-$ENV{USER}";
527 mkdir $cache_path, oct 700 if (! -d $cache_path);
528 PPI::Cache->import(path => $cache_path);
529
11f53956
ES
530This cachedir should perhaps include the PPI version number! At least until
531PPI incorporates its own version number in the cache.
9b8f64cf 532
de8cacdb 533(see F<t/40_criticize.t> for a more robust implementation)
79b72614 534
2b7b1533 535
c5bd966c
JRT
536=item * Use hash-lookup instead of C<List::MoreUtils::any> function.
537
11f53956
ES
538In several places, Perl::Critic uses C<List::MoreUtils::any> to see if a
539string is a member of a list. Instead, I suggest using a named subroutine
540that does a hash-lookup like this:
c5bd966c
JRT
541
542 my %logical_ops = hashify( qw( ! || && ||= &&= and or not ) );
543 sub is_logical_op { return exists $logical_ops{ $_[0] }; }
544
c7830a4f 545Question: Why?
2b7b1533 546
c7830a4f 547Answer: Readability, mostly. Performance, maybe.
2b7b1533 548
dc4cdd33 549=item * Refactor guts of perlcritic into a module (Perl::Critic::App ??)
233e8deb 550
c7830a4f 551Because it is getting unwieldy in there.
2b7b1533 552
9b8f64cf
CD
553=back
554
555=head1 PPI BUGS
556
557We're waiting on the following bugs to get fixed in a CPAN release of PPI:
558
2b7b1533
ES
559
560=over
9b8f64cf 561
dd813c73
ES
562=item PPI::Token::descendant_of()
563
564Exists in svn. Replace _descendant_of() in RequireCheckingReturnValueOfEval
565with that, once it is released, because it's faster and native.
566
c38138ab 567=item Newlines
b589a229
CD
568
569PPI does not preserve newlines. That makes
c38138ab
CD
570CodeLayout::RequireConsistentNewlines impossible to implement under PPI. For
571now, it's implemented by pulling the source out of the file and skipping PPI.
572
573It's unlikely that PPI will support mixde newlines anytime soon.
574
2b7b1533 575
ef919624
CD
576=item Operators
577
11f53956
ES
578ValuesAndExpressions::ProhibitMismatchedOperators has two workarounds for PPI
579bugs with parsing operators. Many of these bugs have been fixed in PPI, so it
580would be good to check if those workarounds are still needed.
ef919624 581
2b7b1533 582
6a4d1045
CD
583=item Regexp methods
584
585Not strictly a bug -- the PPI Regexp classes have a dearth of accessor methods
586as of v1.118, meaning that we have to do messy digging into internals. I
587wrote Perl::Critic:Utils::PPIRegexp to encapsulate this messiness, but it
588would be nicer to have an official interface in PPI.
589
2b7b1533 590
b5366318
ES
591=item QuoteLike::Words in the place of a ForLoop
592
593PPI mis-parses C<<for qw<blah> {}>>.
594
595
daad783f 596=back
501ee198 597
85e38a07 598=cut
dfe2eb3f
JRT
599
600##############################################################################
601# Local Variables:
602# mode: cperl
603# cperl-indent-level: 4
604# fill-column: 78
605# indent-tabs-mode: nil
606# c-indentation-style: bsd
607# End:
96fed375 608# ex: set ts=8 sts=4 sw=4 tw=78 ft=pod expandtab shiftround :