Perl critic error on splitting string into array using regexp

240 Views Asked by At
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)
2

There are 2 best solutions below

3
choroba On BEST ANSWER

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.

0
haukex On

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.