Rubocop: How to Ignore Code at all costs

[schneems]

This needs a bit of a backstory:

I am a maintainer of the Heroku Ruby Buildpack.

We recently started supporting CI runs. To do this one thing we have to do is support running tasks like rake db:schema:create, to do this we must "clear" out tasks such as db:drop that we cannot perform on the platform. The way we've decided to do this is to "inject" code into customer's lib/tasks/ directory.

if Rake::Task.task_defined?('db:drop')
  Rake::Task['db:drop'].clear
  task 'db:drop' do
  end
end

As you can imagine this started throwing rubocop errors on people's CI due to being configured to single quoted strings. We did some digging and found we could disable rubocop errors by wrapping it:

# rubocop:disable all"
...
# rubocop:enable all

But here's the problem: It seems on some versions of Rubocop (maybe this is a new feature) but disabling failures apparently seems to actually __cause__ a failure if there were no cop failures in the block:

Offenses:
lib/tasks/heroku_clear_tasks.rake:1:1: W: Lint/UnneededDisable: Unnecessary disabling of all cops. 
# rubocop:disable all

They are using version 0.49.1.

This seems a bit counter-intuitive. I disabled ALL of rubocop (or at least thought i did, so I would guarantee no failures, but this in turn caused a failure).

Question is this: How can I disable all failures from rubocop in a section of code without also triggering a failure that i've disabled rubocop?

[danielpclark]

You can try appending both an exclude and include overwrite to .rubocops.yml as the quick and dirty way.

"AllCops:\n  Exclude:\n    - '**/*'"

This will overwrite all existing excludes from the YAML file.

[schneems]

Won't that stop all cops from running?

I still want cops to run for customer code, if they've violated their own style guides and have enabled rubocop presumably they want their tests to notify them.

A co-worker @maxbeizer tried writing a .rubocops.yml just for that directory, but it caused other failures. Not totally sure, but we think that it wiped out top level config instead of only applying to that directory.

[backus]

@schneems if you know what cops are going to produce offenses then you could check if they have --auto-correct enabled and then just run that on the code you generate

[danielpclark]

Won't that stop all cops from running?

Yes. It rewrites them, you'd have to do some fancy search and insert if you wanted to add just the new task file.

Have you tried:

# rubocop:disable Lint/UnneededDisable
# rubocop:disable all"
...
# rubocop:enable all

[schneems]

Have you tried:

I have not, but I will. Thanks for the help!

[maxbeizer]

A co-worker @maxbeizer tried writing a .rubocops.yml just for that directory, but it caused other failures. Not totally sure, but we think that it wiped out top level config instead of only applying to that directory.

I tried working with Rubocop's inheritance functionality since presumably @schneems could just write a file and exclude the file he's trying to disable cops for (instead of trying to write to the top-level .rubocop.yml). Adding the file I want to exclude to AllCops > Exclude in a more localized .rubocop.yml did not work as expected for me.

I created this repo to reproduce the issue as I see it--basing my code off of the manual on this subject. Am I doing it wrong?

[bbatsov]

But here's the problem: It seems on some versions of Rubocop (maybe this is a new feature) but disabling failures apparently seems to actually cause a failure if there were no cop failures in the block:

Well, the idea was to show people when they added some disables, which are not really necessary. Obviously the easiest way to get this of this behavior is to disable the cop that actually does those checks.

[jonas054]

@maxbeizer The manual says:

Note: The Include and Exclude parameters are special. They are valid for the directory tree starting where they are defined. They are not shadowed by the setting of Include and Exclude in other .rubocop.yml files in subdirectories. This is different from all other parameters, who follow RuboCop's general principle that configuration for an inspected file is taken from the nearest .rubocop.yml, searching upwards.

It could perhaps be stated more clearly, but the meaning is that Include/Exclude settings are taken from the top level (usually project root) .rubocop.yml file. This feature was designed with vendor directories in mind. When a project wants to exclude -- or not exlcude -- a subdirectory, the subdirectory doesn't have a say in what gets excluded.

[maxbeizer]

@jonas054 that makes sense and thanks for your reply. ❤️

I'm not sure if @schneems is still having this issue or not, so I'll leave it to him to follow up as the original issue is a bit different from my question.

[jaredbeck]

If Bozhidar's suggestion works ...

.. the easiest way to get this of this behavior is to disable the cop that actually does those checks.

.. (ie. disable Lint/UnneededDisable) then I think we can close this issue.

[benkimball]

It looks like #4938 may have provided the support that @schneems needed. Is this issue still a problem?

[veganstraightedge]

I'm seeing this same error when Heroku runs our rubocop in Heroku CI.

@schneems Has this been resolved in Heroku Land?

[stale[bot]]

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[bot]]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

[schneems]

We need to re-open this issue. Adding this extra directive:

# rubocop:disable Lint/UnneededDisable

Causes additional failures as reported in https://github.com/heroku/heroku-buildpack-ruby/commit/1dc39bb0ca86bb5e3dd3287609e471f639952f95#commitcomment-35070260

[bbatsov]

Done.

[schneems]

Thanks! Copying the text of the failure mode from a comment in that thread

---

The problem we were trying to solve with this code is different projects will have rules like "must use double-quoted strings" or "must use single-quoted strings" so we added the rubocop:disable all magic comment to let rubocop know we don't care about this code. Unfortunately, for some reason, that magic comment itself triggers failures of Lint/UnneededDisable i guess for the cases where there were no failures, so we're trying to disable that cop too.

My best guess is that for @pixeltrix's app rubocop:disable all worked correctly and actually disabled all cops so Style/FrozenStringLiteralComment would not fire, however when we added the extra Lint/UnneededDisable it did disable the unneeded cop, however the disable all directive wasn't until the next line so therefore rubocop ran it's cop that evaluates the first line which was checking for the string literal pragma.

The crux of the issue is...I still don't know how to generate code into an arbitrary codebase with arbitrary rules and tell rubocop to not care about that generated code since it's orthogonal to the actual project and is needed to get CI to run in the first place.

---

I'm still trying to get information about this failure mode such as what version of rubocop they were using, it may be that #4938 might have been successful and they're on an older version of Rubocop. If that's the case then I don't need to add the extra Lint/UnneededDisable comment and we can instead warn when we see an older version of rubocop. If they're using a recent version of rubocop then the issue would like in the implementation.

[jonas054]

that magic comment itself triggers failures of Lint/UnneededDisable

The cop name was changed to Lint/UnneededCopDisableDirective in Rubocop 0.53.0, so if you see offense reports from Lint/UnneededDisable that's from an older version.

In the current version you only need # rubocop:disable all to disable all cops, including Lint/UnneededCopDisableDirective.

[abodelot]

Hi,

I'm using Heroku CI and I ran into this issue after adding the following rules to .rubocop.yml:

Style/FrozenStringLiteralComment:
  Enabled: false

Style/MutableConstant:
  Enabled: false

The CI job failed with the following message:

Offenses:
lib/tasks/heroku_clear_tasks.rake:1:1: W: Lint/UnneededCopDisableDirective: Unnecessary disabling of all cops.
# rubocop:disable all
^^^^^^^^^^^^^^^^^^^^^
1857 files inspected, 1 offense detected
rake aborted!

The CI was running successfully until I added (or rather, disabled) the 2 rules above.

To fix my issue with heroku-ci, I also added the following:

Lint/UnneededCopDisableDirective:
  Enabled: false

This works, but I'm unpleased with this solution. I only want to disable Style/FrozenStringLiteralComment and Style/MutableConstant.

Is there a better fix available?

Thanks!

[pixeltrix]

@abodelot I added lib/tasks/heroku_clear_tasks.rake to the list of ignored files in my .rubocop.yml file.

[schneems]

Would it be reasonable to inject an ignore in .rubocop.yml from the Heroku buildpack?

I’m also open to other suggestions for how to disable all cops in a way that doesn’t trigger other cops.

[pixeltrix]

Would it be reasonable to inject an ignore in .rubocop.yml from the Heroku buildpack?

I’m also open to other suggestions for how to disable all cops in a way that doesn’t trigger other cops.

For a Heroku generated file? Totally reasonable.

[abodelot]

Thank your @pixeltrix, this is a much better solution.

[stale[bot]]

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!

[jonas054]

I don't think there are any unresolved problems remaining, so I'm closing the issue.