Fix ReturnInVoidContext cop when return is not inside a method. #4737
Conversation
| @@ -45,7 +45,7 @@ def on_return(return_node) | |||
|
|
|||
| def method_name(return_node) | |||
There was a problem hiding this comment.
I am inclined to split this into two methods, one that finds the context, and one that extracts the name from a def node.
Right now these two are mixed, and causes this method to have a "special meaning nil" mixed return type.
I would rather see something like:
context_node = non_void_context(return_node)
return unless context_node && ...
add_offense(return_node, :keyword, format(message, method_name(context_node))
CHANGELOG.md
Outdated
| @@ -2,6 +2,10 @@ | |||
|
|
|||
| ## master (unreleased) | |||
|
|
|||
| ### Bug fixes | |||
|
|
|||
| * [#4737](https://github.com/bbatsov/rubocop/pull/4737): Fix ReturnInVoidContext cop when `return` is not inside a method. ([@frodsan][]) | |||
There was a problem hiding this comment.
Technically "not inside a context", or "in top scope". 🙂
There was a problem hiding this comment.
You can use return in any scope, so the description seems reasonable to me, unless I missing something @Drenmi.
There was a problem hiding this comment.
Actually I'm wrong - I tested this and you can use it only in method/lambda/proc or in the top scope. My bad.
| @@ -80,4 +80,12 @@ def self.initialize | |||
| RUBY | |||
| end | |||
| end | |||
|
|
|||
| context 'with no method' do | |||
There was a problem hiding this comment.
Here, too: "when return is in top scope".
There was a problem hiding this comment.
Guess we can also have some tests with procs and lambdas. Guess we didn't think of all of them in the past.
|
@Drenmi updated |
| end | ||
|
|
||
| def useless_return_method?(method_name) | ||
| method_name == :initialize || method_setter?(method_name) | ||
| end | ||
|
|
||
| def method_setter?(method_name) |
There was a problem hiding this comment.
#method_setter? -> #setter_method? 🙂
| method_name.to_s.end_with?('=') && | ||
| !AST::Node::COMPARISON_OPERATORS.include?(method_name) | ||
| def method_name(context_node) | ||
| context_node.children.first | ||
| end | ||
|
|
||
| def useless_return_method?(method_name) |
There was a problem hiding this comment.
Should this be #void_context_method?
|
@Drenmi updated |
|
Thanks! |
Fixes #4736