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