Rubocop: Introduce new cops with a special status (instead of enabled/disabled)

Created on 9 Jun 2018  路  13Comments  路  Source: rubocop-hq/rubocop

We have to add a way to mark recently added cops as new and have the users decide themselves whether they want to enable them or not.

I think that we can have Enabled feature a new value (e.g. New or nil) and cops marked this way will just produce a message on RuboCop start saying something like:

The following cops were added and you need to decided whether to enable or disable them.

This is going to solve the commonly reported problem that RuboCop updates are very painful for end users as usually they break their builds due to new cops being added. This would allow people go just bugfixes without having to mess much with the config. Going forward we'll take a page from ESLint and enable/disable cops upstream only in major releases.

This ticket is somewhat related to #5978, as we'd use the version data in the message we print to the users.

enhancement high priority

Most helpful comment

@mikegee We can infer the cop status, but we still have to set it to something different than enable or disabled IMO. I don't like the idea of having cops set as disabled by default, because than you can't differentiate if you disabled something on purpose or it's disabled simply because it's new. What exactly is going to be the Enabled state for cops introduced in 1.1, 1.2 and so on if we don't add something for this?

All 13 comments

This seems a bit duplicative with the VersionAdded and VersionChanged metadata. Do you think we can infer "newness" from versions?

@mikegee We can infer the cop status, but we still have to set it to something different than enable or disabled IMO. I don't like the idea of having cops set as disabled by default, because than you can't differentiate if you disabled something on purpose or it's disabled simply because it's new. What exactly is going to be the Enabled state for cops introduced in 1.1, 1.2 and so on if we don't add something for this?

I hadn鈥檛 thought it through as much as you. That makes sense. 馃憤

I think we can have something like 'LockVersion' in config - and then cops from a newer non-permitted versions can switch into non-error mode, or even just become disabled and only produce a warning that new cops were added and user should consider raising locked version.

@Vasfed Perhaps. Generally all of this is trivial once someone has the down the ground work in #5978.

@Darhazer I guess it makes sense for you to reap the benefits of your work on the huge metadata tasks, so I'm assigning this to you.

@Darhazer would you be able to tackle this anytime soon or should we find someone else from @rubocop-hq/rubocop-core to work on it? It's one of the most important tasks on the way to RuboCop 1.0 and since it's not a big task it'd be nice if we wrapped it up in the next couple of releases.

Sure. I'll do it in the next couple of weeks.
What about the wording? Something along the lines of

The following cops were added to RuboCop, but not configured. Please set Enabled to either `true` or `false` in your `.rubocop.yml` config.

Also, since it would require copy/pasting cop names to the .rubocop.yml, perhaps a rake task to enable/disable a cop /all non-configured cops/ would be useful?

@Darhazer The wording sounds great.

Also, since it would require copy/pasting cop names to the .rubocop.yml, perhaps a rake task to enable/disable a cop /all non-configured cops/ would be useful?

Great idea! This might also be some command-line option, but I rake task would be enough.

@rubocop-hq/rubocop-core I won't be able to complete this at least two more weeks, so if someone wants to take it, feel free to do so

Elaborating on @Vasfed's idea.

Imagine an acknowledged_version setting:

require:
  - rubocop/cop/internal_affairs
  - rubocop-performance
  - rubocop-rspec

acknowledged_version:
  rubocop: 0.72.0
  rubocop-performance: 0.60
  rubocop-rspec: 1.34.1

Imagine we set Enabled: true for all cops.
When the user updates RuboCop or extension version, when we run rubocop, we have information about the original version and the destination version.
Thus we know the "diff", e.g. which new cops were introduced between those two versions. E.g.:

There are some newly added cops that are not acknowledged.
The following cops were added to RuboCop.
Run `rubocop --auto-gen-config` to update your `.rubocop_todo.yml`, and optionally disable them completely if they don't match your project's established style.

RuboCop: 0.72.0 -> 0.74.0
  Style/MultilineWhenThen
RuboCop/Performance: 0.60.0 -> 1.4.1
  Performance/OpenStruct 
RuboCop/RSpec: 1.34.1 -> 1.35.0
  RSpec/ImplicitBlockExpectation http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ImplicitBlockExpectation

Benefits:

  1. no need to replicate the whole list of cops in their configs, only disable the cops.
  2. less effort, it's possible to rubocop --auto-gen-config to disable the new cops that have offences. Users might decide to disable them completely or leave in todo.

@bbatsov @Darhazer @mikegee WDYT?

Please disregard the above comment, I've completely missed the discussion in the related pull request and this comment.

In Cookstyle, which is a wrapper for RuboCop, we've worked around this issue by setting the default fail level to convention and introducing all our new cops at refactor level. That way we can introduce new cops that will show up on the CLI and autocorrect, but won't fail CI builds. That allows our users to always run the latest version of our tool without constantly spending time updating their codebases when new versions are released.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

benoittgt picture benoittgt  路  3Comments

cabello picture cabello  路  3Comments

david942j picture david942j  路  3Comments

Aqualon picture Aqualon  路  3Comments

bquorning picture bquorning  路  3Comments