From 4733d704cdbdb808264870d721db94f3b3b4c333 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 9 Oct 2020 23:35:38 +0900 Subject: [PATCH 1/2] [Fix #8875] Fix incorrect autocorrect for `Style/ClassEqualityComparison` Fixes #8875. Fix an incorrect auto-correct for `Style/ClassEqualityComparison` when comparing string class name. --- CHANGELOG.md | 1 + .../cop/style/class_equality_comparison.rb | 13 +++++- .../style/class_equality_comparison_spec.rb | 41 ++++++++++++++++++- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 009622afbf9..ad557c04537 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/lib/rubocop/cop/style/class_equality_comparison.rb b/lib/rubocop/cop/style/class_equality_comparison.rb index 57c3b533308..cba4b9e76e0 100644 --- a/lib/rubocop/cop/style/class_equality_comparison.rb +++ b/lib/rubocop/cop/style/class_equality_comparison.rb @@ -36,13 +36,22 @@ 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) add_offense(range) do |corrector| - corrector.replace(range, "instance_of?(#{class_node.source})") + class_name = class_node.source + class_name = class_name.delete('"').delete("'") if node.children.first.method?(:name) + + corrector.replace(range, "instance_of?(#{class_name})") end end end + + private + + def offense_range(receiver_node, node) + range_between(receiver_node.loc.selector.begin_pos, node.source_range.end_pos) + end end end end diff --git a/spec/rubocop/cop/style/class_equality_comparison_spec.rb b/spec/rubocop/cop/style/class_equality_comparison_spec.rb index b0261adb1e1..bc64c78e5bc 100644 --- a/spec/rubocop/cop/style/class_equality_comparison_spec.rb +++ b/spec/rubocop/cop/style/class_equality_comparison_spec.rb @@ -5,22 +5,59 @@ { '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. + 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. + 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. RUBY + + expect_correction(<<~RUBY) + var.instance_of?(Date) + RUBY + end + + it 'registers an offense and corrects when comparing single quoted class name for equality' do + expect_offense(<<~RUBY) + var.class.name == 'Date' + ^^^^^^^^^^^^^^^^^^^^ Use `Object.instance_of?` 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 double quoted class name for equality' do expect_offense(<<~RUBY) var.class.name == "Date" ^^^^^^^^^^^^^^^^^^^^ Use `Object.instance_of?` instead of comparing classes. RUBY + + expect_correction(<<~RUBY) + var.instance_of?(Date) + RUBY end it 'does not register an offense when using `instance_of?`' do From ec93d26ff00afcf7c26001e74417b54c4cd8c400 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 9 Oct 2020 23:48:06 +0900 Subject: [PATCH 2/2] Tweak the offense message for `Style/ClassEqualityComparison` --- .../cop/style/class_equality_comparison.rb | 16 +++++++++++----- .../cop/style/class_equality_comparison_spec.rb | 10 +++++----- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/rubocop/cop/style/class_equality_comparison.rb b/lib/rubocop/cop/style/class_equality_comparison.rb index cba4b9e76e0..066a3221eef 100644 --- a/lib/rubocop/cop/style/class_equality_comparison.rb +++ b/lib/rubocop/cop/style/class_equality_comparison.rb @@ -21,7 +21,7 @@ class ClassEqualityComparison < Base include IgnoredMethods extend AutoCorrector - MSG = 'Use `Object.instance_of?` instead of comparing classes.' + MSG = 'Use `instance_of?(%s)` instead of comparing classes.' RESTRICT_ON_SEND = %i[== equal? eql?].freeze @@ -37,11 +37,9 @@ def on_send(node) class_comparison_candidate?(node) do |receiver_node, class_node| range = offense_range(receiver_node, node) + class_name = class_name(class_node, node) - add_offense(range) do |corrector| - class_name = class_node.source - class_name = class_name.delete('"').delete("'") if node.children.first.method?(:name) - + add_offense(range, message: format(MSG, class_name: class_name)) do |corrector| corrector.replace(range, "instance_of?(#{class_name})") end end @@ -49,6 +47,14 @@ def on_send(node) 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 diff --git a/spec/rubocop/cop/style/class_equality_comparison_spec.rb b/spec/rubocop/cop/style/class_equality_comparison_spec.rb index bc64c78e5bc..d402e0df67c 100644 --- a/spec/rubocop/cop/style/class_equality_comparison_spec.rb +++ b/spec/rubocop/cop/style/class_equality_comparison_spec.rb @@ -8,7 +8,7 @@ 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) @@ -19,7 +19,7 @@ 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) @@ -30,7 +30,7 @@ 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) @@ -41,7 +41,7 @@ it 'registers an offense and corrects when comparing single 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) @@ -52,7 +52,7 @@ 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)