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