-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix an incorrect auto-correct for Style/MultilineTernaryOperator
#8661
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 an incorrect auto-correct for Style/MultilineTernaryOperator
#8661
Conversation
@@ -5777,6 +5777,9 @@ a = if cond | |||
else | |||
c | |||
end | |||
return cond ? |
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 guess we shouldn't auto-correct this, but I think we should still flag this, as it looks quite weird.
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.
Ah, I get it and updated this PR!
Style/MultilineTernaryOperator
Style/MultilineTernaryOperator
c7ce426
to
838bb4e
Compare
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.
You can add in the docs some explanation why won't auto-correct a ternary op in return
.
|
||
add_offense(node) do |corrector| | ||
next unless offense?(node) && !node.parent.return_type? |
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.
You can add a comment here explaining why this is needed for future reference.
838bb4e
to
6794f3f
Compare
Thank you for your advice. I added a note to the doc. |
9abfb7a
to
4c4c50e
Compare
This PR fixes the following incorrect auto-correct for `Style/MultilineTernaryOperator` when returning a multiline ternary operator expression. ```console % cat example.rb return distance_in_minutes == 0 ? locale.t(:less_than_x_minutes, count: 1) : locale.t(:x_minutes, count: distance_in_minutes) % bundle exec rubocop -a --only Style/MultilineTernaryOperator (snip) Inspecting 2 files C. Offenses: example.rb:1:8: C: [Corrected] Style/MultilineTernaryOperator: Avoid multi-line ternary operators, use if or unless instead. return distance_in_minutes == 0 ? ... ^^^^^^^^^^^^^^^^^^^^^^^^^^ 2 files inspected, 1 offense detected, 1 offense corrected % cat example.rb return if distance_in_minutes == 0 locale.t(:less_than_x_minutes, count: 1) else locale.t(:x_minutes, count: distance_in_minutes) end % ruby -c example.rb example.rb:3: syntax error, unexpected `else', expecting end-of-input ``` I found it with the following code. https://github.com/rails/rails/blob/v6.0.3.2/actionview/lib/action_view/helpers/date_helper.rb#L109-L111
4c4c50e
to
8a1776f
Compare
Follow up rubocop#8661. This PR fixes an incorrect auto-correct for `Style/MultilineTernaryOperator` when returning a multiline ternary operator expression. Using `break`, `next`, or method call also causes the same syntax error as `return`. And this PR will change these auto-correct to single-line ternary operator as valid syntax.
Follow up #8661. This PR fixes an incorrect auto-correct for `Style/MultilineTernaryOperator` when returning a multiline ternary operator expression. Using `break`, `next`, or method call also causes the same syntax error as `return`. And this PR will change these auto-correct to single-line ternary operator as valid syntax.
This PR fixes the following incorrect auto-correct for
Style/MultilineTernaryOperator
when returning a multiline ternary operator expression.I found it with the following code.
https://github.com/rails/rails/blob/v6.0.3.2/actionview/lib/action_view/helpers/date_helper.rb#L109-L111
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.