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