-1
sub func {
    my ($n) = @_;
    return unless ($n);
    my @array;
    push @array, $1 while $n =~ /
            ((?:
              [^(),]+ |
              ( \(
                (?: [^()]+ | (?2) )*
              \) )
            )+)
            (?: ,\s* | $)
            /xg;

    return \@array;
    }

    for my $item (@array) {
        if (index($item, '$n') != -1) {
           print "HELLO\n";
        }
} 

I have above regex to split some string into array. It works fine.

Problem is :

Perl critic gives below errors. Please advise how do i fix this.

Capture variable used outside conditional at line 150, 
    near 'push @array, $1 while $n =~ /'.  (Severity: 3)
Use '{' and '}' to delimit multi-line regexps at line 150, 
    near 'push @input_expression, $1 while $n =~ /'.  (Severity: 1)
String *may* require interpolation at line 168, 
    near '$item, '$n''.  (Severity: 1)
zdim
  • 64,580
  • 5
  • 52
  • 81
zubug55
  • 729
  • 7
  • 27

2 Answers2

2

The first one is IMO a false positive, since while is acting as the conditional here - the push @array, $1 won't get executed unless the regexp matched, which is what the policy wants (add --verbose 11 to the perlcritic invocation to see the explanations). This is a case where I think it's safe to suppress the policy, as I show below.

The second one is easy to fix, simply replace $n =~ /.../xg with $n =~ m{...}xg.

push @array, $1  ## no critic (ProhibitCaptureWithoutTest)
    while $n =~ m{ ... }xg;

That suppresses those two messages.

As a side note, running perlcritic at brutal severity is IMO a little extreme, and it'll complain about a lot of other stuff in that snippet. Personally, when I use it, I run perlcritic at harsh (-3) with a few policies at customized levels.

Edit: As for your third perlcritic message, which you added to your post later, it looks like that's been answered in your other post.

haukex
  • 2,973
  • 9
  • 21
2

Perl Critic doesn't give any errors. It gives policy violations.

To fix the first one, change the loop from modifier to normal while loop and name the variable:

    while ($n =~ /
        ...

    /xg) {
        my $match = $1;
        push @array, $match;
    }

To fix the second one, just change /.../ to m{...}.

In my opinion, it makes little sense to use policies you don't understand well. Sometimes, there might be very good reasons to break some of them; blindly following Perl Critic (especially at more stern levels) doesn't bring you anything.

choroba
  • 231,213
  • 25
  • 204
  • 289
  • please check the edited question, when i loop around that array and look for string $n in the item of an array, perl critic gives one more error. – zubug55 Apr 29 '18 at 22:02
  • 2
    Perl critic gives no errors. It just tells you `$n` looks like a variable, but won't be interpolated in single quotes. If your process needs to silent severity 1 warnings, add a `## no critic (ValuesAndExpressions::RequireInterpolationOfMetachars)` comment plus explanation why `$n` shouldn't be interpolated here. – choroba Apr 29 '18 at 23:02
  • 2
    Also, next time, use a new question to ask a new question. You may link to the previous one if you think their relevance is important. – choroba Apr 29 '18 at 23:03