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 #8875] Fix incorrect autocorrect for Style/ClassEqualityComparison #8876

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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@
* [#8862](https://github.com/rubocop-hq/rubocop/issues/8862): Fix an error for `Lint/AmbiguousRegexpLiteral` when using regexp without method calls in nested structure. ([@koic][])
* [#8872](https://github.com/rubocop-hq/rubocop/issues/8872): Fix an error for `Metrics/ClassLength` when multiple assignments to constants. ([@koic][])
* [#8871](https://github.com/rubocop-hq/rubocop/issues/8871): Fix a false positive for `Style/RedundantBegin` when using `begin` for method argument or part of conditions. ([@koic][])
* [#8875](https://github.com/rubocop-hq/rubocop/issues/8875): Fix an incorrect auto-correct for `Style/ClassEqualityComparison` when comparing class name. ([@koic][])

## 0.93.0 (2020-10-08)

Expand Down
23 changes: 19 additions & 4 deletions lib/rubocop/cop/style/class_equality_comparison.rb
Expand Up @@ -21,7 +21,7 @@ class ClassEqualityComparison < Base
include IgnoredMethods
extend AutoCorrector

MSG = 'Use `Object.instance_of?` instead of comparing classes.'
MSG = 'Use `instance_of?(%<class_name>s)` instead of comparing classes.'

RESTRICT_ON_SEND = %i[== equal? eql?].freeze

Expand All @@ -36,13 +36,28 @@ def on_send(node)
return if def_node && ignored_method?(def_node.method_name)

class_comparison_candidate?(node) do |receiver_node, class_node|
range = range_between(receiver_node.loc.selector.begin_pos, node.source_range.end_pos)
range = offense_range(receiver_node, node)
class_name = class_name(class_node, node)

add_offense(range) do |corrector|
corrector.replace(range, "instance_of?(#{class_node.source})")
add_offense(range, message: format(MSG, class_name: class_name)) do |corrector|
corrector.replace(range, "instance_of?(#{class_name})")
end
end
end

private

def class_name(class_node, node)
if node.children.first.method?(:name)
class_node.source.delete('"').delete("'")
else
class_node.source
end
end

def offense_range(receiver_node, node)
range_between(receiver_node.loc.selector.begin_pos, node.source_range.end_pos)
end
end
end
end
Expand Down
49 changes: 43 additions & 6 deletions spec/rubocop/cop/style/class_equality_comparison_spec.rb
Expand Up @@ -5,21 +5,58 @@
{ 'IgnoredMethods' => [] }
end

it 'registers an offense and corrects when comparing class for equality' do
it 'registers an offense and corrects when comparing class using `==` for equality' do
expect_offense(<<~RUBY)
var.class == Date
^^^^^^^^^^^^^ Use `Object.instance_of?` instead of comparing classes.
^^^^^^^^^^^^^ Use `instance_of?(Date)` instead of comparing classes.
RUBY

expect_correction(<<~RUBY)
var.instance_of?(Date)
RUBY
end

it 'registers an offense and corrects when comparing class using `equal?` for equality' do
expect_offense(<<~RUBY)
var.class.equal?(Date)
^^^^^^^^^^^^^^^^^^ Use `Object.instance_of?` instead of comparing classes.
^^^^^^^^^^^^^^^^^^ Use `instance_of?(Date)` instead of comparing classes.
RUBY

expect_correction(<<~RUBY)
var.instance_of?(Date)
RUBY
end

it 'registers an offense and corrects when comparing class using `eql?` for equality' do
expect_offense(<<~RUBY)
var.class.eql?(Date)
^^^^^^^^^^^^^^^^ Use `Object.instance_of?` instead of comparing classes.
^^^^^^^^^^^^^^^^ Use `instance_of?(Date)` instead of comparing classes.
RUBY

expect_correction(<<~RUBY)
var.instance_of?(Date)
RUBY
end

it 'registers an offense and corrects when comparing class name for equality' do
it 'registers an offense and corrects when comparing single quoted class name for equality' do
expect_offense(<<~RUBY)
var.class.name == 'Date'
^^^^^^^^^^^^^^^^^^^^ Use `instance_of?(Date)` instead of comparing classes.
RUBY

expect_correction(<<~RUBY)
var.instance_of?(Date)
RUBY
end

it 'registers an offense and corrects when comparing double quoted class name for equality' do
expect_offense(<<~RUBY)
var.class.name == "Date"
^^^^^^^^^^^^^^^^^^^^ Use `Object.instance_of?` instead of comparing classes.
^^^^^^^^^^^^^^^^^^^^ Use `instance_of?(Date)` instead of comparing classes.
RUBY

expect_correction(<<~RUBY)
var.instance_of?(Date)
RUBY
end

Expand Down