1 ##############################################################################
2 # $URL: http://perlcritic.tigris.org/svn/perlcritic/trunk/Perl-Critic/lib/Perl/Critic/Policy/ErrorHandling/RequireCheckingReturnValueOfEval.pm $
3 # $Date: 2008-07-03 10:19:10 -0500 (Thu, 03 Jul 2008) $
6 ##############################################################################
8 package Perl::Critic::Policy::ErrorHandling::RequireCheckingReturnValueOfEval;
16 use Scalar::Util qw< refaddr >;
18 use Perl::Critic::Utils qw< :booleans :characters :severities hashify >;
19 use base 'Perl::Critic::Policy';
21 our $VERSION = '1.088';
23 #-----------------------------------------------------------------------------
25 Readonly::Scalar my $DESC => 'Return value of eval not tested.';
26 ## no critic (RequireInterpolationOfMetachars)
27 Readonly::Scalar my $EXPL =>
28 q<You can't depend upon the value of $@/$EVAL_ERROR to tell whether an eval failed.>;
31 Readonly::Hash my %BOOLEAN_OPERATORS => hashify qw< || && // or and >;
32 Readonly::Hash my %POSTFIX_OPERATORS =>
33 hashify qw< for foreach if unless while until >;
35 #-----------------------------------------------------------------------------
37 sub supported_parameters { return () }
38 sub default_severity { return $SEVERITY_MEDIUM }
39 sub default_themes { return qw( core bugs ) }
40 sub applies_to { return 'PPI::Token::Word' }
42 #-----------------------------------------------------------------------------
45 my ( $self, $elem, undef ) = @_;
47 return if $elem->content() ne 'eval';
49 my $evaluated = $elem->snext_sibling() or return; # Nothing to eval!
50 my $following = $evaluated->snext_sibling();
52 return if _is_in_right_hand_side_of_assignment($elem);
53 return if _is_in_postfix_expression($elem);
55 _is_in_correct_position_in_a_condition_or_foreach_loop_collection(
62 and $following->isa('PPI::Token::Operator')
63 and $BOOLEAN_OPERATORS{ $following->content() }
68 return $self->violation($DESC, $EXPL, $elem);
71 #-----------------------------------------------------------------------------
73 sub _is_in_right_hand_side_of_assignment {
76 my $previous = $elem->sprevious_sibling();
80 _grandparent_for_is_in_right_hand_side_of_assignment($elem);
84 my $base_previous = $previous;
88 if ( $previous->isa('PPI::Token::Operator') ) {
89 return $TRUE if $previous->content() eq q<=>;
90 last EQUALS_SCAN if _is_effectively_a_comma($previous);
92 $previous = $previous->sprevious_sibling();
96 _grandparent_for_is_in_right_hand_side_of_assignment($base_previous);
102 sub _grandparent_for_is_in_right_hand_side_of_assignment {
105 my $parent = $elem->parent() or return;
106 $parent->isa('PPI::Statement') or return;
108 my $grandparent = $parent->parent() or return;
111 $grandparent->isa('PPI::Structure::Constructor')
112 or $grandparent->isa('PPI::Structure::List')
120 #-----------------------------------------------------------------------------
122 Readonly::Scalar my $CONDITION_POSITION_IN_C_STYLE_FOR_LOOP => 1;
124 sub _is_in_correct_position_in_a_condition_or_foreach_loop_collection {
125 my ($elem, $following) = @_;
127 my $parent = $elem->parent();
129 if ( $parent->isa('PPI::Structure::Condition') ) {
131 _is_in_correct_position_in_a_structure_condition(
132 $elem, $parent, $following,
136 if ( $parent->isa('PPI::Structure::ForLoop') ) {
137 my @for_loop_components = $parent->schildren();
139 return $TRUE if 1 == @for_loop_components;
141 $for_loop_components[$CONDITION_POSITION_IN_C_STYLE_FOR_LOOP]
144 return _descendant_of($elem, $condition);
147 $parent = $parent->parent();
153 sub _is_in_correct_position_in_a_structure_condition {
154 my ($elem, $parent, $following) = @_;
157 while ($level and refaddr $level != $parent) {
158 my $cursor = refaddr $elem == refaddr $level ? $following : $level;
160 IS_FINAL_EXPRESSION_AT_DEPTH:
162 if ( _is_effectively_a_comma($cursor) ) {
163 $cursor = $cursor->snext_sibling();
164 while ( _is_effectively_a_comma($cursor) ) {
165 $cursor = $cursor->snext_sibling();
168 # Semicolon would be a syntax error here.
170 last IS_FINAL_EXPRESSION_AT_DEPTH;
173 $cursor = $cursor->snext_sibling();
176 my $statement = $level->parent();
177 return $TRUE if not $statement; # Shouldn't happen.
178 return $TRUE if not $statement->isa('PPI::Statement'); # Shouldn't happen.
180 $level = $statement->parent();
184 not $level->isa('PPI::Structure::List')
185 and not $level->isa('PPI::Structure::Condition')
196 # Replace with PPI implementation once it is released.
198 my ($cursor, $potential_ancestor) = @_;
200 return $EMPTY if not $potential_ancestor;
202 while ( refaddr $cursor != refaddr $potential_ancestor ) {
203 $cursor = $cursor->parent() or return $EMPTY;
209 #-----------------------------------------------------------------------------
211 sub _is_in_postfix_expression {
214 my $previous = $elem->sprevious_sibling();
217 $previous->isa('PPI::Token::Word')
218 and $POSTFIX_OPERATORS{ $previous->content() }
222 $previous = $previous->sprevious_sibling();
228 #-----------------------------------------------------------------------------
230 sub _is_effectively_a_comma {
236 $elem->isa('PPI::Token::Operator')
238 $elem->content() eq $COMMA
239 || $elem->content() eq $FATCOMMA
243 #-----------------------------------------------------------------------------
249 #-----------------------------------------------------------------------------
253 =for stopwords destructors
257 Perl::Critic::Policy::ErrorHandling::RequireCheckingReturnValueOfEval - You can't depend upon the value of C<$@>/C<$EVAL_ERROR> to tell whether an C<eval> failed.
261 This Policy is part of the core L<Perl::Critic> distribution.
266 A common idiom in perl for dealing with possible errors is to use
267 C<eval> followed by a check of C<$@>/C<$EVAL_ERROR>:
276 There's a problem with this: the value of C<$EVAL_ERROR> can change
277 between the end of the C<eval> and the C<if> statement. The issue is
293 my $foo = Foo->new();
300 Assuming there are no other references to C<$foo> created, when the
301 C<eval> block in C<main> is exited, C<Foo::DESTROY()> will be invoked,
302 regardless of whether the C<eval> finished normally or not. If the
303 C<eval> in C<main> fails, but the C<eval> in C<Foo::DESTROY()>
304 succeeds, then C<$EVAL_ERROR> will be empty by the time that the C<if>
305 is executed. Additional issues arise if you depend upon the exact
306 contents of C<$EVAL_ERROR> and both C<eval>s fail, because the
307 messages from both will be concatenated.
309 Even if there isn't an C<eval> directly in the C<DESTROY()> method
310 code, it may invoke code that does use C<eval> or otherwise affects
313 The solution is to ensure that, upon normal exit, an C<eval> returns a
314 true value and to test that value:
316 # Constructors are no problem.
317 my $object = eval { Class->new() };
319 # To cover the possiblity that an operation may correctly return a
320 # false value, end the block with "1":
321 if ( eval { something(); 1 } ) {
330 # Error handling here
333 Unfortunately, you can't use the C<defined> function to test the
334 result; C<eval> returns an empty string on failure.
336 "But we don't use DESTROY() anywhere in our code!" you say. That may
337 be the case, but do any of the third-party modules you use have them?
338 What about any you may use in the future or updated versions of the
339 ones you already use?
344 This Policy is not configurable except for the standard options.
349 See thread on perl5-porters starting here:
350 L<http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2008-06/msg00537.html>.
355 Elliot Shank C<< <perl@galumph.com> >>
359 Copyright (c) 2008 Elliot Shank. All rights reserved.
361 This program is free software; you can redistribute it and/or modify
362 it under the same terms as Perl itself. The full text of this license
363 can be found in the LICENSE file included with this module.
369 # cperl-indent-level: 4
371 # indent-tabs-mode: nil
372 # c-indentation-style: bsd
374 # ex: set ts=8 sts=4 sw=4 tw=78 ft=perl expandtab shiftround :