Change superclass for many cops

[bquorning]

Let's depend as little as possible on ::RuboCop::Cop::RSpec::Base as cop superclass.

Now, only InferredSpecType inherits from rubocop-rspec's base class.

See also rubocop/rubocop-rspec#1511

---

Before submitting the PR make sure the following are checked:

• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Updated documentation.
• Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
• The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

[pirj]

Awesome!

How hard do you think it will be to untangle the remaining one?

[bquorning]

How hard do you think it will be to untangle the remaining one?

I would need to add these few lines to the cop:

include RuboCop::RSpec::Language

def on_new_investigation
  super
  RuboCop::RSpec::Language.config = config['RSpec']['Language']
end

The first part seems fine, but overwriting RuboCop::RSpec::Language.config is just wrong.

The problem seems to be that rubocop-rspec doesn’t define RuboCop::RSpec::Language.config at load time, but at runtime. But it would be much simpler if this rubocop-rspec_rails cop could just rely on that config being set at load time.

[pirj]

It can’t be at load time, as the language config can vary in sub-directories.
Indeed it feels improper to set the config from another extension’s on_new_investigation.
Do we have a choice?
It always felt weird to use a shared config while RuboCop can now work in parallel. I guess either failures didn’t happen, or noone really found the reason. Is rubocop parallelization threaded or proccess-based?

Can we subscribe somehow to the configloader, and set the languageconfig there? That shared memory issue again.

I can’t recall what the original issue was. And do we really compile node patterns from scratch on new investigation (as the language config may have changed).

The topic is not trivial.
As a quick solution, can we expose some “reload config” hook in rspec cop base class, at least not to call that config= directly?

[bquorning]

RuboCop Would have the same problem then, right? I guess there is a per-directory(or per-configfile) cache that maybe we could rely on?

[pirj]

RuboCop doesn’t seem to need this to update dynamic matchers like Example.all, so we’re on our own here.
For the cache - definitely. Config loading is an expensive operation, and I hope it is already optimized.

[bquorning]

It always felt weird to use a shared config while RuboCop can now work in parallel. I guess either failures didn’t happen, or noone really found the reason. Is rubocop parallelization threaded or proccess-based?

Oh, good point. Changing a class variable when doing parallel processing isn’t so good 😅 RuboCop uses the Parallel gem which uses processes, threads or ractors. As I read RuboCop, Parallel.each and Parallel.map it looks like we default to running in threads. (Maybe RuboCop should be explicit about that…)

[bquorning]

Anyway, I declare scope creep 😄 Would the PR be okay to merge as-is, and then I’ll look into opening another PR for fixing InferredSpecType.

[ydah]

Looks fine. Let's follow up with another PR.

[bquorning]

Looks fine. Let's follow up with another PR.

See #19.