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

Rails/CreateTableWithTimestamps gets disabled when disabling other cops in .rubocop.yml that's in a subfolder #8240

Open
unikitty37 opened this issue Jul 5, 2020 · 3 comments
Labels
enhancement stale Issues that haven't been active in a while

Comments

@unikitty37
Copy link
Contributor

unikitty37 commented Jul 5, 2020

I am using a .rubocop.yml in the db/migrate folder to add specific rules for migrations (mainly disabling Metrics/MethodLength and Style/Documentation). It inherits from the project's root .rubocop.yml.

A couple of migrations had timestamps missing, and made it through CI to production. Specifically enabling Rails/CreateTableWithTimestamps in the migration-specific config file did nothing.

Experimentation showed that disabling any other cop in the migration-specific config file also disabled this cop — if the only things happening in the non-root config file were enabling cops or changing their settings, the missing timestamps were detected.

The same behaviour occurs if Rails/CreateTableWithTimestamps is commented out in db/migrate/.rubocop.yml on the grounds that it defaults to being enabled anyway.


Expected behaviour

Disabling other cops in a non-root .rubocop.yml that inherits from the root does not disable Rails/CreateTableWithTimestamps.

Actual behaviour

Rails/CreateTableWithTimestamps is only enabled in db/migrate if no other cops have Enabled: false set on them.

With debug output enabled, the output is as follows:

With no other cops disabled:

└─▷ bundle exec rubocop --debug --only Rails/CreateTableWithTimestamps db/migrate/*.rb
For /private/tmp/rubocoptest: configuration from /private/tmp/rubocoptest/.rubocop.yml
configuration from /Users/unikitty37/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-rails-2.6.0/config/default.yml
configuration from /Users/unikitty37/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-rails-2.6.0/config/default.yml
Default configuration from /Users/unikitty37/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-0.86.0/config/default.yml
Inspecting 1 file
Scanning /private/tmp/rubocoptest/db/migrate/20200705130640_create_pages.rb
For /private/tmp/rubocoptest/db/migrate: configuration from /private/tmp/rubocoptest/db/migrate/.rubocop.yml
Inheriting configuration from /private/tmp/rubocoptest/.rubocop.yml
AllCops/Exclude configuration from /private/tmp/rubocoptest/.rubocop.yml
C

Offenses:

db/migrate/20200705130640_create_pages.rb:3:5: C: Rails/CreateTableWithTimestamps: Add timestamps when creating a new table.
    create_table :pages do |t| ...
    ^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
Finished in 0.4792379999998957 seconds

With other cops disabled:

└─▷ bundle exec rubocop --debug --only Rails/CreateTableWithTimestamps db/migrate/*.rb
For /private/tmp/rubocoptest: configuration from /private/tmp/rubocoptest/.rubocop.yml
configuration from /Users/unikitty37/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-rails-2.6.0/config/default.yml
configuration from /Users/unikitty37/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-rails-2.6.0/config/default.yml
Default configuration from /Users/unikitty37/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-0.86.0/config/default.yml
Inspecting 1 file
Scanning /private/tmp/rubocoptest/db/migrate/20200705130640_create_pages.rb
For /private/tmp/rubocoptest/db/migrate: configuration from /private/tmp/rubocoptest/db/migrate/.rubocop.yml
Inheriting configuration from /private/tmp/rubocoptest/.rubocop.yml
AllCops/Exclude configuration from /private/tmp/rubocoptest/.rubocop.yml
.

1 file inspected, no offenses detected
Finished in 0.4813480000011623 seconds

Steps to reproduce the problem

  1. Clone https://github.com/unikitty37/rubocop-issue.
  2. Run bundle exec rubocop --only Rails/CreateTableWithTimestamps db/migrate/*.rb.
  3. Observe that the missing timestamps are detected.
  4. In db/migrate/.rubocop.yml, uncomment either the block disabling Metrics/MethodLength or Style/Documentation.
  5. Run bundle exec rubocop --only Rails/CreateTableWithTimestamps db/migrate/*.rb.
  6. Observe that the missing timestamps are no longer detected.

RuboCop version

Include the output of rubocop -V or bundle exec rubocop -V if using Bundler. Here's an example:

$ [bundle exec] rubocop -V
0.86.0 (using Parser 2.7.1.4, rubocop-ast 0.1.0, running on ruby 2.6.6 x86_64-darwin18)
@deivid-rodriguez
Copy link
Contributor

Hi @unikitty37!

I'm not sure of the specific reason why the cop gets run or not depending on whether you comment or uncomment other cops, but the reason for the cop being ignored is that the Include patterns are matched relatively to the configuration file.

So, in your case, the default Include pattern for this cop (db/migrate/*.rb) is getting matched against the relative path to the file from the configuration file enabling the cop. Since the configuration file lives in db/migrate/.rubocop.yml, it's getting matched against 20200705130640_create_pages.rb. And since db/migrate/*.rb doesn't match 20200705130640_create_pages.rb, the cop is ignored as not relevant.

This behaviour feels quite weird to me, since it forces you to override all default inclusion and exclusion patterns of all cops that you enable in a file that's not the root folder of your project.

In any case, you should be able to fix this, by either removing the nested configuration and move everything to the root of your project, or by overriding the inclusion pattern inside db/migrate/.rubocop.yml like this:

Rails/CreateTableWithTimestamps:
  Enabled: true
  Include:
    - "*.rb"

@unikitty37
Copy link
Contributor Author

Thanks! That's fixed it.

I've added a PR to hopefully make the documentation a bit clearer on this.

@stale
Copy link

stale bot commented Jan 3, 2021

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 the stale Issues that haven't been active in a while label Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement stale Issues that haven't been active in a while
Projects
None yet
Development

No branches or pull requests

3 participants