Skip Menu |
 

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Perl-Critic CPAN distribution.

Report information
The Basics
Id: 46980
Status: new
Priority: 0/
Queue: Perl-Critic

People
Owner: Nobody in particular
Requestors: user42 [...] zip.com.au
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: RequireCheckingReturnValueOfEval allow test script ok() [patch]
Date: Tue, 16 Jun 2009 10:26:44 +1000
To: bug-Perl-Critic [...] rt.cpan.org
From: Kevin Ryde <user42 [...] zip.com.au>
Download (untitled) / with headers
text/plain 433b
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$


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.