From d59d18764d88b8f6058bd50d85b6aa2c5f52faf6 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Sat, 16 May 2020 19:49:59 +0200 Subject: [PATCH] Make the config loader Bundler-aware 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. --- CHANGELOG.md | 2 + lib/rubocop/config_loader_resolver.rb | 10 +++- spec/rubocop/config_loader_spec.rb | 78 +++++++++++++++++++-------- 3 files changed, 66 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e689b7ca1091..1f11e182d7a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -4538,3 +4539,4 @@ [@jeffcarbs]: https://github.com/jeffcarbs [@laurmurclar]: https://github.com/laurmurclar [@jethrodaniel]: https://github.com/jethrodaniel +[@CvX]: https://github.com/CvX diff --git a/lib/rubocop/config_loader_resolver.rb b/lib/rubocop/config_loader_resolver.rb index 76230de346ca..9d5aab1f73f3 100644 --- a/lib/rubocop/config_loader_resolver.rb +++ b/lib/rubocop/config_loader_resolver.rb @@ -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}" diff --git a/spec/rubocop/config_loader_spec.rb b/spec/rubocop/config_loader_spec.rb index 0043452a7033..5affe9012cf2 100644 --- a/spec/rubocop/config_loader_spec.rb +++ b/spec/rubocop/config_loader_spec.rb @@ -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