Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style/RedundantRegexpEscape false positive when used with multiline regexp with comments #8805

Closed
rafal-brize opened this issue Sep 28, 2020 · 4 comments

Comments

@rafal-brize
Copy link

Provided regexp written as

        regexp = %r{
          \A[a-z#]            # letters or #
          \.[a-z]\z           # dot + letters
        }x

that enforces:

regexp.match?("#.a") => true
regexp.match?("#aa") => false

i receive

Style/RedundantRegexpEscape: Redundant escape inside regexp literal
          \.[a-z]\z           # dot + letters
          ^^

that after autocorrect changes the behaviour to:

regexp.match?("#.a") => true
regexp.match?("#aa") => true

However

regexp = %r{\A[a-z#]\.[a-z]\z}

passes the Style/RedundantRegexpEscape check

RuboCop version

$ bundle exec rubocop -V
0.92.0 (using Parser 2.7.1.5, rubocop-ast 0.7.0, running on ruby 2.6.5 x86_64-darwin18)
@rafal-brize
Copy link
Author

@owst Tagging you, since i've read in changelog you've been working on those false positives, maybe you can pitch in.

@owst
Copy link
Contributor

owst commented Sep 28, 2020

Hi @rafal-brize, thanks for tagging me and apologies for the bug.

It looks like the code that attempts to ignore comments is a bit overzealous and is treating the # within the character class as starting a comment, so your pattern is incorrectly processed as something like:

        regexp = %r{
          \A[a-z
          \.[a-z]\z
        }x

which makes the \. appear to be redundant as it's inside a character class.

I've recently been working on moving to use regexp_parser for this cop (rather than using a home-grown "parser") in #8625 and the parser correctly handles your pattern (I'll also add it as an explicit test). So when #8625 is merged, this bug will be fixed.

owst added a commit to owst/rubocop that referenced this issue Sep 28, 2020
@rafal-brize
Copy link
Author

Thanks @owst for all your work, awesome!

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 5, 2020

The fix was merged a while ago, but we forgot to close this issue.

@bbatsov bbatsov closed this as completed Dec 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants