Skip to content

Commit

Permalink
[Fix #12106] Fix a false negative for Style/RedundantReturn
Browse files Browse the repository at this point in the history
Fixes #12106.

This PR fixes a false negative for `Style/RedundantReturn`
when returning value with guard clause and `return` is used.
  • Loading branch information
koic authored and bbatsov committed Aug 8, 2023
1 parent 235f749 commit f871c38
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12106](https://github.com/rubocop/rubocop/issues/12106): Fix a false negative for `Style/RedundantReturn` when returning value with guard clause and `return` is used. ([@koic][])
4 changes: 2 additions & 2 deletions lib/rubocop/config_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ def find_user_dotfile

file = File.join(Dir.home, DOTFILE)

return file if File.exist?(file)
file if File.exist?(file)
end

def find_user_xdg_config
xdg_config_home = expand_path(ENV.fetch('XDG_CONFIG_HOME', '~/.config'))
xdg_config = File.join(xdg_config_home, 'rubocop', XDG_CONFIG)

return xdg_config if File.exist?(xdg_config)
xdg_config if File.exist?(xdg_config)
end

def expand_path(path)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/layout/empty_line_after_guard_clause.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def last_heredoc_argument(node)
return node if node
end

return last_heredoc_argument(n.receiver) if n.respond_to?(:receiver)
last_heredoc_argument(n.receiver) if n.respond_to?(:receiver)
end

def last_heredoc_argument_node(node)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/layout/indentation_width.rb
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ def skip_check?(base_loc, body_node)
# Don't check indentation if the line doesn't start with the body.
# For example, lines like "else do_something".
first_char_pos_on_line = body_node.source_range.source_line =~ /\S/
return true unless body_node.loc.column == first_char_pos_on_line
true unless body_node.loc.column == first_char_pos_on_line
end

def offending_range(body_node, indentation)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/layout/space_inside_parens.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def can_be_ignored?(token1, token2)
# follows, and that the rules for space inside don't apply.
return true if token2.comment?

return true unless same_line?(token1, token2) && !token1.space_after?
true unless same_line?(token1, token2) && !token1.space_after?
end
end
end
Expand Down
24 changes: 12 additions & 12 deletions lib/rubocop/cop/lint/struct_new_override.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,25 +32,25 @@ class StructNewOverride < Base
# @!method struct_new(node)
def_node_matcher :struct_new, <<~PATTERN
(send
(const ${nil? cbase} :Struct) :new ...)
(const {nil? cbase} :Struct) :new ...)
PATTERN

def on_send(node)
return unless struct_new(node) do
node.arguments.each_with_index do |arg, index|
# Ignore if the first argument is a class name
next if index.zero? && arg.str_type?
return unless struct_new(node)

# Ignore if the argument is not a member name
next unless STRUCT_MEMBER_NAME_TYPES.include?(arg.type)
node.arguments.each_with_index do |arg, index|
# Ignore if the first argument is a class name
next if index.zero? && arg.str_type?

member_name = arg.value
# Ignore if the argument is not a member name
next unless STRUCT_MEMBER_NAME_TYPES.include?(arg.type)

next unless STRUCT_METHOD_NAMES.include?(member_name.to_sym)
member_name = arg.value

message = format(MSG, member_name: member_name.inspect, method_name: member_name.to_s)
add_offense(arg, message: message)
end
next unless STRUCT_METHOD_NAMES.include?(member_name.to_sym)

message = format(MSG, member_name: member_name.inspect, method_name: member_name.to_s)
add_offense(arg, message: message)
end
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/style/block_delimiters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,8 @@ def special_method?(method_name)
def special_method_proper_block_style?(node)
method_name = node.method_name
return true if allowed_method?(method_name) || matches_allowed_pattern?(method_name)
return node.braces? if braces_required_method?(method_name)

node.braces? if braces_required_method?(method_name)
end

def braces_required_method?(method_name)
Expand Down
9 changes: 7 additions & 2 deletions lib/rubocop/cop/style/redundant_return.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,18 @@ module Style
# return something
# end
#
# # good
# # bad
# def test
# return something if something_else
# end
#
# # good
# def test
# something if something_else
# end
#
# # good
# def test
# if x
# elsif y
# else
Expand Down Expand Up @@ -136,7 +141,7 @@ def check_case_node(node)
end

def check_if_node(node)
return if node.modifier_form? || node.ternary?
return if node.ternary?

check_branch(node.if_branch)
check_branch(node.else_branch)
Expand Down
19 changes: 17 additions & 2 deletions spec/rubocop/cop/style/redundant_return_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,25 @@ def self.func
RUBY
end

it 'accepts return in a non-final position' do
expect_no_offenses(<<~RUBY)
it 'registers an offense when returning value with guard clause and `return` is used' do
expect_offense(<<~RUBY)
def func
return something if something_else
^^^^^^ Redundant `return` detected.
end
RUBY

expect_correction(<<~RUBY)
def func
something if something_else
end
RUBY
end

it 'does not register an offense when returning value with guard clause and `return` is not used' do
expect_no_offenses(<<~RUBY)
def func
something if something_else
end
RUBY
end
Expand Down

0 comments on commit f871c38

Please sign in to comment.