Rubocop: inherit_mode Documentation Improvements

[jfelchner]

I'm having some major issues with the new inherit_mode. Specifically around superfluous warning messages being shown that I don't care about and don't know how to get rid of. The fact that messages are being written to the output which is not consistent with other warning or errors is also breaking build tooling which parse those messages to determine if they're warnings or errors.

What I currently have:

# .rubocop.yml

inherit_from:
  - '.rubocop_core.yml'

Lint/HandleExceptions:
  Enabled: false

# .rubocop_core.yml

Lint/HandleExceptions:
  Enabled: true

What I see in my output:

.rubocop.yml: Lint/HandleExceptions:Enabled overrides the same parameter in .rubocop_core.yml

I've tried changing .rubocop.yml to add:

inherit_from:
  - '.rubocop_core.yml'

inherit_mode:
  override:
    - 'Enabled'

Lint/HandleExceptions:
  Enabled: false

I've tried both override and merge and neither of them fix the issue.

RuboCop version

$ rubocop -V
0.53.0 (using Parser 2.5.0.2, running on ruby 2.4.3 x86_64-darwin16)

/cc @leklund

[leklund]

Those warnings come from a separate PR unrelated to the inherit_mode changes: https://github.com/bbatsov/rubocop/pull/5371 It will warn on duplicate settings regardless of inherit_mode

It would probably be nice to have a way to disable those warnings (cc: @jonas054) without having to disable all warnings.

[leklund]

I'm not sure what that would look like though. Changing inherit_from would require a lot of changes but perhaps we change inherit_mode to inherit_settings:

inherit_settings:
  inherit_mode:
    override:
      - 'Enabled'
  duplicate_warnings: false

[jfelchner]

@leklund thanks for the clarification! @jonas054 you know I :heart: you but those warnings need to be able to be disabled. 😂

[jonas054]

@jfelchner Yes I know. :smile: I'll try to come up with something ASAP.

[jonas054]

The inherit_mode parameter was added to address the problem of arrays always being overridden and never merged during configuration inheritance resolution. But now we have a new problem, one that I was unaware of before; that some users want to override parameters in exactly the way that I added a warning for in #5371.

I think that the least intrusive solution would be to make it work (almost) like @jfelchner expected and let

Lint/HandleExceptions:
  inherit_mode:
    override:
      - Enabled

inhibit the warning. The user has shown with this configuration that the override is intentional. This is a deviation from @leklund's original idea with inherit_mode, but maybe acceptable for everyone?

I also think that the warning message should direct the user to the relevant passage in the manual.

If the above solution doesn't hold water, I could just change the warning logic so that the message only appears if the --debug/-d option is given.

[jfelchner]

@jonas054 I personally like the --debug idea the best.

The reason for that is because I think it's logical that most people would assume if you inherit a file that sets something, then you inherit another file that sets the same thing, that the second file's value will override the first file's value. If we go with the override configuration requirement, you're going to have a lot of boilerplate for people who override a lot of settings.

Include/Exclude / Array merging are special cases IMHO.

I think if people are having issues, and they pull out the --debug tool, they'll get the information they're looking for.

I think the other benefit to --debug is that I can optionally see the information it provides any time. If we go with the override option, that information will always be hidden. What if I've set override but I'm having issues and want to see what's being overridden?

[leklund]

I think the other benefit to --debug is that I can optionally see the information it provides any time. If we go with the override option, that information will always be hidden. What if I've set override but I'm having issues and want to see what's being overridden?

I agree and think --debug is a good idea. Especially as a way to see what has been overridden

[jfelchner]

@jonas054 @bbatsov don't mean to pester, but any chance this can go out in a bug fix release ASAP? I think this is breaking a lot of tools that use rubocop output.