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