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
Use regexp_parser
to improve Style/RedundantRegexp...
cops
#8625
Merged
bbatsov
merged 37 commits into
rubocop:master
from
owst:use_regexp_parser_in_redundant_regexp_cops
Oct 4, 2020
Merged
Changes from 4 commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
c474274
Use `regexp_parser` to improve `Style/RedundantRegexp...` cops
owst 4bbf4cb
fixup! Use `regexp_parser` to improve `Style/RedundantRegexp...` cops
owst 1fd5bf6
Merge remote-tracking branch 'upstream/master' into use_regexp_parser…
owst 1608eb7
fixup! Merge remote-tracking branch 'upstream/master' into use_regexp…
owst a591d13
fixup! fixup! Use `regexp_parser` to improve `Style/RedundantRegexp..…
owst 7598a99
Merge remote-tracking branch 'upstream/master' into use_regexp_parser…
owst db7cd65
fixup! Merge remote-tracking branch 'upstream/master' into use_regexp…
owst c1520e9
fixup! fixup! Merge remote-tracking branch 'upstream/master' into use…
owst 9eac941
fixup! fixup! fixup! Merge remote-tracking branch 'upstream/master' i…
owst ad3540b
fixup! fixup! fixup! fixup! Merge remote-tracking branch 'upstream/ma…
owst 33caf59
Merge remote-tracking branch 'upstream/master' into use_regexp_parser…
owst 477e3c8
fixup! Merge remote-tracking branch 'upstream/master' into use_regexp…
owst 6d3c405
fixup! fixup! Merge remote-tracking branch 'upstream/master' into use…
owst 3b777ac
Test parsed_tree
owst cb668c1
Handle comments and blank interpolations in regexp parsed_tree
owst 3e35f83
Merge branch 'handle_comments_and_blank_interpolations_in_regexp_pars…
owst 7693728
Handle comments and blank interpolations in regexp parsed_tree
owst fd8b231
Merge branch 'handle_comments_and_blank_interpolations_in_regexp_pars…
owst 201fc15
fixup! Merge branch 'handle_comments_and_blank_interpolations_in_rege…
owst df4f6b4
fixup! Handle comments and blank interpolations in regexp parsed_tree
owst cfec9ed
fixup! fixup! Handle comments and blank interpolations in regexp pars…
owst 770b91e
Add test for #8805
owst 9b3fe89
Merge remote-tracking branch 'upstream/master' into use_regexp_parser…
owst b25fd8e
Merge remote-tracking branch 'upstream/master' into handle_comments_a…
owst cb8a04b
fixup! Merge remote-tracking branch 'upstream/master' into handle_com…
owst 37b2e89
fixup! Merge remote-tracking branch 'upstream/master' into use_regexp…
owst 064b245
Merge branch 'handle_comments_and_blank_interpolations_in_regexp_pars…
owst ee5bcd8
fixup! Merge branch 'handle_comments_and_blank_interpolations_in_rege…
owst 85d7f4b
fixup! fixup! Merge branch 'handle_comments_and_blank_interpolations_…
owst a73a64f
Merge remote-tracking branch 'upstream/master' into handle_comments_a…
owst 48ed439
Merge remote-tracking branch 'upstream/master' into use_regexp_parser…
owst 86f6e12
fixup! Merge remote-tracking branch 'upstream/master' into use_regexp…
owst a6b3cd1
Merge remote-tracking branch 'upstream/master' into handle_comments_a…
owst 9b61289
fixup! Merge remote-tracking branch 'upstream/master' into handle_com…
owst 62007c3
Merge branch 'handle_comments_and_blank_interpolations_in_regexp_pars…
owst 71fe7da
fixup! Merge branch 'handle_comments_and_blank_interpolations_in_rege…
owst a21a007
Merge remote-tracking branch 'upstream/master' into use_regexp_parser…
owst File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should use
node.parsed_tree
here...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately
parsed_tree
doesn't support patterns containing an interpolation. @marcandre I see you added that method - do you recall why you returnnil
for patterns containing interpolations? As it stands, this diff causes a bunch of spec failures: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 patternThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we could incorporate the "interpolation-blanking" into
parsed_tree
, or some other mechanism of supporting interpolation?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've had a go at this - please let me know what you think @marcandre!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.