Skip to content

Commit

Permalink
Make the config loader Bundler-aware
Browse files Browse the repository at this point in the history
Before this change, when inheriting a configuration from a gem, rubocop:

1. would fail to find it, if the gem specified in the Gemfile has a `path` or `git` option (because rubygems is unaware of those gems)
2. would always load the config of the latest installed version of a gem, regardless of the version specified in the Gemfile

This commit changes that, so if executed in the context of Bundler (`defined?(Bundler)`) it will first try to get the gem path via Bundler, and fallback to the previous `Gem::Specification` path when not found.

This solves the two mentioned issues while maintaining the possibility to run rubocop in a globally installed context, i.e. without Bundler.
  • Loading branch information
CvX committed May 25, 2020
1 parent 8a383ac commit d59d187
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 24 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -26,6 +26,7 @@
* [#7886](https://github.com/rubocop-hq/rubocop/issues/7886): Fix a bug in `AllowComments` logic in `Lint/SuppressedException`. ([@jonas054][])
* [#7991](https://github.com/rubocop-hq/rubocop/issues/7991): Fix an error for `Layout/EmptyLinesAroundAttributeAccessor` when attribute method is method chained. ([@koic][])
* [#7993](https://github.com/rubocop-hq/rubocop/issues/7993): Fix a false positive for `Migration/DepartmentName` when a disable comment contains an unexpected character for department name. ([@koic][])
* [#7983](https://github.com/rubocop-hq/rubocop/pull/7983): Make the config loader Bundler-aware. ([@CvX][])

### Changes

Expand Down Expand Up @@ -4538,3 +4539,4 @@
[@jeffcarbs]: https://github.com/jeffcarbs
[@laurmurclar]: https://github.com/laurmurclar
[@jethrodaniel]: https://github.com/jethrodaniel
[@CvX]: https://github.com/CvX
10 changes: 8 additions & 2 deletions lib/rubocop/config_loader_resolver.rb
Expand Up @@ -208,8 +208,14 @@ def transform(config)
end

def gem_config_path(gem_name, relative_config_path)
spec = Gem::Specification.find_by_name(gem_name)
File.join(spec.gem_dir, relative_config_path)
if defined?(Bundler)
gem = Bundler.load.specs[gem_name].first
gem_path = gem.full_gem_path if gem
end

gem_path ||= Gem::Specification.find_by_name(gem_name).gem_dir

File.join(gem_path, relative_config_path)
rescue Gem::LoadError => e
raise Gem::LoadError,
"Unable to find gem #{gem_name}; is the gem installed? #{e}"
Expand Down
78 changes: 56 additions & 22 deletions spec/rubocop/config_loader_spec.rb
Expand Up @@ -823,31 +823,65 @@ class Loop < Cop
YAML
end

it 'returns values from the gem config with local overrides' do
gem_class = Struct.new(:gem_dir)
%w[gemone gemtwo].each do |gem_name|
mock_spec = gem_class.new(File.join(gem_root, gem_name))
allow(Gem::Specification).to receive(:find_by_name)
.with(gem_name).and_return(mock_spec)
context 'and the gem is globally installed' do
before do
gem_class = Struct.new(:gem_dir)
%w[gemone gemtwo].each do |gem_name|
mock_spec = gem_class.new(File.join(gem_root, gem_name))
allow(Gem::Specification).to receive(:find_by_name)
.with(gem_name).and_return(mock_spec)
end
allow(Gem).to receive(:path).and_return([gem_root])
end
allow(Gem).to receive(:path).and_return([gem_root])

expected = { 'Enabled' => true, # overridden in .rubocop.yml
'CountComments' => true, # overridden in local.yml
'Max' => 200 } # inherited from somegem
expect do
expect(configuration_from_file['Metrics/MethodLength']
.to_set.superset?(expected.to_set)).to be(true)
end.to output('').to_stderr
it 'returns values from the gem config with local overrides' do
expected = { 'Enabled' => true, # overridden in .rubocop.yml
'CountComments' => true, # overridden in local.yml
'Max' => 200 } # inherited from somegem
expect do
expect(configuration_from_file['Metrics/MethodLength']
.to_set.superset?(expected.to_set)).to be(true)
end.to output('').to_stderr

expected = { 'Enabled' => true, # gemtwo/config/default.yml
'Max' => 72, # gemtwo/config/strict.yml
'AllowHeredoc' => false, # gemtwo/config/strict.yml
'AllowURI' => false } # overridden in .rubocop.yml
expect(
configuration_from_file['Layout/LineLength']
.to_set.superset?(expected.to_set)
).to be(true)
end
end

context 'and the gem is bundled' do
before do
specs = {
'gemone' => [OpenStruct.new(full_gem_path: File.join(gem_root, 'gemone'))],
'gemtwo' => [OpenStruct.new(full_gem_path: File.join(gem_root, 'gemtwo'))]
}

expected = { 'Enabled' => true, # gemtwo/config/default.yml
'Max' => 72, # gemtwo/config/strict.yml
'AllowHeredoc' => false, # gemtwo/config/strict.yml
'AllowURI' => false } # overridden in .rubocop.yml
expect(
configuration_from_file['Layout/LineLength']
.to_set.superset?(expected.to_set)
).to be(true)
allow(Bundler).to receive(:load).and_return(OpenStruct.new(specs: specs))
end

it 'returns values from the gem config with local overrides' do
expected = { 'Enabled' => true, # overridden in .rubocop.yml
'CountComments' => true, # overridden in local.yml
'Max' => 200 } # inherited from somegem
expect do
expect(configuration_from_file['Metrics/MethodLength']
.to_set.superset?(expected.to_set)).to be(true)
end.to output('').to_stderr

expected = { 'Enabled' => true, # gemtwo/config/default.yml
'Max' => 72, # gemtwo/config/strict.yml
'AllowHeredoc' => false, # gemtwo/config/strict.yml
'AllowURI' => false } # overridden in .rubocop.yml
expect(
configuration_from_file['Layout/LineLength']
.to_set.superset?(expected.to_set)
).to be(true)
end
end
end

Expand Down

0 comments on commit d59d187

Please sign in to comment.