From cada3e09f3f521d5d4c430d838017d3cc48618a0 Mon Sep 17 00:00:00 2001 From: Tejas Bubane Date: Sat, 13 Feb 2021 23:53:07 +0530 Subject: [PATCH] Add new `RSpec/IdenticalEqualityAssertion` cop Closes #1130 --- CHANGELOG.md | 1 + config/default.yml | 6 ++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_rspec.adoc | 31 ++++++++++ .../cop/rspec/identical_equality_assertion.rb | 38 +++++++++++++ lib/rubocop/cop/rspec_cops.rb | 1 + .../identical_equality_assertion_spec.rb | 57 +++++++++++++++++++ 7 files changed, 135 insertions(+) create mode 100644 lib/rubocop/cop/rspec/identical_equality_assertion.rb create mode 100644 spec/rubocop/cop/rspec/identical_equality_assertion_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b1ff8e4c9..f5467fb98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Add missing documentation for `single_statement_only` style of `RSpec/ImplicitSubject` cop. ([@tejasbubane][]) * Fix an exception in `DescribedClass` when accessing a constant on a variable in a spec that is nested in a namespace. ([@rrosenblum][]) +* Add new `RSpec/IdenticalEqualityAssertion` cop. ([@tejasbubane][]) ## 2.3.0 (2021-04-28) diff --git a/config/default.yml b/config/default.yml index 1b3b7d3f4..9da8539cb 100644 --- a/config/default.yml +++ b/config/default.yml @@ -351,6 +351,12 @@ RSpec/HooksBeforeExamples: VersionAdded: '1.29' StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/HooksBeforeExamples +RSpec/IdenticalEqualityAssertion: + Description: Checks for equality assertions with identical expressions on both sides. + Enabled: pending + VersionAdded: '2.4' + StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/IdenticalEqualityAssertion + RSpec/ImplicitBlockExpectation: Description: Check that implicit block expectation syntax is not used. Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 4330d141f..610c03a03 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -35,6 +35,7 @@ * xref:cops_rspec.adoc#rspecfocus[RSpec/Focus] * xref:cops_rspec.adoc#rspechookargument[RSpec/HookArgument] * xref:cops_rspec.adoc#rspechooksbeforeexamples[RSpec/HooksBeforeExamples] +* xref:cops_rspec.adoc#rspecidenticalequalityassertion[RSpec/IdenticalEqualityAssertion] * xref:cops_rspec.adoc#rspecimplicitblockexpectation[RSpec/ImplicitBlockExpectation] * xref:cops_rspec.adoc#rspecimplicitexpect[RSpec/ImplicitExpect] * xref:cops_rspec.adoc#rspecimplicitsubject[RSpec/ImplicitSubject] diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index f866b5c9e..9cd6d51fa 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -1720,6 +1720,37 @@ end * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/HooksBeforeExamples +== RSpec/IdenticalEqualityAssertion + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| No +| 2.4 +| - +|=== + +Checks for equality assertions with identical expressions on both sides. + +=== Examples + +[source,ruby] +---- +# bad +expect(foo.bar).to eq(foo.bar) +expect(foo.bar).to eql(foo.bar) + +# good +expect(foo.bar).to eq(2) +expect(foo.bar).to eql(2) +---- + +=== References + +* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/IdenticalEqualityAssertion + == RSpec/ImplicitBlockExpectation |=== diff --git a/lib/rubocop/cop/rspec/identical_equality_assertion.rb b/lib/rubocop/cop/rspec/identical_equality_assertion.rb new file mode 100644 index 000000000..89f911b99 --- /dev/null +++ b/lib/rubocop/cop/rspec/identical_equality_assertion.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Checks for equality assertions with identical expressions on both sides. + # + # @example + # + # # bad + # expect(foo.bar).to eq(foo.bar) + # expect(foo.bar).to eql(foo.bar) + # + # # good + # expect(foo.bar).to eq(2) + # expect(foo.bar).to eql(2) + # + class IdenticalEqualityAssertion < Base + MSG = 'Identical expressions on both sides of the equality ' \ + 'may indicate a flawed test.' + RESTRICT_ON_SEND = %i[to].freeze + + # @!method equality_check?(node) + def_node_matcher :equality_check?, <<~PATTERN + (send (send nil? :expect $_) :to + {(send nil? {:eql :eq :be} $_) + (send (send nil? :be) :== $_)}) + PATTERN + + def on_send(node) + equality_check?(node) do |left, right| + add_offense(node) if left == right + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index 1de7e025d..922ff591f 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -47,6 +47,7 @@ require_relative 'rspec/focus' require_relative 'rspec/hook_argument' require_relative 'rspec/hooks_before_examples' +require_relative 'rspec/identical_equality_assertion' require_relative 'rspec/implicit_block_expectation' require_relative 'rspec/implicit_expect' require_relative 'rspec/implicit_subject' diff --git a/spec/rubocop/cop/rspec/identical_equality_assertion_spec.rb b/spec/rubocop/cop/rspec/identical_equality_assertion_spec.rb new file mode 100644 index 000000000..0524f4d20 --- /dev/null +++ b/spec/rubocop/cop/rspec/identical_equality_assertion_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::IdenticalEqualityAssertion do + it 'registers an offense when using identical expressions with `eq`' do + expect_offense(<<~RUBY) + expect(foo.bar).to eq(foo.bar) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'registers an offense when using identical expressions with `eql`' do + expect_offense(<<~RUBY) + expect(foo.bar.baz).to eql(foo.bar.baz) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'registers an offense for trivial constants' do + expect_offense(<<~RUBY) + expect(42).to eq(42) + ^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'registers an offense for complex constants' do + expect_offense(<<~RUBY) + expect({a: 1, b: :b}).to eql({a: 1, b: :b}) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'registers an offense for identical expression with be' do + expect_offense(<<~RUBY) + expect(foo.bar).to be(foo.bar) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'registers an offense for identical expression with be ==' do + expect_offense(<<~RUBY) + expect(foo.bar).to be == foo.bar + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'does not register offense for different expressions' do + expect_no_offenses(<<~RUBY) + expect(foo.bar).to eq(bar.foo) + RUBY + end + + it 'checks for whole expression' do + expect_no_offenses(<<~RUBY) + expect(Foo.new(1).foo).to eql(Foo.new(2).bar) + RUBY + end +end