Skip to content

Commit

Permalink
[Fix rubocop#12279] Fix false positives for Lint/EmptyConditionalBody
Browse files Browse the repository at this point in the history
Fixes rubocop#12279.

This PR fixes false positives for `Lint/EmptyConditionalBody`
when missing 2nd `if` body with a comment.
  • Loading branch information
koic committed Oct 13, 2023
1 parent b7fb064 commit 4526653
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12279](https://github.com/rubocop/rubocop/issues/12279): Fix false positives for `Lint/EmptyConditionalBody` when missing 2nd `if` body with a comment. ([@koic][])
28 changes: 16 additions & 12 deletions lib/rubocop/cop/mixin/comments_help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,25 +62,29 @@ def buffer
# Returns the end line of a node, which might be a comment and not part of the AST
# End line is considered either the line at which another node starts, or
# the line at which the parent node ends.
# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity, Lint/DuplicateBranch
# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity
def find_end_line(node)
if node.if_type? && node.else?
node.loc.else.line
elsif node.if_type? && node.ternary?
node.else_branch.loc.line
elsif node.if_type? && node.elsif?
node.each_ancestor(:if).find(&:if?).loc.end.line
if node.if_type?
if node.else?
node.loc.else.line
elsif node.ternary?
node.else_branch.loc.line
elsif node.elsif?
node.each_ancestor(:if).find(&:if?).loc.end.line
end
elsif node.block_type? || node.numblock_type?
node.loc.end.line
elsif (next_sibling = node.right_sibling) && next_sibling.is_a?(AST::Node)
next_sibling.loc.line
elsif (parent = node.parent)
parent.loc.respond_to?(:end) && parent.loc.end ? parent.loc.end.line : parent.loc.line
else
node.loc.end.line
end
if parent.loc.respond_to?(:end) && parent.loc.end
parent.loc.end.line
else
parent.loc.line
end
end || node.loc.end.line
end
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity, Lint/DuplicateBranch
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity
end
end
end
15 changes: 13 additions & 2 deletions spec/rubocop/cop/lint/empty_conditional_body_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,17 @@ class Foo
RUBY
end

it 'does not register an offense for missing 2nd `if` body with a comment' do
expect_no_offenses(<<~RUBY)
if condition1
do_something1
end
if condition2
# noop
end
RUBY
end

it 'does not register an offense for missing 2nd `elsif` body with a comment' do
expect_no_offenses(<<~RUBY)
if condition1
Expand Down Expand Up @@ -347,10 +358,10 @@ class Foo
it 'registers an offense for multi-line value omission in `unless`' do
expect_offense(<<~RUBY)
var =
# This is the value of `other:`, like so: `other: condition || other_condition`
unless object.action value:, other:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid `unless` branches without a body.
condition || other_condition # This is the value of `other:`, like so:
# `other: condition || other_condition`
condition || other_condition
end
RUBY
end
Expand Down

0 comments on commit 4526653

Please sign in to comment.