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

New Cop: UnusedImplicitSubject #1040

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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][])
Expand Down Expand Up @@ -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
7 changes: 7 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
34 changes: 34 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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

|===
Expand Down
63 changes: 63 additions & 0 deletions lib/rubocop/cop/rspec/unused_implicit_subject.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions lib/rubocop/cop/rspec_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
53 changes: 53 additions & 0 deletions spec/rubocop/cop/rspec/unused_implicit_subject_spec.rb
Original file line number Diff line number Diff line change
@@ -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