perlcritic: eval "require $module";

Question

While digging in some old source code I saw the following:

my $module = $some{module};
eval "require $module";
die "Bad module\n$@" if $@;

while I understand what the code does, it tries "require" a module and die when it is unsuccessful - the perlcritic complains about it

Expression form of "eval" at line 331, column 13. See page 161 of PBP. (Severity: 5)

Unfortunately I havent the PBP book, so wondering what is the correct method of the above...

Also, in the same source found:

sub test_repo_file {
    my($self, $repo, $test) = @_;
    my $abspath = repo_abs_path($repo);
    return "eval -$test $abspath";
}

Here doesn't understand what solves the "eval", and the perlcritic complains again for the "string eval"...

Can somebody please explain the basic points about the "string eval" and how to write the above correctly?

Travelled

Answer 1

Running perlcritic --verbose '%d\n' will give you the explanations, too:

The string form of `eval' is recompiled every time it is executed, whereas the block form is only compiled once. Also, the string form doesn't give compile-time warnings.

   eval "print $foo";        # not ok
   eval {print $foo};        # ok

It's applicable to the first case.

The second case doesn't generate any messages for me. Isn't it rather

return eval "-$test $abspath"

You can't use block eval here. You should verify that $test really contains what it should

$test =~ /^[a-z]$/i

and avoid the evaluation of $abspath:

eval "-$test \$abspath"

If you're OK with that, you can than add

## no critic

to the end of the line.

Earn

Answer 2

Few users should ever have to use eval EXPR. Most of the time, it's being used as a template system when it shouldn't (e.g. s/.../eval($repl)/e), or it's used to catch exceptions when eval BLOCK should have been used.

If there is reason to use it, it's to execute generated code or user-submitted code. Generating code is tricky and error-prone, and errors have security implications. Executing user-submitted code is a major security concern. As such, every use of eval EXPR should be carefully reviewed.

It's quite appropriate for perlcritic to flag its usage given that virtually every use is an error with major security implications.

---

In your case, the use of eval EXPR is suboptimal. I'd use

my $path = ( $module =~ s{::}{/}gr ) . ".pm";
require $path;

(Yes, this is portable.)

Better yet, there's Module::Load. It's even a core module.

Oruntha

Module::Load has the simple and straightforward code that shows the sorts of things you have to think about, which is basically what ikegami already showed. – Lector