Remove 'eval' as much as possible (#150)

* Convert eval-as-try{} to Syntax::Keyword::Try

Note that the new Syntax::Keyword::Try dependency increases the
minimum supported Perl version to 5.14.

* Replace eval'd "require" statements with Module::Runtime

This is the easy way out on properly managing $@ in conjunction with eval().

* Localization of $EVAL_ERROR

To preserve $@ ($EVAL_ERROR) when no error occurs, in conjunction
with 'eval()', we need to jump through hoops...

* Change variable name (in code example)

In response to review by @jonasbn.

* Propagate failure to load ID generators

Instead of consuming any errors, but still depending on a positive
outcome (like using it like it had been successfully loaded), send
the error up the call chain.

With tests to verify that the error *is* being propagated.

* Throw an error when the argument fails to evaluate in CheckReturn

Being unable to evaluate the argument constitutes an execution error.
Deal with it that way, instead of acting as if the evaluation had
returned 'undef' (which might later be treated as zero).

dist.ini

@@ -67,7 +67,7 @@ meta_noindex    = 1    ;optional flag
 
 ; REF: Dist::Zilla https://metacpan.org/pod/Dist::Zilla
 [Prereqs]
-perl                       = 5.006
+perl                       = 5.014
 Class::Accessor            = 0.18
 Class::Factory             = 1.00
 DateTime                   = 0.15
@@ -82,6 +82,8 @@ Carp                       = 0
 File::Slurp                = 0
 Data::UUID                 = 0
 Scalar::Util               = 0
+Syntax::Keyword::Try       = 0.25
+Module::Runtime            = 0
 
 ; REF: Dist::Zilla https://metacpan.org/pod/Dist::Zilla
 [Prereqs / TestRequires]
@@ -94,3 +96,4 @@ Test::Kwalitee          = 1.21 ; from Dist::Zilla
 Pod::Coverage::TrustPod = 0    ; from Dist::Zilla
 Test::Pod               = 1.41 ; from Dist::Zilla
 Test::Pod::Coverage     = 1.08 ; from Dist::Zilla
+Test::Without::Module   = 0.20

lib/Workflow.pm

@@ -10,7 +10,7 @@ use Workflow::Exception qw( workflow_error );
 use Exception::Class;
 use Workflow::Factory qw( FACTORY );
 use Carp qw(croak carp);
-use English qw( -no_match_vars );
+use Syntax::Keyword::Try;
 
 my @FIELDS   = qw( id type description state last_update time_zone );
 my @INTERNAL = qw( _factory _observers );
@@ -120,8 +120,10 @@ sub execute_action {
                 );
 
             if ( $wf_state->may_stop() ) {
-                $action_name =
-                    eval { $wf_state->get_autorun_action_name($self); };
+                try {
+                    $action_name = $wf_state->get_autorun_action_name($self);
+                }
+                catch { }
             }
             else {
                 $action_name = $wf_state->get_autorun_action_name($self);
@@ -275,7 +277,7 @@ sub _execute_single_action {
     my $old_state = $self->state;
     my ( $new_state, $action_return );
 
-    eval {
+    try {
         $action->validate($self);
         $self->log->is_debug && $self->log->debug("Action validated ok");
         $action_return = $action->execute($self);
@@ -305,13 +307,8 @@ sub _execute_single_action {
 
         $self->log->is_info
             && $self->log->info("Saved workflow with possible new state ok");
-    };
-
-    # If there's an exception, reset the state to the original one and
-    # rethrow
-
-    if ($EVAL_ERROR) {
-        my $error = $EVAL_ERROR;
+     }
+    catch ($error) {
         $self->log->error(
             "Caught exception from action: $error; reset ",
             "workflow to old state '$old_state'"
@@ -323,7 +320,7 @@ sub _execute_single_action {
         # If it is a validation error we rethrow it so it can be evaluated
         # by the caller to provide better feedback to the user
         if (Exception::Class->caught('Workflow::Exception::Validation')) {
-            $EVAL_ERROR->rethrow();
+            $error->rethrow();
         }
 
         # Don't use 'workflow_error' here since $error should already

lib/Workflow/Action.pm

@@ -190,6 +190,7 @@ This documentation describes version 1.56 of this package
 
  use base qw( Workflow::Action );
  use Workflow::Exception qw( workflow_error );
+ use Syntax::Keyword::Try;
 
  sub execute {
      my ( $self, $wf ) = @_;
@@ -198,16 +199,15 @@ This documentation describes version 1.56 of this package
      # Since 'username' and 'email' have already been validated we
      # don't need to check them for uniqueness, well-formedness, etc.
 
-     my $user = eval {
-         User->create({ username => $context->param( 'username' ),
-                        email    => $context->param( 'email' ) })
-     };
-
-     # Wrap all errors returned...
-
-     if ( $@ ) {
+     my $user;
+     try {
+         $user = User->create({ username => $context->param( 'username' ),
+                                email    => $context->param( 'email' ) })
+     }
+     catch ($error) {
+        # Wrap all errors returned...
          workflow_error
-             "Cannot create new user with name '", $context->param( 'username' ), "': $EVAL_ERROR";
+             "Cannot create new user with name '", $context->param( 'username' ), "': $error";
      }
 
      # Set the created user in the context for the application and/or

lib/Workflow/Condition.pm

@@ -4,7 +4,6 @@ use warnings;
 use strict;
 use base qw( Workflow::Base );
 use Carp qw(croak);
-use English qw( -no_match_vars );
 use Log::Log4perl qw( get_logger );
 use Workflow::Exception qw( workflow_error );
 

lib/Workflow/Condition/CheckReturn.pm

@@ -6,8 +6,7 @@ use warnings;
 our $VERSION = '1.56';
 
 use base qw( Workflow::Condition );
-use Workflow::Exception qw( configuration_error );
-use English qw( -no_match_vars );
+use Workflow::Exception qw( configuration_error workflow_error );
 
 __PACKAGE__->mk_accessors( 'condition', 'operator', 'argument' );
 
@@ -63,11 +62,22 @@ sub evaluate {
     } elsif ( $arg =~ /^[a-zA-Z0-9_]+$/ ) {    # alpha-numeric, plus '_'
         $argval = $wf->context->param($arg);
     } else {
-        $argval = eval $arg;
+        my $error;
+        my $success = do {
+            local $@;
+            my $rv = eval "\$argval = do { $arg }; 1;";
+            $error = $@;
+            $rv;
+        };
+        if (not $success) {
+            workflow_error
+                "Unable to evaluate condition expression '$arg': $error";
+        }
     }
 
     my $condval = $self->evaluate_condition( $wf, $cond );
 
+    local $@;
     return eval "\$condval $op \$argval";
 }
 

lib/Workflow/Condition/Evaluate.pm

@@ -6,7 +6,6 @@ use base qw( Workflow::Condition );
 use Log::Log4perl qw( get_logger );
 use Safe;
 use Workflow::Exception qw( configuration_error );
-use English qw( -no_match_vars );
 
 $Workflow::Condition::Evaluate::VERSION = '1.56';
 

lib/Workflow/Condition/GreedyOR.pm

@@ -7,7 +7,6 @@ our $VERSION = '1.56';
 
 use base qw( Workflow::Condition );
 use Workflow::Exception qw( configuration_error );
-use English qw( -no_match_vars );
 
 __PACKAGE__->mk_accessors('conditions');
 

lib/Workflow/Condition/LazyAND.pm

@@ -7,7 +7,6 @@ our $VERSION = '1.56';
 
 use base qw( Workflow::Condition );
 use Workflow::Exception qw( configuration_error );
-use English qw( -no_match_vars );
 
 __PACKAGE__->mk_accessors('conditions');
 

lib/Workflow/Condition/LazyOR.pm

@@ -7,7 +7,6 @@ our $VERSION = '1.56';
 
 use base qw( Workflow::Condition );
 use Workflow::Exception qw( configuration_error );
-use English qw( -no_match_vars );
 
 __PACKAGE__->mk_accessors('conditions');
 

lib/Workflow/Config/Perl.pm

@@ -84,10 +84,18 @@ sub _translate_perl {
     my $log = get_logger();
 
     no strict 'vars';
-    my $data = eval $config;
-    if ($EVAL_ERROR) {
+    my $data;
+    my $error;
+    my $success = do {
+        local $@;
+
+        my $rv = eval "\$data = do { $config }; 1;";
+        $error = $EVAL_ERROR;
+        $rv;
+    };
+    if (not $success) {
         configuration_error "Cannot evaluate perl data structure ",
-            "in '$file': $EVAL_ERROR";
+            "in '$file': $error";
     }
     return $data;
 }

lib/Workflow/Config/XML.pm

@@ -6,7 +6,7 @@ use base qw( Workflow::Config );
 use Log::Log4perl qw( get_logger );
 use Workflow::Exception qw( configuration_error );
 use Carp qw(croak);
-use English qw( -no_match_vars );
+use Syntax::Keyword::Try;
 
 $Workflow::Config::XML::VERSION = '1.56';
 
@@ -55,11 +55,15 @@ sub parse {
         my $file_name = ( ref $item ) ? '[scalar ref]' : $item;
         $log->info("Will parse '$type' XML config file '$file_name'");
         my $this_config;
-        eval { $this_config = $self->_translate_xml( $type, $item ); };
-
-        # If processing multiple config files, this makes it much easier
-        # to find a problem.
-        croak "Processing $file_name: $EVAL_ERROR" if $EVAL_ERROR;
+        try {
+            $this_config = $self->_translate_xml( $type, $item );
+        }
+        catch ($error) {
+            # If processing multiple config files, this makes it much easier
+            # to find a problem.
+            $log->error("Processing $file_name: ", $error);
+            croak "Processing $file_name: $error";
+        }
         $log->info("Parsed XML '$file_name' ok");
 
         # This sets the outer-most tag to use
@@ -81,14 +85,16 @@ sub parse {
 sub _translate_xml {
     my ( $self, $type, $config ) = @_;
     unless ($XML_REQUIRED) {
-        eval { require XML::Simple };
-        if ($EVAL_ERROR) {
+        try {
+            require XML::Simple;
+        }
+        catch ($error) {
             configuration_error "XML::Simple must be installed to parse ",
                 "configuration files/data in XML format";
-        } else {
-            XML::Simple->import(':strict');
-            $XML_REQUIRED++;
         }
+
+        XML::Simple->import(':strict');
+        $XML_REQUIRED++;
     }
     my $options = $XML_OPTIONS{$type} || {};
     my $data = XMLin( $config, %{$options} );