Rubocop: Extend cop metadata

[bbatsov]

Each cop should gain the following additional metadata:

• VersionAdded - the RuboCop version in which it was added
• VersionChanged - the RuboCop version in which it was changed in a user-impacting way (new config, updated defaults, etc)
• VersionRemoved - the RuboCop version in which the cop was removed for whatever reason.

Those will be pretty useful for the documentation (so the manual generation has to be enhanced to include them) and keeping track of changes. Adding the fields is easy - the real work is going to be populating this metadata for existing cops (I guess this will happen gradually).

[bbatsov]

Oh, I forgot the most important metadata.

• Safe (true/false) - indicates whether the cop can yield false positives (by design) or not.
• SafeAutoCorrect (true/false) - indicates whether the auto-correct the cop does is safe (equivalent) by design.

We'll use those two pieces of metadata to add options to run only safe cops or only safe auto-corrections. Again the real work here is just going through all the cops to fill the data in. Guess we can just assume that by default everything is safe unless marked otherwise to reduce the amount of work needed here.

[To1ne]

This one sounds interesting, and doable.

Also, it shouldn’t be that hard to write a script to find theVersionAdded.

How would so consider handling cases where you need multiple VersionChanged?

[bbatsov]

I think in practice we need just the last one.

[bbatsov]

@rubocop-hq/rubocop-core @Darhazer Our volunteer for this just pulled out of it and it's probably the most important ticket we have to resolve in the direction of RuboCop 1.0 (as it blocks everything related to using this metadata). Would one of you like to tackle this? The task is pretty simple and you don't really need to dig through all the versions - it'd be fine if those are filled just for the last release and fallback to nil until someone can do a bit of archeology.

[bbatsov]

Same with the Safe data - we just need to put the support for it, fallback to "everything's safe" and fill in the data as we go.

And update the manual generation script, of course.

[Darhazer]

I'll take on this

[bbatsov]

Thanks, Max! Much appreciated!

[Darhazer]

I've done a bit of archeology and have some questions:

• Not all cops are listed in default.yml - it inherits from enabled.yml and disabled.yml, where all cops are listed. So either all should be moved in the default (and enabled and disabled would be somehow obsolete), or the metadata can be added to those base configs instead. WDYT?
• I guess adding or removing autocorrect counts towards VersionChanged.
• What about renaming cops and or/moving them to other departments? Is that a VersionChanged or VersionRemoved for the old name and VersionAdded for the new one?
• Same goes for splitting cops in two - in that case, I guess it's really VersionRemoved for the original one and VersionAdded for the 2 new cops.
• I guess changing enabled by default (both for the cop or its autocorrect) counts towards VersionChanged.

[Drenmi]

Not all cops are listed in default.yml - it inherits from enabled.yml and disabled.yml, where all cops are listed.

It only contains cops that have custom configuration options.

So either all should be moved in the default (and enabled and disabled would be somehow obsolete), or the metadata can be added to those base configs instead. WDYT?

Very much in favour. The split configuration was confusing to me at first, and keeps annoying me on a regular basis. 😬

[bbatsov]

Not all cops are listed in default.yml - it inherits from enabled.yml and disabled.yml, where all cops are listed. So either all should be moved in the default (and enabled and disabled would be somehow obsolete), or the metadata can be added to those base configs instead. WDYT?

All should be moved to default. The enabled/disabled files are just remnants of the very first days of the project where cops didn't have any config/metadata and those files were the way to figure out if some cop should be enabled or not. We should have removed them a very long time ago...

I guess adding or removing autocorrect counts towards VersionChanged.

Yes. Any config changes count towards it.

What about renaming cops and or/moving them to other departments? Is that a VersionChanged or VersionRemoved for the old name and VersionAdded for the new one?

If something was truly renamed (like we've done in the past) it should be just VersionAdded for the renamed cop. After the metadata is introduced it will be VersionRemoved for the old cop and VersionAdded for the new one.

Same goes for splitting cops in two - in that case, I guess it's really VersionRemoved for the original one and VersionAdded for the 2 new cops.

Yes.

I guess changing enabled by default (both for the cop or its autocorrect) counts towards VersionChanged.

Yes.

[Drenmi]

Amazing work @Darhazer! 🙇 There's another small remnant still there, I think. We used to have a few SafeMode flags, which probably were a failed attempt at introducing the Safe flag. Perhaps we should replace instances of this with the new flag?

[ilyakatz]

I just tried the latest default config with the latest version of rubocop, and I'm getting a bunch of warnings

Warning: unrecognized parameter Bundler/DuplicatedGem:VersionAdded found in .rubocop.yml

% rubocop -v                                                                                                                                                                      
0.59.2

Any ideas on why this is happening?

[bbatsov]

@ilyakatz Because the new metadata as added after 0.59.2. :-) There's no released version that supports it yet - 0.60 will be the first one when we cut it.

[ilyakatz]

ha, got it, i jumped them gun. will wait for it to come out. thanks