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

Rubocop finds .rubocop.yml in parent directories #536

Closed
b4hand opened this issue Oct 3, 2013 · 14 comments
Closed

Rubocop finds .rubocop.yml in parent directories #536

b4hand opened this issue Oct 3, 2013 · 14 comments

Comments

@b4hand
Copy link

b4hand commented Oct 3, 2013

I have a project that vendors in another project using git submodules both of which I work inside. So my file system looks something like this:

src/
src/project1
src/project1/.rubocop.yml
src/project1/vendor
src/project1/vendor/project2
src/project1/vendor/project2/.rubocop.yml

Inside src/project1/.rubocop.yml I exclude the vendor directory using vendor/**. Unfortunately, this means that when I run rubocop from within project2 it picks up the Exclude and then rubocop says it is "Inspecting 0 files". This is not what I intended. I simply don't want to see rubocop issues for vendor when I'm building within project1, but I do want to see them when I'm building within project2. Here's some output from rubocop --debug:

For /home/brandon/src/mozlinks/vendor/askari: configuration from /home/brandon/src/mozlinks/vendor/askari/.rubocop.yml
Default configuration from /home/brandon/.rbenv/versions/jruby-1.7.4/lib/ruby/gems/shared/gems/rubocop-0.12.0/config/default.yml
Inheriting configuration from /home/brandon/.rbenv/versions/jruby-1.7.4/lib/ruby/gems/shared/gems/rubocop-0.12.0/config/enabled.yml
Inheriting configuration from /home/brandon/.rbenv/versions/jruby-1.7.4/lib/ruby/gems/shared/gems/rubocop-0.12.0/config/disabled.yml
AllCops/Excludes configuration from /home/brandon/src/mozlinks/.rubocop.yml

That last line is the issue. Is there a reason rubocop walks the directory tree up looking for configuration higher than my current invocation path even when it has already found a .rubocop.yml?

@jonas054
Copy link
Collaborator

jonas054 commented Oct 3, 2013

Yes, that's by design. This behavior is governed by two principles.

  1. The configuration for each file comes from the nearest .rubocop.yml seaching upwards, except for AllCops/Excludes, which comes from the highest .rubocop.yml found.
  2. Current directory where rubocop is invoked is not considered.

The reason for (1) is that we don't want AllCops/Excludes in project1/vendor/project2/.rubocop.yml to override the setting in project1/.rubocop.yml, because then the vendor directory will not be excluded if it contains a project that sets AllCops/Excludes.
The reason for (2) is that you should be able to run rubocop in a shell, let's say, from the project root, but also start it from an editor deep within the source directory tree, and get the same configuration.

The purpose of the design is to make configuration of RuboCop work "as expected". I'm sorry to see that it didn't for you in this case, but I'm not sure how we can change it without breaking other behavior.

A work-around that lets you inspect excluded files is to give them as explicit arguments. For example:

src/project1> find vendor/project2 -name "*.rb" | xargs rubocop

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 4, 2013

@jonas054 Maybe you could add more information on the subject to the README?

@ghost ghost assigned jonas054 Oct 4, 2013
@jonas054
Copy link
Collaborator

jonas054 commented Oct 4, 2013

Sure.

@b4hand
Copy link
Author

b4hand commented Oct 4, 2013

I understand the rationale for point number 2, but I wasn't suggesting to stop that behavior. The only difference would be that if you started it deep in a sub-tree that you stop after you saw the first .rubocop.yml file. It doesn't seem like it is a common use case to have multiple different per directory .rubocop.yml files. In fact, I think that is the source of confusion for me. My guess is that the vast majority of all projects have a single .rubocop.yml file. Personally, if I see a rubocop failure I want to look in one place and see all of the rule modifications. I don't want to have to recursively search my project and gather up all of the files to concatenate the combined configuration. That just seems like overuse of configuration.

jonas054 added a commit to jonas054/rubocop that referenced this issue Oct 4, 2013
This is for rubocop#536, but we may want to keep the issue open for discussion a while longer.

Move the sentence about what the configuration file is for, to the first paragraph.

Make the example .rubocop.yml file more realistic by setting parameter values that are different from the default ones.

Clarify the note about AllCops/Excludes.

Make first paragraph under "Including/Excluding files" more clear and simple.

Make "Inheritance" a subchapter, and try to use the word "inherit" rather than "include" when describing inherit_from.

Replace the word folder with directory so we use the same nomenclature throughout the README.

Make "Disabling Cops within Source Code" its own subchapter, not under "Configuration", since it has nothing to do with .rubocop.yml.
@jonas054
Copy link
Collaborator

jonas054 commented Oct 5, 2013

Before continuing to discuss how to allow inspection of files that would normally be excluded, I want to give some background so we're all on the same page.

The reason for the special handling of AllCops/Excludes can be found in issue #288.

The algorithm RuboCop follows to determine which files to inspect is this:

  1. Files given on the command line are always inspected, never excluded.
  2. Directories given on the command line (current directory being the default) are searched for files.
  3. Each file is associated with a configuration by searching upwards for .rubocop.yml. AllCops/Excludes is taken from the highest .rubocop.yml found. All other parameters are taken from the lowest.
  4. If the file matches any of the AllCops/Excludes patterns, it is excluded from the list.
  5. If the file matches "*.rb", any of the AllCops/Includes patterns, or is extensionless and has a ruby hashbang line, it is considered a Ruby file. Other files are excluded at this point.
  6. The remaining files in the list are then inspected.

@jonas054
Copy link
Collaborator

jonas054 commented Oct 5, 2013

So if I understand your proposal correctly, @b4hand, you want rubocop, when invoked from the src/project1/vendor/project2 to find the .rubocop.yml there but not search upwards for other .rubocop.yml files to get the Excludes parameter from.

This has an unwanted implication that would have to be fixed. A vendor project with a .rubocop.yml file would not be excluded, but another project lacking such a file would still be excluded. [Edited: This is inconsistent and I don't have a good solution. Disregarding configuration from above the current directory would perhaps work for vendor directories because the code there belongs to other projects, but for other directories such as lib, we can't do that. ]

@agibralter
Copy link

I know this is super old... but it might be nice if it was configurable to stop the traversal through parent directories. I just had some confusing/surprising behavior because I accidentally had a .rubocop file in my home directory.

@jonas054
Copy link
Collaborator

jonas054 commented Aug 1, 2018

@agibralter Your problem only occurs if you have no .rubocop.yml in the project where you're running rubocop, but there's one higher up, or in your home directory. You can get RuboCop to not look for any configuration file and just run with default configuration like this:

rubocop --force-default-config

@vStone
Copy link

vStone commented Jul 16, 2019

using rubocop -c .rubocop.yml also does seem to prevent looking for other .rubocop.yml files in parent directories.

@Kache
Copy link

Kache commented Jun 17, 2020

I just ran into this behavior, and also found it surprising that the first .rubocop.yml configuration found going up doesn't take full precedence.

This has an unwanted implication that would have to be fixed.

@jonas054 can you elaborate? I don't understand your example why taking the first .rubocop.yml looking upwards would break expected behavior. (Isn't that how bundler works, btw?)

Are you saying that it's intended to have multiple nested .rubocop.yml files in a single project, e.g. one at PROJ_ROOT/, one in PROJ_ROOT/lib, so rubocop always searches up?

@jonas054
Copy link
Collaborator

Are you saying that it's intended to have multiple nested .rubocop.yml files in a single project, e.g. one at PROJ_ROOT/, one in PROJ_ROOT/lib, so rubocop always searches up?

Yes, that is supported. I don't think that feature is used much, having multiple .rubocop.yml within a project, but the situation with vendor directories, as described at the top of this page, is probably common.

I don't understand your example why taking the first .rubocop.yml looking upwards would break expected behavior.

Think of vendor directories, i.e. code from other projects being copied in to your project. If you say in your project that you don't want to inspect the files in vendor directories, the .rubocop.yml files in those directories should not be able to override your preference.

Note that it's only Exclude parameters that are taken from a higher level. Everything else comes from the nearest .rubocop.yml. Note also that since these comments were written, the command line option --ignore-parent-exclusion has been added.

@deivid-rodriguez
Copy link
Contributor

@jonas054 Could you give me a practical example in which the current behaviour is expected?

Say I have a layout like the one mentioned originally

src/
src/project1
src/project1/.rubocop.yml
src/project1/vendor
src/project1/vendor/project2
src/project1/vendor/project2/.rubocop.yml

Whereas if you run rubocop from the projec1 root folder, you do want your exclusions to be applied, if for some reason you explicitly switch to the vendored project2 in order to run rubocop, I can't think of a reason why it shouldn't go ahead and inspect the files and disregard exclusions further up.

You mentioned #288 as the original motivation for the current special handling of AllCops/Exclude but that I don't think removing the special handling would break exclusions like the ones in that issue.

For what it's worth, I created at PR that fixes this: #8176.

@jonas054
Copy link
Collaborator

jonas054 commented Jul 4, 2020

I can't think of a reason why it shouldn't go ahead and inspect the files and disregard exclusions further up

From the perspective of running RuboCop on the command line, standing in different directories, you may have a point. But it would mean that we have to consider current directory in the exclusion logic, and that logic is already pretty complex. Wouldn't it affect RuboCop editor integration if current directory is added as a factor in the inspection algorithm?

@deivid-rodriguez
Copy link
Contributor

You're right, although I made some testing in #8176 when the initial approach was to use CWD, and results seemed either the same or superior. But I only checked my particular integration. Anyways, I dropped the CWD approach because that reason: it felt potentially breaking even if I couldn't figure out how.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants