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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
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.md
if 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).