1

perlcritic is complaining with Expression form of "eval" [BuiltinFunctions::ProhibitStringyEval] on the first eval line from the code below:

use strict;
use warnings;
use feature qw/say/;

my $hasTwitter = 1;
my $validEmail = 0;

my $rule   = '${hasTwitter} | ${validEmail}';
my $result = eval $rule;
say "Result ->  $result";

$result = eval { $rule };
say "Result -> $result";

I tried to use eval {} to fix the perlCritic but then It does not return the expected result.

The response is:

Result ->  1
Result -> ${hasTwitter} | ${validEmail}

Is there a workaround using the string interpolation? The idea is to have a set of rules on a configuration file and let the code read and eval them.

Thanks

toolic
  • 57,801
  • 17
  • 75
  • 117
Albert
  • 57
  • 3
  • 3
    perlcritic can always be "fixed" by adding `## no critic` to the offending line – mob Sep 23 '15 at 15:45
  • 3
    ... specifically: `my $result = eval $rule; ## no critic (ProhibitStringyEval);` – toolic Sep 23 '15 at 15:47
  • 1
    By the way, that `|` (bitwise-or) should probably be `||` (logical-or). – ikegami Sep 23 '15 at 15:51
  • 1
    By the way, `${hasTwitter}` is a very weird way of writing `$hasTwitter` – ikegami Sep 23 '15 at 15:54
  • @ikegami You are right, it works without {} and with ||. I don't remember why I changed that, some of my tests failed without the {} and the bitwise-or. I'll check it out. – Albert Sep 24 '15 at 07:21

3 Answers3

7

The critic is there to make you think: Is there a reason for the expression to be there as a string? Can it be avoided given the numerous pitfalls and high security risks? Or rather, are the risks and problems worth avoiding a little work?

For starters, could you have used the following?

my $rule = sub { $hasTwitter || $validEmail };

my $result = $rule->();

Or maybe

my $rule = 'has_twitter_or_email';

my %rules = (
   has_twitter_or_email => sub { $hasTwitter || $validEmail },
);
my $result = $rules{$rule}->();
ThisSuitIsBlackNot
  • 23,492
  • 9
  • 63
  • 110
ikegami
  • 367,544
  • 15
  • 269
  • 518
  • If the config-file should define new rules, that will not actually help. I believe there is an example in _Higher Order Perl_ for pretty much exactly that, though. – simbabque Sep 23 '15 at 16:54
  • 3
    @simbabque, That's why I didn't say "you should use the following". In such a situation, it would be best to create a rule-defining language. A config file is no place for Perl code. – ikegami Sep 23 '15 at 17:01
5

You can turn individual Perl::Critic rules off for specific blocks. Add a comment like this. Note the double comment ## is on purpose.

my $rule   = '${hasTwitter} | ${validEmail}';

## no critic 'ProhibitStringyEval'
my $result = eval $rule;

As this works per block, you will want to do it in the smallest possible scope, just like a use or no.

It makes sense to explain in a comment why you did that. Usually your team would have good reasons to pick the rules, and you should only turn them off if you have a better reason for your specific case.

simbabque
  • 53,749
  • 8
  • 73
  • 136
  • 1
    I knew I can turn them off, just wanted to know if the given code could be changed somehow to avoid the PerlCritic warning – Albert Sep 24 '15 at 08:00
1

Perlcritic is not the ultimate authority. If you know what you're doing and why, just disable the given "sin" (either globally in the configuration, or by adding ## no critic to the offending line).

In this case, you can use double quotes instead of single quotes and verify there are only zeros and ones in the resulting strings before evaluating them. Implementing a parser and evaluator of logic formulas is not that hard, either.

choroba
  • 231,213
  • 25
  • 204
  • 289