Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't load configuration files outside of the current project unless there's no project specific configuration #8314

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -22,6 +22,7 @@
* [#8289](https://github.com/rubocop-hq/rubocop/issues/8289): Fix `Style/AccessorGrouping` to not register offense for accessor with comment. ([@tejasbubane][])
* [#8310](https://github.com/rubocop-hq/rubocop/pull/8310): Handle major version requirements in `Gemspec/RequiredRubyVersion`. ([@eugeneius][])
* [#8315](https://github.com/rubocop-hq/rubocop/pull/8315): Fix crash for `Style/PercentLiteralDelimiters` when the source contains invalid characters. ([@eugeneius][])
* [#8239](https://github.com/rubocop-hq/rubocop/pull/8239): Don't load `.rubocop.yml` files at all outside of the current project, unless they are personal configuration files and the project has no configuration. ([@deivid-rodriguez][])

### Changes

Expand Down
20 changes: 17 additions & 3 deletions lib/rubocop/config_loader.rb
Expand Up @@ -24,7 +24,7 @@ class << self

attr_accessor :debug, :ignore_parent_exclusion,
:disable_pending_cops, :enable_pending_cops
attr_writer :default_configuration
attr_writer :default_configuration, :project_root
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only for test code to be able to reset state, the project_root doesn't really change at runtime. I can alternatively revert this change and use instance_variable_set inside the test code to reset it.


alias debug? debug
alias ignore_parent_exclusion? ignore_parent_exclusion
Expand Down Expand Up @@ -118,7 +118,7 @@ def possible_new_cops?(config)
end

def add_excludes_from_files(config, config_file)
exclusion_file = find_last_file_upwards(DOTFILE, config_file)
exclusion_file = find_last_file_upwards(DOTFILE, config_file, project_root)

return unless exclusion_file
return if PathUtil.relative_path(exclusion_file) == PathUtil.relative_path(config_file)
Expand All @@ -134,6 +134,12 @@ def default_configuration
end
end

# Returns the path rubocop inferred as the root of the project. No file
# searches will go past this directory.
def project_root
@project_root ||= find_project_root
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intentionally left this method above in the public section and documented it since I believe it could be useful for extensions. At least it's useful for a cop I'm helping with in the rubocop-packaging extension.

def warn_on_pending_cops(pending_cops)
return if pending_cops.empty?

Expand All @@ -158,7 +164,15 @@ def merge_with_default(config, config_file, unset_nil: true)
private

def find_project_dotfile(target_dir)
find_file_upwards(DOTFILE, target_dir)
find_file_upwards(DOTFILE, target_dir, project_root)
end

def find_project_root
pwd = Dir.pwd
gems_file = find_last_file_upwards('Gemfile', pwd) || find_last_file_upwards('gems.rb', pwd)
return unless gems_file

File.dirname(gems_file)
end

def find_user_dotfile
Expand Down
18 changes: 9 additions & 9 deletions lib/rubocop/file_finder.rb
Expand Up @@ -9,33 +9,33 @@ def self.root_level=(level)
@root_level = level
end

def self.root_level?(path)
@root_level == path.to_s
def self.root_level?(path, stop_dir)
(@root_level || stop_dir) == path.to_s
end

def find_file_upwards(filename, start_dir)
traverse_files_upwards(filename, start_dir) do |file|
def find_file_upwards(filename, start_dir, stop_dir = nil)
traverse_files_upwards(filename, start_dir, stop_dir) do |file|
# minimize iteration for performance
return file if file
end
end

def find_last_file_upwards(filename, start_dir)
def find_last_file_upwards(filename, start_dir, stop_dir = nil)
last_file = nil
traverse_files_upwards(filename, start_dir) do |file|
traverse_files_upwards(filename, start_dir, stop_dir) do |file|
last_file = file
end
last_file
end

private

def traverse_files_upwards(filename, start_dir)
def traverse_files_upwards(filename, start_dir, stop_dir)
Pathname.new(start_dir).expand_path.ascend do |dir|
break if FileFinder.root_level?(dir)

file = dir + filename
yield(file.to_s) if file.exist?

break if FileFinder.root_level?(dir, stop_dir)
end
end
end
Expand Down
19 changes: 11 additions & 8 deletions lib/rubocop/rspec/shared_contexts.rb
Expand Up @@ -12,17 +12,20 @@
# get mismatched pathnames when loading config files later on.
tmpdir = File.realpath(tmpdir)

virtual_home = File.expand_path(File.join(tmpdir, 'home'))
Dir.mkdir(virtual_home)
ENV['HOME'] = virtual_home
ENV.delete('XDG_CONFIG_HOME')

base_dir = example.metadata[:project_inside_home] ? virtual_home : tmpdir
root = example.metadata[:root]
working_dir = root ? File.join(base_dir, 'work', root) : File.join(base_dir, 'work')

# Make upwards search for .rubocop.yml files stop at this directory.
RuboCop::FileFinder.root_level = tmpdir
RuboCop::FileFinder.root_level = working_dir

begin
virtual_home = File.expand_path(File.join(tmpdir, 'home'))
Dir.mkdir(virtual_home)
ENV['HOME'] = virtual_home
ENV.delete('XDG_CONFIG_HOME')

working_dir = File.join(tmpdir, 'work')
Dir.mkdir(working_dir)
FileUtils.mkdir_p(working_dir)

Dir.chdir(working_dir) do
example.run
Expand Down
49 changes: 49 additions & 0 deletions spec/rubocop/config_loader_spec.rb
Expand Up @@ -82,6 +82,26 @@
end
end

context 'when there is a spurious rubocop config outside of the project', root: 'dir' do
let(:dir_path) { 'dir' }

before do
# Force reload of project root
described_class.project_root = nil
create_empty_file('Gemfile')
create_empty_file('../.rubocop.yml')
end

after do
# Don't leak project root change
described_class.project_root = nil
end

it 'ignores the spurious config and falls back to the provided default file if run from the project' do
expect(configuration_file_for).to end_with('config/default.yml')
end
end

context 'when a config file exists in the parent directory' do
let(:dir_path) { 'dir' }

Expand Down Expand Up @@ -197,6 +217,35 @@
end
end

context 'when project has a Gemfile', :project_inside_home do
let(:file_path) { '.rubocop.yml' }

before do
create_empty_file('Gemfile')

create_file(file_path, <<~YAML)
AllCops:
Exclude:
- vendor/**
YAML
end

context 'and there is a personal config file in the home folder' do
before do
create_file('~/.rubocop.yml', <<~YAML)
AllCops:
Exclude:
- tmp/**
YAML
end

it 'ignores personal AllCops/Exclude' do
excludes = configuration_from_file['AllCops']['Exclude']
expect(excludes).to eq([File.expand_path('vendor/**')])
end
end
end

context 'when a parent file specifies DisabledByDefault: true' do
let(:file_path) { '.rubocop.yml' }

Expand Down