Skip to content

Commit

Permalink
RSpec/SubjectStub. Code review. Handle case with several explicit sub…
Browse files Browse the repository at this point in the history
…jects in example group
  • Loading branch information
andrykonchin committed Jun 8, 2020
1 parent 817a77e commit 7e76bd7
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 23 deletions.
37 changes: 14 additions & 23 deletions lib/rubocop/cop/rspec/subject_stub.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,20 +76,12 @@ class SubjectStub < Cop
PATTERN

def on_block(node)
# process only describe/context/... example groups
return unless example_group?(node)

# skip already processed example group
# it's processed if is nested in one of the processed example groups
return unless (processed_example_groups & node.ancestors).empty?

# add example group to already processed
processed_example_groups << node
@explicit_subjects = find_all_explicit_subjects(node)

# find all custom subjects e.g. subject(:foo) { ... }
@named_subjects = find_all_named_subjects(node)

# look for method expectation matcher
find_subject_expectations(node) do |stub|
add_offense(stub)
end
Expand All @@ -98,32 +90,31 @@ def on_block(node)
private

def processed_example_groups
@processed_example_groups ||= Set[]
@processed_example_groups ||= Set.new
end

def find_all_named_subjects(node)
def find_all_explicit_subjects(node)
node.each_descendant(:block).each_with_object({}) do |child, h|
name = subject(child)
h[child.parent.parent] = name if name
if name
h[child.parent.parent] ||= []
h[child.parent.parent] << name
end
end
end

def find_subject_expectations(node, subject_name = nil, &block)
# if it's a new example group - check whether new named subject is
# defined there
if example_group?(node)
subject_name = @named_subjects[node] || subject_name
def find_subject_expectations(node, subject_names = [], &block)
if example_group?(node) && @explicit_subjects[node]
subject_names = @explicit_subjects[node]
end

# check default :subject and then named one (if it's present)
expectation_detected = message_expectation?(node, :subject) || \
(subject_name && message_expectation?(node, subject_name))

expectation_detected = (subject_names + [:subject]).any? do |name|
message_expectation?(node, name)
end
return yield(node) if expectation_detected

# Recurse through node's children looking for a message expectation.
node.each_child_node do |child|
find_subject_expectations(child, subject_name, &block)
find_subject_expectations(child, subject_names, &block)
end
end
end
Expand Down
20 changes: 20 additions & 0 deletions spec/rubocop/cop/rspec/subject_stub_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,26 @@
RUBY
end

it 'flags when subject is stubbed and there are several named subjects ' \
'in the same example group', :wip do
expect_offense(<<-RUBY)
describe Foo do
subject(:foo) { described_class.new }
subject(:bar) { described_class.new }
subject(:baz) { described_class.new }
before do
allow(bar).to receive(:bar).and_return(baz)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not stub methods of the object under test.
end
it 'uses expect twice' do
expect(foo.bar).to eq(baz)
end
end
RUBY
end

it 'flags when subject is mocked' do
expect_offense(<<-RUBY)
describe Foo do
Expand Down

0 comments on commit 7e76bd7

Please sign in to comment.