File Coverage

blib/lib/Perl/Critic/Policy/Subroutines/ProhibitExplicitReturnUndef.pm
Criterion Covered Total %
statement 29 29 100.0
branch 10 10 100.0
condition n/a
subroutine 11 11 100.0
pod 4 5 80.0
total 54 55 98.1


line stmt bran cond sub pod time code
1             package Perl::Critic::Policy::Subroutines::ProhibitExplicitReturnUndef;
2              
3 40     40   26552 use 5.010001;
  40         165  
4 40     40   240 use strict;
  40         123  
  40         819  
5 40     40   228 use warnings;
  40         95  
  40         1091  
6 40     40   234 use Readonly;
  40         99  
  40         2135  
7              
8 40     40   277 use Perl::Critic::Utils qw{ :severities :classification };
  40         123  
  40         2150  
9 40     40   14439 use parent 'Perl::Critic::Policy';
  40         123  
  40         280  
10              
11             our $VERSION = '1.146';
12              
13             #-----------------------------------------------------------------------------
14              
15             Readonly::Scalar my $DESC => q{"return" statement with explicit "undef"};
16             Readonly::Scalar my $EXPL => [ 199 ];
17              
18             #-----------------------------------------------------------------------------
19              
20 91     91 0 1989 sub supported_parameters { return () }
21 78     78 1 349 sub default_severity { return $SEVERITY_HIGHEST }
22 92     92 1 390 sub default_themes { return qw(core pbp bugs certrec ) }
23 38     38 1 121 sub applies_to { return 'PPI::Token::Word' }
24              
25             #-----------------------------------------------------------------------------
26              
27             sub violates {
28 389     389 1 745 my ( $self, $elem, undef ) = @_;
29 389 100       802 return if $elem->content() ne 'return';
30 13 100       99 return if is_hash_key($elem);
31              
32 12         51 my $sib = $elem->snext_sibling();
33 12 100       285 return if !$sib;
34 11 100       64 return if !$sib->isa('PPI::Token::Word');
35 5 100       20 return if $sib->content() ne 'undef';
36              
37             # Must be 'return undef'
38 4         43 return $self->violation( $DESC, $EXPL, $elem );
39             }
40              
41             1;
42              
43             __END__
44              
45             #-----------------------------------------------------------------------------
46              
47             =pod
48              
49             =head1 NAME
50              
51             Perl::Critic::Policy::Subroutines::ProhibitExplicitReturnUndef - Return failure with bare C<return> instead of C<return undef>.
52              
53             =head1 AFFILIATION
54              
55             This Policy is part of the core L<Perl::Critic|Perl::Critic>
56             distribution.
57              
58              
59             =head1 DESCRIPTION
60              
61             Returning C<undef> upon failure from a subroutine is pretty common.
62             But if the subroutine is called in list context, an explicit C<return
63             undef;> statement will return a one-element list containing
64             C<(undef)>. Now if that list is subsequently put in a boolean context
65             to test for failure, then it evaluates to true. But you probably
66             wanted it to be false.
67              
68             sub read_file {
69             my $file = shift;
70             -f $file || return undef; #file doesn't exist!
71              
72             #Continue reading file...
73             }
74              
75             #and later...
76              
77             if ( my @data = read_file($filename) ){
78              
79             # if $filename doesn't exist,
80             # @data will be (undef),
81             # but I'll still be in here!
82              
83             process(@data);
84             }
85             else{
86              
87             # This is my error handling code.
88             # I probably want to be in here
89             # if $filname doesn't exist.
90              
91             die "$filename not found";
92             }
93              
94             The solution is to just use a bare C<return> statement whenever you
95             want to return failure. In list context, Perl will then give you an
96             empty list (which is false), and C<undef> in scalar context (which is
97             also false).
98              
99             sub read_file {
100             my $file = shift;
101             -f $file || return; #DWIM!
102              
103             #Continue reading file...
104             }
105              
106              
107             =head1 CONFIGURATION
108              
109             This Policy is not configurable except for the standard options.
110              
111              
112             =head1 NOTES
113              
114             You can fool this policy pretty easily by hiding C<undef> in a boolean
115             expression. But don't bother trying. In fact, using return values to
116             indicate failure is pretty poor technique anyway. Consider using
117             C<die> or C<croak> with C<eval>, or the L<Error|Error> module for a
118             much more robust exception-handling model. Conway has a real nice
119             discussion on error handling in chapter 13 of PBP.
120              
121              
122             =head1 SEE ALSO
123              
124             There's a discussion of the appropriateness of this policy at
125             L<http://perlmonks.org/index.pl?node_id=741847>.
126              
127              
128             =head1 AUTHOR
129              
130             Jeffrey Ryan Thalhammer <jeff@imaginative-software.com>
131              
132             =head1 COPYRIGHT
133              
134             Copyright (c) 2005-2011 Imaginative Software Systems. All rights reserved.
135              
136             This program is free software; you can redistribute it and/or modify
137             it under the same terms as Perl itself. The full text of this license
138             can be found in the LICENSE file included with this module.
139              
140             =cut
141              
142             # Local Variables:
143             # mode: cperl
144             # cperl-indent-level: 4
145             # fill-column: 78
146             # indent-tabs-mode: nil
147             # c-indentation-style: bsd
148             # End:
149             # ex: set ts=8 sts=4 sw=4 tw=78 ft=perl expandtab shiftround :