File Coverage

blib/lib/Perl/Critic/Policy/Community/ConditionalImplicitReturn.pm
Criterion Covered Total %
statement 43 44 97.7
branch 12 14 85.7
condition 12 17 70.5
subroutine 15 16 93.7
pod 4 5 80.0
total 86 96 89.5


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