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

RSpec/LeadingSubject is crashing if subject[] is used inside let #1150

Closed
ojab opened this issue May 10, 2021 · 5 comments
Closed

RSpec/LeadingSubject is crashing if subject[] is used inside let #1150

ojab opened this issue May 10, 2021 · 5 comments

Comments

@ojab
Copy link

ojab commented May 10, 2021

subject { {foo: 1} }
let(:bar) { subject[:foo] }

leads to

$ rubocop -rrubocop-rspec -V
1.14.0 (using Parser 3.0.1.1, rubocop-ast 1.5.0, running on ruby 3.0.0 x86_64-linux)
  - rubocop-rspec 2.2.0
$ rubocop -rrubocop-rspec --only RSpec/LeadingSubject /tmp/email_spec.rb
Inspecting 1 file
An error occurred while RSpec/LeadingSubject cop was inspecting /tmp/email_spec.rb:1:0.
To see the complete backtrace run rubocop -d.
.

1 file inspected, no offenses detected

1 error occurred:
An error occurred while RSpec/LeadingSubject cop was inspecting /tmp/email_spec.rb:1:0.
Errors are usually caused by RuboCop bugs.
Please, report your problems to RuboCop's issue tracker.
https://github.com/rubocop/rubocop/issues

Mention the following information in the issue report:
1.14.0 (using Parser 3.0.1.1, rubocop-ast 1.5.0, running on ruby 3.0.0 x86_64-linux)
@pirj
Copy link
Member

pirj commented May 10, 2021

I can't reproduce this with an example group:

RSpec.describe 'A' do
  subject { { foo: 1 } }

  let(:bar) { subject[:foo] }

  it { foo }
end

Do you use subject/let outside an example group?

@ojab
Copy link
Author

ojab commented May 10, 2021

The code above is the whole reproducer, found the crash when playing with subject in #1149.
Probably it can't be reproduced in the real example group, but should be fixed anyway.

@pirj
Copy link
Member

pirj commented May 11, 2021

Sorry, RuboCop RSpec is not supposed to work on source files that are not valid RSpec files.

@pirj pirj closed this as completed May 11, 2021
@ojab
Copy link
Author

ojab commented May 11, 2021

I can provide valid spec then :)

def example_setup
  subject { { foo: 1 } }
  let(:bar) { subject[:foo] }
end

RSpec.describe Mail do
  instance_exec(&method(:example_setup))

  specify do
    p subject
    p bar
  end
end

Anyway, while IMHO all the crashes should be handled, I understand if this one will be WONTIX though.

@pirj
Copy link
Member

pirj commented May 11, 2021

We make assumptions that RSpec DSL elements are properly structured, and LeadingSubject expects that subject is wrapped in a block. Please take into consideration that it is very flexible, and doesn't care if it's a describe do block.

Do you suggest we made explicit checks to all the cops that the RSpec DSL is in order, and skip checking those files?

RSpec DSL configuration feature (#956) has been released not long ago, to tell RuboCop RSpec about project-specific custom example group aliases. I believe if we start skipping those blocks that are actually example groups, but just are not configured as such in .rubocop.yml, and only in spec/spec_helper.rb, we risk skipping the inspection of those files that we detect as non-RSpec, and risk false negatives.

In any case, a pull request is welcome if you have an idea how to elegantly fix such a case with no side effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants