Login
Fix typos and add to TODO.
[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
de8cacdb
CD
20=head1 SEE ALSO
21
22Perl-Critic-More is a separate distribution for less-widely-accepted
ab1cd5d7 23policies. It contains its own TODO.pod.
de8cacdb 24
daad783f
AL
25=head1 NEW FEATURES
26
27=over 4
28
8b5892fc 29=item * Report Safari sections in addition to book page numbers.
9bc546d7 30
039ec6b9
ES
31=item * Allow policies to say that they've had enough and to not use them for the rest of the current document.
32
33Primarily for things like C<RequireUseStrict> and C<ProhibitMagicNumbers>.
2ad35e86 34Replace current workaround for C<RequireUseStrict>.
039ec6b9 35
d0e776fc
ES
36=item * Add --prohibit-unrestricted-no-critic option to F<perlcritic>.
37
38Requires C<## no critic> to take an argument:
39
40 ## no critic (SomePolicyPattern) # ok
41 ## no critic # not ok
42
43Can't be done as a Policy because any line that violated it would disable it.
44
d1d6429f
AL
45=back
46
daad783f
AL
47=head1 BUGS/LIMITATIONS
48
49=over 4
50
daad783f
AL
51=item * Modules::RequireVersionVar
52
53Doesn't enforce three-part versions
54
55=item * NamingConventions::ProhibitAmbiguousNames
56
57Don't allow compound names with forbidden words, like "last_record".
58Allow forbidden words in RHS of variable declarations
59
fd5bd7b5
JRT
60Also, we should make it easeir to add (or delete) words from the
61forbbiden list.
62
daad783f
AL
63=item * Subroutines::ProtectPrivateSubs
64
65Doesn't forbid C<< $pkg->_foo() >> because it can't tell the
66difference between that and C<< $self->_foo() >>
67
d1d6429f
AL
68=item * ErrorHandling::RequireCarping
69
63124782 70This should not complain about using C<warn> or C<die> if it's not in a
df88f751 71function, or if it's not in a non-main:: package.
d1d6429f 72
63124782 73Also, should allow C<die> when it is obvious that the "message" is a reference.
5c77583a 74
4c542233
AL
75=item * RegularExpressions::ProhibitCaptureWithoutTest
76
77Allow this construct:
78
79 for ( ... ) {
80 next unless /(....)/;
81 if ( $1 ) {
82 ....
83 }
84 }
85
86Right now, P::C thinks that the C<$1> isn't legal to use because it's
87"outside" of the match. The thing is, we can only get to the C<if>
88if the regex matched.
dfe2eb3f 89 while ( $str =~ /(expression)/ )
4c542233 90
d9abc689
JRT
91=item * CodeLayout::ProhibitParensWithBuiltins
92
93Some builtin functions (particularly those that take a variable number of
94scalar arguments) should probably get parens. This policy should be enhanced
95to allow the user to specify a list of builtins that are expempt from the
96policy.
97
9fb2d1dc
AM
98=item * InputOutput::RequireCheckedOpen and RequireCheckedClose
99
100These policies should not report violations if 'use Fatal' is in effect.
101
96280e8c
ES
102=item * TestingAndDebugging::RequireUseWarnings
103
104Check for -w on the shbang line.
105
365e3d9d
AL
106=back
107
daad783f
AL
108=head1 OTHER PBP POLICIES THAT SEEM FEASIBLE TO IMPLEMENT
109
369abea9
CD
110Chris Dolan is in the process of implementing the ideas below (but don't let
111that stop you from contributing!!!). That work is supported by a Perl
112Foundation grant. More details about the grant:
113L<http://www.perlfoundation.org/april_1_2007_new_grant_awards>
0ab066f7 114
369abea9 115=over 4
0ab066f7 116
9fb2d1dc 117=item * InputOutput::RequireChecked* for system calls (p208)
daad783f 118
654129c7
CD
119Add policies to ensure checking the return values of system calls. See
120InputOutput::RequireCheckedOpen and InputOutput::RequireCheckedClose. Needs
121to take into account the Fatal and Fatal::Exception modules.
daad783f
AL
122
123=item * InputOutput::RequireBriefOpen (p209)
124
125Make sure there's a close within N statements of an open, both with
126same lexical FH
127
daad783f
AL
128=item * RegularExpressions::RequireBracesForMultiline (p242)
129
130=item * RegularExpressions::ProhibitUnusualDelimiters (p246)
131
132=item * RegularExpressions::ProhibitEscapedMetacharacters (p247)
133
134=item * RegularExpressions::ProhibitEnumeratedClasses (p248)
135
df88f751 136This will be avoided for ASCII-only code.
daad783f
AL
137
138=item * RegularExpressions::ProhibitUnusedCapture (p252)
139
140Look for LHS of regexp or use of C<$1>, C<$2>, ... before next
df88f751 141regexp.
daad783f 142
66186ba3 143=item * RegularExpressions::ProhibitComplexRegexes (p261)
daad783f
AL
144
145If regexp is longer than N characters/lines, require it be split
146into C<qr//> pieces.
147
66186ba3
ES
148Even if the x modifier is used and the regex is split across
149multiple lines?
150
daad783f
AL
151=item * RegularExpressions::ProhibitSingleCharAlternation (p265)
152
153Not sure if this is easy or hard. Need to look at what PPI emits
1fd10f9b 154for regexps. Make an exception for qr/ [ ] /x.
daad783f 155
daad783f
AL
156=back
157
158=head1 NON-PBP POLICIES WANTED
159
160=over 4
161
df88f751
AL
162=item * TBD::VariableNotUsed
163
164Detect a variable that has a value assigned to it, but never used.
165
166=item * TBD::AllProgramsNeedShebangs
d92d2828
AL
167
168Anything that is a program should have a shebang line. This includes .t files.
169
daad783f
AL
170=item * BuiltInFunctions::RequireConstantSprintfFormat
171
172=item * BuiltInFunctions::RequireConstantUnpackFormat
173
1cc04072 174L<http://diotalevi.isa-geek.net/~josh/yapc-lint/slides/slide5.html>
daad783f 175
dfe2eb3f
JRT
176=item * Miscellanea::ProhibitObnoxiousComments
177
178Forbid excessive hash marks e.g. "#### This is a loud comment ####".
179Make the obnoxious pattern configurable
180
181=item * ValuesAndExpressions::RequireNotOperator
182
183Require the use of "not" instead of "!", except when this would contradict
184ProhibitMixedBooleanOperators. This may be better suited for
185Perl::Critic::More.
186
187=item * Modules::RequireExplicitImporting
188
189Require every C<use> statement to have an explicit import list. You could
190still get around this by calling C<import> directly.
191
192=item * Modules::ForbidImporting
193
194Require every C<use> to have an explicitly empty import list. This is for
195folks who like to see fully-qualified function names. Should probably provide
196a list of exempt modules (like FindBin);
197
daad783f
AL
198=item * ControlStructures::ProhibitIncludeViaDo
199
200Forbid C<do "foo.pl">. Not sure about this policy name.
201
daad783f
AL
202=item * Variables::ProhibitUseVars
203
f707b14a
CD
204Disallow C<use vars qw(...)> and require C<our $foo> instead. This
205contradicts Miscellanea::Prohibit5006isms. Maybe verify C<use 5.6>
206before applying this policy. Low severity.
daad783f
AL
207
208=item * VariablesAndExpressions::ProhibitQuotedHashKeys
209
210Forbid quotes around hash keys, unless they are really needed. This
211is against what Damian says. Suggested by Adam Kennedy. Low
212severity.
213
b589a229 214=item * CodeLayout::ProhibitFunctionalNew
daad783f
AL
215
216Good: C<< Foo::Bar->new >>, Bad: C<< new Foo::Bar >>
217
218=item * VariablesAndExpressions::RequireConstantVersion (low severity)
219
f707b14a
CD
220=item * VariablesAndExpressions::ProhibitComplexVersion (medium severity)
221
daad783f
AL
222L<http://rt.cpan.org/Ticket/Display.html?id=20439>
223
79b72614
CD
224=item * Documentation::RequireSynopsis
225
226=item * Documentation::RequireLicense
227
228These are simplified versions of Documentation::RequirePodSections.
229
79b72614
CD
230=item * Miscellaneous::ProhibitBoilerplate
231
b589a229 232Complain about copy-and-paste code or docs from h2xs, Module::Starter::*,
79b72614 233etc.
c6e7b236 234
ef919624
CD
235Here's a non-PPI implementation:
236L<http://search.cpan.org/src/JJORE/Carp-Clan-5.8/t/04boilerplate.t>
237
956825ec
CD
238=item * BuiltinFunctions::ProhibitExtraneousScalarCall
239
240Recommend that C<if (scalar @array)> be rewritten as C<if (@array)>.
241
daad783f
AL
242=back
243
244=head1 REFACTORINGS and ENHANCEMENTS
245
246=over 4
247
628facda
ES
248=item * Create constants for the PPI location array elements.
249
992600be
JRT
250=item * MOVE THE LINE-DISABLING INTO P::C::Document
251
252All the code that deals with finding all the '##no critic' comments and noting
253which policies are disabled at each line seems like it would be better placed
254in Perl::Critic::Document. P::C::Document could then provide methods to
255indicate if a policy is disabled at a particular line. So the basic algorithm
256in Perl::Critic might look something like this:
257
258 foreach $element (@PPI_ELEMENTS) {
259 foreach $policy (@POLICIES) {
260 $line = $element->location->[0];
261 next if $doc->policy_is_disabled_at_line( $policy, $line );
262 push @violations, $policy->violates( $elem, $doc );
263 }
264 }
265
aae75e29
ES
266=item * Some means of detecting "runnaway" C<##no critic>
267
268Elliot was talking to a couple of users at ETech and one of their major
269concerns was that they were using C<##no critic> and forgetting to do a
270C<##use critic> after the problematic section. Perhaps an option to
271F<perlcritic> to scan for such things is in order.
272
dfe2eb3f
JRT
273=item * Change API to use named parameters
274
275Most of the methods on the public classes use named parameters for passing
276arguments. I'd like to extend that pattern to include all object-methods.
277Static methods can still use positional parameters.
278
279=item * Allow more flexible Policy parameter parsing
280
281Several policies use C<words_from_string()> to split their parameters into
282words. This function is currently limited to splitting on whitespace. It
283would be nice to allow some lattitude for users who might try and use commas
284or some other kind of delimiter.
285
286=item * Enhance P::C::critique() to accept files, directories, or code strings
501ee198 287
9b8f64cf 288Just like F<bin/perlcritic> does now.
daad783f 289
9b8f64cf
CD
290=item * Add C<-cache> flag to F<bin/perlcritic>
291
292If enabled, this turns on L<PPI::Cache>:
293
294 require PPI::Cache;
295 my $cache_path = "/tmp/test-perl-critic-cache-$ENV{USER}";
296 mkdir $cache_path, oct 700 if (! -d $cache_path);
297 PPI::Cache->import(path => $cache_path);
298
de8cacdb 299This cachedir should perhaps include the PPI version number! At least
84e87307 300until PPI incorporates its own version number in the cache.
9b8f64cf 301
de8cacdb 302(see F<t/40_criticize.t> for a more robust implementation)
79b72614 303
c5bd966c
JRT
304=item * Use hash-lookup instead of C<List::MoreUtils::any> function.
305
306In several places, Perl::Critic uses C<List::MoreUtils::any> to see if
307a string is a member of a list. Instead, I suggest using a named
308subroutine that does a hash-lookup like this:
309
310 my %logical_ops = hashify( qw( ! || && ||= &&= and or not ) );
311 sub is_logical_op { return exists $logical_ops{ $_[0] }; }
312
9b8f64cf
CD
313=back
314
315=head1 PPI BUGS
316
317We're waiting on the following bugs to get fixed in a CPAN release of PPI:
318
319=over 4
320
b589a229
CD
321=item literal()
322
323ValuesAndExpressions::RequireNumberSeparators uses a stringy eval to
324numify. Current PPI SVN has code for the
325PPI::Token::Number->literal() method which numifies from source. When
326we depend on a PPI version higher than 1.118, the _to_number()
327function in that policy can be removed in favor of literal().
328
c38138ab 329=item Newlines
b589a229
CD
330
331PPI does not preserve newlines. That makes
c38138ab
CD
332CodeLayout::RequireConsistentNewlines impossible to implement under PPI. For
333now, it's implemented by pulling the source out of the file and skipping PPI.
334
335It's unlikely that PPI will support mixde newlines anytime soon.
336
337=item Anonymous constructors in lists
338
339The following parses wrong in PPI v1.118. A PPI fix is in progress.
340
341 bless( {} );
342
343When this is fixed, uncomment a few tests in t/20_policies_classhierarchies.t
b589a229 344
d2d8975c
ES
345=item Hash constructors with a parenthesis directly to the left.
346
347The L<PPI::Statement> surrounding the L<PPI::Constructor> returns undef
348for C<location()> for the following:
349
350 ({})
351
352The same problem exists for
353
354 ({} )
355
356but not for
357
358 ( {})
359
360Logged as RT #23788.
361
362Remove trinary operator usage in RequireUseStrict, RequireUseWarnings, and
363RequireExplicitPackage once this is fixed.
364
c948abe5
ES
365=item L<PPI::Structure::Block>s being generated instead of L<PPI::Structure::Constructor>
366
367For complicated data structures, C<< { blah => blah } >> will result in a
368L<PPI::Structure::Block> being created, instead of a
369L<PPI::Structure::Constructor>.
370
371ValuesAndExpressions::ProhibitCommaSeparatedStatements and other policies have
372workarounds for this.
373
ef919624
CD
374=item Operators
375
376ValuesAndExpressions::ProhibitMismatchedOperators has two workarounds
377for PPI bugs with parsing operators. Many of these bugs have been
378fixed in PPI, so it would be good to check if those workarounds are
379still needed.
380
daad783f 381=back
501ee198 382
85e38a07 383=cut
dfe2eb3f
JRT
384
385##############################################################################
386# Local Variables:
387# mode: cperl
388# cperl-indent-level: 4
389# fill-column: 78
390# indent-tabs-mode: nil
391# c-indentation-style: bsd
392# End:
74dfa143 393# ex: set ts=8 sts=4 sw=4 tw=78 ft=pod expandtab :