rubocop-rspec
rubocop-rspec copied to clipboard
Extract part of project to rubocop-extension
The Inject.defaults!
code is duplicated across many third party gems
-
lib/rubocop/rspec/inject.rb@
d25070
-
lib/rubocop/devtools/inject.rb@
fefa8d
-
lib/rubocop/cask/inject.rb@
c2456c
-
lib/rubocop/yast/config.rb@
0cdddc
This code (plus a few other pieces of rubocop-rspec) would probably benefit from being extracted to a gem that unifies logic for making a rubocop extension. This would also make the eventual transition to https://github.com/bbatsov/rubocop/issues/3414 easier.
It looks like RuboCop RSpec’s Inject
code has become the standard way of adding 3rd party cops to RuboCop.
- https://github.com/rubocop-hq/rubocop-rubycw/blob/da0c25d3bce406ed19a1066e5a2c91f05165736c/lib/rubocop/rubycw/inject.rb#L10-L17
- https://github.com/rubocop-hq/rubocop-rspec/blob/fc8422ae65907721a0be36167fbdcfff0159a414/lib/rubocop/rspec/inject.rb#L8-L17
- https://github.com/rubocop-hq/rubocop-performance/blob/eb26b4ae942704be1dd2b5c14ef613692c1747d1/lib/rubocop/performance/inject.rb#L8-L15
- https://github.com/rubocop-hq/rubocop-md/blob/6e074498a0949fcc20b3696a4f9f07a42506d717/lib/rubocop/markdown/rubocop_ext.rb#L25-L32
- https://github.com/rubocop-hq/rubocop-rails/blob/41a0401a2144cdcac76ba08b21a7fa13bb09a5fc/lib/rubocop/rails/inject.rb#L8-L15
- https://github.com/rubocop-hq/rubocop-minitest/blob/5e488f0978aabd26fd2e72eb174ae83f0e3a7851/lib/rubocop/minitest/inject.rb#L8-L15
- https://github.com/rubocop-hq/rubocop-rake/blob/f4d49f0eeb4c52eb8bef055aefb125cfd1e57d5d/lib/rubocop/rake/inject.rb#L10-L17
We can do better than this!
The simplest approach I can think of, is to copy our code into RuboCop::ConfigLoader
, e.g.
RuboCop
class ConfigLoader
class << self
def load_plugin!(path)
path = path.to_s
hash = load_yaml_configuration(path)
config = Config.new(hash, path)
puts "Plugin configuration from #{path}" if debug?
config = merge_with_default(config, path)
self.default_configuration = config
end
end
end
end
In lib/rubocop_rspec.rb we can delete the RuboCop::RSpec::Inject.defaults!
call and instead do
RuboCop::ConfigLoader.load_plugin!(
File.expand_path('../config/default.yml', __dir__)
)
I know nothing about the internals of ConfigLoader
, so maybe there is some code duplication or some obvious refactoring that can be done. But what do you think of this API? Do we need to register anything else? The RuboCop::RSpec
namespace? Or is passing a path (as a string) enough?
/cc @Darhazer @pirj @bbatsov
Adding a method to RuboCop for extensions to load ownself's config looks strange to me overall. Shouldn't RuboCop decide on its own which config to load depending on require
in .rubocop.yml
?
There's not much harm in adding e.g. rubocop-rspec
's configuration when rubocop-rspec
is loaded by Bundler but not required from .rubocop.yml
, but it is useless.
I don't know about ConfigLoader
. From a newbie's point of view I would expect it to load extensions' configs, especially ConfigLoaderResolver#resolve_requires
goes as far as require
gems.
I might be mistaken, but it seems that this scheme circumvents the ability of extensions to require other extensions without explicitly require
'ing them from Ruby code. When extension's config/default.yml
gets loaded, its require
s are ignored.
It's not a problem with the current Inject code, but that would load rubocop-factory_bot
code when running rubocop-rspec
specs, while it's only a sidecar, and we only need it when running rubocop
from the application that specifies rubocop-rspec
in its .rubocop.yml
.
Fixed in https://github.com/rubocop/rubocop/pull/11265 There's https://github.com/rubocop/rubocop-rspec/pull/1522 to use it, but it broke after capybara extraction, needs some debugging. I'll close as completed.