houndci / hound

Automated code review for GitHub pull requests.
https://houndci.com
MIT License
1.95k stars 402 forks source link

Hound fails to merge RuboCop configuration properly #1842

Open skalee opened 3 years ago

skalee commented 3 years ago

Hound inherits organization-wide Ruby style book in a different way than RuboCop.

Given organization-wide .rubocop.yml

Metrics/MethodLength:
  Description: Avoid methods longer than 10 lines of code.
  Max: 10

and project-specific .rubocop.yml:

# This is for locally installed RuboCop. Hound ignores it.
inherit_from:
  - https://raw.githubusercontent.com/path/to/organization-wide/.rubocop.yml

Metrics:
  Enabled: false

Hound reports violations of Metrics/MethodLength, whereas local RuboCop does not (cop is disabled as expected). IMO Hound's behaviour is incorrect.

RuboCop 1.5.2 in both cases.

skalee commented 3 years ago

This is because Hound simply merges organization-wide configuration into project-specific one, instead of relying on RuboCop's inheritance rules:

https://github.com/houndci/hound/blob/fe43c074728438cae520d4f14d36926206200d2e/app/models/config/rubocop.rb#L12

skalee commented 3 years ago

To clarify: It isn't a problem when overriding specific cop, but it is a problem when overriding the whole department. Due to incorrect merging, more specific Metrics/MethodLength takes precedence over Metrics. That would be correct if they were set in the same file. But they are not, hence Metrics should take precedence over Metrics/MethodLength, as the latter is defined in the inherited style book.