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

Ability to specify files/directories to look at and ignore everything else #4247

Closed
olegantonyan opened this issue Apr 5, 2017 · 18 comments
Closed
Labels

Comments

@olegantonyan
Copy link

Situation:
A big old project written without any specific style. It's impossible to integrate Rubocop to the whole project. But, it's possible to enable it for new code only and incrementally add new directories as we go.

What I want to do:
Add global .rubocop.yml to the project and specify directories to look at. Everything else should be ignored. We're going to add more directories to that list as we go, but we need to start with a small step.

Right now the only way to do this looks very ugly and unreadable:

Exclude:
    - !ruby/regexp /^((?!(something|some\/other\/dir)).)*$/

I bet everyone will give up on this after a 10 directories added.

My first try was this:

Include:
    - 'something/**/*.rb'
Exclude:
  - '**/*'

I.e. exclude everything, but include these.

Second try was:

Include:
    - 'something/**/*.rb'

Ok, just include these, and I'm not telling you to include anything else, so why do you even look at ./*?

I'm not sure how syntax should look like, but I hope the problem was clear. Any thoughts appretiated

@Drenmi
Copy link
Collaborator

Drenmi commented Apr 5, 2017

Welcome over from StackOverflow land. 🙂 Will have a look at the existing code and give this a think over the weekend. Any input from others is appreciated. 😊

@jonas054
Copy link
Collaborator

jonas054 commented Aug 2, 2017

The best way to do this currently is to build up a list of AllCops: Exclude patterns. They can be regexp or glob patterns, and you don't have to cram everything into a single pattern.

The alternative is that you build a wrapper script that finds the files to inspect (using the version control history for example) and gives them as command line arguments to rubocop.

@jonas054 jonas054 closed this as completed Aug 2, 2017
@gettalong
Copy link

I would also prefer to specify what to include instead of excluding all the things I don't want. A typical Ruby project has bin, lib and test/spec folders. So a config like

AllCops:
  Include:
    - bin/**/*
    - lib/**/*.rb
    - test/**/*.rb

would allow one to use Rubocop easily. No temporary Ruby files would raise a false alarm.

Would it be possible to add a new config option to AllCops to specify the exclude/include order? The default could be the current "find all files, exclude those from exclude list, use ones that are Ruby files, use ones that are specially included", with the alternative being "find all files, only use the ones from the include list, exclude the ones from the exclude list".

@revolter
Copy link
Contributor

I thought that the default Include is **/* and as soon as you set it to my_folder/**/*, it overwrites the default "all folders and files" setting, so it would only look in the files inside the my_folder.

@revolter
Copy link
Contributor

revolter commented Mar 2, 2018

So, what is the solution to this closed issue?!

@olegantonyan
Copy link
Author

I've added all existing files in the project to Exclude. It's very long list, but it seems to work just fine. Any new file created since then will be inspected by rubocop. And when refactoring old code we usually remove the file from the rubocop's exceptions.

@revolter
Copy link
Contributor

revolter commented Mar 3, 2018

That's not an elegant solution at all :( Couldn't there be a DisableImplicitIncludes option instead?

@jonas054
Copy link
Collaborator

jonas054 commented Mar 3, 2018

Yes, I guess so. Opening again.

Either we add a DisableImplicitIncludes option, or we add an IncludeOnly option. It would be nil or empty by default, and if it contained any files or patterns, the implicit includes and the Include option would be disregarded.

Either way, the Exclude option could still work as today, I think.

@jonas054 jonas054 reopened this Mar 3, 2018
@revolter
Copy link
Contributor

revolter commented Mar 3, 2018

I, too, think that the Exclude option should not be modified.

IncludeOnly sounds like the best way to achieve this, because that's exactly what I expected when first started using it: only run against the specified files in the Include option.

jonas054 added a commit to jonas054/rubocop that referenced this issue May 3, 2018
This feature makes it possible to add RuboCop inspection slowly
in a legacy project. Files and directories can be added for
inspection incrementally.

It's an alternative or complement to the existing workflow of
adding cops one at a time using the --auto-gen-config feature.
jonas054 added a commit to jonas054/rubocop that referenced this issue May 13, 2018
Before this change, we were hard-coding file extensions, file
names, and ruby interpreters, and then adding configured
includes from AllCops/Include.

Now we add the parameter AllCops/RubyInterpreters and add
**/*.rb to AllCops/Include.

By removing the hard-coded lists of things to inspect, this
feature makes it possible to introduce RuboCop inspection
slowly in a legacy project. Files and directories can be added
for inspection incrementally. It's an alternative or complement
to the existing workflow of adding cops one at a time using the
--auto-gen-config feature.

Closes rubocop#5847, which was my first attempt at a fix for rubocop#4247.
@chuckd
Copy link

chuckd commented May 16, 2018

Just got bitten by this - took 10 minutes of reading to work out why rubocop went from inspecting 950 files to only 5. We had an explicit Include list (and an Exclude list, if that is relevant). Was only listed as a New Feature, could it be listed as a Change with a warning?

Adding - '**/*.rb' to the Include: seemed to fix it, although I'm not 100% sure this was the only hardcoded path before.

@nunosilva800
Copy link

nunosilva800 commented May 16, 2018

Same here!

BTW - @chuckd full list here: https://github.com/jonas054/rubocop/blob/e64d9419b9e053cd26ec2919ad69caa6d973725d/config/default.yml#L17

In my case, I simply removed the AllCops/Include rule in rubocop.yml

@chuckd
Copy link

chuckd commented May 17, 2018

Aha, thanks @Onumis - I'd missed *.rake.

cupakromer added a commit to RadiusNetworks/radius-spec that referenced this issue Jun 14, 2018
I'm not sure why we added this in the first place. Looking at the
Rubocop default setup these files have been included for over a year
now. This custom include also breaks Rubocop 0.56.0 due to changes in
how it handles the merging (it no longer merges include by default for
the all cops setting).

For more details see:

  - [Incorrect number of inspected files](rubocop/rubocop#5822)
  - [Ability to specify files/directories to look at and ignore everything else](rubocop/rubocop#4247)
@joshuapinter
Copy link
Contributor

I got burned by this, too. Just noticed our rubocop tests were only doing 40 or so files instead of 1000+.

Just to be clear, was this a change in functionality between one of the releases? If so, which one?

I could have sworn that adding entries to the Include would append to the existing list, not overwrite the existing list.

Thanks!

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 3, 2018

I think we changed this in 0.56 or 0.57.

I could have sworn that adding entries to the Include would append to the existing list, not overwrite the existing list.

True. The problem was that this way there was no way to modify the hardcoded defaults, which was frustrating for some people. Now people have the ultimate flexibility, as nothing's hardcoded anymore.

@joshuapinter
Copy link
Contributor

Thanks for the clarification. So, quick question, how does one go about adding a specific file, whilst keeping the default files included?

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 3, 2018

See leklund@6c7e5ed

Probably we should make this more prominent in the docs.

@gresbtim
Copy link

gresbtim commented Apr 21, 2020

what happened to IncludeOnly?

we have a huge codebase and want to enable it step by step for small parts of the codebase (mainly new written things or things we touched)

config looks

AllCops:
  IncludeOnly:
    - 'app/models/building_data/*'

however this gives me a warning: Warning: AllCops does not support IncludeOnly parameter.

@jonas054
Copy link
Collaborator

IncludeOnly was never added, but if you override AllCops:Include locally it will do what you want.

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

No branches or pull requests

10 participants