File Coverage

blib/lib/Perl/Critic/Policy/Subroutines/RequireFinalReturn.pm
Criterion Covered Total %
statement 117 125 93.6
branch 52 68 76.4
condition 44 53 83.0
subroutine 26 26 100.0
pod 4 5 80.0
total 243 277 87.7


line stmt bran cond sub pod time code
1             package Perl::Critic::Policy::Subroutines::RequireFinalReturn;
2              
3 40     40   28141 use 5.010001;
  40         167  
4 40     40   240 use strict;
  40         89  
  40         872  
5 40     40   210 use warnings;
  40         115  
  40         1084  
6 40     40   249 use Readonly;
  40         98  
  40         2172  
7              
8 40     40   298 use List::SomeUtils qw(any);
  40         101  
  40         2207  
9 40     40   342 use Perl::Critic::Exception::Fatal::Internal qw{ throw_internal };
  40         144  
  40         2352  
10 40     40   306 use Perl::Critic::Utils qw{ :characters :severities :data_conversion };
  40         120  
  40         2091  
11 40     40   13509 use parent 'Perl::Critic::Policy';
  40         102  
  40         233  
12              
13             our $VERSION = '1.148';
14              
15             #-----------------------------------------------------------------------------
16              
17             Readonly::Scalar my $EXPL => [ 197 ];
18              
19             Readonly::Hash my %CONDITIONALS => hashify( qw(if unless for foreach) );
20              
21             #-----------------------------------------------------------------------------
22              
23             sub supported_parameters {
24             return (
25             {
26 115     115 0 2863 name => 'terminal_funcs',
27             description => 'The additional subroutines to treat as terminal.',
28             default_string => $EMPTY,
29             behavior => 'string list',
30             list_always_present_values =>
31             [ qw< croak confess die exec exit throw Carp::confess Carp::croak ...> ],
32             },
33             {
34             name => 'terminal_methods',
35             description => 'The additional methods to treat as terminal.',
36             default_string => $EMPTY,
37             behavior => 'string list',
38             list_always_present_values => [],
39             },
40             );
41             }
42              
43 95     95 1 408 sub default_severity { return $SEVERITY_HIGH }
44 92     92 1 437 sub default_themes { return qw( core bugs pbp certrec ) }
45 56     56 1 183 sub applies_to { return 'PPI::Statement::Sub' }
46              
47             #-----------------------------------------------------------------------------
48              
49             sub violates {
50 59     59 1 148 my ( $self, $elem, undef ) = @_;
51              
52             # skip BEGIN{} and INIT{} and END{} etc
53 59 100       349 return if $elem->isa('PPI::Statement::Scheduled');
54              
55 55         207 my @blocks = grep {$_->isa('PPI::Structure::Block')} $elem->schildren();
  165         1244  
56 55 50       263 if (@blocks > 1) {
    100          
57             # sanity check
58 0         0 throw_internal 'Subroutine should have no more than one block';
59             }
60             elsif (@blocks == 0) {
61             #Technically, subroutines don't have to have a block at all. In
62             # that case, its just a declaration so this policy doesn't really apply
63 1         4 return; # ok!
64             }
65              
66              
67 54         130 my ($block) = @blocks;
68 54 100 100     170 if ($self->_block_is_empty($block) || $self->_block_has_return($block)) {
69 33         370 return; # OK
70             }
71              
72             # Must be a violation
73 21         48 my $desc;
74 21 50       90 if ( my $name = $elem->name() ) {
75 21         1333 $desc = qq<Subroutine "$name" does not end with "return">;
76             }
77             else {
78 0         0 $desc = q<Subroutine does not end with "return">;
79             }
80              
81 21         102 return $self->violation( $desc, $EXPL, $elem );
82             }
83              
84             #-----------------------------------------------------------------------------
85              
86             sub _block_is_empty {
87 54     54   133 my ( $self, $block ) = @_;
88 54         162 return $block->schildren() == 0;
89             }
90              
91             #-----------------------------------------------------------------------------
92              
93             sub _block_has_return {
94 84     84   1154 my ( $self, $block ) = @_;
95 84         215 my @blockparts = $block->schildren();
96 84         873 my $final = $blockparts[-1]; # always defined because we call _block_is_empty first
97 84 100       308 return if !$final;
98 83   100     260 return $self->_is_explicit_return($final)
99             || $self->_is_given_when_return($final)
100             || $self->_is_compound_return($final);
101             }
102              
103             #-----------------------------------------------------------------------------
104              
105             sub _is_explicit_return {
106 83     83   187 my ( $self, $final ) = @_;
107              
108 83 100       236 return if $self->_is_conditional_stmnt( $final );
109 62   100     205 return $self->_is_return_or_goto_stmnt( $final )
110             || $self->_is_terminal_stmnt( $final );
111             }
112              
113             #-----------------------------------------------------------------------------
114              
115             sub _is_compound_return {
116 35     35   94 my ( $self, $final ) = @_;
117              
118 35 100       136 if (!$final->isa('PPI::Statement::Compound')) {
119 20         97 return; #fail
120             }
121              
122 15         46 my $begin = $final->schild(0);
123 15 50       248 return if !$begin; #fail
124 15 100 100     79 if (!($begin->isa('PPI::Token::Word') &&
      66        
125             ($begin->content() eq 'if' || $begin->content() eq 'unless'))) {
126 1         17 return; #fail
127             }
128              
129 14   100     108 my @blocks = grep {!$_->isa('PPI::Structure::Condition') &&
  76         580  
130             !$_->isa('PPI::Token')} $final->schildren();
131             # Sanity check:
132 14 50   30   97 if (any { !$_->isa('PPI::Structure::Block') } @blocks) {
  30         96  
133 0         0 throw_internal
134             'Expected only conditions, blocks and tokens in the if statement';
135             }
136              
137 14         57 for my $block (@blocks) {
138 25 100       144 if (! $self->_block_has_return($block)) {
139 6         29 return; #fail
140             }
141             }
142              
143 8         88 return 1;
144             }
145              
146             #-----------------------------------------------------------------------------
147              
148             sub _is_given_when_return {
149 37     37   604 my ( $self, $final ) = @_;
150              
151 37 100       188 if ( ! $final->isa( 'PPI::Statement::Given' ) ) {
152 33         171 return; #fail
153             }
154              
155 4         16 my $begin = $final->schild(0);
156 4 50       73 return if !$begin; #fail
157 4 50 33     31 if ( ! ( $begin->isa( 'PPI::Token::Word' ) &&
158             $begin->content() eq 'given' ) ) {
159 0         0 return; #fail
160             }
161              
162 4   100     37 my @blocks = grep {!$_->isa( 'PPI::Structure::Given' ) &&
  12         124  
163             !$_->isa( 'PPI::Token' )} $final->schildren();
164             # Sanity check:
165 4 50   4   34 if (any { !$_->isa('PPI::Structure::Block') } @blocks) {
  4         33  
166 0         0 throw_internal
167             'Expected only givens, blocks and tokens in the given statement';
168             }
169 4 50       39 if (@blocks > 1) {
170             # sanity check
171 0         0 throw_internal 'Given statement should have no more than one block';
172             }
173 4 50       14 @blocks or return; #fail
174              
175 4         10 my $have_default; # We have to fail unless a default block is present
176              
177 4         17 foreach my $stmnt ( $blocks[0]->schildren() ) {
178              
179 7 100       104 if ( $stmnt->isa( 'PPI::Statement::When' ) ) {
180              
181             # Check for the default block.
182 6         16 my $first_token;
183 6 100 66     23 $first_token = $stmnt->schild( 0 )
184             and 'default' eq $first_token->content()
185             and $have_default = 1;
186              
187 6 100       130 $self->_is_when_stmnt_with_return( $stmnt )
188             or return; #fail
189              
190             } else {
191              
192 1 50       5 $self->_is_suffix_when_with_return( $stmnt )
193             or return; #fail
194              
195             }
196              
197             }
198              
199 3         22 return $have_default;
200             }
201              
202             #-----------------------------------------------------------------------------
203              
204             sub _is_return_or_goto_stmnt {
205 63     63   147 my ( $self, $stmnt ) = @_;
206 63 100       310 return if not $stmnt->isa('PPI::Statement::Break');
207 34   50     120 my $first_token = $stmnt->schild(0) || return;
208 34   100     535 return $first_token->content() eq 'return'
209             || $first_token->content() eq 'goto';
210             }
211              
212             #-----------------------------------------------------------------------------
213              
214             sub _is_terminal_stmnt {
215 31     31   102 my ( $self, $stmnt ) = @_;
216              
217 31 50       127 return if not $stmnt->isa('PPI::Statement');
218              
219 31   50     99 my $first_token = $stmnt->schild(0) || return;
220 31 100       470 return 1 if exists $self->{_terminal_funcs}->{$first_token};
221              
222 19   100     144 my $second_token = $stmnt->schild(1) || return;
223 18 100 100     399 return if not ( $second_token->isa('PPI::Token::Operator') && ($second_token eq q{->}) );
224              
225 6   50     107 my $third_token = $stmnt->schild(2) || return;
226 6         101 return exists $self->{_terminal_methods}->{$third_token};
227             }
228              
229             #-----------------------------------------------------------------------------
230              
231             sub _is_conditional_stmnt {
232 83     83   194 my ( $self, $stmnt ) = @_;
233 83 50       318 return if not $stmnt->isa('PPI::Statement');
234 83         241 for my $elem ( $stmnt->schildren() ) {
235             return 1 if $elem->isa('PPI::Token::Word')
236 196 100 100     2543 && exists $CONDITIONALS{$elem};
237             }
238 62         348 return;
239             }
240              
241             #-----------------------------------------------------------------------------
242              
243             sub _is_when_stmnt_with_return {
244 6     6   16 my ( $self, $stmnt ) = @_;
245              
246 6   100     18 my @inner = grep { ! $_->isa( 'PPI::Token' ) &&
  15         151  
247             ! $_->isa( 'PPI::Structure::When' ) }
248             $stmnt->schildren();
249 6 50   6   34 if ( any { ! $_->isa( 'PPI::Structure::Block' ) } @inner ) {
  6         31  
250 0         0 throw_internal 'When statement should contain only tokens, conditions, and blocks';
251             }
252 6 50       28 @inner > 1
253             and throw_internal 'When statement should have no more than one block';
254 6 50       19 @inner or return; #fail
255              
256 6         18 foreach my $block ( @inner ) {
257 6 100       20 if ( ! $self->_block_has_return( $block ) ) {
258 1         6 return; #fail
259             }
260             }
261              
262 5         60 return 1; #succeed
263             }
264              
265             #-----------------------------------------------------------------------------
266              
267             sub _is_suffix_when_with_return {
268 1     1   4 my ( $self, $stmnt ) = @_;
269 1 50       9 return if not $stmnt->isa('PPI::Statement');
270 1         6 foreach my $elem ( $stmnt->schildren() ) {
271 3 100 33     36 return ( $self->_is_return_or_goto_stmnt( $stmnt ) ||
      100        
272             $self->_is_terminal_stmnt( $stmnt ) )
273             if $elem->isa( 'PPI::Token::Word' )
274             && 'when' eq $elem->content();
275             }
276 0           return;
277             }
278              
279             1;
280              
281             __END__
282              
283             #-----------------------------------------------------------------------------
284              
285             =pod
286              
287             =head1 NAME
288              
289             Perl::Critic::Policy::Subroutines::RequireFinalReturn - End every path through a subroutine with an explicit C<return> statement.
290              
291             =head1 AFFILIATION
292              
293             This Policy is part of the core L<Perl::Critic|Perl::Critic>
294             distribution.
295              
296              
297             =head1 DESCRIPTION
298              
299             Require all subroutines to terminate explicitly with one of the
300             following: C<return>, C<carp>, C<croak>, C<die>, C<exec>, C<exit>,
301             C<goto>, or C<throw>.
302              
303             Subroutines without explicit return statements at their ends can be
304             confusing. It can be challenging to deduce what the return value will
305             be.
306              
307             Furthermore, if the programmer did not mean for there to be a
308             significant return value, and omits a return statement, some of the
309             subroutine's inner data can leak to the outside. Consider this case:
310              
311             package Password;
312             # every time the user guesses the password wrong, its value
313             # is rotated by one character
314             my $password;
315             sub set_password {
316             $password = shift;
317             }
318             sub check_password {
319             my $guess = shift;
320             if ($guess eq $password) {
321             unlock_secrets();
322             } else {
323             $password = (substr $password, 1).(substr $password, 0, 1);
324             }
325             }
326             1;
327              
328             In this case, the last statement in check_password() is the
329             assignment. The result of that assignment is the implicit return
330             value, so a wrong guess returns the right password! Adding a
331             C<return;> at the end of that subroutine solves the problem.
332              
333             The only exception allowed is an empty subroutine.
334              
335             Be careful when fixing problems identified by this Policy; don't
336             blindly put a C<return;> statement at the end of every subroutine.
337              
338             =head1 CONFIGURATION
339              
340             If you've created your own terminal functions that behave like C<die>
341             or C<exit>, then you can configure Perl::Critic to recognize those
342             functions as well. Just put something like this in your
343             F<.perlcriticrc>:
344              
345             [Subroutines::RequireFinalReturn]
346             terminal_funcs = quit abort bailout
347              
348             If you've created your own terminal methods, then you can configure
349             Perl::Critic to recognize those methods as well, but the class won't
350             be considered. For example if you define throw_exception as terminal,
351             then any method of that name will be terminal, regardless of class.
352             Just put something like this in your
353             F<.perlcriticrc>:
354              
355             [Subroutines::RequireFinalReturn]
356             terminal_methods = throw_exception
357              
358             =head1 BUGS
359              
360             We do not look for returns inside ternary operators. That
361             construction is too complicated to analyze right now. Besides, a
362             better form is the return outside of the ternary like this: C<return
363             foo ? 1 : bar ? 2 : 3>
364              
365             =head1 AUTHOR
366              
367             Chris Dolan <cdolan@cpan.org>
368              
369             =head1 COPYRIGHT
370              
371             Copyright (c) 2005-2023 Chris Dolan.
372              
373             This program is free software; you can redistribute it and/or modify
374             it under the same terms as Perl itself. The full text of this license
375             can be found in the LICENSE file included with this module.
376              
377             =cut
378              
379             ##############################################################################
380             # Local Variables:
381             # mode: cperl
382             # cperl-indent-level: 4
383             # fill-column: 78
384             # indent-tabs-mode: nil
385             # c-indentation-style: bsd
386             # End:
387             # ex: set ts=8 sts=4 sw=4 tw=78 ft=perl expandtab shiftround :