Fix subject nesting detection in RSpec/LeadingSubject #1011
Conversation
|
@fabioxgn can you please check if this is fixing your case? |
It does, thanks! 🎉 |
| break if sibling.equal?(node) | ||
|
|
||
| yield sibling if offending?(sibling) | ||
| end | ||
| end | ||
|
|
||
| def parent(node) | ||
| node.each_ancestor(:block).first.body |
There was a problem hiding this comment.
@pirj Could you help me understand how this fixed the bug? (Extra points for adding the explanation to the commit message ❤️)
There was a problem hiding this comment.
Sure. Described what was wrong here: 87562ac
By picking the nearest outer block body, we make sure we only pick real siblings inside that block.
pirj
force-pushed
the
fix-leading-subject
branch
from
1f57db4
to
87562ac
Compare
For the following code:
RSpec.describe User do
let(:foo) { 'bar' }
it_behaves_like 'a good citizen' do
subject { described_class.new }
end
end
when `on_block` is triggered for `subject`,
`node.parent.each_child_node.to_a` is:
[
s(:send, nil, :it_behaves_like, s(:str, "a good citizen")),
s(:args),
s(:block, s(:send, nil, :subject), s(:args), s(:send, s(:send, nil, :described_class), :new))
]
and autocorrect was putting the subject block above all "offenders"
coming before it, including the `it_behaves_like` send node.
pirj
force-pushed
the
fix-leading-subject
branch
from
87562ac
to
fefb978
Compare
|
@bquorning Was going to say it's high time for 1.43.2 with two recent bugfixes, but it turns out we've merged a new cop, |
Fix subject nesting detection in RSpec/LeadingSubject
I’ve pushed a branch with cherry-picked commits: https://github.com/rubocop-hq/rubocop-rspec/compare/1.43-bugfixes We can create a release from that branch, then merge it back into Good idea or bad idea? |
|
@bquorning Great idea! 👏 Can you please glance over #1018? We have different points of view with |
Maybe some temp branch branched of the last release? |
|
Maybe add your doc fix as well? #1020 skips |
|
Opened #1022. Let’s talk there. |
Fixes #1010
Before submitting the PR make sure the following are checked:
master(if not - rebase it).CHANGELOG.mdif the new code introduces user-observable changes.bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).