Skip to content

Commit

Permalink
Fix a false positive for Style/CaseLikeIf when conditional contains…
Browse files Browse the repository at this point in the history
… comparison with a class
  • Loading branch information
fatkodima authored and marcandre committed Aug 12, 2020
1 parent 764fcf5 commit 636b547
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 5 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,10 @@

* [#8474](https://github.com/rubocop-hq/rubocop/pull/8474): Add new `Lint/DuplicateRequire` cop. ([@fatkodima][])

### Bug fixes

* [#8508](https://github.com/rubocop-hq/rubocop/pull/8508): Fix a false positive for `Style/CaseLikeIf` when conditional contains comparison with a class. Mark `Style/CaseLikeIf` as not safe. ([@fatkodima][])

### Changes

* [#8362](https://github.com/rubocop-hq/rubocop/issues/8362): Add numbers of correctable offenses to summary. ([@nguyenquangminh0711][])
Expand Down
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -2596,6 +2596,7 @@ Style/CaseLikeIf:
Description: 'This cop identifies places where `if-elsif` constructions can be replaced with `case-when`.'
StyleGuide: '#case-vs-if-else'
Enabled: 'pending'
Safe: false
VersionAdded: '0.88'

Style/CharacterLiteral:
Expand Down
4 changes: 2 additions & 2 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -865,8 +865,8 @@ some_string =~ /something/
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| Yes
| No
| Yes (Unsafe)
| 0.88
| -
|===
Expand Down
22 changes: 19 additions & 3 deletions lib/rubocop/cop/style/case_like_if.rb
Expand Up @@ -159,9 +159,10 @@ def condition_from_send_node(node, target)
case node.method_name
when :is_a?
node.arguments.first if node.receiver == target
when :==, :eql?, :equal?, :=~, :match, :match?
lhs, _method, rhs = *node
condition_from_binary_op(lhs, rhs, target)
when :==, :eql?, :equal?
condition_from_equality_node(node, target)
when :=~, :match, :match?
condition_from_match_node(node, target)
when :===
lhs, _method, rhs = *node
lhs if rhs == target
Expand All @@ -172,6 +173,17 @@ def condition_from_send_node(node, target)
end
# rubocop:enable Metrics/CyclomaticComplexity

def condition_from_equality_node(node, target)
lhs, _method, rhs = *node
condition = condition_from_binary_op(lhs, rhs, target)
condition if condition && !class_reference?(condition)
end

def condition_from_match_node(node, target)
lhs, _method, rhs = *node
condition_from_binary_op(lhs, rhs, target)
end

def condition_from_binary_op(lhs, rhs, target)
lhs = deparenthesize(lhs)
rhs = deparenthesize(rhs)
Expand Down Expand Up @@ -202,6 +214,10 @@ def const_reference?(node)
name == name.upcase
end

def class_reference?(node)
node.const_type? && node.children[1].match?(/[[:lower:]]/)
end

def deparenthesize(node)
node = node.children.last while node.begin_type?
node
Expand Down
9 changes: 9 additions & 0 deletions spec/rubocop/cop/style/case_like_if_spec.rb
Expand Up @@ -77,6 +77,15 @@
RUBY
end

it 'does not register an offense when one of the branches contains `==` with class reference' do
expect_no_offenses(<<~RUBY)
if x == 1
elsif x == Foo
else
end
RUBY
end

it 'does not register an offense when using `==` with constant containing 1 letter in name' do
expect_no_offenses(<<~RUBY)
if x == F
Expand Down

0 comments on commit 636b547

Please sign in to comment.