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

Ability to use wildcards when disabling cops #9519

Closed
unikitty37 opened this issue Feb 16, 2021 · 8 comments
Closed

Ability to use wildcards when disabling cops #9519

unikitty37 opened this issue Feb 16, 2021 · 8 comments
Assignees

Comments

@unikitty37
Copy link
Contributor

Is your feature request related to a problem? Please describe.

When it is necessary to write a method that RuboCop decides is too complex, it tends to trigger multiple cops. If there is no easy way to rewrite the method in a reasonable amount of time, it is necessary to disable multiple cops on one line:

def complex_method # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity

This is a lot of typing! However, disabling all cops for the method with a simple rubocop:disable is overkill.

Describe the solution you'd like

It would be useful to have wildcard support:

def complex_method # rubocop:disable Metrics/*

or

def complex_method # rubocop:disable Metrics

(arguably, the first is more explicit and guards against typos better)

Describe alternatives you've considered

Obviously, I considered breaking the method down into simpler code 😁 , but it doesn't seem to be particularly easy in this case, and the required dev time is a factor.

Supporting bash-style globbing would be nice, but would probably be a lot more hassle to implement for something that only makes the line slightly shorter:

def complex_method # rubocop:disable Metrics/{AbcSize,CyclomaticComplexity,MethodLength,PerceivedComplexity}

Ideally, it would be possible to define groups in rubocop.yml, and then disable those:

Groups:
  ComplexityChecks:
    - Metrics/AbcSize
    - Metrics/CyclomaticComplexity
    - Metrics/MethodLength
    - Metrics/PerceivedComplexity
def complex_method # rubocop:disable Groups/ComplexityChecks

but I suspect that will be a lot of work to implement, and I'm not sure if there's enough demand for this feature to justify it.

Additional context

none

@koic
Copy link
Member

koic commented Feb 16, 2021

def complex_method # rubocop:disable Metrics

It seems good to me that it can be specified by department. Because there are similar specification options such as rubocop --only department and enabling/disabling by department in .rubocop.yml. It does not bring in new complexity.

The only concern is that the specified department's cops hidden in the department may not be able to detect an offense. However, it shouldn't be the issue as user will accept the risk and specify it.

@AndreiEres
Copy link

I'm working on the class that's responsible for this thing. So further I can implement that. @koic could you assign this issue to me?

@bbatsov
Copy link
Collaborator

bbatsov commented Mar 14, 2021

@AndreiEres Done.

@AndreiEres
Copy link

#9630

  • Added department checking to Registry
  • Added list of names for department to Registry

@AndreiEres
Copy link

I think it can be done with #9626

  • Added checking for department only to directive comment.
  • Added note about new feature to documentation.

@bbatsov or other, could you take a look?

@AndreiEres
Copy link

Guys, I discovered that disabling of all department has consequences. How we can deal with redundant department disabling? I realised that the best way — do not use RedundantCopDisabling, but create another cop — RedundantDepartmentDisabling. So we will ignore department disabling in RedundantCop processing and check it only for RedundantDepartment.

What do you think?

@dvandersluis
Copy link
Member

Don't they have to work together? As I mentioned in #9563, if a department is disabled, disabling a specific cop below it is redundant.

# rubocop:disable Metrics
...
# rubocop:disable Metrics/Foo <-- redundant

@AndreiEres
Copy link

Oh, it's true. If we split that cops we can't catch that interference. Hard thing :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants