Commit | Line | Data |
---|---|---|
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 | 9 | Perl::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 | ||
23 | Perl-Critic-More is a separate distribution for less-widely-accepted | |
ab1cd5d7 | 24 | policies. 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 | ||
36 | Just print out file names. I could have used this at work when combined with | |
37 | C<--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 | ||
47 | For the benefit of PodSpelling, etc. | |
48 | ||
49 | ||
50 | =item * Enhance string-list Behavior to allow specification of delimiters. | |
51 | ||
52 | For things like RequirePodSections. | |
53 | ||
54 | ||
afceaa1d ES |
55 | =item * Add queries to --list option to F<perlcritic>. |
56 | ||
57 | List Policies based upon severity, theme, and (what I want this second) | |
58 | applies_to. | |
59 | ||
a9243c21 ES |
60 | =item * Add formatting of --list output. |
61 | ||
62 | Support Jeff Bisbee's use case (he dumps all the policies in severity order | |
63 | with full descriptions and other metadata). | |
64 | ||
ad5f2c12 ES |
65 | =item * Support for C<#line 123 "filename"> directives. |
66 | ||
67 | For code generators and template languages that allow inline Perl code. | |
68 | ||
96fa2db5 ES |
69 | Yes, somebody has an in-house templating system where they've written a custom |
70 | test module that extracts the perl code from a template and critiques it. | |
71 | ||
1d077d29 ES |
72 | Actually, this would be useful for programs: Module::Build "fixes" shebang |
73 | lines so that there's the bit about invoking perl if the program is attempted | |
74 | to be run by a Bourne shell, which throws the line numbers off when using | |
75 | Test::P::C on the contents of a C<blib> directory. | |
76 | ||
a6539177 ES |
77 | This 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 | ||
93 | For 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 | ||
98 | For example, the self compliance test now depends upon a Policy in the More | |
99 | distribution. | |
100 | ||
101 | Something like using a "+" sign in front of the Policy name in its | |
102 | configuration block, analogous to the "-" sign used for disabling a policy, | |
103 | e.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 | ||
111 | L<Alias on P::C|http://use.perl.org/article.pl?sid=08/09/24/1957256>. | |
112 | ||
113 | I<For example, Perl::Critic helps both helps educate developers and reduce the | |
114 | time cost of bugs and weird uses of Perl, but at the expense of having to | |
115 | maintain nocritic entries permanently, which themselves have education and | |
116 | time costs.> | |
117 | ||
118 | I<And the goal of the Perl Critic team is that the education and time cost of | |
119 | using Perl::Critic is significantly less than the education and time costs of | |
120 | NOT using Perl::Critic.> | |
121 | ||
122 | In order to allow people to get rid of "## no critic" markers, create a mode | |
123 | that 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 | ||
135 | Look for the do {} while case and change the explanation to point to page 123 | |
2e69f9b8 | 136 | when it is found. RT #37905. |
bd1718ea ES |
137 | |
138 | ||
daad783f AL |
139 | =item * NamingConventions::ProhibitAmbiguousNames |
140 | ||
141 | Don't allow compound names with forbidden words, like "last_record". | |
142 | Allow forbidden words in RHS of variable declarations | |
143 | ||
fd5bd7b5 JRT |
144 | Also, we should make it easeir to add (or delete) words from the |
145 | forbbiden list. | |
146 | ||
2b7b1533 | 147 | |
daad783f AL |
148 | =item * Subroutines::ProtectPrivateSubs |
149 | ||
150 | Doesn't forbid C<< $pkg->_foo() >> because it can't tell the | |
151 | difference between that and C<< $self->_foo() >> | |
152 | ||
2b7b1533 | 153 | |
d1d6429f AL |
154 | =item * ErrorHandling::RequireCarping |
155 | ||
63124782 | 156 | This should not complain about using C<warn> or C<die> if it's not in a |
dd813c73 | 157 | function, or if it's in main::. |
d1d6429f | 158 | |
11f53956 ES |
159 | Also, should allow C<die> when it is obvious that the "message" is a |
160 | reference. | |
5c77583a | 161 | |
2b7b1533 | 162 | |
4c542233 AL |
163 | =item * RegularExpressions::ProhibitCaptureWithoutTest |
164 | ||
165 | Allow this construct: | |
166 | ||
167 | for ( ... ) { | |
168 | next unless /(....)/; | |
169 | if ( $1 ) { | |
170 | .... | |
171 | } | |
172 | } | |
173 | ||
174 | Right 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> | |
176 | if the regex matched. | |
11f53956 | 177 | |
dfe2eb3f | 178 | while ( $str =~ /(expression)/ ) |
4c542233 | 179 | |
2b7b1533 | 180 | |
d9abc689 JRT |
181 | =item * CodeLayout::ProhibitParensWithBuiltins |
182 | ||
183 | Some builtin functions (particularly those that take a variable number of | |
c296c678 ES |
184 | scalar arguments) should probably get parentheses. This policy should be |
185 | enhanced to allow the user to specify a list of builtins that are expempt | |
186 | from the policy. | |
d9abc689 | 187 | |
2b7b1533 | 188 | |
c59ac28c ES |
189 | =item * ValuesAndExpressions::ProhibitCommaSeparatedStatements |
190 | ||
191 | Needs to check for C<scalar( something, something )>. | |
192 | ||
193 | ||
326d1973 ES |
194 | =item * Variables::ProhibitPunctuationVars |
195 | ||
196 | Needs to look inside strings. RT #35970. | |
197 | ||
198 | ||
96280e8c ES |
199 | =item * TestingAndDebugging::RequireUseWarnings |
200 | ||
201 | Check 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 | ||
227 | The idea behind this policy is to encourage better names for variables | |
228 | and subroutines by enforcing correct spelling and prohibiting the use of | |
229 | home-grown abbreviations. Assumng that the author uses underscores or | |
230 | camel-case, it should be possible to split symbols into words, and then look | |
231 | them up in a dictionary (see PodSpelling). This policy should probably have | |
232 | a similar stopwords feature as well. | |
233 | ||
45136ef4 ES |
234 | =item * Documentation::RequireModuleAbstract |
235 | ||
236 | Require a C<=head1 NAME> POD section with content that matches | |
237 | C<\A \s* [\w:]+ \s+ - \s+ \S>. The single hyphen is the important bit. Also, | |
238 | must be a single line. | |
239 | ||
be48b0c6 ES |
240 | =item * Expressions::RequireFatCommasInHashConstructors |
241 | ||
f3bcb2ca ES |
242 | =item * ErrorHandling::RequireLocalizingEvalErrorInDESTROY |
243 | ||
244 | Prevent 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 |
261 | See 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 | 270 | Steal the idea from L<B::Lint|B::Lint>. |
3264a614 ES |
271 | |
272 | ||
df88f751 | 273 | =item * TBD::AllProgramsNeedShebangs |
d92d2828 | 274 | |
11f53956 ES |
275 | Anything that is a program should have a shebang line. This includes .t |
276 | files. | |
d92d2828 | 277 | |
2b7b1533 | 278 | |
daad783f AL |
279 | =item * BuiltInFunctions::RequireConstantSprintfFormat |
280 | ||
2b7b1533 | 281 | |
daad783f AL |
282 | =item * BuiltInFunctions::RequireConstantUnpackFormat |
283 | ||
1cc04072 | 284 | L<http://diotalevi.isa-geek.net/~josh/yapc-lint/slides/slide5.html> |
daad783f | 285 | |
2b7b1533 | 286 | |
dfe2eb3f JRT |
287 | =item * Miscellanea::ProhibitObnoxiousComments |
288 | ||
289 | Forbid excessive hash marks e.g. "#### This is a loud comment ####". | |
290 | Make the obnoxious pattern configurable | |
291 | ||
2b7b1533 | 292 | |
dfe2eb3f JRT |
293 | =item * ValuesAndExpressions::RequireNotOperator |
294 | ||
295 | Require the use of "not" instead of "!", except when this would contradict | |
296 | ProhibitMixedBooleanOperators. This may be better suited for | |
297 | Perl::Critic::More. | |
298 | ||
2b7b1533 | 299 | |
dfe2eb3f JRT |
300 | =item * Modules::RequireExplicitImporting |
301 | ||
302 | Require every C<use> statement to have an explicit import list. You could | |
303 | still get around this by calling C<import> directly. | |
304 | ||
2b7b1533 | 305 | |
dfe2eb3f JRT |
306 | =item * Modules::ForbidImporting |
307 | ||
308 | Require every C<use> to have an explicitly empty import list. This is for | |
309 | folks who like to see fully-qualified function names. Should probably provide | |
310 | a list of exempt modules (like FindBin); | |
311 | ||
2b7b1533 | 312 | |
daad783f AL |
313 | =item * ControlStructures::ProhibitIncludeViaDo |
314 | ||
315 | Forbid C<do "foo.pl">. Not sure about this policy name. | |
316 | ||
2b7b1533 | 317 | |
daad783f AL |
318 | =item * Variables::ProhibitUseVars |
319 | ||
f707b14a | 320 | Disallow C<use vars qw(...)> and require C<our $foo> instead. This |
11f53956 ES |
321 | contradicts Miscellanea::Prohibit5006isms. Maybe verify C<use 5.6> before |
322 | applying this policy. Low severity. | |
daad783f | 323 | |
2b7b1533 | 324 | |
daad783f AL |
325 | =item * VariablesAndExpressions::ProhibitQuotedHashKeys |
326 | ||
11f53956 ES |
327 | Forbid quotes around hash keys, unless they are really needed. This is |
328 | against what Damian says. Suggested by Adam Kennedy. Low severity. | |
daad783f | 329 | |
2b7b1533 | 330 | |
b589a229 | 331 | =item * CodeLayout::ProhibitFunctionalNew |
daad783f AL |
332 | |
333 | Good: C<< Foo::Bar->new >>, Bad: C<< new Foo::Bar >> | |
334 | ||
2b7b1533 | 335 | |
7eec5b31 ES |
336 | =item * RegularExpressions::ProhibitSWSWSW |
337 | ||
338 | Require 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 |
346 | L<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 | ||
354 | These are simplified versions of Documentation::RequirePodSections. | |
355 | ||
2b7b1533 | 356 | |
6a4d1045 CD |
357 | =item * Documentation::RequireValidSynopsis |
358 | ||
359 | The 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 | ||
365 | Any C<=headN> and C<=over> sections must not be empty. This helps catch | |
366 | boilerplate (althought Test::Pod should catch empty C<=over> blocks). | |
367 | ||
368 | On the other hand, C<=item ...> sections can be empty, since the item label is | |
369 | content. | |
370 | ||
2b7b1533 | 371 | |
79b72614 CD |
372 | =item * Miscellaneous::ProhibitBoilerplate |
373 | ||
11f53956 | 374 | Complain about copy-and-paste code or docs from h2xs, Module::Starter::*, etc. |
c6e7b236 | 375 | |
ef919624 CD |
376 | Here's a non-PPI implementation: |
377 | L<http://search.cpan.org/src/JJORE/Carp-Clan-5.8/t/04boilerplate.t> | |
378 | ||
2b7b1533 | 379 | |
956825ec CD |
380 | =item * BuiltinFunctions::ProhibitExtraneousScalarCall |
381 | ||
382 | Recommend that C<if (scalar @array)> be rewritten as C<if (@array)>. | |
383 | ||
2b7b1533 | 384 | |
6a4d1045 CD |
385 | =item * RegularExpressions::ProhibitMixedDelimiters |
386 | ||
387 | Ban s{foo}(bar) | |
388 | ||
2b7b1533 | 389 | |
c79c8b5b CD |
390 | =item * RegularExpressions::ProhibitScalarAsRegexp |
391 | ||
392 | Ban naked srtings as regexps, like: | |
393 | ||
394 | print 1 if $str =~ $regexp; | |
395 | ||
396 | Instead, it should be: | |
397 | ||
398 | print 1 if $str =~ m/$regexp/; | |
399 | ||
400 | or | |
401 | ||
402 | print 1 if $str =~ m/$regexp/xms; | |
403 | ||
404 | ||
6a4d1045 CD |
405 | =item * ValuesAndExpressions::RequireInterpolatedStringyEval |
406 | ||
407 | Ensure that the argument to a stringy eval is not a constant string. That's | |
408 | just wasteful. Real world examples include: | |
409 | ||
410 | eval 'use Optional::Module'; | |
411 | ||
412 | which is better written as | |
413 | ||
414 | eval { require Optional::Module; Optional::Module->import }; | |
415 | ||
416 | for performance gains and compile-time syntax checking. | |
417 | ||
2b7b1533 | 418 | |
76a987f3 CD |
419 | =item * RegularExpressions::ProhibitUnnecessaryEscapes |
420 | ||
11f53956 ES |
421 | Complain if user puts a backslash escape in front of non-special characters. |
422 | For example: | |
76a987f3 CD |
423 | |
424 | m/\!/; | |
425 | ||
426 | Make exceptions for C<\">, C<\'> and C<\`> since those are often inserted to | |
427 | workaround bugs in syntax highlighting. | |
428 | ||
429 | Note that this is different inside character classes, where only C<^>, C<]> | |
430 | and C<-> need to be escaped, I think. Caret only needs to be escaped at the | |
431 | beginning, and dash does NOT need to be escaped at the beginning and end. See | |
11f53956 | 432 | L<perlreref|perlreref>. |
76a987f3 | 433 | |
2b7b1533 | 434 | |
11f53956 | 435 | =item * Steal ideas from L<Dunce::Files|Dunce::Files>. |
fef95802 | 436 | |
18658b1a CD |
437 | Can someone expand this entry, please? |
438 | ||
439 | =item * ControlStructures::ProhibitAssigmentInConditional | |
440 | ||
441 | =item * ValuesAndExpressions::RequireConstantBeforeEquals | |
442 | ||
443 | =item * ValuesAndExpressions::RequireConstantBeforeOperator | |
444 | ||
445 | L<http://use.perl.org/~stu42j/journal/36412> | |
446 | ||
447 | Just about everyone has been bitten by C<if ($x = 10) { ... }> when they meant | |
448 | to use C<==>. A safer style is C<10 == $x> because omitting the second C<=> | |
449 | yields a noisy compile-time failure instead of silent runtime error. | |
450 | ||
451 | ProhibitAssigmentInConditional complains if the condition of a while, until, | |
452 | if or unless is solely an assignment. If it's anything more complex (like | |
453 | C<if (($x=10)){}> or C<while ($x=$y=$z){}>), there is no warning. | |
454 | ||
455 | RequireConstantBeforeEquals complains if the left side of an C<==> is a | |
456 | variable while the right side is a constant. | |
457 | ||
458 | RequireConstantBeforeOperator complains if the left side of any comparison | |
459 | operator (C<==>, C<eq>, C<<>, etc) is a variable while the right side is a | |
460 | constant. | |
fef95802 | 461 | |
3f64b6c8 ES |
462 | |
463 | =item * InputOutput::ProhibitUTF8IOLayer | |
464 | ||
465 | http://www.perlfoundation.org/perl5/index.cgi?the_utf8_perlio_layer | |
466 | ||
b61a141c | 467 | =item * BuiltinFunctions::ProhibitExit(?:InModules)? |
f0a7988d CD |
468 | |
469 | Forbid C<exit()> in files that lack a shebang. Inspired by | |
470 | L<http://use.perl.org/~Ovid/journal/36746> and an analgous checker in | |
471 | FindBugs. | |
3f64b6c8 | 472 | |
c7830a4f JRT |
473 | =item * Modules::ProhibitRedundantLoading |
474 | ||
475 | Don't allow a package to "use" the same module more than once, unless | |
476 | there is a "no <module>" between them. | |
477 | ||
478 | See 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 | ||
492 | All the code that deals with finding all the '##no critic' comments and noting | |
493 | which policies are disabled at each line seems like it would be better placed | |
494 | in Perl::Critic::Document. P::C::Document could then provide methods to | |
495 | indicate if a policy is disabled at a particular line. So the basic algorithm | |
496 | in 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 | ||
509 | Elliot was talking to a couple of users at ETech and one of their major | |
510 | concerns was that they were using C<##no critic> and forgetting to do a | |
511 | C<##use critic> after the problematic section. Perhaps an option to | |
512 | F<perlcritic> to scan for such things is in order. | |
513 | ||
2b7b1533 | 514 | |
dfe2eb3f JRT |
515 | =item * Change API to use named parameters |
516 | ||
517 | Most of the methods on the public classes use named parameters for passing | |
518 | arguments. I'd like to extend that pattern to include all object-methods. | |
519 | Static 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 | 524 | Just like F<bin/perlcritic> does now. |
daad783f | 525 | |
2b7b1533 | 526 | |
9b8f64cf CD |
527 | =item * Add C<-cache> flag to F<bin/perlcritic> |
528 | ||
11f53956 | 529 | If 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 |
536 | This cachedir should perhaps include the PPI version number! At least until |
537 | PPI 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 |
544 | In several places, Perl::Critic uses C<List::MoreUtils::any> to see if a |
545 | string is a member of a list. Instead, I suggest using a named subroutine | |
546 | that 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 | 551 | Question: Why? |
2b7b1533 | 552 | |
c7830a4f | 553 | Answer: Readability, mostly. Performance, maybe. |
2b7b1533 | 554 | |
dc4cdd33 | 555 | =item * Refactor guts of perlcritic into a module (Perl::Critic::App ??) |
233e8deb | 556 | |
c7830a4f | 557 | Because it is getting unwieldy in there. |
2b7b1533 | 558 | |
9b8f64cf CD |
559 | =back |
560 | ||
561 | =head1 PPI BUGS | |
562 | ||
563 | We'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 | ||
570 | Exists in svn. Replace _descendant_of() in RequireCheckingReturnValueOfEval | |
571 | with that, once it is released, because it's faster and native. | |
572 | ||
c38138ab | 573 | =item Newlines |
b589a229 CD |
574 | |
575 | PPI does not preserve newlines. That makes | |
c38138ab CD |
576 | CodeLayout::RequireConsistentNewlines impossible to implement under PPI. For |
577 | now, it's implemented by pulling the source out of the file and skipping PPI. | |
578 | ||
579 | It's unlikely that PPI will support mixde newlines anytime soon. | |
580 | ||
2b7b1533 | 581 | |
ef919624 CD |
582 | =item Operators |
583 | ||
11f53956 ES |
584 | ValuesAndExpressions::ProhibitMismatchedOperators has two workarounds for PPI |
585 | bugs with parsing operators. Many of these bugs have been fixed in PPI, so it | |
586 | would be good to check if those workarounds are still needed. | |
ef919624 | 587 | |
2b7b1533 | 588 | |
6a4d1045 CD |
589 | =item Regexp methods |
590 | ||
591 | Not strictly a bug -- the PPI Regexp classes have a dearth of accessor methods | |
592 | as of v1.118, meaning that we have to do messy digging into internals. I | |
593 | wrote Perl::Critic:Utils::PPIRegexp to encapsulate this messiness, but it | |
594 | would 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 | ||
599 | PPI 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 : |