Skip to content
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

Add simple autocorrect for Style/GuardClause #10255

Merged
merged 1 commit into from Oct 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1 @@
* Add simple autocorrect for `Style/GuardClause`. ([@FnControlOption][])
83 changes: 62 additions & 21 deletions lib/rubocop/cop/style/guard_clause.rb
Expand Up @@ -10,6 +10,9 @@ module Style
# one of `return`, `break`, `next`, `raise`, or `fail` is used
# in the body of the conditional expression.
#
# NOTE: Autocorrect works in most cases except with if-else statements
# that contain logical operators such as `foo || raise('exception')`
#
# @example
# # bad
# def test
Expand Down Expand Up @@ -90,6 +93,7 @@ module Style
# end
#
class GuardClause < Base
extend AutoCorrector
include MinBodyLength
include StatementModifier

Expand All @@ -101,40 +105,49 @@ def on_def(node)

return unless body

if body.if_type?
check_ending_if(body)
elsif body.begin_type?
final_expression = body.children.last
check_ending_if(final_expression) if final_expression&.if_type?
end
check_ending_body(body)
end
alias on_defs on_def

def on_if(node)
return if accepted_form?(node)

guard_clause_in_if = node.if_branch&.guard_clause?
guard_clause_in_else = node.else_branch&.guard_clause?
guard_clause = guard_clause_in_if || guard_clause_in_else
return unless guard_clause
if (guard_clause = node.if_branch&.guard_clause?)
kw = node.loc.keyword.source
guard = :if
elsif (guard_clause = node.else_branch&.guard_clause?)
kw = node.inverse_keyword
guard = :else
else
return
end

kw = if guard_clause_in_if
node.loc.keyword.source
else
node.inverse_keyword
end
guard = nil if and_or_guard_clause?(guard_clause)

register_offense(node, guard_clause_source(guard_clause), kw)
register_offense(node, guard_clause_source(guard_clause), kw, guard)
end

private

def check_ending_body(body)
return if body.nil?

if body.if_type?
check_ending_if(body)
elsif body.begin_type?
final_expression = body.children.last
check_ending_if(final_expression) if final_expression&.if_type?
end
end

def check_ending_if(node)
return if accepted_form?(node, ending: true) || !min_body_length?(node)
return if allowed_consecutive_conditionals? &&
consecutive_conditionals?(node.parent, node)

register_offense(node, 'return', node.inverse_keyword)

check_ending_body(node.if_branch)
end

def consecutive_conditionals?(parent, node)
Expand All @@ -145,28 +158,56 @@ def consecutive_conditionals?(parent, node)
end
end

def register_offense(node, scope_exiting_keyword, conditional_keyword)
def register_offense(node, scope_exiting_keyword, conditional_keyword, guard = nil)
condition, = node.node_parts
example = [scope_exiting_keyword, conditional_keyword, condition.source].join(' ')
if too_long_for_single_line?(node, example)
return if trivial?(node)

example = "#{conditional_keyword} #{condition.source}; #{scope_exiting_keyword}; end"
replacement = <<~RUBY.chomp
#{conditional_keyword} #{condition.source}
#{scope_exiting_keyword}
end
RUBY
end

add_offense(node.loc.keyword, message: format(MSG, example: example))
add_offense(node.loc.keyword, message: format(MSG, example: example)) do |corrector|
next if node.else? && guard.nil?

autocorrect(corrector, node, condition, replacement || example, guard)
end
end

def guard_clause_source(guard_clause)
parent = guard_clause.parent
def autocorrect(corrector, node, condition, replacement, guard)
corrector.replace(node.loc.keyword.join(condition.loc.expression), replacement)
corrector.remove(node.loc.end)
return unless node.else?

corrector.remove(node.loc.else)
corrector.remove(branch_to_remove(node, guard))
end

def branch_to_remove(node, guard)
case guard
when :if then node.if_branch
when :else then node.else_branch
end
end

if parent.and_type? || parent.or_type?
def guard_clause_source(guard_clause)
if and_or_guard_clause?(guard_clause)
guard_clause.parent.source
else
guard_clause.source
end
end

def and_or_guard_clause?(guard_clause)
parent = guard_clause.parent
parent.and_type? || parent.or_type?
end

def too_long_for_single_line?(node, example)
max = max_line_length
max && node.source_range.column + example.length > max
Expand Down
4 changes: 3 additions & 1 deletion spec/rubocop/cli/autocorrect_spec.rb
Expand Up @@ -1179,7 +1179,9 @@ class Goo
def something
first call
do_other 'things'
more_work if other > 34
return unless other > 34

more_work
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/rubocop/cli/options_spec.rb
Expand Up @@ -1577,7 +1577,7 @@ def badName
'Use snake_case for method names.',
'def badName',
' ^^^^^^^',
'example3.rb:2:3: C: Style/GuardClause: ' \
'example3.rb:2:3: C: [Correctable] Style/GuardClause: ' \
'Use a guard clause (return unless something) instead of ' \
'wrapping the code inside a conditional expression.',
' if something',
Expand All @@ -1592,7 +1592,7 @@ def badName
' end',
' ^^^',
'',
'3 files inspected, 15 offenses detected, 12 offenses autocorrectable',
'3 files inspected, 15 offenses detected, 13 offenses autocorrectable',
''
].join("\n"))
end
Expand Down