[Fix #13963] Fix wrong autocorrect for Lint/LiteralAsCondition when the literal is followed by return, break, or next
#13966
Conversation
Earlopain
force-pushed
the
literal-condition-void-expr
branch
from
bab13f9 to
92e6464
Compare
Earlopain
changed the title
Lint/LiteralAsCondition when the literal is followed by return, break, or nextLint/LiteralAsCondition when the literal is followed by return, break, or next
| RUBY | ||
| end | ||
|
|
||
| %w[return break next].each do |keyword| |
There was a problem hiding this comment.
Can you separate the examples into three different ones instead of using each?
There was a problem hiding this comment.
Sure. I tweaked examples to remove the block when not necessary for it to be valid syntax for return.
|
|
||
| expect_no_corrections | ||
| end | ||
| end |
There was a problem hiding this comment.
Can you also add a spec for something like:
puts "foo" && return if 2 && foo?There was a problem hiding this comment.
Ruby seems to parse this as puts ("foo" && return) if 2 && foo? so I don't think that's necessary.
| add_offense(node.lhs) do |corrector| | ||
| # Don't autocorrect `'foo' && return` because having `return` as | ||
| # the leftmost node can lead to a void value expression syntax error. | ||
| next if node.rhs.type?(:return, :break, :next) |
There was a problem hiding this comment.
I guess we also need to handle other cases like raising exceptions?
puts "foo" && raise("bar") if baz?puts "a" && fail("b") if c?puts "a" && exit if c?There was a problem hiding this comment.
Ruby doesn't have a problem with these, they are just ordinary method calls. The code itself doesn't make much sense though.
Maybe there are more where this syntax error could happen but the rules must be pretty complex because even the parser gem doesn't implement this as a syntax error.
…n` when the literal is followed by `return`, `break`, or `next` It still makes sense to register an offense in this case since the code is likely faulty and not what the user intended. There's just not much to reasonably autocorrect to. For example, OP of the issue noticed that parens where missing.
Earlopain
force-pushed
the
literal-condition-void-expr
branch
from
92e6464 to
8b83cec
Compare
|
Thanks! |
3 participants
Fix #13963
It still makes sense to register an offense in this case since the code is likely faulty and not what the user intended. There's just not much to reasonably autocorrect to.
For example, OP of the issue noticed that parens where missing.
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 runs RuboCop on its own code.{change_type}_{change_description}.mdif the new code introduces user-observable changes. See changelog entry format for details.