Login
Add TODO from Andy about a policy about not using
[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
d0e776fc
ES
61=item * Add --prohibit-unrestricted-no-critic option to F<perlcritic>.
62
63Requires C<## no critic> to take an argument:
64
65 ## no critic (SomePolicyPattern) # ok
66 ## no critic # not ok
67
e95460bd 68Can't be done as a regular Policy because any line that violated it would disable it.
d0e776fc 69
2b7b1533 70
ad5f2c12
ES
71=item * Support for C<#line 123 "filename"> directives.
72
73For code generators and template languages that allow inline Perl code.
74
96fa2db5
ES
75Yes, somebody has an in-house templating system where they've written a custom
76test module that extracts the perl code from a template and critiques it.
77
2b7b1533 78
a889d362
ES
79=item * Enhance statistics.
80
81- Blank line count
82
83- POD line count
84
85- Comment line count
86
87- Data section count
88
a889d362 89
afb00bb2
ES
90=item * Detect 5.10 source and enable stuff for that.
91
92For example, treat C<say> as equivalent to C<print>.
93
94
d1d6429f
AL
95=back
96
2b7b1533 97
daad783f
AL
98=head1 BUGS/LIMITATIONS
99
2b7b1533 100=over
daad783f 101
517c4d69
ES
102=item * Don't kill entire run if PPI::Document cannot be instantiated.
103
104Just say that the individual file is bad and skip to the next one.
105
106
daad783f
AL
107=item * Modules::RequireVersionVar
108
109Doesn't enforce three-part versions
110
2b7b1533 111
daad783f
AL
112=item * NamingConventions::ProhibitAmbiguousNames
113
114Don't allow compound names with forbidden words, like "last_record".
115Allow forbidden words in RHS of variable declarations
116
fd5bd7b5
JRT
117Also, we should make it easeir to add (or delete) words from the
118forbbiden list.
119
2b7b1533 120
daad783f
AL
121=item * Subroutines::ProtectPrivateSubs
122
123Doesn't forbid C<< $pkg->_foo() >> because it can't tell the
124difference between that and C<< $self->_foo() >>
125
2b7b1533 126
d1d6429f
AL
127=item * ErrorHandling::RequireCarping
128
63124782 129This should not complain about using C<warn> or C<die> if it's not in a
df88f751 130function, or if it's not in a non-main:: package.
d1d6429f 131
63124782 132Also, should allow C<die> when it is obvious that the "message" is a reference.
5c77583a 133
2b7b1533 134
4c542233
AL
135=item * RegularExpressions::ProhibitCaptureWithoutTest
136
137Allow this construct:
138
139 for ( ... ) {
140 next unless /(....)/;
141 if ( $1 ) {
142 ....
143 }
144 }
145
146Right now, P::C thinks that the C<$1> isn't legal to use because it's
147"outside" of the match. The thing is, we can only get to the C<if>
148if the regex matched.
dfe2eb3f 149 while ( $str =~ /(expression)/ )
4c542233 150
2b7b1533 151
d9abc689
JRT
152=item * CodeLayout::ProhibitParensWithBuiltins
153
154Some builtin functions (particularly those that take a variable number of
c296c678
ES
155scalar arguments) should probably get parentheses. This policy should be
156enhanced to allow the user to specify a list of builtins that are expempt
157from the policy.
d9abc689 158
2b7b1533 159
9fb2d1dc
AM
160=item * InputOutput::RequireCheckedOpen and RequireCheckedClose
161
162These policies should not report violations if 'use Fatal' is in effect.
163
2b7b1533 164
c59ac28c
ES
165=item * ValuesAndExpressions::ProhibitCommaSeparatedStatements
166
167Needs to check for C<scalar( something, something )>.
168
169
d6dc5ff8
ES
170=item * ValuesAndExpressions::ProhibitMagicNumbers
171
172The C<allowed_values> option needs to handle ranges.
173
174Needs to be tested against a wider range of $VERSION declarations.
175
176
96280e8c
ES
177=item * TestingAndDebugging::RequireUseWarnings
178
179Check for -w on the shbang line.
180
2b7b1533 181
365e3d9d
AL
182=back
183
2b7b1533 184
daad783f
AL
185=head1 OTHER PBP POLICIES THAT SEEM FEASIBLE TO IMPLEMENT
186
5331273a
ES
187=over
188
189=item * Modules::RequireUseVersion [405-406]
190
191=item * Modules::RequireThreePartVersion [405-406]
192
193=back
daad783f 194
2b7b1533 195
daad783f
AL
196=head1 NON-PBP POLICIES WANTED
197
2b7b1533 198=over
daad783f 199
be48b0c6
ES
200=item * Expressions::RequireFatCommasInHashConstructors
201
0b15f123
ES
202=item * ErrorHandling::RequireCheckOfEvalErrorAfterEval
203
77fa21bd
ES
204=item * Expressions::ProhibitDecimalWithBitwiseOperator
205
206=item * Expressions::ProhibitStringsWithBitwiseOperator
207
2b7b1533 208
3264a614
ES
209=item * InputOutput::ProhibitMagicDiamond
210
211Steal the idea from L<B::Lint>.
212
213
df88f751
AL
214=item * TBD::VariableNotUsed
215
d19608d1 216Detect a variable that is declared, but never used.
df88f751 217
2b7b1533 218
df88f751 219=item * TBD::AllProgramsNeedShebangs
d92d2828
AL
220
221Anything that is a program should have a shebang line. This includes .t files.
222
2b7b1533 223
daad783f
AL
224=item * BuiltInFunctions::RequireConstantSprintfFormat
225
2b7b1533 226
daad783f
AL
227=item * BuiltInFunctions::RequireConstantUnpackFormat
228
1cc04072 229L<http://diotalevi.isa-geek.net/~josh/yapc-lint/slides/slide5.html>
daad783f 230
2b7b1533 231
dfe2eb3f
JRT
232=item * Miscellanea::ProhibitObnoxiousComments
233
234Forbid excessive hash marks e.g. "#### This is a loud comment ####".
235Make the obnoxious pattern configurable
236
2b7b1533 237
dfe2eb3f
JRT
238=item * ValuesAndExpressions::RequireNotOperator
239
240Require the use of "not" instead of "!", except when this would contradict
241ProhibitMixedBooleanOperators. This may be better suited for
242Perl::Critic::More.
243
2b7b1533 244
dfe2eb3f
JRT
245=item * Modules::RequireExplicitImporting
246
247Require every C<use> statement to have an explicit import list. You could
248still get around this by calling C<import> directly.
249
2b7b1533 250
dfe2eb3f
JRT
251=item * Modules::ForbidImporting
252
253Require every C<use> to have an explicitly empty import list. This is for
254folks who like to see fully-qualified function names. Should probably provide
255a list of exempt modules (like FindBin);
256
2b7b1533 257
daad783f
AL
258=item * ControlStructures::ProhibitIncludeViaDo
259
260Forbid C<do "foo.pl">. Not sure about this policy name.
261
2b7b1533 262
daad783f
AL
263=item * Variables::ProhibitUseVars
264
f707b14a
CD
265Disallow C<use vars qw(...)> and require C<our $foo> instead. This
266contradicts Miscellanea::Prohibit5006isms. Maybe verify C<use 5.6>
267before applying this policy. Low severity.
daad783f 268
2b7b1533 269
daad783f
AL
270=item * VariablesAndExpressions::ProhibitQuotedHashKeys
271
272Forbid quotes around hash keys, unless they are really needed. This
273is against what Damian says. Suggested by Adam Kennedy. Low
274severity.
275
2b7b1533 276
b589a229 277=item * CodeLayout::ProhibitFunctionalNew
daad783f
AL
278
279Good: C<< Foo::Bar->new >>, Bad: C<< new Foo::Bar >>
280
2b7b1533 281
7eec5b31
ES
282=item * RegularExpressions::ProhibitSWSWSW
283
284Require C<split> instead of C<m/\s*\w*\s*\w*\s*/>. From MJD's Red Flags.
285
286
daad783f
AL
287=item * VariablesAndExpressions::RequireConstantVersion (low severity)
288
2b7b1533 289
f707b14a
CD
290=item * VariablesAndExpressions::ProhibitComplexVersion (medium severity)
291
daad783f
AL
292L<http://rt.cpan.org/Ticket/Display.html?id=20439>
293
2b7b1533 294
79b72614
CD
295=item * Documentation::RequireSynopsis
296
2b7b1533 297
79b72614
CD
298=item * Documentation::RequireLicense
299
300These are simplified versions of Documentation::RequirePodSections.
301
2b7b1533 302
6a4d1045
CD
303=item * Documentation::RequireValidSynopsis
304
305The Synopsis section must be all indented and must be syntactically valid Perl
306(as validated by PPI).
307
2b7b1533 308
90c0067c
CD
309=item * Documentation::ProhibitEmptySections
310
311Any C<=headN> and C<=over> sections must not be empty. This helps catch
312boilerplate (althought Test::Pod should catch empty C<=over> blocks).
313
314On the other hand, C<=item ...> sections can be empty, since the item label is
315content.
316
2b7b1533 317
79b72614
CD
318=item * Miscellaneous::ProhibitBoilerplate
319
b589a229 320Complain about copy-and-paste code or docs from h2xs, Module::Starter::*,
79b72614 321etc.
c6e7b236 322
ef919624
CD
323Here's a non-PPI implementation:
324L<http://search.cpan.org/src/JJORE/Carp-Clan-5.8/t/04boilerplate.t>
325
2b7b1533 326
956825ec
CD
327=item * BuiltinFunctions::ProhibitExtraneousScalarCall
328
329Recommend that C<if (scalar @array)> be rewritten as C<if (@array)>.
330
2b7b1533 331
6a4d1045
CD
332=item * RegularExpressions::ProhibitMixedDelimiters
333
334Ban s{foo}(bar)
335
2b7b1533 336
c79c8b5b
CD
337=item * RegularExpressions::ProhibitScalarAsRegexp
338
339Ban naked srtings as regexps, like:
340
341 print 1 if $str =~ $regexp;
342
343Instead, it should be:
344
345 print 1 if $str =~ m/$regexp/;
346
347or
348
349 print 1 if $str =~ m/$regexp/xms;
350
351
6a4d1045
CD
352=item * ValuesAndExpressions::RequireInterpolatedStringyEval
353
354Ensure that the argument to a stringy eval is not a constant string. That's
355just wasteful. Real world examples include:
356
357 eval 'use Optional::Module';
358
359which is better written as
360
361 eval { require Optional::Module; Optional::Module->import };
362
363for performance gains and compile-time syntax checking.
364
2b7b1533 365
76a987f3
CD
366=item * RegularExpressions::ProhibitUnnecessaryEscapes
367
368Complain if user puts a backslash escape in front of non-special characters. For example:
369
370 m/\!/;
371
372Make exceptions for C<\">, C<\'> and C<\`> since those are often inserted to
373workaround bugs in syntax highlighting.
374
375Note that this is different inside character classes, where only C<^>, C<]>
376and C<-> need to be escaped, I think. Caret only needs to be escaped at the
377beginning, and dash does NOT need to be escaped at the beginning and end. See
378L<perlreref>.
379
2b7b1533 380
9579156b
ES
381=item * TBD::ProhibitLabelsWithSpecialBlockNames
382
383BEGIN { <...code...> }
384is a BEGIN block that gets executed at compile time.
385
386BEGIN: { <...code...> }
387is a labeled block that gets executed during run time.
388
389I suspect that any block named BEGIN, END, INIT or CHECK is an error.
390This should be a standard rule.
391
392xoxo,
393Andy
394
395
fef95802
ES
396=item * Steal ideas from L<Dunce::Files>.
397
398
daad783f
AL
399=back
400
2b7b1533 401
daad783f
AL
402=head1 REFACTORINGS and ENHANCEMENTS
403
2b7b1533 404=over
daad783f 405
628facda
ES
406=item * Create constants for the PPI location array elements.
407
2b7b1533 408
992600be
JRT
409=item * MOVE THE LINE-DISABLING INTO P::C::Document
410
411All the code that deals with finding all the '##no critic' comments and noting
412which policies are disabled at each line seems like it would be better placed
413in Perl::Critic::Document. P::C::Document could then provide methods to
414indicate if a policy is disabled at a particular line. So the basic algorithm
415in Perl::Critic might look something like this:
416
417 foreach $element (@PPI_ELEMENTS) {
418 foreach $policy (@POLICIES) {
419 $line = $element->location->[0];
420 next if $doc->policy_is_disabled_at_line( $policy, $line );
421 push @violations, $policy->violates( $elem, $doc );
422 }
423 }
424
2b7b1533 425
aae75e29
ES
426=item * Some means of detecting "runnaway" C<##no critic>
427
428Elliot was talking to a couple of users at ETech and one of their major
429concerns was that they were using C<##no critic> and forgetting to do a
430C<##use critic> after the problematic section. Perhaps an option to
431F<perlcritic> to scan for such things is in order.
432
2b7b1533 433
dfe2eb3f
JRT
434=item * Change API to use named parameters
435
436Most of the methods on the public classes use named parameters for passing
437arguments. I'd like to extend that pattern to include all object-methods.
438Static methods can still use positional parameters.
439
2b7b1533 440
dfe2eb3f
JRT
441=item * Allow more flexible Policy parameter parsing
442
443Several policies use C<words_from_string()> to split their parameters into
444words. This function is currently limited to splitting on whitespace. It
445would be nice to allow some lattitude for users who might try and use commas
446or some other kind of delimiter.
447
2b7b1533 448
dfe2eb3f 449=item * Enhance P::C::critique() to accept files, directories, or code strings
501ee198 450
9b8f64cf 451Just like F<bin/perlcritic> does now.
daad783f 452
2b7b1533 453
9b8f64cf
CD
454=item * Add C<-cache> flag to F<bin/perlcritic>
455
456If enabled, this turns on L<PPI::Cache>:
457
458 require PPI::Cache;
459 my $cache_path = "/tmp/test-perl-critic-cache-$ENV{USER}";
460 mkdir $cache_path, oct 700 if (! -d $cache_path);
461 PPI::Cache->import(path => $cache_path);
462
de8cacdb 463This cachedir should perhaps include the PPI version number! At least
84e87307 464until PPI incorporates its own version number in the cache.
9b8f64cf 465
de8cacdb 466(see F<t/40_criticize.t> for a more robust implementation)
79b72614 467
2b7b1533 468
c5bd966c
JRT
469=item * Use hash-lookup instead of C<List::MoreUtils::any> function.
470
471In several places, Perl::Critic uses C<List::MoreUtils::any> to see if
472a string is a member of a list. Instead, I suggest using a named
473subroutine that does a hash-lookup like this:
474
475 my %logical_ops = hashify( qw( ! || && ||= &&= and or not ) );
476 sub is_logical_op { return exists $logical_ops{ $_[0] }; }
477
2b7b1533
ES
478
479=item * Allow color output to work through a pipe.
480
481http://rt.cpan.org/Ticket/Display.html?id=30140
482
233e8deb
ES
483F<ack> now supports this.
484
2b7b1533 485
9b8f64cf
CD
486=back
487
488=head1 PPI BUGS
489
490We're waiting on the following bugs to get fixed in a CPAN release of PPI:
491
2b7b1533
ES
492
493=over
9b8f64cf 494
c38138ab 495=item Newlines
b589a229
CD
496
497PPI does not preserve newlines. That makes
c38138ab
CD
498CodeLayout::RequireConsistentNewlines impossible to implement under PPI. For
499now, it's implemented by pulling the source out of the file and skipping PPI.
500
501It's unlikely that PPI will support mixde newlines anytime soon.
502
2b7b1533 503
ef919624
CD
504=item Operators
505
506ValuesAndExpressions::ProhibitMismatchedOperators has two workarounds
507for PPI bugs with parsing operators. Many of these bugs have been
508fixed in PPI, so it would be good to check if those workarounds are
509still needed.
510
2b7b1533 511
6a4d1045
CD
512=item Regexp methods
513
514Not strictly a bug -- the PPI Regexp classes have a dearth of accessor methods
515as of v1.118, meaning that we have to do messy digging into internals. I
516wrote Perl::Critic:Utils::PPIRegexp to encapsulate this messiness, but it
517would be nicer to have an official interface in PPI.
518
2b7b1533 519
d6dc5ff8
ES
520=item Hash constructors being parsed as blocks
521
522ValuesAndExpressions::ProhibitMagicNumbers has a performance optimization that
523can't be done because it can't tell whether a block is really a block or not.
524
525When fixed, uncomment the lines in
526_element_is_in_an_include_or_readonly_statement().
527
528
529=item literal()
530
531This method has been added in PPI v1.199_xx. When v1.200 is released,
532we can remove the special 1.118 checks from
533ValuesAndExpressions::ProhibitMagicNumbers and
534F<t/20_policies_prohibitmagicnumbers.t>.
535
536
daad783f 537=back
501ee198 538
85e38a07 539=cut
dfe2eb3f
JRT
540
541##############################################################################
542# Local Variables:
543# mode: cperl
544# cperl-indent-level: 4
545# fill-column: 78
546# indent-tabs-mode: nil
547# c-indentation-style: bsd
548# End:
96fed375 549# ex: set ts=8 sts=4 sw=4 tw=78 ft=pod expandtab shiftround :