Bug #46980 for Perl-Critic: RequireCheckingReturnValueOfEval allow test script ok() [patch]

I've been using test script bits like ok (eval { foo(); 1}); ok (eval { MyModule->VERSION(1); 1}); It'd be good if RequireCheckingReturnValueOfEval recognised ok() as using the return value. Possible change along those lines below, with ok() restricted to when Test::Simple or Test::More are seen. Perhaps a func named ok() coming from anywhere would be enough, but the test module ones are certainly using the return.

Index: lib/Perl/Critic/Policy/ErrorHandling/RequireCheckingReturnValueOfEval.pm =================================================================== --- lib/Perl/Critic/Policy/ErrorHandling/RequireCheckingReturnValueOfEval.pm (revision 3341) +++ lib/Perl/Critic/Policy/ErrorHandling/RequireCheckingReturnValueOfEval.pm (working copy) @@ -13,6 +13,7 @@ use Readonly; +use List::MoreUtils qw( any ); use Scalar::Util qw< refaddr >; use Perl::Critic::Utils qw< :booleans :characters :severities hashify >; @@ -51,6 +52,7 @@ return if _is_in_right_hand_side_of_assignment($elem); return if _is_in_postfix_expression($elem); + return if _is_test_ok_parameter($elem); return if _is_in_correct_position_in_a_condition_or_foreach_loop_collection( $elem, @@ -239,6 +241,35 @@ #----------------------------------------------------------------------------- +sub _is_test_ok_parameter { + my ($elem) = @_; + + my $p = $elem->parent || return; + $p->isa ('PPI::Statement::Expression') or return; + + $p = $p->parent || return; + $p->isa ('PPI::Structure::List') or return; + + $p = $p->sprevious_sibling || return; + $p->isa ('PPI::Token::Word') or return; + $p->content eq 'ok' or return; + + return _document_uses_test_module ($elem->top); +} + +my %test_modules = ('Test::Simple' => 1, + 'Test::More' => 1); + +sub _document_uses_test_module { + my ($document) = @_; + + # ENHANCE-ME: maybe cache this search result against the document ... + my $includes = $document->find('PPI::Statement::Include') || return; + return any { $test_modules{$_->module||''} } @$includes; +} + +#----------------------------------------------------------------------------- + 1; __END__ Index: t/ErrorHandling/RequireCheckingReturnValueOfEval.run =================================================================== --- t/ErrorHandling/RequireCheckingReturnValueOfEval.run (revision 3341) +++ t/ErrorHandling/RequireCheckingReturnValueOfEval.run (working copy) @@ -385,6 +385,36 @@ #----------------------------------------------------------------------------- +## name ok() not in a test script is not ok +## failures 1 +## cut + +ok(eval { foo() }); + +## name Test::Simple ok() call +## failures 0 +## cut + +use Test::Simple; +ok(eval { foo() }); + +## name Test::More ok() call +## failures 0 +## cut + +use Test::More; +ok(eval { foo() }); + +## name Test::More ok() call, plus perl version +## failures 0 +## cut + +use 5.006; +use Test::More; +ok(eval { foo() }); + +#----------------------------------------------------------------------------- + ############################################################################## # $URL$ # $Date$