search

rubocop / rubocop-rspec

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

Allow to catch RSpec/ExpectInHook offenses in the setup block #835

Closed roman-dubrovsky closed 3 years ago

roman-dubrovsky commented 4 years ago

RSpec/ExpectInHook is a great cop that allows us to detected expectation usage in the hook. Unfortunately, it does not work with setup block which is a part of rspec-rails gem.

Here is a valid Rspec example that won't be detected via RuboCop. 

setup do
  expect(something).to eq 'foo'
end

Probably, for fixing that issue we just need to extend the language dictionary and add new hook. https://github.com/rubocop-hq/rubocop-rspec/blob/0036374b8bd3cea61d9a61a911cbaeaf3541883e/lib/rubocop/rspec/language.rb#L85-L97

P.S. Probably, also we can add a new cop which can detect setup usage and suggest to use before block.

pirj commented 4 years ago

Related to https://github.com/rubocop-hq/rubocop-rspec/issues/333 In addition to aliases to examples/example groups, consider also configuring hooks and matchers that we treat as "built-in".

pirj commented 4 years ago

Would you like to take a stab on this @roman-dubrovsky ? It encompasses renaming ALL in Language::Hooks to BUILTIN, adding a CUSTOM that would load custom hooks from AllCops/RSpec, and adding an ALL as a union of BUILTIN and CUSTOM. Plus, documentation on how to configure it in the project using RuboCop RSpec.

That would be a solid first step to solve a number of outstanding issues related to RSpec DSL extensions that RuboCop RSpec previously did not consider.

lazycoder9 commented 4 years ago

@roman-dubrovsky let me try to handle this. I will work on it @pirj

pirj commented 3 years ago

@roman-dubrovsky I'm closing this, since RSpec Rails doesn't seem to recommend using setup, but rather use more conventional before, after and around hooks. setup comes from Minitest, and RSpec Rails wraps it into hooks.

In any case, this will be handled in #1077