Skip to content

Commit

Permalink
Add autocorrection to Lint/EmptyConditionalBody.
Browse files Browse the repository at this point in the history
  • Loading branch information
dvandersluis authored and bbatsov committed Aug 4, 2022
1 parent 45bb2bd commit cbe9248
Show file tree
Hide file tree
Showing 5 changed files with 171 additions and 2 deletions.
1 change: 1 addition & 0 deletions changelog/change_add_autocorrection_to.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#10862](https://github.com/rubocop/rubocop/pull/10862): Add autocorrection to `Lint/EmptyConditionalBody`. ([@dvandersluis][])
1 change: 1 addition & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1755,6 +1755,7 @@ Lint/EmptyConditionalBody:
Enabled: true
AllowComments: true
VersionAdded: '0.89'
VersionChanged: '<<next>>'

Lint/EmptyEnsure:
Description: 'Checks for empty ensure block.'
Expand Down
61 changes: 60 additions & 1 deletion lib/rubocop/cop/lint/empty_conditional_body.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ module RuboCop
module Cop
module Lint
# Checks for the presence of `if`, `elsif` and `unless` branches without a body.
#
# NOTE: empty `else` branches are handled by `Style/EmptyElse`.
#
# @example
# # bad
# if condition
Expand Down Expand Up @@ -53,15 +56,71 @@ module Lint
# end
#
class EmptyConditionalBody < Base
extend AutoCorrector
include CommentsHelp
include RangeHelp

MSG = 'Avoid `%<keyword>s` branches without a body.'

def on_if(node)
return if node.body
return if cop_config['AllowComments'] && contains_comments?(node)

add_offense(node, message: format(MSG, keyword: node.keyword))
add_offense(node, message: format(MSG, keyword: node.keyword)) do |corrector|
autocorrect(corrector, node)
end
end

private

def autocorrect(corrector, node)
remove_comments(corrector, node)
remove_empty_branch(corrector, node)
correct_other_branches(corrector, node)
end

def remove_comments(corrector, node)
comments_in_range(node).each do |comment|
range = range_by_whole_lines(comment.loc.expression, include_final_newline: true)
corrector.remove(range)
end
end

def remove_empty_branch(corrector, node)
corrector.remove(deletion_range(branch_range(node)))
end

def correct_other_branches(corrector, node)
return unless (node.if? || node.unless?) && node.else_branch

if node.else_branch.if_type?
# Replace an orphaned `elsif` with `if`
corrector.replace(node.else_branch.loc.keyword, 'if')
else
# Flip orphaned `else`
corrector.replace(node.loc.else, "#{node.inverse_keyword} #{node.condition.source}")
end
end

def branch_range(node)
if node.loc.else
node.source_range.with(end_pos: node.loc.else.begin_pos - 1)
else
node.source_range
end
end

def deletion_range(range)
# Collect a range between the start of the `if` node and the next relevant node,
# including final new line.
# Based on `RangeHelp#range_by_whole_lines` but allows the `if` to not start
# on the first column.
buffer = @processed_source.buffer

last_line = buffer.source_line(range.last_line)
end_offset = last_line.length - range.last_column + 1

range.adjust(end_pos: end_offset).intersect(buffer.source_range)
end
end
end
Expand Down
6 changes: 5 additions & 1 deletion lib/rubocop/cop/mixin/comments_help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@ def source_range_with_comment(node)
end

def contains_comments?(node)
comments_in_range(node).any?
end

def comments_in_range(node)
start_line = node.source_range.line
end_line = find_end_line(node)

processed_source.each_comment_in_lines(start_line...end_line).any?
processed_source.each_comment_in_lines(start_line...end_line)
end

private
Expand Down
104 changes: 104 additions & 0 deletions spec/rubocop/cop/lint/empty_conditional_body_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
^^^^^^^^^^^^ Avoid `if` branches without a body.
end
RUBY

expect_correction('')
end

it 'does not register an offense for missing `if` body with a comment' do
Expand All @@ -27,6 +29,64 @@
^^^^^^^^^^^^^^^^^^^^^ Avoid `elsif` branches without a body.
end
RUBY

expect_correction(<<~RUBY)
if condition
do_something
end
RUBY
end

it 'registers an offense for missing `if` body with `else`' do
expect_offense(<<~RUBY)
if condition
^^^^^^^^^^^^ Avoid `if` branches without a body.
else
do_something
end
RUBY

expect_correction(<<~RUBY)
unless condition
do_something
end
RUBY
end

it 'registers an offense for missing `unless` body with `else`' do
expect_offense(<<~RUBY)
unless condition
^^^^^^^^^^^^^^^^ Avoid `unless` branches without a body.
else
do_something
end
RUBY

expect_correction(<<~RUBY)
if condition
do_something
end
RUBY
end

it 'registers an offense for missing `if` body with `elsif`' do
expect_offense(<<~RUBY)
if condition
^^^^^^^^^^^^ Avoid `if` branches without a body.
elsif other_condition
do_something
elsif another_condition
do_something_else
end
RUBY

expect_correction(<<~RUBY)
if other_condition
do_something
elsif another_condition
do_something_else
end
RUBY
end

it 'does not register an offense for missing `elsif` body with a comment' do
Expand All @@ -49,6 +109,14 @@
# noop
end
RUBY

expect_correction(<<~RUBY)
if condition
do_something
else
# noop
end
RUBY
end

it 'does not register an offense for missing `elsif` body with an inline comment' do
Expand All @@ -72,6 +140,14 @@
^^^^^^^^^ Avoid `elsif` branches without a body.
end
RUBY

expect_correction(<<~RUBY)
if foo
do_foo
elsif bar
do_bar
end
RUBY
end

it 'registers an offense for missing `unless` body' do
Expand All @@ -80,6 +156,8 @@
^^^^^^^^^^^^^^^^ Avoid `unless` branches without a body.
end
RUBY

expect_correction('')
end

it 'does not register an offense for missing `unless` body with a comment' do
Expand All @@ -90,6 +168,22 @@
RUBY
end

it 'autocorrects properly when the if is assigned to a variable' do
expect_offense(<<~RUBY)
x = if foo
^^^^^^ Avoid `if` branches without a body.
elsif bar
5
end
RUBY

expect_correction(<<~RUBY)
x = if bar
5
end
RUBY
end

context 'when AllowComments is false' do
let(:cop_config) { { 'AllowComments' => false } }

Expand All @@ -100,6 +194,8 @@
# noop
end
RUBY

expect_correction('')
end

it 'registers an offense for missing `elsif` body with a comment' do
Expand All @@ -111,6 +207,12 @@
# noop
end
RUBY

expect_correction(<<~RUBY)
if condition
do_something
end
RUBY
end

it 'registers an offense for missing `unless` body with a comment' do
Expand All @@ -120,6 +222,8 @@
# noop
end
RUBY

expect_correction('')
end
end
end

0 comments on commit cbe9248

Please sign in to comment.