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
Override disabling a department #7390
Comments
I also find this behavior unintuitive, and it seems also inconsistent. For example, I was using rubocop 0.63.0 (when Rails cops lived in the core repository) and I wanted to only enable the AllCops:
DisabledByDefault: true
Rails:
Enabled: true
Rails/FilePath:
Enabled: true After upgrading to Rails:
Enabled: true from my configuration to get back to the original situation. So the department level configuration seems to have a different meaning for external departments vs internal departments... or something? |
I agree with @ybiquitous. Certainly a behavior change, but it will be easier to use for intuition. |
Yep, I also agree the suggested behaviour makes more sense. |
Thank you for your feedback! I think that we need to fix the BeforeAfter# Priority order:
# 1. Cop
# 2. Department
# 3. All
def enable_cop?(qualified_cop_name, cop_options)
return cop_options.fetch('Enabled') if cop_options.key?('Enabled')
cop_department, = qualified_cop_name.split('/')
department_options = self[cop_department]
if department_options&.key?('Enabled')
return department_options.fetch('Enabled')
end
!for_all_cops['DisabledByDefault']
end If this is not a big mistake, I will open a pull request including the code and tests. Also, it seems that we need to fix the following document: |
The problem I envision is that almost all cops have an It needs to be clarified how this department–cop overriding interacts with inheritance from other files (be it user-defined files or |
@buehmann Thank you for the feedback! Surely, my PoC code is wrong. 😅 It may not be enough to just fix the |
What is the status of this issue? Do you think we can leverage the en/dis-abledByDefault and bring it to the departments and cops levels? The enabled : true / false would have priority over dis/en-abledByDefault So if if have
by default, all
Here in my config, all cops As a rubocop noob, I don't want to say it's a good way to solve the issue nor pretend it's a strong alternative to what we have. Just trying to help here |
Hi guys, me again sorry, is it possible to know the state of this issue? I am willing to work on this, I am just not sure yet which direction should be taken. |
Sorry, I'm not so familiar with RuboCop internal implementation, so I don't know how to solve this issue well... 🤷♂ |
…artment [Fix #7390] Override disabled department for enabled cops
Yeah, I didn't comment but I love this!! 😍 |
yay 🎉 |
Is your feature request related to a problem? Please describe.
Hi, I am so confused about the current behavior to disable a department.
For example, assume that there are the below two files (
.rubocop.yml
andtest.rb
):When I looked at the
.rubocop.yml
above at first, I misunderstood thatStyle/StringHashKeys
cop is enabled. 😓However,
Style/StringHashKeys
cop is actually disabled in the settings above. The manual is here.I think this behavior is very misleading. Because the settings above are equivalent to:
The settings which look different are actually the same. In the settings above, I think the behavior to "disable all Style cops except for Style/StringHashKeys" should be more natural. What do you think?
The reproduction example via Docker:
In this case, I expect that a
Style/StringHashKeys
violation should be reported.Describe the solution you'd like
I want a behavior that
<Department>/<Cop>: { Enabled: true }
overrides<Department>: { Enabled: true }
. For example:Describe alternatives you've considered
The solution above may be a breaking change, but I don't have good alternatives to avoid the breaking change. 🤷♂
Additional context
The current behavior has been added via the commit 37943df.
The commit has been released with the version 0.36.0:
https://github.com/rubocop-hq/rubocop/releases/tag/v0.36.0
The text was updated successfully, but these errors were encountered: