Defining cops' include/exclude overwrites department configuration (and there's no way to merge them)

[Drowze]

Expected behavior

I expect that when I define inherit_mode.merge = ['Exclude'] on my rubocop.yml, the department exclude configuration is merged with the individual cops' Exclude. This should actually happen with Include too.

Actual behavior

When I have an Exclude option configured at the department level, it is always overwritten on individual cops if they define any Exclude themselves.

Steps to reproduce the problem

Given test.rb:

1 + 1

And given a .rubocop.yml:

inherit_mode:
  merge:
  - Exclude

AllCops:
  NewCops: disable

Style/FrozenStringLiteralComment:
  Exclude:
    - foo.rb

Style:
  Exclude:
    - test.rb

When I run rubocop --debug --only Style/FrozenStringLiteralComment test.rb I get:

For /Users/Drowze/workspace/ruby-scripts/rubocop_bug6: configuration from /Users/Drowze/workspace/ruby-scripts/rubocop_bug6/.rubocop.yml
Default configuration from /Users/Drowze/.asdf/installs/ruby/2.7.3/lib/ruby/gems/2.7.0/gems/rubocop-1.18.4/config/default.yml
Inspecting 1 file
Scanning /Users/Drowze/workspace/ruby-scripts/rubocop_bug6/test.rb
Loading cache from /Users/Drowze/.cache/rubocop_cache/f0c6549b3ba85dee6fbdac52c969fbbd0c5e7eeb/71f7dda238ff76ce0d032ce86f7a4921eb79aa1e/fcd33c8d415b48f56d850a02862f251c3520b52b
C

Offenses:

test.rb:1:1: C: [Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
1 + 1
^

1 file inspected, 1 offense detected, 1 offense auto-correctable
Finished in 0.157611999893561 seconds

RuboCop version

$ rubocop -V
1.18.4 (using Parser 3.0.2.0, rubocop-ast 1.8.0, running on ruby 2.7.3 x86_64-darwin20)

[dvandersluis]

#9952 might have fixed this already (or #9624?)

[Drowze]

I don't think it's related to #9952 as the issue is still reproducible using the steps given.

I would like to give a bit more details of our setup and our particular use-case. We currently use:

• a gem to centralize our configuration (using inherit_gem to include it)
• rubocop-rspec to check our tests styling
• rspec to write our automated tests
• rswag to write our request tests

rswag uses a DSL written on top of RSpec to describe requests tests. We do not want to run rubocop-rspec on our request tests as there would be a lot of false positives (see rswag/rswag#138).

That being said, what we would like to have (but doesn't currently work) is:

our_gem's rubocop.yml:

RSpec:
  Enabled: true
  Excludes:
    - "spec/requests/**/*.rb"

project's .rubocop.yml

inherit_from: rubocop_todo.yml # line auto-added by rubocop auto-gen-conf

inherit_gem:
  our_gem: rubocop.yml

project's .rubocop_todo.yml (generated by rubocop auto-gen-conf)

# ... some comments added automatically ...
RSpec/LetSetup:
  # note: rubocop auto-gen-conf does not populate Excludes with anything from `spec/requests` (which is expected)
  Excludes:
    - spec/controllers/foo_controller.rb

With this particular setup, if we run bundle exec rubocop --only LetSetup, we would still get dozens of issues on spec/requests/

For this reason, as a temporary workaround we are not defining excludes at RSpec department level anymore: Over-haul/rubocop-overhaul#24

[dvandersluis]

@Drowze I commented on that rswag issue, but it should be possible to use rubocop-rspec now with it's configurable syntax. That's not really the point of this issue, but I thought I'd mention it.

I still think the fix in #9624 may solve your problem. Can you try checking out the branch from that PR and running it against your code?

[Drowze]

@dvandersluis checking out to #9624 unfortunately did not fix anything for me.

Using example from first post

$ (9325_merge_exclude_by_default) git branch
* 9325_merge_exclude_by_default
  master

$ (9325_merge_exclude_by_default) git status
On branch 9325_merge_exclude_by_default
Your branch is up to date with 'jonas054/9325_merge_exclude_by_default'.

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        deleted:    .rubocop.yml

no changes added to commit (use "git add" and/or "git commit -a")

$ (9325_merge_exclude_by_default) cat /Users/Drowze/workspace/ruby-scripts/rubocop_bug6/.rubocop.yml
inherit_mode:
  merge:
  - Exclude

AllCops:
  NewCops: disable

Style/FrozenStringLiteralComment:
  Exclude:
    - foo.rb

Style:
  Exclude:
    - test.rb

$ (9325_merge_exclude_by_default) exe/rubocop --debug /Users/Drowze/workspace/ruby-scripts/rubocop_bug6/test.rb
warning: parser/current is loading parser/ruby27, which recognizes
warning: 2.7.4-compliant syntax, but you are running 2.7.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
For /Users/Drowze/workspace/rubocop: Default configuration from /Users/Drowze/workspace/rubocop/config/default.yml
Inspecting 1 file
Scanning /Users/Drowze/workspace/ruby-scripts/rubocop_bug6/test.rb
For /Users/Drowze/r/rubocop_bug6: configuration from /Users/Drowze/workspace/ruby-scripts/rubocop_bug6/.rubocop.yml
Loading cache from /Users/Drowze/.cache/rubocop_cache/efca944991c69c5a1cb1b4e2d92b6d039ab41014/6d7a3b621ca1730e04accd938619e4bdab66cfb1/82108402598d5a1ba449fe9300886deee1d98b9b
C

Offenses:

/Users/Drowze/workspace/ruby-scripts/rubocop_bug6/test.rb:1:1: C: [Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
1 + 1
^

1 file inspected, 1 offense detected, 1 offense auto-correctable
Finished in 0.482789000030607 seconds

Using inherit_gem/rubocop_todo/etc

$ cat Gemfile | grep -E "\"rubocop\"|\"rubocop-overhaul\""
  gem "rubocop", path: "../../rubocop"
  gem "rubocop-overhaul", github: "Over-haul/rubocop-overhaul", ref: "a80d6eb7"

$ cat .rubocop_todo.yml
RSpec/LetSetup:
  Exclude:
    - 'test.rb'

$ cat .rubocop.yml
inherit_from: .rubocop_todo.yml

inherit_mode:
  merge:
    - Exclude

require:
  - rubocop-rspec

inherit_gem:
  rubocop-overhaul:
    - rubocop.yml
    - rubocop-rspec.yml

$ bundle exec rubocop --debug --cache false --only RSpec/LetSetup spec/requests/foo.rb
warning: parser/current is loading parser/ruby27, which recognizes
warning: 2.7.4-compliant syntax, but you are running 2.7.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
For /Users/Drowze/company/project: configuration from /Users/Drowze/company/project/.rubocop.yml
configuration from /Users/Drowze/.asdf/installs/ruby/2.7.3/lib/ruby/gems/2.7.0/gems/rubocop-rspec-2.4.0/config/default.yml
configuration from /Users/Drowze/.asdf/installs/ruby/2.7.3/lib/ruby/gems/2.7.0/gems/rubocop-rspec-2.4.0/config/default.yml
Default configuration from /Drowze/rubocop/config/default.yml
Inheriting configuration from /Users/Drowze/.asdf/installs/ruby/2.7.3/lib/ruby/gems/2.7.0/bundler/gems/rubocop-overhaul-a80d6eb7f493/rubocop.yml
Inheriting configuration from /Users/Drowze/.asdf/installs/ruby/2.7.3/lib/ruby/gems/2.7.0/bundler/gems/rubocop-overhaul-a80d6eb7f493/rubocop-rspec.yml
Inheriting configuration from /Users/Drowze/company/project/.rubocop_todo.yml
.rubocop.yml: inherit_mode:merge overrides the same parameter in .rubocop_todo.yml
Inspecting 1 file
Scanning /Users/Drowze/company/project/spec/requests/foo.rb
C

Offenses:

spec/requests/foo.rb:44:9: C: RSpec/LetSetup: Do not use let! to setup objects not referenced in tests.
        let!(:id) { shipment.id }

[jonas054]

A problem I see here is that it's not entirely clear what we mean by inheritance in RuboCop.

(1): A cop's parameter overrides the same cop parameter in another file. We reference that other file with inherit_from (for example), the documentation calls this inheritance, and we can use inherit_mode to modify how it works.

(2): A cop's parameter overrides the cop's department's value for the same parameter in the same file or in another file. This is actually not called inheritance in the documentation (as far as I could see), and inherit_mode doesn't affect it.

But maybe it's reasonable to call (2) inheritance too, and to let inherit_mode affect it. It might be hard to get it right, though, as the logic is already complex, so an alternative idea is to just clarify the documentation.