search

rubocop / rubocop-rspec

Code style checking for RSpec files.
https://docs.rubocop.org/rubocop-rspec
MIT License
810 stars 276 forks source link

`RSpec/ScatteredSetup` is marked as safe but misforms case statements #1995

Closed corsonknowles closed 1 week ago

corsonknowles commented 1 week ago

Cop Doc: https://www.rubydoc.info/gems/rubocop-rspec/1.10.0/RuboCop/Cop/RSpec/ScatteredSetup

RSpec/ScatteredSetup is not safe for autocorrection because it will move before logic out of case statements and into one giant and many empty case statements

pirj commented 1 week ago

Can you please give an example?

corsonknowles commented 1 week ago

Yes, here you go!

pirj commented 1 week ago

I don’t think we have to account for RSpec DSL in case/if. RSpec has rich metadata capabilities, and those should be used instead

corsonknowles commented 1 week ago

Thanks @pirj -- It took my a moment to figure out what that means, I think it's that code like this should be refactored to pass params into the before block, etc and bring the case statements inside so that these errors are not generated by the autocorrect.

I'm convinced that the Rubocop rule here doesn't need to accomadate these cases, but I'm not convinced that it should remain marked as safe for autocorrection.

The examples here were perfectly validy Ruby/RSpec before, but are not after the autocorrect runs.

corsonknowles commented 1 week ago

In fact, for the example of hoisting to a higher context, I'm not convinced that is good behavior at all.

But I'm confident neither case should be considered equivalent by design, as is the guideline for cops marked SafeForAutocorrect.