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

New file excludes in auto-gen-config overriden by local config, can leave project with offences #2102

Closed
DMA57361 opened this issue Aug 4, 2015 · 2 comments

Comments

@DMA57361
Copy link

DMA57361 commented Aug 4, 2015

I'm playing around with master at the moment and have come across a case where it is possible for me to run rubocop --auto-gen-config and end up with a configuration that has offences (unless I manually fix things). This is with my .rubocop.yml inheriting from the generated .rubocop_todo.yml, and to date every run of auto-gen-config has then left that project in a state where rubocop is clean, so to not have the happen feels like it's breaking the expected result of auto-gen-config. I'm not sure there's really a need to fix this, or if it'll be possible to do so, but I thought I'd at least raise the issue for awareness or discussion.

My .rubocop.yml contains (this is a common file we share between many projects, to give us a consistent base for checking, which is why it includes cops which still pop up in the local TODO):

Style/RegexpLiteral:
  EnforcedStyle: mixed
  Exclude:
    - Guardfile

While the generated .rubocop_todo.yml contains:

Style/RegexpLiteral:
  Exclude:
    - 'app/controllers/application_controller.rb'
    - 'app/models/message.rb'
    - 'config/routes.rb'
    # etc

Now it seems that, regardless of if inherit_from: .rubocop_todo.yml is above or below the config in the main file, rubocop only sees the Guardfile exclusion and so I get several offences from the files the TODO is trying to exclude. Previously, since the TODO disabled the cop entirely, this did not produce the unexpected behaviour.

I can of course remove the offences this by merging the two configs and placing them in the main or TODO files (neither of which is a great fix) or by fixing all the outstanding problems.

If I understand correctly, your default config is a YAML, which I guess you overwrite with our local file, so having another layer of that with the TODO file and somehow expecting those excludes to merge together (given some people will want replacement instead!) feels like it could get very messy. As said, I'm OK with this not getting fixed, but thought I'd at least draw some attention to it.

@jonas054
Copy link
Collaborator

jonas054 commented Aug 4, 2015

I think it's a bug, and I think it should be fixed. The easiest way is probably to copy any existing Exclude elements to .rubocop_todo.yml when generating it. That should work.

@whithajess
Copy link

Just encountered this same bug:

.rubocop.yml

Style/RescueModifier:
  Exclude:
  - 'lib/spam_protection.rb'
  - 'app/controllers/admin/dashboard_controller.rb'

.rubocop_todo.yml

# Offense count: 2
Style/RescueModifier:
  Exclude:
    - 'app/helpers/admin/base_helper.rb'
    - 'app/models/theme.rb'

Like you said if I comment out that part in .rubocop.yml and re-generate .rubocop_todo.yml it will be different and copying those values across works but would be better to fix the problem.

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

3 participants