Skip to content

Commit

Permalink
Add simple autocorrect for Style/GuardClause
Browse files Browse the repository at this point in the history
  • Loading branch information
FnControlOption authored and bbatsov committed Oct 28, 2022
1 parent 99d13e6 commit 9228ed3
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Add simple autocorrect for `Style/GuardClause`. ([@FnControlOption][])
83 changes: 62 additions & 21 deletions lib/rubocop/cop/style/guard_clause.rb
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -1197,7 +1197,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
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit 9228ed3

Please sign in to comment.