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

Fix a false positive for Style/CaseLikeIf when conditional contains comparison with a class #8511

Merged
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
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