From befa4f7b840aef2a783c218bc24d0d4cfffe4889 Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Tue, 13 Oct 2020 00:17:51 -0400 Subject: [PATCH] New Cop: `RSpec/UnusedImplicitSubject` --- CHANGELOG.md | 2 + config/default.yml | 7 +++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_rspec.adoc | 34 ++++++++++ .../cop/rspec/unused_implicit_subject.rb | 63 +++++++++++++++++++ lib/rubocop/cop/rspec_cops.rb | 1 + .../cop/rspec/unused_implicit_subject_spec.rb | 53 ++++++++++++++++ 7 files changed, 161 insertions(+) create mode 100644 lib/rubocop/cop/rspec/unused_implicit_subject.rb create mode 100644 spec/rubocop/cop/rspec/unused_implicit_subject_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b0221b04f..f695997db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Master (Unreleased) +* Add `RSpec/UnusedImplicitSubject` cop. ([@marcandre][]) * Move our documentation from rubocop-rspec.readthedocs.io to docs.rubocop.org/rubocop-rspec. ([@bquorning][]) * Add `RSpec/RepeatedIncludeExample` cop. ([@biinari][]) * Add `RSpec/StubbedMock` cop. ([@bquorning][], [@pirj][]) @@ -565,3 +566,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@biinari]: https://github.com/biinari [@koic]: https://github.com/koic [@Rafix02]: https://github.com/Rafix02 +[@marcandre]: https://github.com/marcandre diff --git a/config/default.yml b/config/default.yml index 7f7daf15d..d9503aa31 100644 --- a/config/default.yml +++ b/config/default.yml @@ -577,6 +577,13 @@ RSpec/UnspecifiedException: VersionAdded: '1.30' StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/UnspecifiedException +RSpec/UnusedImplicitSubject: + Description: Checks for usage of explicit subject that could be implicit. + Enabled: false + VersionAdded: '1.44' + StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/UnusedImplicitSubject + Safe: false + RSpec/VariableDefinition: Description: Checks that memoized helpers names are symbols or strings. Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 24d8e9ae9..a1bd8511b 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -89,6 +89,7 @@ * xref:cops_rspec.adoc#rspecstubbedmock[RSpec/StubbedMock] * xref:cops_rspec.adoc#rspecsubjectstub[RSpec/SubjectStub] * xref:cops_rspec.adoc#rspecunspecifiedexception[RSpec/UnspecifiedException] +* xref:cops_rspec.adoc#rspecunusedimplicitsubject[RSpec/UnusedImplicitSubject] * xref:cops_rspec.adoc#rspecvariabledefinition[RSpec/VariableDefinition] * xref:cops_rspec.adoc#rspecvariablename[RSpec/VariableName] * xref:cops_rspec.adoc#rspecverifieddoubles[RSpec/VerifiedDoubles] diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index f14ccce99..744a71bf2 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -3949,6 +3949,40 @@ expect { do_something }.not_to raise_error * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/UnspecifiedException +== RSpec/UnusedImplicitSubject + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Disabled +| No +| Yes (Unsafe) +| 1.44 +| - +|=== + +Checks for usage of explicit subject that could be implicit. + +This Cop is not safe as it might be confused by what is the subject. + + # bad + subject(:foo) { :bar } + it { expect(foo).to eq :bar } + it { foo.should eq :bar } + + # good + subject(:foo) { :bar } + it { is_expected.to eq :bar } + it { should eq :bar } + + # also good + it { expect { foo }.to raise_error } + it { expect(foo.to_s).to eq 'bar' } + +=== References + +* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/UnusedImplicitSubject + == RSpec/VariableDefinition |=== diff --git a/lib/rubocop/cop/rspec/unused_implicit_subject.rb b/lib/rubocop/cop/rspec/unused_implicit_subject.rb new file mode 100644 index 000000000..fa7cadb6d --- /dev/null +++ b/lib/rubocop/cop/rspec/unused_implicit_subject.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Checks for usage of explicit subject that could be implicit. + # + # This Cop is not safe as it might be confused by what is the subject. + # + # # bad + # subject(:foo) { :bar } + # it { expect(foo).to eq :bar } + # it { foo.should eq :bar } + # + # # good + # subject(:foo) { :bar } + # it { is_expected.to eq :bar } + # it { should eq :bar } + # + # # also good + # it { expect { foo }.to raise_error } + # it { expect(foo.to_s).to eq 'bar' } + # + class UnusedImplicitSubject < Base + extend AutoCorrector + + MSG = 'Use implicit subject.' + RESTRICT_ON_SEND = %i[expect should subject subject!].freeze + + def_node_matcher :subject_definition, + '(send #rspec? {:subject :subject!} (sym $_name))' + def_node_matcher :subject_should?, + '(send (send nil? %subject) :should ...)' + def_node_matcher :expect_subject?, + '(send nil? :expect (send nil? %subject))' + + def on_send(node) + send(node.method_name, node) + end + + private + + def subject(node) + @cur_subject = subject_definition(node) + end + alias subject! subject + + def should(node) + return unless subject_should?(node, subject: @cur_subject) + + range = node.receiver.loc.expression.join(node.loc.selector) + add_offense(range) { |c| c.replace(range, 'should') } + end + + def expect(node) + return unless expect_subject?(node, subject: @cur_subject) + + add_offense(node) { |c| c.replace(node, 'is_expected') } + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index 22599a760..d514f9766 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -89,6 +89,7 @@ require_relative 'rspec/stubbed_mock' require_relative 'rspec/subject_stub' require_relative 'rspec/unspecified_exception' +require_relative 'rspec/unused_implicit_subject' require_relative 'rspec/variable_definition' require_relative 'rspec/variable_name' require_relative 'rspec/verified_doubles' diff --git a/spec/rubocop/cop/rspec/unused_implicit_subject_spec.rb b/spec/rubocop/cop/rspec/unused_implicit_subject_spec.rb new file mode 100644 index 000000000..9314cc26e --- /dev/null +++ b/spec/rubocop/cop/rspec/unused_implicit_subject_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::UnusedImplicitSubject, :config do + context 'with a subject that can be used implicitly' do + it 'flags expect(that_subject)' do + expect_offense(<<-RUBY) + subject(:example) {} + it { expect(example).to be_good } + ^^^^^^^^^^^^^^^ Use implicit subject. + RUBY + + expect_correction(<<-RUBY) + subject(:example) {} + it { is_expected.to be_good } + RUBY + end + + it 'flags that_subject.should' do + expect_offense(<<-RUBY) + subject(:example) {} + it { example.should be_good } + ^^^^^^^^^^^^^^ Use implicit subject. + it { example.should == 42 } + ^^^^^^^^^^^^^^ Use implicit subject. + RUBY + + expect_correction(<<-RUBY) + subject(:example) {} + it { should be_good } + it { should == 42 } + RUBY + end + end + + context 'with a subject that can not be used implicitly' do + it 'does not flag similar cases' do + expect_no_offenses(<<-RUBY) + let(:example) {} + it { expect(example).to be_good } + it { example.should be_good } + RUBY + end + + it 'does not flag non-simplifyable cases' do + expect_no_offenses(<<-RUBY) + subject(:example) {} + it { expect(example.foo).to be_good } + it { example.foo.should be_good } + it { expect { example }.to be_good } + RUBY + end + end +end