Skip to content

Cop from RuboCop takes precedence over cop with the same name (but different department) from extension and the require is done in an inherited file/gem #5251

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

Closed
rymai opened this issue Dec 14, 2017 · 6 comments · Fixed by #11216
Labels

Comments

@rymai
Copy link

rymai commented Dec 14, 2017

First of all, thanks for this awesome project!

I've encountered a bug where we're using a gem to define common rules for multiple projects and the require is done in the gem.

Then in a project that uses this gem, I wanted to disable the RSpec/FilePath cop.

That wasn't working until I added

require:
  - rubocop-rspec

in the .rubocop.yml of my project.

I've set up a reproduction repo here: https://github.com/rymai/reproduce-rubocop-bug


Expected behavior

  1. When you inherit a config from a file or gem,
  2. and this config includes a require,
  3. and the extension defines a cop with the same name (but different department) as a cop from RuboCop itself (e.g. FilePath, rubocop defines Rails/FilePath and rubocop-rspec defines RSpec/FilePath)
  4. Then any rule that acts on the extension cop should be taken in account.

Actual behavior

The rules that act on the extension cop isn't taken in account.

Steps to reproduce the problem

See https://github.com/rymai/reproduce-rubocop-bug.

  1. git clone https://github.com/rymai/reproduce-rubocop-bug.git
  2. cd reproduce-rubocop-bug
  3. bundle install
  4. bundle exec rubocop -c .rubocop-inherit-from-gem.yml --show-cops RSpec/FilePath or bundle exec rubocop -c .rubocop-inherit-from-file.yml --show-cops RSpec/FilePath
  5. You'll see Enabled: true instead of Enabled: false

Work-around

You can work-around this bug by re-requiring the extension in the final RuboCop config file.

RuboCop version

$ rubocop -V
0.52.0 (using Parser 2.4.0.2, running on ruby 2.3.5 x86_64-darwin16)

also, rubocop-rspec (1.21.0).

@bbatsov bbatsov added the bug label Sep 23, 2018
@bbatsov
Copy link
Collaborator

bbatsov commented Sep 23, 2018

@bquorning @Darhazer Can you confirm this?

@stale
Copy link

stale bot commented May 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added stale Issues that haven't been active in a while and removed stale Issues that haven't been active in a while labels May 8, 2019
@rymai
Copy link
Author

rymai commented May 9, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

After upgrading rubocop and rubocop-rspec to 0.68.1 and `1.32.0 respectively, it seems that the bug is still occurring:

› bundle exec rubocop -c .rubocop-inherit-from-file.yml --show-cops RSpec/FilePath
.rubocop-inherit-from-file.yml: RSpec/FilePath has the wrong namespace - should be Rails
RSpec/FilePath:
  Description: Checks that spec file paths are consistent with the test subject.
  Enabled: true
  CustomTransform:
    RuboCop: rubocop
    RSpec: rspec
  IgnoreMethods: false
  StyleGuide: http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FilePath

and

› bundle exec rubocop -c .rubocop-inherit-from-gem.yml --show-cops RSpec/FilePath 
.rubocop-inherit-from-gem.yml: RSpec/FilePath has the wrong namespace - should be Rails
RSpec/FilePath:
  Description: Checks that spec file paths are consistent with the test subject.
  Enabled: true
  CustomTransform:
    RuboCop: rubocop
    RSpec: rspec
  IgnoreMethods: false
  StyleGuide: http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FilePath

@bkuhlmann
Copy link

bkuhlmann commented May 31, 2022

Hello. 👋 I'm seeing this issue as well. Here's the contents of my .rubocop.yml:

Configuration
inherit_gem:
  caliber: config/all.yml

require:
  - rubocop-rails

RSpec/FilePath:
  CustomTransform:
    URLs: urls

If you are curious how the Caliber gem works, here is the configuration being used and inherited from the gem. Interestingly, when I add this statement to the configuration (as shown above):

require:
  - rubocop-rails

...and then run rubocop on my project, I end up with the following errors:

Error Output
.rubocop.yml: RSpec/FilePath has the wrong namespace - should be Rails
Warning: Rails/FilePath does not support CustomTransform parameter.

Supported parameters are:

  - Enabled
  - EnforcedStyle
  - SupportedStyles
  
Offenses:

spec/lib/okta/urls/login_spec.rb:6:1: C: RSpec/FilePath: Spec path should end with okta/ur_ls/login*_spec.rb. (https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FilePath)
RSpec.describe Okta::URLs::Login do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/lib/okta/urls/logout_spec.rb:6:1: C: RSpec/FilePath: Spec path should end with okta/ur_ls/logout*_spec.rb. (https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FilePath)
RSpec.describe Okta::URLs::Logout do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  

So when adding the rubocop-rails requirement, this voids the RuboCop RSpec configuration and make it look like RSpec/FilePath is no longer legit. When using rubocop --debug, the output gets more interesting (notice the conflict with the RuboCop Rails gem):

Debug Details
For /Users/bkuhlmann/Engineering/Companies/<redacted>/auth: configuration from /Users/bkuhlmann/Engineering/Companies/<redacted>/auth/.rubocop.yml
configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-rails-2.14.2/config/default.yml
configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-rails-2.14.2/config/default.yml
Default configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.30.0/config/default.yml
.rubocop.yml: RSpec/FilePath has the wrong namespace - should be Rails
Inheriting configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/caliber-0.9.0/config/all.yml
Inheriting configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/caliber-0.9.0/config/ruby.yml
Inheriting configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/caliber-0.9.0/config/rake.yml
configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-rake-0.6.0/config/default.yml
configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-rake-0.6.0/config/default.yml
Inheriting configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/caliber-0.9.0/config/performance.yml
configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-performance-1.14.0/config/default.yml
configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-performance-1.14.0/config/default.yml
Inheriting configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/caliber-0.9.0/config/rspec.yml
configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-rspec-2.11.1/config/default.yml
configuration from /Users/bkuhlmann/.cache/frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-rspec-2.11.1/config/default.yml
Warning: Rails/FilePath does not support CustomTransform parameter.

Here's my environment when running rubocop -V on my project:

1.30.0 (using Parser 3.1.2.0, rubocop-ast 1.18.0, running on ruby 3.1.2 arm64-darwin21.4.0)
  - rubocop-performance 1.14.0
  - rubocop-rails 2.14.2
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.11.1

Workaround

After studying Rémy's bug reproduction -- as stated in this issue -- I was able to use this configuration as a workaround where I have to re-require all gem dependencies:

Workaround Solution
# You have to re-require all dependencies even though the Caliber gem does this for you.
require:
  - rubocop-performance
  - rubocop-rake
  - rubocop-rails
  - rubocop-rspec

inherit_gem:
  caliber: config/all.yml

RSpec/FilePath:
  CustomTransform:
    URLs: urls

@bquorning bquorning removed their assignment May 31, 2022
@NobodysNightmare
Copy link
Contributor

Just stumbled upon this one. I was not aware of the workaround using an additional require in .rubocop.yml directly, so that helped already.

In case this helps (also debugging): Another workaround was to have the configuration inside .rubocop_todo.yml, where it would also be correctly recognized.

Can confirm that this is still the case for rubocop 1.39.0.

@NobodysNightmare
Copy link
Contributor

Attempting a fix in #11216. Hope this does not break anything else 🙈

NobodysNightmare added a commit to NobodysNightmare/rubocop that referenced this issue Dec 1, 2022
Ingredients for the edge-case:
* .rubocop.yml inherits another config (inherit_from/inherit_gem)
* .other config requires an additional gem (e.g. rubocop-rspec)
* .rubocop.yml configures a cop defined in additional gem (e.g. RSpec/VariableName)
  whose name already exists in rubocop (e.g. Naming/VariableName)

What would have happened before is that during the call of add_missing_namespaces,
the registry would have replaced the configuration of the RSpec/VariableName with
Naming/VariableName, because that's the only cop that it knew at that point.

By changing the call order we make sure that requires of external files are
resolved before letting the registry replace badge names.
bbatsov pushed a commit that referenced this issue Dec 5, 2022
Ingredients for the edge-case:
* .rubocop.yml inherits another config (inherit_from/inherit_gem)
* .other config requires an additional gem (e.g. rubocop-rspec)
* .rubocop.yml configures a cop defined in additional gem (e.g. RSpec/VariableName)
  whose name already exists in rubocop (e.g. Naming/VariableName)

What would have happened before is that during the call of add_missing_namespaces,
the registry would have replaced the configuration of the RSpec/VariableName with
Naming/VariableName, because that's the only cop that it knew at that point.

By changing the call order we make sure that requires of external files are
resolved before letting the registry replace badge names.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants