If ~/.rubocop.yml contains a Rails (or Performance) cop, loading excludes causes "Rails cops have been extracted" error

[martinemde]

When running rubocop in a project that does not load Rails (in this case I'm running rubocop on rubygems), if you have Rails/ cops in your ~/.rubycop.yml file, then they will be loaded, validated, and the obsolete checker will raise.

However, the docs seem to explicitly say that this file should not be loaded since the project root contains a .rubocop.yml.

This is happening because the tests do not test the common case where projects are in a subdirectory of the user's home dir.

When the .rubocop.yml file in $HOME are in an ancestor folder of the current project, the ConfigLoader looks for the "last file upwards" matching the file pattern ".rubocop.yml" and finds ~/.rubocop.yml.

      def add_excludes_from_files(config, config_file)
        exclusion_file = find_last_file_upwards(DOTFILE, config_file, ConfigFinder.project_root) # finds ~/.rubocop.yml
        # **snip**
        config.add_excludes_from_higher_level(load_file(exclusion_file)) # load_file loads and validates, causing obsolete errors.
      end

This results in fully loading the ~/.rubocop.yml and loading the excludes contained therein. However, since the validation and obsolete checks are run, errors are generated for not requiring rubocop-rails.

My offered fix is to specifically exclude this file, but I can reduce this PR back to just the failing spec if another solution is desired.

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

[martinemde]

I can squash when we agree this solution is acceptable.

[koic]

Can you refactor to the below?

Suggested change

	return if exclusion_relative_path == PathUtil.relative_path(config_file)
	return if exclusion_relative_path == PathUtil.relative_path(File.join(Dir.home, DOTFILE))
	return if exclusion_relative_path == PathUtil.relative_path(config_file) ||
	exclusion_relative_path == PathUtil.relative_path(File.join(Dir.home, DOTFILE))

[koic]

I leave one small comment, but overall it's LGTM. Can you squash your commits into one?

[martinemde]

Squashed. Thank you!

[martinemde]

Hey, I just realize this is not the right fix. Please hold off on merging. We need to instead return the file before the $HOME/.rubocop.yml in the directory hierarchy. As is this code will skip the actual project root yml if there is a nested config deeper in the project.

[martinemde]

I'll get these fixes tomorrow.

[martinemde]

Actual fix needs to modify find_last_file_upwards to not return the home .yml file, but whatever config it finds before that home dotfile.

[martinemde]

Worth noting, @deivid-rodriguez linked me to a similar fix #8176 from a few years ago trying to address a similar problem. Hopefully we can get this specified clearly enough this time.

[martinemde]

I chose to attempt forcing all specs in the isolated environment to stay within the directory tree that is allowed. Many of the specs were placing files outside of the tmpdir, then testing that those files weren't loaded (relying on the root_level behavior which is only enabled in tests). This meant that some of the tests only passed because the test environment kept the file from being found. In the real world it would pick up the file (which is the point of this PR).

However, I'm having trouble fixing a few of the specs. They should all be caused by this situation of previously loading or relying on files that are now off limits (or assuming they wouldn't be loaded). They might also be outside of the search path, or some other magic is happening that I don't understand. Maybe someone with more experience in RuboCop code could help?

Another thing that might be causing it is just some implied directory locations based on the old dir structure.. I'll try adjusting that and see if it fixes it. (edit: nope, removing the root/ from the tmpdir causes the same failures and more).