Skip to content

Commit

Permalink
we no longer throw a violation if both regexes are modified in the sa…
Browse files Browse the repository at this point in the history
…me way. This avoids violations for people who always append "smx" onto their regexes like good PBP followers.

We now throw violations for different uses of 's' modifier.
We do not throw violations for different uses of 'x' modifiers.

M    t/Bangs/ProhibitUselessRegexModifiers.run
M    lib/Perl/Critic/Policy/Bangs/ProhibitUselessRegexModifiers.pm
  • Loading branch information
amoore committed Dec 23, 2007
1 parent b8f3cd0 commit a40fe21
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 10 deletions.
31 changes: 22 additions & 9 deletions lib/Perl/Critic/Policy/Bangs/ProhibitUselessRegexModifiers.pm
Expand Up @@ -29,31 +29,37 @@ sub violates {
my ( $self, $elem, undef ) = @_;


# we throw a violation if both of two conditions are true:
# we throw a violation if all these conditions are true:
# 1) there's an 'm' modifier
# 2) the *only* thing in the regex is a compiled regex from a previous qr().
# 3) the modifiers are not the same in both places
my %mods = get_modifiers($elem);
if ( $mods{'m'} ) {
if ( $mods{'m'} || $mods{'s'} ) {
my $match = get_match_string( $elem );
if ( $match =~ /^\$\w+$/smx ) { # It looks like a single variable in there
if ( _is_previously_assigned_quote_like_operator( $elem, $match ) ) {
return $self->violation( $DESC, $EXPL, $elem );
}
if ( my $qr = _previously_assigned_quote_like_operator( $elem, $match ) ) {
# don't violate if both regexes are modified in the same way
if ( _sorted_modifiers( $elem ) ne _sorted_modifiers( $qr ) ) {
return $self->violation( $DESC, $EXPL, $elem );
}
}
}
}
return; #ok!;
}

sub _is_previously_assigned_quote_like_operator {
sub _previously_assigned_quote_like_operator {
my ( $elem, $match ) = @_;

my $qlop = _find_previous_quote_like_regexp( $elem ) or return;

# find if this previous quote-like-regexp assigned to the variable in $match
my $parent = $qlop->parent();
my $found = $parent->find_any( sub { $_[1]->isa( 'PPI::Token::Symbol' ) and
$_[1]->content eq $match } );
return $found;
if ( $parent->find_any( sub { $_[1]->isa( 'PPI::Token::Symbol' ) and
$_[1]->content eq $match } ) ) {
return $qlop;
}
return;
}


Expand All @@ -68,6 +74,13 @@ sub _find_previous_quote_like_regexp {
return $qlop;
}

sub _sorted_modifiers {
my $elem = shift;

my %mods = get_modifiers( $elem );
return join( '', sort keys %mods );
}

1;

=pod
Expand Down
21 changes: 20 additions & 1 deletion t/Bangs/ProhibitUselessRegexModifiers.run
Expand Up @@ -40,11 +40,30 @@ if ( $string =~ /foo$regex/m ) {

## name Do not warn if the qr() is modified in the same way
## failures 0
## TODO not handled yet
## cut

my $regex = qr(asdf)m;
if ( $string =~ /$regex/m ) {
}

#-----------------------------------------------------------------------------

## name 's' modifier should fail
## failures 1
## cut

my $regex = qr(asdf);
if ( $string =~ /$regex/s ) {
}

#-----------------------------------------------------------------------------

## name 'x' modifier should not trigger warning
## failures 0
## cut

my $regex = qr(asdf);
if ( $string =~ /$regex/x ) {
}

#-----------------------------------------------------------------------------

0 comments on commit a40fe21

Please sign in to comment.