Avoiding stringy eval in Perl
Tags: perl
Published: 2024-05-14 10:19:37

---
title: Avoiding stringy eval in Perl
published: true
date: 2024-04-23 22:00:00 UTC
tags: Perl
canonical_url: https://peateasea.de/avoiding-stringy-eval-in-perl/
cover_image: https://peateasea.de/assets/images/prohibited-stringy-eval-cheese-pizza.png
---

Once upon a time, I used to take an active part in the [CPAN Pull Request Challenge](http://cpan-prc.org/) (and later in the [Pull Request Club](https://pullrequest.club)). One task I tackled as part of each monthly assignment was to try to remove as many [`Perl::Critic`](https://metacpan.org/dist/Perl-Critic) violations as possible. One violation which tended to crop up often as part of module-loading guards in tests was [stringy `eval`](https://metacpan.org/pod/Perl::Critic::Policy::BuiltinFunctions::ProhibitStringyEval). Generally, this is something to be avoided. But how? Here I discuss recommended [best practices](https://www.oreilly.com/library/view/perl-best-practices/0596001738/) and some other options.

## Stringy `eval` is bad, mkay?

Unlike [chunky](https://peateasea.de/assets/images/chunky-bacon.png) [bacon](https://poignant.guide/book/chapter-3.html), [stringy `eval`](https://metacpan.org/pod/Perl::Critic::Policy::BuiltinFunctions::ProhibitStringyEval) is something one usually wants to avoid in Perl code. It’s hopefully obvious that one shouldn’t wantonly use `eval` in one’s code (mainly because it could potentially run _any_ code and [that would be bad](https://peateasea.de/assets/images/ghostbusters-it-would-be-bad.gif)). Even so, it does have its uses in the appropriate context.

Often one uses `eval` to check if a module is available before continuing with further processing. For instance, if you want to run [Pod checking tests](https://metacpan.org/pod/Test::Pod) but only if `Test::Pod` has already been installed. In this case, you’d try to load `Test::Pod` within an `eval` and then skip the tests if the [`$EVAL_ERROR` variable `$@`](https://perldoc.perl.org/perlvar#$@) is a truthy value<sup id="fnref:eval-success-value" role="doc-noteref"><a href="#fn:eval-success-value" rel="footnote">1</a></sup>. It turns out that one can’t always rely on this behaviour either and that there is a more secure technique; more on this later.

Module-loading guard code tends to look like this:

```perl
use strict;
use warnings;

use Test::More;

# check for Test::Pod existence
eval "use Test::Pod";
plan skip_all => "Test::Pod required to test Pod" if $@;

# check all Pod files if Test::Pod is available
all_pod_files_ok();
```

where we try to `use` a module with an `eval` and if that fails, skip the remaining tests and print an appropriate warning message. Here we check the `eval` status as part of a postfix `if` statement. Of course, one can run the `eval` status check earlier, in which case the code takes this shape:<sup id="fnref:avoid-postfix-controls" role="doc-noteref"><a href="#fn:avoid-postfix-controls" rel="footnote">2</a></sup>

```perl
use strict;
use warnings;

use Test::More;

# check for Test::Pod existence
eval "use Test::Pod";
if ($@) {
    plan skip_all => "Test::Pod required to test Pod";
}

# check all Pod files if Test::Pod is available
all_pod_files_ok();
```

Either way, both of these code snippets are **discouraged** because we pass a string to `eval`. The [`ProhibitStringyEval` `Perl::Critic`documentation](https://metacpan.org/pod/Perl::Critic::Policy::BuiltinFunctions::ProhibitStringyEval) explains why:

> The Perl interpreter recompiles the string form of `eval` for each execution, whereas the block form is only compiled once. Also, the string form doesn’t give compile-time warnings.

In other words, one should use the block form of the `eval` statement. Not only is the block form more performant, but the compiler checks our code at compile time rather than at runtime. Thus, it finds any syntax errors and other silly bugs earlier.<sup id="fnref:silly-runtime-bugs" role="doc-noteref"><a href="#fn:silly-runtime-bugs" rel="footnote">3</a></sup>

## The simple fix

The fix is very simple:<sup id="fnref:docs-need-updating" role="doc-noteref"><a href="#fn:docs-need-updating" rel="footnote">4</a></sup>

```diff
  use strict;
  use warnings;

  use Test::More;

  # check for Test::Pod existence
- eval "use Test::Pod";
+ eval { use Test::Pod };
  if ($@) {
      plan skip_all => "Test::Pod required to test Pod";
  }

  # check all Pod files if Test::Pod is available
  all_pod_files_ok();
```

So that’s all, right? Just change stringy `eval` to use the block form and everything’s fixed and you can carry on with your day? Not so fast…

## It’s never as simple as you first think…

The problem is that, in general, [you can’t depend upon the value of `$@`/`$EVAL_ERROR` to tell whether an `eval` failed](https://metacpan.org/pod/Perl::Critic::Policy::ErrorHandling::RequireCheckingReturnValueOfEval).

The reasoning behind the issue is a bit involved but the short explanation is that

> [it’s possible for] the value of `$@` to change between the end of the `eval` and the `if`.

This is unfortunate because it is precisely this behaviour we’re relying upon for the module-loading guard code to work.

Help is at hand: use the [`try_load_class` function from the `Class::Load`module](https://metacpan.org/pod/Class::Load). If you want to be absolutely sure that you’re testing that the module loads correctly before running tests, you can change the guard code to this:

```perl
use strict;
use warnings;

use Test::More;
use Class::Load qw( try_load_class );

# check for Test::Pod existence
try_load_class('Test::Pod') or plan skip_all => "Test::Pod required to test Pod";

# check all Pod files if Test::Pod is available
all_pod_files_ok();
```

which makes the code shorter and a bit more declarative. The downside is that your project needs to rely on another module. If the extra dependency isn’t a problem for you, then using `Class::Load` to safely check that your optional modules have loaded is a viable solution.

The nice thing with this solution is that `try_load_class()` returns true if the class loaded and false otherwise. This is a positive formulation. By this, I mean that the function returns true if the thing that we wanted to happen succeeded. In contrast, with the `$@` check, the formulation is negative: did the thing we want to happen fail? If yes, barf with an appropriate message. Both formulations do their job, but I find the “yup, everything worked, let’s continue doing stuff” formulation nicer.

## Sometimes you need to be eval

Of course, `eval` isn’t always bad.<sup id="fnref:things-are-nuanced" role="doc-noteref"><a href="#fn:things-are-nuanced" rel="footnote">5</a></sup> Sometimes, it can be exactly what you need. Consider, for instance, [this code](https://github.com/karpet/dezi-app/blob/f804982b50168f3f92e5558a11ef695ccdefc835/lib/Dezi/ReplaceRules.pm#L174):

```perl
# $a, $b, $o, and $orig set earlier from other code
my $code = "\$str =~ s/$b/$a/$o";

try {
    eval "$code";
}
catch {
    die "Bad rule: $orig ($_)";
}
```

Here one constructs the code to be run from other information that is only available at runtime, thus requiring a stringy `eval`. In other words, _sometimes_ one needs to construct code dynamically and evaluate it. In that case, creating the code to run in a string and then using stringy `eval` is perfectly acceptable. Of course, in such situations, you need to know what you’re doing. But if you know the risks and take relevant precautions, this is still valid code. The only issue here is that [`perlcritic`](https://metacpan.org/dist/Perl-Critic/view/bin/perlcritic) will complain, this time unnecessarily. How to get around this? Simply annotate the code with a `no critic` exception:

```diff
  # $a, $b, $o, and $orig set earlier from other code
  my $code = "\$str =~ s/$b/$a/$o";

  try {
+ ## no critic (ProhibitStringyEval)
      eval "$code";
  }
  catch {
      die "Bad rule: $orig ($_)";
  }
```

where we’re being explicit about which `Perl::Critic` warning we’re keeping quiet to signal to future readers of the code that we genuinely _do_ want to do this.

## Best practices are best

Best practices are just that: best. They’re not hard-and-fast rules. Yet they’re called best practices for a reason: hard-won experience has shown them to be the safer bet in most situations.

In our case here: it’s usually best to replace stringy `eval` with its equivalent block form. If you want to be a bit more careful and don’t mind an extra dependency, use `Class::Load`. But if you really, really need a stringy `eval`, that’s also ok, Perl will still let you. Make sure you know what you’re doing though.

## Update

Since I published this post, [FErki](@ferki@fosstodon.org) [pinged me on Mastodon](https://mastodon.social/@ferki@fosstodon.org/112333931604765650) to let me know that there are core modules available which do a similar job
to that performed by `Class::Load`.  Many thanks for the heads up!

Here's what FErki had to say:

-------

If one would like to use a core module instead of `try_load_class()` from `Class::Load`, they might be interested in `can_load()` from [`Module::Load::Conditional`](https://metacpan.org/pod/Module::Load::Conditional).

Particulary in tests, one may also be interested in either of:

- [`Test::Requires`](https://metacpan.org/pod/Test::Requires)
- [`Test2::Require::Module`](https://metacpan.org/pod/Test2::Require::Module)

Hope these are useful additions for readers wishing to avoid stringy evals! Happy hacking :)

-------

1. If the `eval` was successful `$@` contains the empty string which evaluates as false. [↩](#fnref:eval-success-value)

2. It turns out that [avoiding postfix controls](https://metacpan.org/pod/Perl::Critic::Policy::ControlStructures::ProhibitPostfixControls) is also something one should do. [↩](#fnref:avoid-postfix-controls)

3. Finding silly bugs at runtime can be rather frustrating. [↩](#fnref:silly-runtime-bugs)

4. It’s unfortunate that [some modules](https://metacpan.org/pod/Test::Pod::Coverage) still recommend the outdated `eval` form for module-guard code. Oh well. [↩](#fnref:docs-need-updating)

5. Things are nuanced. Who would have thought? :wink: [↩](#fnref:things-are-nuanced)