-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow parens for calls in range literals (Style/MethodCallWithArgsParentheses) #6763
Allow parens for calls in range literals (Style/MethodCallWithArgsParentheses) #6763
Conversation
expect_no_offenses(<<-RUBY) | ||
1..limit(n) | ||
RUBY | ||
end |
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! On the other hand, there is also erange
in the similar case :-)
1...limit(n)
Would you please add along with the CHANGELOG entry?
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.
Oh, didn't know that. Thanks for the catch!
4611c7b
to
f2216da
Compare
@@ -259,13 +259,16 @@ def legitimate_call_with_parentheses?(node) | |||
allowed_chained_call_with_parentheses?(node) | |||
end | |||
|
|||
# rubocop:disable Metrics/CyclomaticComplexity |
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.
The whole omit_parentheses
style is based around the many edge-cases where we actually do need the parentheses. I have tried to cater to the Metrics/CyclomaticComplexity
until now, but the extra helpers I introduce to hide the conditions blow up the line length cop. I think the code will be clearer if we don't measure the cyclomatic complexity for this whole cop and eventually inline the helpers where they are used.
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.
Introduction of range_type?
meaning both irange_type?
and erange_type?
is proposed.
#6126 (comment)
We will be able to remove the disabling cop comment when replaced by this proposal . So I think this is fine.
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.
#6773 will remove the disabling cop comment.
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.
Great!
Before: ```ruby 1..limit(n) ^^^ Omit parentheses for method calls with arguments. ``` After: ```ruby 1..limit(n) ``` 馃憣
f2216da
to
9fc434e
Compare
Thanks! |
With the style of
omit_parentheses
in theStyle/MethodCallWithArgsParentheses
cop, we got:Before:
After:
馃憣