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
Add Lint/MixedRegexpCaptureTypes
cop
#7749
Conversation
end | ||
|
||
def each_regexp_expr(tree) | ||
tree.to_enum(:each_expression) |
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'll refactor it after the new version of regexp_parser gem is released. ref: ammar/regexp_parser#62
Co-Authored-By: Koichi ITO <koic.ito@gmail.com>
Might also be a good idea to mention this in the style guide. |
I opened a pull request for the style guide. rubocop/ruby-style-guide#793 By the way, I think this pull request is ready to review. If you need, I'll squash the commits to one. |
# /(?<foo>FOO)(BAR)/ | ||
# | ||
# # good | ||
# /(?<foo>FOO)(?<bar>BAR)/ |
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 it's even more helpful to show an example of numbered capture for users.
# # good
# /(FOO)(BAR)/
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.
Good point, thanks!
I added it as a good example. d8144b8
I think RuboCop prefers named capture than numbered capture. For example, we have a rule in the style guide. https://github.com/rubocop-hq/ruby-style-guide#no-numbered-regexes
So I added the suggested example to the bottom of the examples.
It is not bad, and I prefer it with a simple Regexp, but named capture is more descriptive.
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 get it! Thanks for the explanation.
Co-Authored-By: Koichi ITO <koic.ito@gmail.com>
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.
Looks good! Let's merge it! :-)
@@ -1478,6 +1478,11 @@ Lint/MissingCopEnableDirective: | |||
# .inf for any size | |||
MaximumRangeSize: .inf | |||
|
|||
Lint/MixedRegexpCaptureTypes: | |||
Description: 'Do not mix named captures and numbered captures in a Regexp literal.' |
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.
rubocop/ruby-style-guide#793 has been merged. Can you add the StyleGuide address?
StyleGuide: '#do-not-mix-named-and-numbered-captures'
❤️💛💚💙💜 |
@pocke ping :-) |
@rubocop-hq/rubocop-core Any volunteer to wrap this up? Most a matter of amending this commit with fresh docs. |
Thanks! |
This PR has the following error, so it seems like it needs to be fixed. I will investigate later. % cd repo/to/rubocop-hq/rubocop
% bundle exec rubocop -d --only Lint/MixedRegexpCaptureTypes lib/rubocop/cop/utils/format_string.rb
For /Users/koic/src/github.com/rubocop-hq/rubocop: configuration from /Users/koic/src/github.com/rubocop-hq/rubocop/.rubocop.yml
configuration from /Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-performance-1.6.0/config/default.yml
configuration from /Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-performance-1.6.0/config/default.yml
Default configuration from /Users/koic/src/github.com/rubocop-hq/rubocop/config/default.yml
configuration from /Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-rspec-1.39.0/config/default.yml
configuration from /Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-rspec-1.39.0/config/default.yml
Inheriting configuration from /Users/koic/src/github.com/rubocop-hq/rubocop/.rubocop_todo.yml
Inspecting 1 file
Scanning /Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/cop/utils/format_string.rb
An error occurred while Lint/MixedRegexpCaptureTypes cop was inspecting /Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/cop/utils/format_string.rb:18:19.
No valid target found for '*'
/Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/regexp_parser-1.7.0/lib/regexp_parser/parser.rb:432:in `quantifier'
/Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/regexp_parser-1.7.0/lib/regexp_parser/parser.rb:102:in `parse_token'
/Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/regexp_parser-1.7.0/lib/regexp_parser/parser.rb:39:in `block in parse'
/Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/regexp_parser-1.7.0/lib/regexp_parser/lexer.rb:55:in `block in lex'
/Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/regexp_parser-1.7.0/lib/regexp_parser/lexer.rb:55:in `map'
/Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/regexp_parser-1.7.0/lib/regexp_parser/lexer.rb:55:in `lex'
/Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/regexp_parser-1.7.0/lib/regexp_parser/lexer.rb:15:in `lex'
/Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/regexp_parser-1.7.0/lib/regexp_parser/parser.rb:38:in `parse'
/Users/koic/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/regexp_parser-1.7.0/lib/regexp_parser/parser.rb:22:in `parse'
/Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/cop/lint/mixed_regexp_capture_types.rb:28:in `on_regexp'
/Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/cop/commissioner.rb:56:in `block (2 levels) in trigger_responding_cops'
/Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/cop/commissioner.rb:115:in `with_cop_error_handling'
/Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/cop/commissioner.rb:55:in `block in trigger_responding_cops'
/Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/cop/commissioner.rb:54:in `each'
/Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/cop/commissioner.rb:54:in `trigger_responding_cops'
/Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/cop/commissioner.rb:32:in `block (2 levels) in <class:Commissioner>' |
Close rubocop#7749 and Reopen rubocop#7749.
I've opened #8074. |
Close #7746
This pull request will add
Lint/MixedRegexpCaptureTypes
cop that I described in #7746 .Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.