line |
stmt |
bran |
cond |
sub |
pod |
time |
code |
1
|
|
|
|
|
|
|
|
2
|
|
|
|
|
|
|
use strict; |
3
|
1
|
|
|
1
|
|
363
|
use warnings; |
|
1
|
|
|
|
|
2
|
|
|
1
|
|
|
|
|
22
|
|
4
|
1
|
|
|
1
|
|
4
|
|
|
1
|
|
|
|
|
2
|
|
|
1
|
|
|
|
|
23
|
|
5
|
|
|
|
|
|
|
use Perl::Critic::Utils qw(:severities :classification :ppi); |
6
|
1
|
|
|
1
|
|
4
|
use parent 'Perl::Critic::Policy'; |
|
1
|
|
|
|
|
1
|
|
|
1
|
|
|
|
|
41
|
|
7
|
1
|
|
|
1
|
|
296
|
|
|
1
|
|
|
|
|
1
|
|
|
1
|
|
|
|
|
5
|
|
8
|
|
|
|
|
|
|
use List::Util 'any'; |
9
|
1
|
|
|
1
|
|
43
|
use Perl::Critic::Community::Utils qw(is_empty_return is_structural_block); |
|
1
|
|
|
|
|
2
|
|
|
1
|
|
|
|
|
58
|
|
10
|
1
|
|
|
1
|
|
6
|
|
|
1
|
|
|
|
|
2
|
|
|
1
|
|
|
|
|
71
|
|
11
|
|
|
|
|
|
|
our $VERSION = 'v1.0.3'; |
12
|
|
|
|
|
|
|
|
13
|
|
|
|
|
|
|
use constant DESC => 'Subroutine may implicitly return a conditional statement'; |
14
|
1
|
|
|
1
|
|
6
|
use constant EXPL => 'When the last statement in a subroutine is a conditional, the return value may unexpectedly be the evaluated condition.'; |
|
1
|
|
|
|
|
2
|
|
|
1
|
|
|
|
|
43
|
|
15
|
1
|
|
|
1
|
|
5
|
|
|
1
|
|
|
|
|
1
|
|
|
1
|
|
|
|
|
354
|
|
16
|
|
|
|
|
|
|
|
17
|
5
|
|
|
5
|
0
|
36200
|
my %conditionals = map { ($_ => 1) } qw(if unless); |
18
|
2
|
|
|
2
|
1
|
27
|
|
19
|
0
|
|
|
0
|
1
|
0
|
my ($self, $elem) = @_; |
20
|
5
|
|
|
5
|
1
|
43475
|
|
21
|
|
|
|
|
|
|
my $block = $elem->block || return (); |
22
|
|
|
|
|
|
|
my $returns = $block->find(sub { |
23
|
|
|
|
|
|
|
my ($elem, $child) = @_; |
24
|
|
|
|
|
|
|
# Don't search in blocks unless we know they are structural |
25
|
5
|
|
|
5
|
1
|
104
|
if ($child->isa('PPI::Structure::Block')) { |
26
|
|
|
|
|
|
|
return undef unless is_structural_block($child); |
27
|
5
|
|
50
|
|
|
23
|
} |
28
|
|
|
|
|
|
|
return 1 if $child->isa('PPI::Token::Word') and $child eq 'return'; |
29
|
147
|
|
|
147
|
|
1374
|
return 0; |
30
|
|
|
|
|
|
|
}); |
31
|
147
|
100
|
|
|
|
351
|
|
32
|
7
|
50
|
|
|
|
29
|
# Check the last statement if any non-empty return is present |
33
|
|
|
|
|
|
|
if ($returns and any { !is_empty_return($_) } @$returns) { |
34
|
147
|
100
|
100
|
|
|
403
|
my $last = $block->schild(-1); |
35
|
139
|
|
|
|
|
335
|
# Check if last statement is a conditional |
36
|
5
|
|
|
|
|
145
|
if ($last and $last->isa('PPI::Statement::Compound') |
37
|
|
|
|
|
|
|
and $last->schildren and exists $conditionals{$last->schild(0)}) { |
38
|
|
|
|
|
|
|
# Make sure there isn't an "else" |
39
|
5
|
100
|
66
|
5
|
|
83
|
unless (any { $_->isa('PPI::Token::Word') and $_ eq 'else' } $last->schildren) { |
|
5
|
|
|
|
|
18
|
|
40
|
4
|
|
|
|
|
25
|
return $self->violation(DESC, EXPL, $last); |
41
|
|
|
|
|
|
|
} |
42
|
4
|
50
|
66
|
|
|
72
|
} |
|
|
|
66
|
|
|
|
|
|
|
|
66
|
|
|
|
|
43
|
|
|
|
|
|
|
} |
44
|
|
|
|
|
|
|
|
45
|
3
|
100
|
|
13
|
|
115
|
return (); |
|
13
|
100
|
|
|
|
115
|
|
46
|
2
|
|
|
|
|
23
|
} |
47
|
|
|
|
|
|
|
|
48
|
|
|
|
|
|
|
1; |
49
|
|
|
|
|
|
|
|
50
|
|
|
|
|
|
|
=head1 NAME |
51
|
3
|
|
|
|
|
29
|
|
52
|
|
|
|
|
|
|
Perl::Critic::Policy::Community::ConditionalImplicitReturn - Don't end a |
53
|
|
|
|
|
|
|
subroutine with a conditional block |
54
|
|
|
|
|
|
|
|
55
|
|
|
|
|
|
|
=head1 DESCRIPTION |
56
|
|
|
|
|
|
|
|
57
|
|
|
|
|
|
|
If the last statement in a subroutine is a conditional block such as |
58
|
|
|
|
|
|
|
C<if ($foo) { ... }>, and the C<else> condition is not handled, the subroutine |
59
|
|
|
|
|
|
|
will return an unexpected value when the condition fails, and it is most likely |
60
|
|
|
|
|
|
|
a logic error. Specify a return value after the conditional, or handle the |
61
|
|
|
|
|
|
|
C<else> condition. |
62
|
|
|
|
|
|
|
|
63
|
|
|
|
|
|
|
sub { ... if ($foo) { return 1 } } # not ok |
64
|
|
|
|
|
|
|
sub { ... if ($foo) { return 1 } return 0 } # ok |
65
|
|
|
|
|
|
|
sub { ... if ($foo) { return 1 } else { return 0 } } # ok |
66
|
|
|
|
|
|
|
|
67
|
|
|
|
|
|
|
This policy only applies if the subroutine contains a return statement with an |
68
|
|
|
|
|
|
|
explicit return value, indicating it is not intended to be used in void |
69
|
|
|
|
|
|
|
context. |
70
|
|
|
|
|
|
|
|
71
|
|
|
|
|
|
|
=head1 CAVEATS |
72
|
|
|
|
|
|
|
|
73
|
|
|
|
|
|
|
This policy currently only checks for implicitly returned conditionals in named |
74
|
|
|
|
|
|
|
subroutines, anonymous subroutines are not checked. Also, return statements |
75
|
|
|
|
|
|
|
within blocks, other than compound statements like C<if> and C<foreach>, are |
76
|
|
|
|
|
|
|
not considered when determining if a function is intended to be used in void |
77
|
|
|
|
|
|
|
context. |
78
|
|
|
|
|
|
|
|
79
|
|
|
|
|
|
|
=head1 AFFILIATION |
80
|
|
|
|
|
|
|
|
81
|
|
|
|
|
|
|
This policy is part of L<Perl::Critic::Community>. |
82
|
|
|
|
|
|
|
|
83
|
|
|
|
|
|
|
=head1 CONFIGURATION |
84
|
|
|
|
|
|
|
|
85
|
|
|
|
|
|
|
This policy is not configurable except for the standard options. |
86
|
|
|
|
|
|
|
|
87
|
|
|
|
|
|
|
=head1 AUTHOR |
88
|
|
|
|
|
|
|
|
89
|
|
|
|
|
|
|
Dan Book, C<dbook@cpan.org> |
90
|
|
|
|
|
|
|
|
91
|
|
|
|
|
|
|
=head1 COPYRIGHT AND LICENSE |
92
|
|
|
|
|
|
|
|
93
|
|
|
|
|
|
|
Copyright 2015, Dan Book. |
94
|
|
|
|
|
|
|
|
95
|
|
|
|
|
|
|
This library is free software; you may redistribute it and/or modify it under |
96
|
|
|
|
|
|
|
the terms of the Artistic License version 2.0. |
97
|
|
|
|
|
|
|
|
98
|
|
|
|
|
|
|
=head1 SEE ALSO |
99
|
|
|
|
|
|
|
|
100
|
|
|
|
|
|
|
L<Perl::Critic> |