Fix a false negative for Lint/ReturnInVoidContext when returning out of a block#13960
Conversation
|
|
||
| return unless context_node&.def_type? | ||
| return unless context_node&.void_context? | ||
| method_node = return_node.each_ancestor(:def).first |
There was a problem hiding this comment.
Can you rename from method_node to def_node?
| method_node = return_node.each_ancestor(:def).first | |
| def_node = return_node.each_ancestor(:def).first |
| expect_offense(<<~RUBY) | ||
| class A | ||
| def initialize | ||
| Proc.new do |
There was a problem hiding this comment.
Is the intended explanation referring to proc?
| Proc.new do | |
| proc do |
There was a problem hiding this comment.
Either works, I don't think there is any difference between the two. I've updated to proc to be more inline with the spec description (it's also what Style/Proc would want).
There was a problem hiding this comment.
Yep, the behavior is the same in both cases. This is about ensuring the spec description matches the example.
…t of a block Should be block happen to be called, the return will break out of the block and attribute the return value to the method. This has been that way since the cop was added and there were no tests for blocks. The only expection to the rule is when returning from inside a lambda. Additionally, I simplified the implementation. The cop looked for `sdef` but then immediatly after required that the node be simply `def`.
2febab9 to
0fd4829
Compare
|
Thanks! |
|
I think there's a false positive for The following example is a little bit contrived but I got the errors in a more complicated case and narrowed it down to this: class Something
def initialize
self.class.define_method :some_method do
return 'abc' # rubocop: Lint/ReturnInVoidContext: Do not return a value in initialize
end
end
end
instance = Something.new
instance.some_method
# => 'abc'Or am I wrong? |
|
I opened #14062 with more context and a working example for a false positive. |
Should be block happen to be called, the return will break out of the block and attribute the return value to the method.
This has been that way since the cop was added and there were no tests for blocks.
The only expection to the rule is when returning from inside a lambda.
Additionally, I simplified the implementation. The cop looked for
sdefbut then immediatly after required that the node be simplydef.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.