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

auto-gen-conf breaks default exclude in Rails/ApplicationRecord cop #1381

Closed
Aqualon opened this issue Oct 23, 2024 · 3 comments
Closed

auto-gen-conf breaks default exclude in Rails/ApplicationRecord cop #1381

Aqualon opened this issue Oct 23, 2024 · 3 comments

Comments

@Aqualon
Copy link

Aqualon commented Oct 23, 2024

Not sure if this is a bug in the Rails/ApplicationRecord cop or in rubocop itself


Expected behavior

Default exclude from the cop should always be considered

Actual behavior

I get complaints about

Rails/ApplicationRecord: Models should subclass ApplicationRecord.

in some spec files. After bundle exec rubocop --auto-gen-conf I got the same complaints about files in db/migrate/ directory, that should be excluded since #1349.

Generate config in .rubocop_todo.yml

# Offense count: 5
# This cop supports unsafe autocorrection (--autocorrect-all).
Rails/ApplicationRecord:
  Exclude:
    - 'spec/feature_helper.rb'
    - 'spec/models/concerns/paperclip_image_spec.rb'
    - 'spec/models/concerns/stat_counters_spec.rb'

Steps to reproduce the problem

  • Create a db migration file with inheritance from ActiveRecord::Base (see Rails/ApplicationModel should ignore db/migrate/** #1342 for example)
  • Run rubocop and get no complaints about the file
  • Add config for Rails/ApplicationRecord excluding any file you want
  • Rerun rubocop and get complaint about the migration file

RuboCop version

❯ bundle exec rubocop -V
1.67.0 (using Parser 3.3.5.0, rubocop-ast 1.32.3, analyzing as Ruby 3.3, running on ruby 3.3.5) [arm64-darwin23]
  - rubocop-capybara 2.21.0
  - rubocop-factory_bot 2.26.1
  - rubocop-performance 1.22.1
  - rubocop-rails 2.26.1
  - rubocop-rspec 3.1.0
@dvandersluis
Copy link
Member

I don’t think this is specific to any cop. When multiple configuration files are loaded, by default keys present in both are overridden.

You can change this behavior by specifying inherit_mode: https://docs.rubocop.org/rubocop/configuration.html#merging-arrays-using-inherit_mode

# in .rubocop.yml

inherit_mode:
  merge:
    - Exclude

@Aqualon
Copy link
Author

Aqualon commented Oct 23, 2024

Thanks, seems this was supposed to be addressed in rubocop/rubocop#9624 to make it less confusing, but got nowhere.

@Aqualon Aqualon closed this as completed Oct 23, 2024
@Earlopain
Copy link
Contributor

It's pretty unfortunate, but the behaviour can only be changed in a major version bump of rubocop.

I do believe there is a bug here however. When generating a todo, it's supposed to warn you about this. https://github.com/rubocop/rubocop/blob/75160946ac3620005874d5188101dab77d585be2/lib/rubocop/formatter/disabled_config_formatter.rb#L229-L248

But because rubocop-rails own config already specifies a inherit_mode, it falls through. I don't know what the appropriate fix would be to show this warning, if even possible.

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

No branches or pull requests

3 participants