Use regexp_parser to improve Style/RedundantRegexp... cops

[owst]

As mentioned in #8593 by @marcandre

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

[marcandre]

Good stuff 😄. I just had a quick look, here are some comments

[marcandre]

I think you should use node.parsed_tree here...

[owst]

Unfortunately parsed_tree doesn't support patterns containing an interpolation. @marcandre I see you added that method - do you recall why you return nil for patterns containing interpolations? As it stands, this diff causes a bunch of spec failures:

diff --git a/lib/rubocop/cop/style/redundant_regexp_character_class.rb b/lib/rubocop/cop/style/redundant_regexp_character_class.rb
index 214a84743..7a3251994 100644
--- a/lib/rubocop/cop/style/redundant_regexp_character_class.rb
+++ b/lib/rubocop/cop/style/redundant_regexp_character_class.rb
@@ -54,7 +54,8 @@ module RuboCop
         end
 
         def each_single_element_character_class(node)
-          Regexp::Parser.parse(pattern_source(node)).each_expression do |expr|
+          # Regexp::Parser.parse(pattern_source(node)).each_expression do |expr|
+          node.parsed_tree.each_expression do |expr|
             next if expr.type != :set || expr.expressions.size != 1
             next if expr.negative? || %i[posixclass set].include?(expr.expressions.first.type)

mostly of the form NoMethodError: undefined method each_expression' for nil:NilClass`.

N.B. I introduced pattern_source (which replaces interpolation and comments with spaces) to remove (as far as the cops are concerned) the "effect" of the interpolation, whilst preserving the indentation/width of the interpolation in the pattern

[owst]

Perhaps we could incorporate the "interpolation-blanking" into parsed_tree, or some other mechanism of supporting interpolation?

[marcandre]

I like the idea of blanking the interpolations, makes sense and could be quite useful.
Maybe parsed_tree could take a keyword argument :interpolation with value :ignore or :blank?

[owst]

I've had a go at this - please let me know what you think @marcandre!

[marcandre]

That's awesome.
Ideally, the changes to parsed_tree would be in a different PR, and with at least a test.
It's not clear to me that replacing the interpolations with spaces has no effect, I thought you were going to simply delete them...
I also don't know why the treatment of the comments.

[bbatsov]

You'll have to move this under master.

[owst]

Thanks for the review @marcandre - I've pushed a conflict resolution (and move of the CHANGELOG entry) and have hopefully adequately responded to your comments

[bbatsov]

The changes look good, but your branch has to be rebased on top of the current master branch due to merge conflicts.

[bbatsov]

@owst Can you rebase and squash, please?

[owst]

@bbatsov sorry, I need to deal with @marcandre's comment here #8625 (comment) - I'll need to pull out another PR for some of this change.

For the record, the comment-in-extended-regexp-blanking is (currently) required because regexp_parser doesn't allow setting Regexp options when parsing a String pattern rather than Regexp directly. Therefore, regexp_parser treats the comment as a literal, which can be flagged by the cops in question e.g.

/
  foo # [x] <- flagged as redundant brackets if comment is not ignored
/x

[jaynetics]

@owst we can add an option to regexp_parser to pass regexp flags / options when parsing a regexp body from a string if that is of help here.

[owst]

Hi @jaynetics, thanks for the suggestion! I was looking to do just that, I'll try and pull something together over the weekend and open a PR 👍

[marcandre]

LGTM

[owst]

@bbatsov / @marcandre - anything more for me to do on this, or can it be merged now?

[bbatsov]

🚀