-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[Fix #7728] Fix an error for Style/OneLineConditional
#7729
[Fix #7728] Fix an error for Style/OneLineConditional
#7729
Conversation
e6393fe
to
35cb74f
Compare
include MethodIdentifierPredicates | ||
|
||
# Always return `false` because `self` cannot have arguments. | ||
def arguments? |
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.
Can you point me to the code that calls #arguments?
on a self
node? I think the call site probably should filter the relevant nodes instead. I can have a look. 🙂
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.
SelfNode#arguments?
is called in the following code.
https://github.com/rubocop-hq/rubocop/blob/v0.80.0/lib/rubocop/cop/style/one_line_conditional.rb#L96
I was worried about choosing between implementing SelfNode#arguments?
or guarding the above with resopnd_to(:arguments?)
.
def keyword_with_changed_precedence?(node)
return false unless node.keyword?
return true if node.prefix_not?
- node.arguments? && !node.parenthesized_call?
+ node.respond_to?(:arguments?) && node.arguments? && !node.parenthesized_call?
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.
@koic Let me have a look. 🙂 Ideally we shouldn't do any of those. We should make sure incompatible nodes don't reach this code.
35cb74f
to
f7b8edc
Compare
cd14a0c
to
30dfc79
Compare
30dfc79
to
b4b5ae2
Compare
@koic Any chance this one can get merged so I can do testing on my side? |
Fixes rubocop#7728. This PR fixes an error for `Style/OneLineConditional` when one of the branches contains a self keyword.
b4b5ae2
to
8815dda
Compare
@Drenmi Can we merge this PR to resolve the error first? |
@koic Yes! Please go ahead. I don't have time to look into it for another couple of weeks or so. 🙂 |
I get it. Thank you for taking your time! |
Fixes rubocop#7829. This PR fixes an error for `Style/OneLineConditional` when one of the branches contains `next` keyword. `SelfNode` was introduced by rubocop#7729 to prevent `NoMethodErrror`, but it is unnecessary because the cop that caused the error were fixed.
Fixes #7728.
This PR fixes an error for
Style/OneLineConditional
when one of the branches contains a self keyword.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.