Rubocop-rspec: Rspec/SubjectStub: false negative with implicit subject

Created on 2 Sep 2019  路  3Comments  路  Source: rubocop-hq/rubocop-rspec

Rubocop-rspec does not detect the SubjectStub offenses when using the implicit subject

For example: no offense is reported with the following code, which it's supposed to be

describe Foo do

  before do
    allow(subject).to receive(:bar).and_return(baz)
  end

  it 'uses expect twice' do
    expect(subject.bar).to eq(baz)
  end
end

On the other hand, the offense can be detected while the subject is explicitly defined.

describe Foo do
  subject { Foo.new }

  before do
    allow(subject).to receive(:bar).and_return(baz)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not stub methods of the object under test.
  end

  it 'uses expect twice' do
    expect(subject.bar).to eq(baz)
  end
end

Rubocop version

0.74.0 (using Parser 2.6.4.0, running on ruby 2.4.0 x86_64-darwin17)
bug

Most helpful comment

@QQism Sorry for not getting back to you in a reasonable time.
I side with you that this is a defect and in this case, it should raise an offence, however not sure what the core team would say.

I've started hacking on this, and I had an impression that this won't be an easy fix due to how the offences are being detected: a subject declaration is found, and then the AST is walked down to find expectations/allowances for this subject.

If we would want to also detect implicit subject, we would have to flip this the other way around, find expectations/allowances, and then walk up the tree to find the nearest subject definition.

There could be another approach, find expectations/allowances for subject in those nodes that don't have subject declarations, and stop recursion once there's one.
If would be plain awesome if you could hack on it. Please feel free to send a pull request on the early stages if you have some doubts about the approach or implementation, I'll be happy to assist you in this journey.

All 3 comments

Hello, if this issue has been overlooked, I'm happy to look into the code and see what I can help.

@QQism Sorry for not getting back to you in a reasonable time.
I side with you that this is a defect and in this case, it should raise an offence, however not sure what the core team would say.

I've started hacking on this, and I had an impression that this won't be an easy fix due to how the offences are being detected: a subject declaration is found, and then the AST is walked down to find expectations/allowances for this subject.

If we would want to also detect implicit subject, we would have to flip this the other way around, find expectations/allowances, and then walk up the tree to find the nearest subject definition.

There could be another approach, find expectations/allowances for subject in those nodes that don't have subject declarations, and stop recursion once there's one.
If would be plain awesome if you could hack on it. Please feel free to send a pull request on the early stages if you have some doubts about the approach or implementation, I'll be happy to assist you in this journey.

I鈥檝e pushed a PR to fix this issue, any comment or feedback is very welcome.

Was this page helpful?
0 / 5 - 0 ratings