File Coverage

blib/lib/Perl/Critic/Policy/ValuesAndExpressions/RequireConstantVersion.pm
Criterion Covered Total %
statement 47 93 50.5
branch 15 78 19.2
condition 1 60 1.6
subroutine 15 18 83.3
pod 4 5 80.0
total 82 254 32.2


line stmt bran cond sub pod time code
1             package Perl::Critic::Policy::ValuesAndExpressions::RequireConstantVersion;
2              
3 40     40   27778 use 5.010001;
  40         153  
4 40     40   228 use strict;
  40         95  
  40         831  
5 40     40   198 use warnings;
  40         104  
  40         1506  
6              
7 40         2280 use Perl::Critic::Utils qw<
8             :booleans :characters :classification :data_conversion :language
9             :severities
10 40     40   303 >;
  40         131  
11 40         2604 use Perl::Critic::Utils::PPI qw{
12             is_ppi_constant_element
13             get_next_element_in_same_simple_statement
14             get_previous_module_used_on_same_line
15 40     40   25208 };
  40         117  
16 40     40   288 use Readonly;
  40         112  
  40         1856  
17              
18 40     40   287 use parent 'Perl::Critic::Policy';
  40         149  
  40         240  
19              
20             our $VERSION = '1.150';
21              
22             #-----------------------------------------------------------------------------
23              
24             Readonly::Scalar my $BIND_REGEX => q<=~>;
25             Readonly::Scalar my $DOLLAR => q<$>;
26             # All uses of the $DOLLAR variable below are to prevent false failures in
27             # xt/93_version.t.
28             Readonly::Scalar my $QV => q<qv>;
29             Readonly::Scalar my $VERSION_MODULE => q<version>;
30             Readonly::Scalar my $VERSION_VARIABLE => $DOLLAR . q<VERSION>;
31              
32             # Operators which would make a new value our of our $VERSION, and therefore
33             # not modify it. I'm sure this list is not exhaustive. The logical operators
34             # generally do not qualify for this list. At least, I think not.
35             Readonly::Hash my %OPERATOR_WHICH_MAKES_NEW_VALUE => hashify( qw{
36             = . + - * ** / % ^ ~ & | > < == != >= <= eq ne gt lt ge le
37             } );
38              
39             Readonly::Scalar my $DESC => $DOLLAR . q<VERSION value must be a constant>;
40             Readonly::Scalar my $EXPL => qq<Computed ${DOLLAR}VERSION may tie the code to a single repository, or cause spooky action from a distance>;
41              
42             #-----------------------------------------------------------------------------
43              
44             sub supported_parameters { return (
45             {
46 90     90 0 2042 name => 'allow_version_without_use_on_same_line',
47             description =>
48             q{Allow qv() and version->new() without a 'use version' on the same line.},
49             default_string => $FALSE,
50             behavior => 'boolean',
51             }
52             );
53             }
54 74     74 1 378 sub default_severity { return $SEVERITY_LOW }
55 74     74 1 283 sub default_themes { return qw( core maintenance ) }
56 30     30 1 101 sub applies_to { return 'PPI::Token::Symbol' }
57              
58             #-----------------------------------------------------------------------------
59              
60             sub violates {
61 172     172 1 327 my ( $self, $elem, $doc ) = @_;
62              
63             # Any variable other than $VERSION is ignored.
64 172 100       326 return if $VERSION_VARIABLE ne $elem->content();
65              
66             # Get the next thing (presumably an operator) after $VERSION. The $VERSION
67             # might be in a list, so if we get nothing we move upwards until we hit a
68             # simple statement. If we have nothing at this point, we do not understand
69             # the code, and so we return.
70 27         152 my $operator;
71             return if
72 27 50       101 not $operator = get_next_element_in_same_simple_statement( $elem );
73              
74             # If the next operator is a regex binding, and its other operand is a
75             # substitution operator, it is an attempt to modify $VERSION, so we
76             # return an error to that effect.
77 27 50       871 return $self->violation( $DESC, $EXPL, $elem )
78             if $self->_validate_operator_bind_regex( $operator, $elem );
79              
80             # If the presumptive operator is not an assignment operator of some sort,
81             # we are not modifying $VERSION at all, and so we just return.
82 27 50       198 return if not $operator = _check_for_assignment_operator( $operator );
83              
84             # If there is no operand to the right of the assignment, we do not
85             # understand the code; simply return.
86 27         287 my $value;
87 27 50       82 return if not $value = $operator->snext_sibling();
88              
89             # If the value is symbol '$VERSION', just return as we will see it again
90             # later.
91             return if
92 27 50 33     745 $value->isa( 'PPI::Token::Symbol' )
93             and $value->content() eq $VERSION_VARIABLE;
94              
95             # If the value is a word, there are a number of acceptable things it could
96             # be. Check for these. If there was a problem, return it.
97 27         125 $value = $self->_validate_word_token( $elem, $value );
98 27 50       160 return $value if $value->isa( 'Perl::Critic::Exception' );
99              
100             # If the value is anything but a constant, we cry foul.
101 27 50       109 return $self->violation( $DESC, $EXPL, $elem )
102             if not is_ppi_constant_element( $value );
103              
104             # If we have nothing after the value, it is OK.
105 27         76 my $structure;
106             return if
107 27 50       99 not $structure = get_next_element_in_same_simple_statement( $value );
108              
109             # If we have a semicolon after the value, it is OK.
110 27 50       1193 return if $SCOLON eq $structure->content();
111              
112             # If there is anything else after the value, we cry foul.
113 0         0 return $self->violation( $DESC, $EXPL, $elem );
114             }
115              
116             #-----------------------------------------------------------------------------
117              
118             # Check if the element is an assignment operator.
119              
120             sub _check_for_assignment_operator {
121 27     27   65 my ( $operator ) = @_;
122              
123 27 50       106 return if not $operator->isa( 'PPI::Token::Operator' );
124 27 50       65 return $operator if is_assignment_operator($operator->content());
125 0         0 return;
126             }
127              
128             #-----------------------------------------------------------------------------
129              
130             # Validate a bind_regex ('=~') operator appearing after $VERSION. We return
131             # true if the operator is in fact '=~', and its next sibling isa
132             # PPI::Token::Regexp::Substitute. Otherwise we return false.
133              
134             sub _validate_operator_bind_regex {
135 27     27   100 my ( $self, $operator, $elem ) = @_;
136              
137             # We are not interested in anything but '=~ s/../../'.
138 27 50       98 return if $BIND_REGEX ne $operator->content();
139 0         0 my $operand;
140 0 0       0 return if not $operand = $operator->snext_sibling();
141 0 0       0 return if not $operand->isa( 'PPI::Token::Regexp::Substitute' );
142              
143             # The substitution is OK if it is of the form
144             # '($var = $VERSION) =~ s/../../'.
145              
146             # We can't look like the desired form if we have a next sig. sib.
147 0 0       0 return $TRUE if $elem->snext_sibling();
148              
149             # We can't look like the desired form if we are not in a list.
150 0         0 my $containing_list;
151 0 0 0     0 $containing_list = $elem->parent()
      0        
      0        
152             and $containing_list->isa( 'PPI::Statement' )
153             and $containing_list = $containing_list->parent()
154             and $containing_list->isa( 'PPI::Structure::List' )
155             or return $TRUE;
156              
157             # If we have no prior element, we're ( $VERSION ) =~ s/../../,
158             # which flunks.
159 0 0       0 my $prior = $elem->sprevious_sibling() or return $TRUE;
160              
161             # If the prior element is an operator which makes a new value, we pass.
162             return if $prior->isa( 'PPI::Token::Operator' )
163 0 0 0     0 && $OPERATOR_WHICH_MAKES_NEW_VALUE{ $prior->content() };
164              
165             # Now things get complicated, as RT #55600 shows. We need to grub through
166             # the entire list, looking for something that looks like a subroutine
167             # call, but without parens around the argument list. This catches the
168             # ticket's case, which was
169             # ( $foo = sprintf '%s/%s', __PACKAGE__, $VERSION ) =~ s/../../.
170 0         0 my $current = $prior;
171 0         0 while( $prior = $current->sprevious_sibling() ) {
172 0 0       0 $prior->isa( 'PPI::Token::Word' ) or next;
173 0 0       0 is_function_call( $prior) or next;
174             # If this function has its own argument list, we need to keep looking;
175             # otherwise we have found a function with no parens, and we can
176             # return.
177 0 0       0 $current->isa( 'PPI::Structure::List' )
178             or return;
179             } continue {
180 0         0 $current = $prior;
181             }
182              
183             # Maybe the whole list was arguments for a subroutine or method call.
184 0 0       0 $prior = $containing_list->sprevious_sibling()
185             or return $TRUE;
186 0 0       0 if ( $prior->isa( 'PPI::Token::Word' ) ) {
187 0 0       0 return if is_method_call( $prior );
188 0 0       0 return if is_function_call( $prior );
189             }
190              
191             # Anything left is presumed a violation.
192 0         0 return $TRUE;
193             }
194              
195             #-----------------------------------------------------------------------------
196              
197             # Validating a PPI::Token::Word is a complicated business, so we split it out
198             # into its own subroutine. The $elem is to be used in forming the error
199             # message, and the $value is the PPI::Token::Word we just encountered. The
200             # return is either a PPI::Element for further analysis, or a
201             # Perl::Critic::Exception to be returned.
202              
203             sub _validate_word_token {
204 27     27   66 my ( $self, $elem, $value ) = @_;
205              
206 27 50       140 if ( $value->isa( 'PPI::Token::Word' ) ) {
207 0         0 my $content = $value->content();
208              
209             # If the word is of the form 'v\d+' it may be the first portion of a
210             # misparsed (by PPI) v-string. It is really a v-string if the next
211             # element is a number. Unless v-strings are allowed, we return an
212             # error.
213 0 0       0 if ( $content =~ m/ \A v \d+ \z /smx ) {
    0          
    0          
214 0         0 $value = $self->_validate_word_vstring( $elem, $value );
215             }
216             elsif ( $QV eq $content ) {
217             # If the word is 'qv' we suspect use of the version module. If
218             # 'use version' appears on the same line, _and_ the remainder of
219             # the expression is of the form '(value)', we extract the value
220             # for further analysis.
221              
222 0         0 $value = $self->_validate_word_qv( $elem, $value );
223             }
224             elsif ( $VERSION_MODULE eq $content ) {
225             # If the word is 'version' we suspect use of the version module.
226             # Check to see if it is properly used.
227 0         0 $value = $self->_validate_word_version( $elem, $value );
228             }
229             }
230              
231 27         77 return $value;
232             }
233              
234             #-----------------------------------------------------------------------------
235              
236             # Validate $VERSION = v1.2.3;
237             # Note that this is needed because PPI mis-parses the 'v1.2.3' construct into
238             # a word ('v1') and a number of some sort ('.2.3'). This method should only be
239             # called if it is already known that the $value is a PPI::Token::Word matching
240             # m/ \A v \d+ \z /smx;
241              
242             sub _validate_word_vstring {
243 0     0     my ( $self, $elem, $value ) = @_;
244              
245             # Check for the second part of the mis-parsed v-string, flunking if it is
246             # not found.
247 0           my $next;
248 0 0 0       return $self->violation( $DESC, $EXPL, $elem )
249             if
250             not $next = $value->snext_sibling()
251             or not $next->isa( 'PPI::Token::Number' );
252              
253             # Return the second part of the v-string for further analysis.
254 0           return $next;
255             }
256              
257             #-----------------------------------------------------------------------------
258              
259             # Validate $VERSION = qv();
260              
261             sub _validate_word_qv {
262 0     0     my ( $self, $elem, $value ) = @_;
263              
264             # Unless we are specifically allowing this construction without the
265             # 'use version;' on the same line, check for it and flunk if we do not
266             # find it.
267             $self->{_allow_version_without_use_on_same_line}
268 0 0         or do {
269 0           my $module;
270 0 0         return $self->violation( $DESC, $EXPL, $elem )
271             if not
272             $module = get_previous_module_used_on_same_line($value);
273 0 0         return $self->violation( $DESC, $EXPL, $elem )
274             if $VERSION_MODULE ne $module->content();
275             };
276              
277             # Dig out the first argument of 'qv()', flunking if we can not find it.
278 0           my $next;
279 0 0 0       return $self->violation( $DESC, $EXPL, $elem )
      0        
      0        
      0        
280             if not (
281             $next = $value->snext_sibling()
282             and $next->isa( 'PPI::Structure::List' )
283             and $next = $next->schild( 0 )
284             and $next->isa( 'PPI::Statement::Expression' )
285             and $next = $next->schild( 0 )
286             );
287              
288             # Return the qv() argument for further analysis.
289 0           return $next;
290             }
291              
292             #-----------------------------------------------------------------------------
293              
294             # Validate $VERSION = version->new();
295              
296             # TODO: Fix this EVIL dual-purpose return value. This is ugggggleeeee.
297             sub _validate_word_version {
298 0     0     my ( $self, $elem, $value ) = @_;
299              
300             # Unless we are specifically allowing this construction without the
301             # 'use version;' on the same line, check for it and flunk if we do not
302             # find it.
303             $self->{_allow_version_without_use_on_same_line}
304 0 0         or do {
305 0           my $module;
306 0 0         return $self->violation( $DESC, $EXPL, $elem )
307             if not
308             $module = get_previous_module_used_on_same_line($value);
309 0 0         return $self->violation( $DESC, $EXPL, $elem )
310             if $VERSION_MODULE ne $module->content();
311             };
312              
313             # Dig out the first argument of '->new()', flunking if we can not find it.
314 0           my $next;
315 0 0 0       return $next if
      0        
      0        
      0        
      0        
      0        
      0        
      0        
      0        
      0        
316             $next = $value->snext_sibling()
317             and $next->isa( 'PPI::Token::Operator' )
318             and q{->} eq $next->content()
319             and $next = $next->snext_sibling()
320             and $next->isa( 'PPI::Token::Word' )
321             and q{new} eq $next->content()
322             and $next = $next->snext_sibling()
323             and $next->isa( 'PPI::Structure::List' )
324             and $next = $next->schild( 0 )
325             and $next->isa( 'PPI::Statement::Expression' )
326             and $next = $next->schild( 0 );
327              
328 0           return $self->violation( $DESC, $EXPL, $elem );
329             }
330              
331             1;
332              
333             __END__
334              
335             #-----------------------------------------------------------------------------
336              
337             =pod
338              
339             =head1 NAME
340              
341             Perl::Critic::Policy::ValuesAndExpressions::RequireConstantVersion - Require $VERSION to be a constant rather than a computed value.
342              
343              
344             =head1 AFFILIATION
345              
346             This Policy is part of the core L<Perl::Critic|Perl::Critic>
347             distribution.
348              
349              
350             =head1 DESCRIPTION
351              
352             The $VERSION variable of a module should be a simple constant - either a
353             number, a single-quotish string, or a 'use version' object. In the latter case
354             the 'use version;' must appear on the same line as the object construction.
355              
356             Computing the version has problems of various severities.
357              
358             The most benign violation is computing the version from (e.g.) a Subversion
359             revision number:
360              
361             our ($VERSION) = q$REVISION: 42$ =~ /(\d+)/;
362              
363             The problem here is that the version is tied to a single repository. The code
364             can not be moved to another repository (even of the same type) without
365             changing its version, possibly in the wrong direction.
366              
367             This policy accepts v-strings (C<v1.2.3> or just plain C<1.2.3>), since these
368             are already flagged by
369             L<Perl::Critic::Policy::ValuesAndExpressions::ProhibitVersionStrings|Perl::Critic::Policy::ValuesAndExpressions::ProhibitVersionStrings>.
370              
371              
372             =head1 CONFIGURATION
373              
374             The proper way to set a module's $VERSION to a C<version> object is to
375             C<use version;> on the same line of code that assigns the value of $VERSION.
376             That way, L<ExtUtils::MakeMaker|ExtUtils::MakeMaker> and
377             L<Module::Build|Module::Build> can extract the version when packaging the
378             module for CPAN. By default, this policy declares an error if this is not
379             done.
380              
381             Should you wish to allow version objects without loading the version module on
382             the same line, add the following to your configuration file:
383              
384             [ValuesAndExpressions::RequireConstantVersion]
385             allow_version_without_use_on_same_line = 1
386              
387              
388             =head1 CAVEATS
389              
390             There will be false negatives if the $VERSION appears on the left-hand side of
391             a list assignment that assigns to more than one variable, or to C<undef>.
392              
393             There may be false positives if the $VERSION is assigned the value of a here
394             document. This will probably remain the case until
395             L<PPI::Token::HereDoc|PPI::Token::HereDoc> acquires the relevant portions of
396             the L<PPI::Token::Quote|PPI::Token::Quote> interface.
397              
398             There will be false positives if $VERSION is assigned the value of a constant
399             created by the L<Readonly|Readonly> module or the L<constant|constant> pragma,
400             because the necessary infrastructure appears not to exist, and the author of
401             the present module lacked the knowledge/expertise/gumption to put it in place.
402              
403             Currently the idiom
404              
405             our $VERSION = '1.005_05';
406             $VERSION = eval $VERSION;
407              
408             will produce a violation on the second line of the example.
409              
410              
411             =head1 AUTHOR
412              
413             Thomas R. Wyant, III F<wyant at cpan dot org>
414              
415              
416             =head1 COPYRIGHT
417              
418             Copyright (c) 2009-2023 Tom Wyant
419              
420             This program is free software; you can redistribute it and/or modify
421             it under the same terms as Perl itself. The full text of this license
422             can be found in the LICENSE file included with this module
423              
424             =cut
425              
426             # Local Variables:
427             # mode: cperl
428             # cperl-indent-level: 4
429             # fill-column: 78
430             # indent-tabs-mode: nil
431             # c-indentation-style: bsd
432             # End:
433             # ex: set ts=8 sts=4 sw=4 tw=78 ft=perl expandtab shiftround :