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

Cops defined in gems can conflict even when they are in different namespaces #8044

Closed
haines opened this issue May 27, 2020 · 2 comments · Fixed by #8490
Closed

Cops defined in gems can conflict even when they are in different namespaces #8044

haines opened this issue May 27, 2020 · 2 comments · Fixed by #8490

Comments

@haines
Copy link

haines commented May 27, 2020

Raising this issue as discussed here: rubocop/rubocop-rspec#834 (comment)

rubocop-rails provides RuboCop::Cop::Rails::HttpStatus, and rubocop-rspec provides RuboCop::Cop::RSpec::Rails::HttpStatus.

Both cops get the same badge, Rails/HttpStatus, and this means that only one cop works (from whichever gem is required last).

Related: rubocop/rubocop-rspec#611

First reported: #5738 (comment)


Expected behavior

Both cops report offenses.

Actual behavior

Only one cop reports offenses (from whichever gem is required last).

Steps to reproduce the problem

https://gist.github.com/haines/369ac2298ce6fe042865def931e8448b

RuboCop version

0.84.0 (using Parser 2.7.1.3, rubocop-ast 0.0.3, running on ruby 2.7.1 x86_64-darwin19)
@pirj
Copy link
Member

pirj commented May 27, 2020

Kind of related #5251, but not exactly the same problem.

@dmytro-savochkin
Copy link

It looks like a pretty deep problem.

So, this happens due to the fact that RuboCop::Cop::Badge class is built in such a way that it defines a department for a cop by its second to last part of the full class name.
So, RuboCop::Cop::RSpec::Rails::HttpStatus becomes HttpStatus cop with Rails department and RuboCop::Cop::Rails::HttpStatus also becomes HttpStatus cop with the same Rails department.

It seems the code in Badge class can be changed easily enough so that departments are actually more like full_class_name.split('::')[2...-1].join('/') instead of the current full_class_name.split('::')[-2]. After such a change it would be required to ensure that all the dependent gems have updated their settings in .rubocop.yml files. For example, rubocop-rspec gem would need to rename Rails/HttpStatus to RSpec/Rails/HttpStatus. This would need to happen at the same time with rubocop gem itself because rubocop references rubocop-rspec the same way as rubocop-rspec references rubocop.

Another alternative I can think of is to prohibit cops from such deep nesting making sure that there are only 4 levels in their full name.

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

Successfully merging a pull request may close this issue.

3 participants