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