line |
stmt |
bran |
cond |
sub |
pod |
time |
code |
1
|
|
|
|
|
|
|
package Perl::Critic::Policy::Subroutines::RequireArgUnpacking; |
2
|
|
|
|
|
|
|
|
3
|
40
|
|
|
40
|
|
28232
|
use 5.010001; |
|
40
|
|
|
|
|
194
|
|
4
|
40
|
|
|
40
|
|
249
|
use strict; |
|
40
|
|
|
|
|
108
|
|
|
40
|
|
|
|
|
955
|
|
5
|
40
|
|
|
40
|
|
264
|
use warnings; |
|
40
|
|
|
|
|
106
|
|
|
40
|
|
|
|
|
1066
|
|
6
|
|
|
|
|
|
|
|
7
|
40
|
|
|
40
|
|
273
|
use Readonly; |
|
40
|
|
|
|
|
120
|
|
|
40
|
|
|
|
|
2084
|
|
8
|
|
|
|
|
|
|
|
9
|
40
|
|
|
|
|
2277
|
use Perl::Critic::Utils qw( |
10
|
|
|
|
|
|
|
:booleans :characters :classification hashify :severities |
11
|
40
|
|
|
40
|
|
297
|
); |
|
40
|
|
|
|
|
153
|
|
12
|
40
|
|
|
40
|
|
21759
|
use parent 'Perl::Critic::Policy'; |
|
40
|
|
|
|
|
98
|
|
|
40
|
|
|
|
|
257
|
|
13
|
|
|
|
|
|
|
|
14
|
|
|
|
|
|
|
our $VERSION = '1.150'; |
15
|
|
|
|
|
|
|
|
16
|
|
|
|
|
|
|
#----------------------------------------------------------------------------- |
17
|
|
|
|
|
|
|
|
18
|
|
|
|
|
|
|
Readonly::Scalar my $AT => q{@}; |
19
|
|
|
|
|
|
|
Readonly::Scalar my $AT_ARG => q{@_}; ## no critic (InterpolationOfMetachars) |
20
|
|
|
|
|
|
|
Readonly::Scalar my $DEREFERENCE => q{->}; |
21
|
|
|
|
|
|
|
Readonly::Scalar my $DOLLAR => q{$}; |
22
|
|
|
|
|
|
|
Readonly::Scalar my $DOLLAR_ARG => q{$_}; ## no critic (InterpolationOfMetaChars) |
23
|
|
|
|
|
|
|
|
24
|
|
|
|
|
|
|
Readonly::Scalar my $DESC => qq{Always unpack $AT_ARG first}; |
25
|
|
|
|
|
|
|
Readonly::Scalar my $EXPL => [178]; |
26
|
|
|
|
|
|
|
|
27
|
|
|
|
|
|
|
#----------------------------------------------------------------------------- |
28
|
|
|
|
|
|
|
|
29
|
|
|
|
|
|
|
sub supported_parameters { |
30
|
|
|
|
|
|
|
return ( |
31
|
|
|
|
|
|
|
{ |
32
|
93
|
|
|
93
|
0
|
3223
|
name => 'short_subroutine_statements', |
33
|
|
|
|
|
|
|
description => |
34
|
|
|
|
|
|
|
'The number of statements to allow without unpacking.', |
35
|
|
|
|
|
|
|
default_string => '0', |
36
|
|
|
|
|
|
|
behavior => 'integer', |
37
|
|
|
|
|
|
|
integer_minimum => 0, |
38
|
|
|
|
|
|
|
}, |
39
|
|
|
|
|
|
|
{ |
40
|
|
|
|
|
|
|
name => 'allow_subscripts', |
41
|
|
|
|
|
|
|
description => |
42
|
|
|
|
|
|
|
'Should unpacking from array slices and elements be allowed?', |
43
|
|
|
|
|
|
|
default_string => $FALSE, |
44
|
|
|
|
|
|
|
behavior => 'boolean', |
45
|
|
|
|
|
|
|
}, |
46
|
|
|
|
|
|
|
{ |
47
|
|
|
|
|
|
|
name => 'allow_delegation_to', |
48
|
|
|
|
|
|
|
description => |
49
|
|
|
|
|
|
|
'Allow the usual delegation idiom to these namespaces/subroutines', |
50
|
|
|
|
|
|
|
behavior => 'string list', |
51
|
|
|
|
|
|
|
list_always_present_values => [ qw< SUPER:: NEXT:: > ], |
52
|
|
|
|
|
|
|
}, |
53
|
|
|
|
|
|
|
{ |
54
|
|
|
|
|
|
|
name => 'allow_closures', |
55
|
|
|
|
|
|
|
description => 'Allow unpacking by a closure', |
56
|
|
|
|
|
|
|
default_string => $FALSE, |
57
|
|
|
|
|
|
|
behavior => 'boolean', |
58
|
|
|
|
|
|
|
}, |
59
|
|
|
|
|
|
|
); |
60
|
|
|
|
|
|
|
} |
61
|
|
|
|
|
|
|
|
62
|
74
|
|
|
74
|
1
|
334
|
sub default_severity { return $SEVERITY_HIGH } |
63
|
86
|
|
|
86
|
1
|
337
|
sub default_themes { return qw( core pbp maintenance ) } |
64
|
32
|
|
|
32
|
1
|
92
|
sub applies_to { return 'PPI::Statement::Sub' } |
65
|
|
|
|
|
|
|
|
66
|
|
|
|
|
|
|
#----------------------------------------------------------------------------- |
67
|
|
|
|
|
|
|
|
68
|
|
|
|
|
|
|
sub violates { |
69
|
5
|
|
|
5
|
1
|
26
|
my ( $self, $elem, undef ) = @_; |
70
|
|
|
|
|
|
|
|
71
|
|
|
|
|
|
|
# forward declaration? |
72
|
5
|
50
|
|
|
|
19
|
return if not $elem->block; |
73
|
|
|
|
|
|
|
|
74
|
5
|
|
|
|
|
125
|
my @statements = $elem->block->schildren; |
75
|
|
|
|
|
|
|
|
76
|
|
|
|
|
|
|
# empty sub? |
77
|
5
|
50
|
|
|
|
152
|
return if not @statements; |
78
|
|
|
|
|
|
|
|
79
|
|
|
|
|
|
|
# Don't apply policy to short subroutines |
80
|
|
|
|
|
|
|
|
81
|
|
|
|
|
|
|
# Should we instead be doing a find() for PPI::Statement |
82
|
|
|
|
|
|
|
# instances? That is, should we count all statements instead of |
83
|
|
|
|
|
|
|
# just top-level statements? |
84
|
5
|
50
|
|
|
|
34
|
return if $self->{_short_subroutine_statements} >= @statements; |
85
|
|
|
|
|
|
|
|
86
|
|
|
|
|
|
|
# look for explicit dereferences of @_, including '$_[0]' |
87
|
|
|
|
|
|
|
# You may use "... = @_;" in the first paragraph of the sub |
88
|
|
|
|
|
|
|
# Don't descend into nested or anonymous subs |
89
|
5
|
|
|
|
|
14
|
my $state = 'unpacking'; # still in unpacking paragraph |
90
|
5
|
|
|
|
|
16
|
for my $statement (@statements) { |
91
|
|
|
|
|
|
|
|
92
|
7
|
|
|
|
|
24
|
my @magic = _get_arg_symbols($statement); |
93
|
|
|
|
|
|
|
|
94
|
7
|
|
|
|
|
126
|
my $saw_unpack = $FALSE; |
95
|
|
|
|
|
|
|
|
96
|
|
|
|
|
|
|
MAGIC: |
97
|
7
|
|
|
|
|
18
|
for my $magic (@magic) { |
98
|
|
|
|
|
|
|
# allow conditional checks on the size of @_ |
99
|
0
|
0
|
|
|
|
0
|
next MAGIC if _is_size_check($magic); |
100
|
|
|
|
|
|
|
|
101
|
0
|
0
|
|
|
|
0
|
if ('unpacking' eq $state) { |
102
|
0
|
0
|
|
|
|
0
|
if ($self->_is_unpack($magic)) { |
103
|
0
|
|
|
|
|
0
|
$saw_unpack = $TRUE; |
104
|
0
|
|
|
|
|
0
|
next MAGIC; |
105
|
|
|
|
|
|
|
} |
106
|
|
|
|
|
|
|
} |
107
|
|
|
|
|
|
|
|
108
|
|
|
|
|
|
|
# allow @$_[] construct in "... for ();" |
109
|
|
|
|
|
|
|
# Check for "print @$_[] for ()" construct (rt39601) |
110
|
|
|
|
|
|
|
next MAGIC |
111
|
0
|
0
|
0
|
|
|
0
|
if _is_cast_of_array($magic) and _is_postfix_foreach($magic); |
112
|
|
|
|
|
|
|
|
113
|
|
|
|
|
|
|
# allow $$_[], which is equivalent to $_->[] and not a use |
114
|
|
|
|
|
|
|
# of @_ at all. |
115
|
|
|
|
|
|
|
next MAGIC |
116
|
0
|
0
|
|
|
|
0
|
if _is_cast_of_scalar( $magic ); |
117
|
|
|
|
|
|
|
|
118
|
|
|
|
|
|
|
# allow delegation of the form "$self->SUPER::foo( @_ );" |
119
|
|
|
|
|
|
|
next MAGIC |
120
|
0
|
0
|
|
|
|
0
|
if $self->_is_delegation( $magic ); |
121
|
|
|
|
|
|
|
|
122
|
|
|
|
|
|
|
# If we make it this far, it is a violation |
123
|
0
|
|
|
|
|
0
|
return $self->violation( $DESC, $EXPL, $elem ); |
124
|
|
|
|
|
|
|
} |
125
|
7
|
50
|
|
|
|
21
|
if (not $saw_unpack) { |
126
|
7
|
|
|
|
|
31
|
$state = 'post_unpacking'; |
127
|
|
|
|
|
|
|
} |
128
|
|
|
|
|
|
|
} |
129
|
5
|
|
|
|
|
22
|
return; # OK |
130
|
|
|
|
|
|
|
} |
131
|
|
|
|
|
|
|
|
132
|
|
|
|
|
|
|
sub _is_unpack { |
133
|
0
|
|
|
0
|
|
0
|
my ($self, $magic) = @_; |
134
|
|
|
|
|
|
|
|
135
|
0
|
|
|
|
|
0
|
my $prev = $magic->sprevious_sibling(); |
136
|
0
|
|
|
|
|
0
|
my $next = $magic->snext_sibling(); |
137
|
|
|
|
|
|
|
|
138
|
|
|
|
|
|
|
# If we have a subscript, we're dealing with an array slice on @_ |
139
|
|
|
|
|
|
|
# or an array element of @_. See RT #34009. |
140
|
0
|
0
|
0
|
|
|
0
|
if ( $next and $next->isa('PPI::Structure::Subscript') ) { |
141
|
0
|
0
|
|
|
|
0
|
$self->{_allow_subscripts} or return; |
142
|
0
|
|
|
|
|
0
|
$next = $next->snext_sibling; |
143
|
|
|
|
|
|
|
} |
144
|
|
|
|
|
|
|
|
145
|
0
|
0
|
0
|
|
|
0
|
return $TRUE if |
|
|
|
0
|
|
|
|
|
|
|
|
0
|
|
|
|
|
|
|
|
0
|
|
|
|
|
146
|
|
|
|
|
|
|
$prev |
147
|
|
|
|
|
|
|
and $prev->isa('PPI::Token::Operator') |
148
|
|
|
|
|
|
|
and is_assignment_operator($prev->content()) |
149
|
|
|
|
|
|
|
and ( |
150
|
|
|
|
|
|
|
not $next |
151
|
|
|
|
|
|
|
or $next->isa('PPI::Token::Structure') |
152
|
|
|
|
|
|
|
and $SCOLON eq $next->content() |
153
|
|
|
|
|
|
|
); |
154
|
0
|
|
|
|
|
0
|
return; |
155
|
|
|
|
|
|
|
} |
156
|
|
|
|
|
|
|
|
157
|
|
|
|
|
|
|
sub _is_size_check { |
158
|
0
|
|
|
0
|
|
0
|
my ($magic) = @_; |
159
|
|
|
|
|
|
|
|
160
|
|
|
|
|
|
|
# No size check on $_[0]. RT #34009. |
161
|
0
|
0
|
|
|
|
0
|
$AT eq $magic->raw_type or return; |
162
|
|
|
|
|
|
|
|
163
|
0
|
|
|
|
|
0
|
my $prev = $magic->sprevious_sibling; |
164
|
0
|
|
|
|
|
0
|
my $next = $magic->snext_sibling; |
165
|
|
|
|
|
|
|
|
166
|
0
|
0
|
0
|
|
|
0
|
if ( $prev || $next ) { |
167
|
|
|
|
|
|
|
|
168
|
0
|
0
|
0
|
|
|
0
|
return $TRUE |
169
|
|
|
|
|
|
|
if _legal_before_size_check( $prev ) |
170
|
|
|
|
|
|
|
and _legal_after_size_check( $next ); |
171
|
|
|
|
|
|
|
} |
172
|
|
|
|
|
|
|
|
173
|
0
|
|
|
|
|
0
|
my $parent = $magic; |
174
|
|
|
|
|
|
|
{ |
175
|
0
|
0
|
|
|
|
0
|
$parent = $parent->parent() |
|
0
|
|
|
|
|
0
|
|
176
|
|
|
|
|
|
|
or return; |
177
|
0
|
|
|
|
|
0
|
$prev = $parent->sprevious_sibling(); |
178
|
0
|
|
|
|
|
0
|
$next = $parent->snext_sibling(); |
179
|
0
|
0
|
0
|
|
|
0
|
$prev |
180
|
|
|
|
|
|
|
or $next |
181
|
|
|
|
|
|
|
or redo; |
182
|
|
|
|
|
|
|
} # until ( $prev || $next ); |
183
|
|
|
|
|
|
|
|
184
|
0
|
0
|
|
|
|
0
|
return $TRUE |
185
|
|
|
|
|
|
|
if $parent->isa( 'PPI::Structure::Condition' ); |
186
|
|
|
|
|
|
|
|
187
|
0
|
|
|
|
|
0
|
return; |
188
|
|
|
|
|
|
|
} |
189
|
|
|
|
|
|
|
|
190
|
|
|
|
|
|
|
{ |
191
|
|
|
|
|
|
|
|
192
|
|
|
|
|
|
|
Readonly::Hash my %LEGAL_NEXT_OPER => hashify( |
193
|
|
|
|
|
|
|
qw{ && || == != > >= < <= and or } ); |
194
|
|
|
|
|
|
|
|
195
|
|
|
|
|
|
|
Readonly::Hash my %LEGAL_NEXT_STRUCT => hashify( qw{ ; } ); |
196
|
|
|
|
|
|
|
|
197
|
|
|
|
|
|
|
sub _legal_after_size_check { |
198
|
0
|
|
|
0
|
|
0
|
my ( $next ) = @_; |
199
|
|
|
|
|
|
|
|
200
|
0
|
0
|
|
|
|
0
|
$next |
201
|
|
|
|
|
|
|
or return $TRUE; |
202
|
|
|
|
|
|
|
|
203
|
|
|
|
|
|
|
$next->isa( 'PPI::Token::Operator' ) |
204
|
0
|
0
|
|
|
|
0
|
and return $LEGAL_NEXT_OPER{ $next->content() }; |
205
|
|
|
|
|
|
|
|
206
|
|
|
|
|
|
|
$next->isa( 'PPI::Token::Structure' ) |
207
|
0
|
0
|
|
|
|
0
|
and return $LEGAL_NEXT_STRUCT{ $next->content() }; |
208
|
|
|
|
|
|
|
|
209
|
0
|
|
|
|
|
0
|
return; |
210
|
|
|
|
|
|
|
} |
211
|
|
|
|
|
|
|
} |
212
|
|
|
|
|
|
|
|
213
|
|
|
|
|
|
|
{ |
214
|
|
|
|
|
|
|
|
215
|
|
|
|
|
|
|
Readonly::Hash my %LEGAL_PREV_OPER => hashify( |
216
|
|
|
|
|
|
|
qw{ && || ! == != > >= < <= and or not } ); |
217
|
|
|
|
|
|
|
|
218
|
|
|
|
|
|
|
Readonly::Hash my %LEGAL_PREV_WORD => hashify( |
219
|
|
|
|
|
|
|
qw{ if unless } ); |
220
|
|
|
|
|
|
|
|
221
|
|
|
|
|
|
|
sub _legal_before_size_check { |
222
|
0
|
|
|
0
|
|
0
|
my ( $prev ) = @_; |
223
|
|
|
|
|
|
|
|
224
|
0
|
0
|
|
|
|
0
|
$prev |
225
|
|
|
|
|
|
|
or return $TRUE; |
226
|
|
|
|
|
|
|
|
227
|
|
|
|
|
|
|
$prev->isa( 'PPI::Token::Operator' ) |
228
|
0
|
0
|
|
|
|
0
|
and return $LEGAL_PREV_OPER{ $prev->content() }; |
229
|
|
|
|
|
|
|
|
230
|
|
|
|
|
|
|
$prev->isa( 'PPI::Token::Word' ) |
231
|
0
|
0
|
|
|
|
0
|
and return $LEGAL_PREV_WORD{ $prev->content() }; |
232
|
|
|
|
|
|
|
|
233
|
0
|
|
|
|
|
0
|
return; |
234
|
|
|
|
|
|
|
} |
235
|
|
|
|
|
|
|
|
236
|
|
|
|
|
|
|
} |
237
|
|
|
|
|
|
|
|
238
|
|
|
|
|
|
|
sub _is_postfix_foreach { |
239
|
0
|
|
|
0
|
|
0
|
my ($magic) = @_; |
240
|
|
|
|
|
|
|
|
241
|
0
|
|
|
|
|
0
|
my $sibling = $magic; |
242
|
0
|
|
|
|
|
0
|
while ( $sibling = $sibling->snext_sibling ) { |
243
|
0
|
0
|
0
|
|
|
0
|
return $TRUE |
244
|
|
|
|
|
|
|
if |
245
|
|
|
|
|
|
|
$sibling->isa('PPI::Token::Word') |
246
|
|
|
|
|
|
|
and $sibling =~ m< \A for (?:each)? \z >xms; |
247
|
|
|
|
|
|
|
} |
248
|
0
|
|
|
|
|
0
|
return; |
249
|
|
|
|
|
|
|
} |
250
|
|
|
|
|
|
|
|
251
|
|
|
|
|
|
|
sub _is_cast_of_array { |
252
|
0
|
|
|
0
|
|
0
|
my ($magic) = @_; |
253
|
|
|
|
|
|
|
|
254
|
0
|
|
|
|
|
0
|
my $prev = $magic->sprevious_sibling; |
255
|
|
|
|
|
|
|
|
256
|
0
|
0
|
0
|
|
|
0
|
return $TRUE |
|
|
|
0
|
|
|
|
|
257
|
|
|
|
|
|
|
if ( $prev && $prev->content() eq $AT ) |
258
|
|
|
|
|
|
|
and $prev->isa('PPI::Token::Cast'); |
259
|
0
|
|
|
|
|
0
|
return; |
260
|
|
|
|
|
|
|
} |
261
|
|
|
|
|
|
|
|
262
|
|
|
|
|
|
|
# This subroutine recognizes (e.g.) $$_[0]. This is a use of $_ (equivalent to |
263
|
|
|
|
|
|
|
# $_->[0]), not @_. |
264
|
|
|
|
|
|
|
|
265
|
|
|
|
|
|
|
sub _is_cast_of_scalar { |
266
|
0
|
|
|
0
|
|
0
|
my ($magic) = @_; |
267
|
|
|
|
|
|
|
|
268
|
0
|
|
|
|
|
0
|
my $prev = $magic->sprevious_sibling; |
269
|
0
|
|
|
|
|
0
|
my $next = $magic->snext_sibling; |
270
|
|
|
|
|
|
|
|
271
|
0
|
|
0
|
|
|
0
|
return $DOLLAR_ARG eq $magic->content() && |
272
|
|
|
|
|
|
|
$prev && $prev->isa('PPI::Token::Cast') && |
273
|
|
|
|
|
|
|
$DOLLAR eq $prev->content() && |
274
|
|
|
|
|
|
|
$next && $next->isa('PPI::Structure::Subscript'); |
275
|
|
|
|
|
|
|
} |
276
|
|
|
|
|
|
|
|
277
|
|
|
|
|
|
|
# A literal @_ is allowed as the argument for a delegation. |
278
|
|
|
|
|
|
|
# An example of the idiom we are looking for is $self->SUPER::foo(@_). |
279
|
|
|
|
|
|
|
# The argument list of (@_) is required; no other use of @_ is allowed. |
280
|
|
|
|
|
|
|
|
281
|
|
|
|
|
|
|
sub _is_delegation { |
282
|
0
|
|
|
0
|
|
0
|
my ($self, $magic) = @_; |
283
|
|
|
|
|
|
|
|
284
|
0
|
0
|
|
|
|
0
|
$AT_ARG eq $magic->content() or return; # Not a literal '@_'. |
285
|
0
|
0
|
|
|
|
0
|
my $parent = $magic->parent() # Don't know what to do with |
286
|
|
|
|
|
|
|
or return; # orphans. |
287
|
0
|
0
|
|
|
|
0
|
$parent->isa( 'PPI::Statement::Expression' ) |
288
|
|
|
|
|
|
|
or return; # Parent must be expression. |
289
|
0
|
0
|
|
|
|
0
|
1 == $parent->schildren() # '@_' must stand alone in |
290
|
|
|
|
|
|
|
or return; # its expression. |
291
|
0
|
0
|
|
|
|
0
|
$parent = $parent->parent() # Still don't know what to do |
292
|
|
|
|
|
|
|
or return; # with orphans. |
293
|
0
|
0
|
|
|
|
0
|
$parent->isa ( 'PPI::Structure::List' ) |
294
|
|
|
|
|
|
|
or return; # Parent must be a list. |
295
|
0
|
0
|
|
|
|
0
|
1 == $parent->schildren() # '@_' must stand alone in |
296
|
|
|
|
|
|
|
or return; # the argument list. |
297
|
0
|
0
|
|
|
|
0
|
my $subroutine_name = $parent->sprevious_sibling() |
298
|
|
|
|
|
|
|
or return; # Missing sub name. |
299
|
0
|
0
|
0
|
|
|
0
|
if ( $subroutine_name->isa( 'PPI::Token::Word' ) ) { |
|
|
0
|
|
|
|
|
|
300
|
0
|
0
|
|
|
|
0
|
$self->{_allow_delegation_to}{$subroutine_name} |
301
|
|
|
|
|
|
|
and return 1; |
302
|
0
|
0
|
|
|
|
0
|
my ($subroutine_namespace) = $subroutine_name =~ m/ \A ( .* ::) \w+ \z /smx |
303
|
|
|
|
|
|
|
or return; |
304
|
0
|
|
|
|
|
0
|
return $self->{_allow_delegation_to}{$subroutine_namespace}; |
305
|
|
|
|
|
|
|
} elsif ( $self->{_allow_closures} && |
306
|
|
|
|
|
|
|
_is_dereference_operator( $subroutine_name ) ) { |
307
|
0
|
|
|
|
|
0
|
my $prev_sib = $subroutine_name; |
308
|
|
|
|
|
|
|
{ # Single-iteration loop |
309
|
0
|
0
|
|
|
|
0
|
$prev_sib = $prev_sib->sprevious_sibling() |
|
0
|
|
|
|
|
0
|
|
310
|
|
|
|
|
|
|
or return; |
311
|
0
|
0
|
|
|
|
0
|
( $prev_sib->isa( 'PPI::Structure::Subscript' || |
312
|
|
|
|
|
|
|
_is_dereference_operator( $prev_sib ) ) ) |
313
|
|
|
|
|
|
|
and redo; |
314
|
|
|
|
|
|
|
} |
315
|
0
|
|
|
|
|
0
|
return $prev_sib->isa( 'PPI::Token::Symbol' ); |
316
|
|
|
|
|
|
|
} |
317
|
0
|
|
|
|
|
0
|
return; |
318
|
|
|
|
|
|
|
} |
319
|
|
|
|
|
|
|
|
320
|
|
|
|
|
|
|
sub _is_dereference_operator { |
321
|
0
|
|
|
0
|
|
0
|
my ( $elem ) = @_; |
322
|
0
|
0
|
|
|
|
0
|
$elem |
323
|
|
|
|
|
|
|
or return; |
324
|
0
|
0
|
|
|
|
0
|
$elem->isa( 'PPI::Token::Operator' ) |
325
|
|
|
|
|
|
|
or return; |
326
|
0
|
|
|
|
|
0
|
return $DEREFERENCE eq $elem->content(); |
327
|
|
|
|
|
|
|
} |
328
|
|
|
|
|
|
|
|
329
|
|
|
|
|
|
|
|
330
|
|
|
|
|
|
|
sub _get_arg_symbols { |
331
|
7
|
|
|
7
|
|
15
|
my ($statement) = @_; |
332
|
|
|
|
|
|
|
|
333
|
7
|
50
|
|
|
|
13
|
return grep {$AT_ARG eq $_->symbol} @{$statement->find(\&_magic_finder) || []}; |
|
0
|
|
|
|
|
0
|
|
|
7
|
|
|
|
|
27
|
|
334
|
|
|
|
|
|
|
} |
335
|
|
|
|
|
|
|
|
336
|
|
|
|
|
|
|
sub _magic_finder { |
337
|
|
|
|
|
|
|
# Find all @_ and $_[\d+] not inside of nested subs |
338
|
39
|
|
|
39
|
|
400
|
my (undef, $elem) = @_; |
339
|
39
|
50
|
|
|
|
116
|
return $TRUE if $elem->isa('PPI::Token::Magic'); # match |
340
|
|
|
|
|
|
|
|
341
|
39
|
50
|
|
|
|
92
|
if ($elem->isa('PPI::Structure::Block')) { |
342
|
|
|
|
|
|
|
# don't descend into a nested named sub |
343
|
0
|
0
|
|
|
|
0
|
return if $elem->statement->isa('PPI::Statement::Sub'); |
344
|
|
|
|
|
|
|
|
345
|
|
|
|
|
|
|
# don't descend into a nested anon sub, either. |
346
|
0
|
0
|
|
|
|
0
|
return if _is_anon_sub( $elem ); |
347
|
|
|
|
|
|
|
|
348
|
|
|
|
|
|
|
} |
349
|
|
|
|
|
|
|
|
350
|
39
|
|
|
|
|
67
|
return $FALSE; # no match, descend |
351
|
|
|
|
|
|
|
} |
352
|
|
|
|
|
|
|
|
353
|
|
|
|
|
|
|
# Detecting anonymous subs is hard, partly because PPI's parse of them, at |
354
|
|
|
|
|
|
|
# least as of 1.220, appears to be a bit dodgy. |
355
|
|
|
|
|
|
|
sub _is_anon_sub { |
356
|
0
|
|
|
0
|
|
|
my ( $elem ) = @_; |
357
|
|
|
|
|
|
|
|
358
|
|
|
|
|
|
|
# If we have no previous element, we can not be an anonymous sub. |
359
|
0
|
0
|
|
|
|
|
my $prev = $elem->sprevious_sibling() |
360
|
|
|
|
|
|
|
or return $FALSE; |
361
|
|
|
|
|
|
|
|
362
|
|
|
|
|
|
|
# The simple case. |
363
|
0
|
0
|
0
|
|
|
|
return $TRUE if $prev->isa( 'PPI::Token::Word' ) |
364
|
|
|
|
|
|
|
and 'sub' eq $prev->content(); |
365
|
|
|
|
|
|
|
|
366
|
|
|
|
|
|
|
# Skip possible subroutine attributes. These appear as words (the names) |
367
|
|
|
|
|
|
|
# or lists (the arguments, if any), or actual attributes (depending on how |
368
|
|
|
|
|
|
|
# PPI handles them). A colon is required before the first, and is optional |
369
|
|
|
|
|
|
|
# in between. |
370
|
0
|
|
0
|
|
|
|
while ( $prev->isa( 'PPI::Token::Word' ) |
|
|
|
0
|
|
|
|
|
|
|
|
0
|
|
|
|
|
|
|
|
0
|
|
|
|
|
371
|
|
|
|
|
|
|
or $prev->isa( 'PPI::Structure::List' ) |
372
|
|
|
|
|
|
|
or $prev->isa( 'PPI::Token::Attribute' ) |
373
|
|
|
|
|
|
|
or $prev->isa( 'PPI::Token::Operator' ) |
374
|
|
|
|
|
|
|
and q<:> eq $prev->content() ) { |
375
|
|
|
|
|
|
|
|
376
|
|
|
|
|
|
|
# Grab the previous significant sib. If there is none, we can not |
377
|
|
|
|
|
|
|
# be an anonymous sub with attributes. |
378
|
0
|
0
|
|
|
|
|
return $FALSE if not $prev = $prev->sprevious_sibling(); |
379
|
|
|
|
|
|
|
} |
380
|
|
|
|
|
|
|
|
381
|
|
|
|
|
|
|
# PPI 1.220 may parse the 'sub :' erroneously as a label. If we find that, |
382
|
|
|
|
|
|
|
# it means our block is the body of an anonymous subroutine. |
383
|
0
|
0
|
0
|
|
|
|
return $TRUE if $prev->isa( 'PPI::Token::Label' ) |
384
|
|
|
|
|
|
|
and $prev->content() =~ m/ \A sub \s* : \z /smx; |
385
|
|
|
|
|
|
|
|
386
|
|
|
|
|
|
|
# At this point we may have a prototype. Skip that too, but there needs to |
387
|
|
|
|
|
|
|
# be something before it. |
388
|
0
|
0
|
0
|
|
|
|
return $FALSE if $prev->isa( 'PPI::Token::Prototype' ) |
389
|
|
|
|
|
|
|
and not $prev = $prev->sprevious_sibling(); |
390
|
|
|
|
|
|
|
|
391
|
|
|
|
|
|
|
# Finally, we can find out if we're a sub |
392
|
0
|
0
|
0
|
|
|
|
return $TRUE if $prev->isa( 'PPI::Token::Word' ) |
393
|
|
|
|
|
|
|
and 'sub' eq $prev->content(); |
394
|
|
|
|
|
|
|
|
395
|
|
|
|
|
|
|
# We are out of options. At this point we can not possibly be an anon sub. |
396
|
0
|
|
|
|
|
|
return $FALSE; |
397
|
|
|
|
|
|
|
} |
398
|
|
|
|
|
|
|
|
399
|
|
|
|
|
|
|
|
400
|
|
|
|
|
|
|
1; |
401
|
|
|
|
|
|
|
|
402
|
|
|
|
|
|
|
__END__ |
403
|
|
|
|
|
|
|
|
404
|
|
|
|
|
|
|
#----------------------------------------------------------------------------- |
405
|
|
|
|
|
|
|
|
406
|
|
|
|
|
|
|
=pod |
407
|
|
|
|
|
|
|
|
408
|
|
|
|
|
|
|
=for stopwords Params::Validate |
409
|
|
|
|
|
|
|
|
410
|
|
|
|
|
|
|
=head1 NAME |
411
|
|
|
|
|
|
|
|
412
|
|
|
|
|
|
|
Perl::Critic::Policy::Subroutines::RequireArgUnpacking - Always unpack C<@_> first. |
413
|
|
|
|
|
|
|
|
414
|
|
|
|
|
|
|
=head1 AFFILIATION |
415
|
|
|
|
|
|
|
|
416
|
|
|
|
|
|
|
This Policy is part of the core L<Perl::Critic|Perl::Critic> |
417
|
|
|
|
|
|
|
distribution. |
418
|
|
|
|
|
|
|
|
419
|
|
|
|
|
|
|
|
420
|
|
|
|
|
|
|
=head1 DESCRIPTION |
421
|
|
|
|
|
|
|
|
422
|
|
|
|
|
|
|
Subroutines that use C<@_> directly instead of unpacking the arguments |
423
|
|
|
|
|
|
|
to local variables first have two major problems. First, they are |
424
|
|
|
|
|
|
|
very hard to read. If you're going to refer to your variables by |
425
|
|
|
|
|
|
|
number instead of by name, you may as well be writing assembler code! |
426
|
|
|
|
|
|
|
Second, C<@_> contains aliases to the original variables! If you |
427
|
|
|
|
|
|
|
modify the contents of a C<@_> entry, then you are modifying the |
428
|
|
|
|
|
|
|
variable outside of your subroutine. For example: |
429
|
|
|
|
|
|
|
|
430
|
|
|
|
|
|
|
sub print_local_var_plus_one { |
431
|
|
|
|
|
|
|
my ($var) = @_; |
432
|
|
|
|
|
|
|
print ++$var; |
433
|
|
|
|
|
|
|
} |
434
|
|
|
|
|
|
|
sub print_var_plus_one { |
435
|
|
|
|
|
|
|
print ++$_[0]; |
436
|
|
|
|
|
|
|
} |
437
|
|
|
|
|
|
|
|
438
|
|
|
|
|
|
|
my $x = 2; |
439
|
|
|
|
|
|
|
print_local_var_plus_one($x); # prints "3", $x is still 2 |
440
|
|
|
|
|
|
|
print_var_plus_one($x); # prints "3", $x is now 3 ! |
441
|
|
|
|
|
|
|
print $x; # prints "3" |
442
|
|
|
|
|
|
|
|
443
|
|
|
|
|
|
|
This is spooky action-at-a-distance and is very hard to debug if it's |
444
|
|
|
|
|
|
|
not intentional and well-documented (like C<chop> or C<chomp>). |
445
|
|
|
|
|
|
|
|
446
|
|
|
|
|
|
|
An exception is made for the usual delegation idiom C<< |
447
|
|
|
|
|
|
|
$object->SUPER::something( @_ ) >>. Only C<SUPER::> and C<NEXT::> are |
448
|
|
|
|
|
|
|
recognized (though this is configurable) and the argument list for the |
449
|
|
|
|
|
|
|
delegate must consist only of C<< ( @_ ) >>. |
450
|
|
|
|
|
|
|
|
451
|
|
|
|
|
|
|
=head1 CONFIGURATION |
452
|
|
|
|
|
|
|
|
453
|
|
|
|
|
|
|
This policy is lenient for subroutines which have C<N> or fewer |
454
|
|
|
|
|
|
|
top-level statements, where C<N> defaults to ZERO. You can override |
455
|
|
|
|
|
|
|
this to set it to a higher number with the |
456
|
|
|
|
|
|
|
C<short_subroutine_statements> setting. This is very much not |
457
|
|
|
|
|
|
|
recommended but perhaps you REALLY need high performance. To do this, |
458
|
|
|
|
|
|
|
put entries in a F<.perlcriticrc> file like this: |
459
|
|
|
|
|
|
|
|
460
|
|
|
|
|
|
|
[Subroutines::RequireArgUnpacking] |
461
|
|
|
|
|
|
|
short_subroutine_statements = 2 |
462
|
|
|
|
|
|
|
|
463
|
|
|
|
|
|
|
By default this policy does not allow you to specify array subscripts |
464
|
|
|
|
|
|
|
when you unpack arguments (i.e. by an array slice or by referencing |
465
|
|
|
|
|
|
|
individual elements). Should you wish to permit this, you can do so |
466
|
|
|
|
|
|
|
using the C<allow_subscripts> setting. This defaults to false. You can |
467
|
|
|
|
|
|
|
set it true like this: |
468
|
|
|
|
|
|
|
|
469
|
|
|
|
|
|
|
[Subroutines::RequireArgUnpacking] |
470
|
|
|
|
|
|
|
allow_subscripts = 1 |
471
|
|
|
|
|
|
|
|
472
|
|
|
|
|
|
|
The delegation logic can be configured to allow delegation other than to |
473
|
|
|
|
|
|
|
C<SUPER::> or C<NEXT::>. The configuration item is |
474
|
|
|
|
|
|
|
C<allow_delegation_to>, and it takes a space-delimited list of allowed |
475
|
|
|
|
|
|
|
delegates. If a given delegate ends in a double colon, anything in the |
476
|
|
|
|
|
|
|
given namespace is allowed. If it does not, only that subroutine is |
477
|
|
|
|
|
|
|
allowed. For example, to allow C<next::method> from C<Class::C3> and |
478
|
|
|
|
|
|
|
_delegate from the current namespace in addition to SUPER and NEXT, the |
479
|
|
|
|
|
|
|
following configuration could be used: |
480
|
|
|
|
|
|
|
|
481
|
|
|
|
|
|
|
[Subroutines::RequireArgUnpacking] |
482
|
|
|
|
|
|
|
allow_delegation_to = next::method _delegate |
483
|
|
|
|
|
|
|
|
484
|
|
|
|
|
|
|
Argument validation tools such as L<Params::Validate|Params::Validate> generate a closure which is |
485
|
|
|
|
|
|
|
used to unpack and validate the arguments of a subroutine. In order to |
486
|
|
|
|
|
|
|
recognize closures as a valid way to unpack arguments you must enable them |
487
|
|
|
|
|
|
|
explicitly: |
488
|
|
|
|
|
|
|
|
489
|
|
|
|
|
|
|
[Subroutines::RequireArgUnpacking] |
490
|
|
|
|
|
|
|
allow_closures = 1 |
491
|
|
|
|
|
|
|
|
492
|
|
|
|
|
|
|
=head1 CAVEATS |
493
|
|
|
|
|
|
|
|
494
|
|
|
|
|
|
|
PPI doesn't currently detect anonymous subroutines, so we don't check |
495
|
|
|
|
|
|
|
those. This should just work when PPI gains that feature. |
496
|
|
|
|
|
|
|
|
497
|
|
|
|
|
|
|
We don't check for C<@ARG>, the alias for C<@_> from English.pm. That's |
498
|
|
|
|
|
|
|
deprecated anyway. |
499
|
|
|
|
|
|
|
|
500
|
|
|
|
|
|
|
=head1 CREDITS |
501
|
|
|
|
|
|
|
|
502
|
|
|
|
|
|
|
Initial development of this policy was supported by a grant from the |
503
|
|
|
|
|
|
|
Perl Foundation. |
504
|
|
|
|
|
|
|
|
505
|
|
|
|
|
|
|
=head1 AUTHOR |
506
|
|
|
|
|
|
|
|
507
|
|
|
|
|
|
|
Chris Dolan <cdolan@cpan.org> |
508
|
|
|
|
|
|
|
|
509
|
|
|
|
|
|
|
=head1 COPYRIGHT |
510
|
|
|
|
|
|
|
|
511
|
|
|
|
|
|
|
Copyright (c) 2007-2023 Chris Dolan |
512
|
|
|
|
|
|
|
|
513
|
|
|
|
|
|
|
This program is free software; you can redistribute it and/or modify |
514
|
|
|
|
|
|
|
it under the same terms as Perl itself. The full text of this license |
515
|
|
|
|
|
|
|
can be found in the LICENSE file included with this module |
516
|
|
|
|
|
|
|
|
517
|
|
|
|
|
|
|
=cut |
518
|
|
|
|
|
|
|
|
519
|
|
|
|
|
|
|
# Local Variables: |
520
|
|
|
|
|
|
|
# mode: cperl |
521
|
|
|
|
|
|
|
# cperl-indent-level: 4 |
522
|
|
|
|
|
|
|
# fill-column: 78 |
523
|
|
|
|
|
|
|
# indent-tabs-mode: nil |
524
|
|
|
|
|
|
|
# c-indentation-style: bsd |
525
|
|
|
|
|
|
|
# End: |
526
|
|
|
|
|
|
|
# ex: set ts=8 sts=4 sw=4 tw=78 ft=perl expandtab shiftround : |