[Fix #5979] Introduce cops with special status #7567

pirj · 2019-12-12T17:00:23Z

New cops, introduced or significantly changed between major versions, are introduced with pending status. This is done to avoid frustration over numerous offences during updates between minor versions. RuboCop will emit a warning that new cops were introduced, and suggest to explicitly enable or disable them in the user configuration file.

Based on #7184

Fixes #5979

Before submitting the PR make sure the following are checked:

Wrote good commit messages.
Commit message starts with [Fix #issue-number] (if the related issue exists).
Feature branch is up-to-date with master (if not - rebase it).
Squashed related commits together.
Added tests.
Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

pirj · 2019-12-12T17:10:29Z

.rubocop_todo.yml

 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 183
+  Max: 197


Sorry about that, but it feels that I might get into a refactoring spiral if I touch ConfigLoader.

pirj · 2019-12-12T17:11:42Z

lib/rubocop/config_loader.rb

@@ -91,7 +91,9 @@ def configuration_from_file(config_file)
        else
          add_excludes_from_files(config, config_file)
        end
-        merge_with_default(config, config_file)
+        merge_with_default(config, config_file).tap do |merged_config|


merge_with_default does not mutate first argument, but rather returns merged config, so I was between tap and using a temporary local variable.

bbatsov · 2019-12-13T06:24:26Z

What I was not 100% certain of the desired effect of EnabledByDefault/DisabledByDefault on new pending status, so I kept it aside. I've added pending spec examples for those cases though.

I guess they should override the new pending status, as that'd be consistent with the previous behaviour.

bbatsov · 2019-12-13T06:47:58Z

A couple of more things:

We should mention the new status in the manual (and the policy that new cops will be using it, until a major release when they'll be enabled/disabled by default)
We should update the template for new cops to use the pending status

CHANGELOG.md

bbatsov · 2019-12-18T12:53:11Z

@pirj Any chance you'll be wrapping this one soon? I'm wondering whether to cut a new release now or wait for this PR.

pirj · 2019-12-18T13:09:47Z

@bbatsov Current status and a couple of questions:

I'm a bit confused with the default behaviour of Enabled/DisabledByDefault, since it seems to only affect RuboCop's own default config, and ignores extensions' configs. So e.g. if rubocop-rspec's config has a pending cop (or, enabled, or, disabled, doesn't matter), those settings don't seem to override extension's cop enabled status.
Is there a distinction between gem's default configs, and user's configs, e.g. if we require .rubocop_base.yml that enables e.g. Lint/A, and also rubocop-rspec/config/default.yml that enables RSpec/B, and the main config has DisabledByDefault: true, shouldn't we disable RSpec/B but keep Lint/A enabled?
docs - Intend to extend manual/configuration.md mentioning default pending status.
Not sure, if a dedicated section mentioning how we are going to introduce cops, and how users are supposed to update RuboCop should be added?

I have some time this week to address this, but if something else pops up, this might take longer, so it's better to postpone to the next release.

bbatsov · 2019-12-20T18:58:38Z

I'm a bit confused with the default behaviour of Enabled/DisabledByDefault, since it seems to only affect RuboCop's own default config, and ignores extensions' configs. So e.g. if rubocop-rspec's config has a pending cop (or, enabled, or, disabled, doesn't matter), those settings don't seem to override extension's cop enabled status.

Well, I think that's an orthogonal problem that has to be fixed separately. This setting dates back from a time where there were almost no extensions.

Is there a distinction between gem's default configs, and user's configs, e.g. if we require .rubocop_base.yml that enables e.g. Lint/A, and also rubocop-rspec/config/default.yml that enables RSpec/B, and the main config has DisabledByDefault: true, shouldn't we disable RSpec/B but keep Lint/A enabled?

Yeah, I agree.

docs - Intend to extend manual/configuration.md mentioning default pending status.

👍

Not sure, if a dedicated section mentioning how we are going to introduce cops, and how users are supposed to update RuboCop should be added?

Some small section on the subject would be useful indeed.

Previously, new cops were introduced as either enabled or disabled. The ones enabled were bothering users with new offences, while disabled cops were often left out and remained under radars, while still being useful. By introducing this special status, users have to decide how to handle new cops, by explicitly enabling or disabling them. Cops are to be introduced with pending status between major releases of RuboCop and its extensions, and they eventually become enabled or disabled on major releases. Co-authored-by: Phil Pirozhkov <hello@fili.pp.ru>

pirj · 2019-12-20T20:48:48Z

@bbatsov All green. Please take a look if something needs to be added.

pirj · 2019-12-20T20:51:03Z

There's one very nasty thing that I've noticed about :isolated_environment. In cases there's something horribly wrong with the example, it interrupts normal RSpec cycle and reports no error. All specs appear to be green, and the only way to figure out that something is wrong is to make sure the number of examples in a spec matches those reported by RSpec.

bbatsov · 2019-12-24T08:11:40Z

Looks good to me. Thanks for bringing this across the finish line!

pirj · 2019-12-24T09:49:29Z

Happy to move this forward 👍

pirj commented Dec 12, 2019

View reviewed changes

pirj marked this pull request as ready for review December 12, 2019 17:13

Darhazer reviewed Dec 13, 2019

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Darhazer mentioned this pull request Dec 13, 2019

[WIP] [Fix #5979] Introduce cops with special status #7184

Closed

8 tasks

Maxim Krizhanovski and others added 7 commits December 20, 2019 23:10

fixup! Adjust changelog

34dc15b

fixup! Adjust cop generator

feefcdd

fixup! Add spec for AllCops/*ByDefault

babf585

fixup! Add doc about versioning

39ce911

fixup! Appease RuboCop

be62a57

fixup! Fix alignment

9b36f1f

pirj changed the title ~~Introduce cops with special status~~ [Fix #5979] Introduce cops with special status Dec 20, 2019

fixup! Add missing parent

42dd985

bbatsov merged commit 5fb9db3 into rubocop:master Dec 24, 2019

pirj deleted the introduce-new-cops branch December 24, 2019 09:49

bbatsov mentioned this pull request Feb 19, 2020

Rubocop 0.80.0 outputs warnings about newly introduced cops #7731

Closed

andyw8 mentioned this pull request Feb 25, 2020

Updated channel and documentation to Rubocop v0.80.0 codeclimate/codeclimate-rubocop#225

Merged

MauricioRobayo mentioned this pull request Apr 16, 2020

Hotfix/rubocop update microverseinc/linters-config#63

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix #5979] Introduce cops with special status #7567

[Fix #5979] Introduce cops with special status #7567

pirj commented Dec 12, 2019 •

edited

pirj Dec 12, 2019

pirj Dec 12, 2019

bbatsov commented Dec 13, 2019

bbatsov commented Dec 13, 2019

bbatsov commented Dec 18, 2019

pirj commented Dec 18, 2019 •

edited

bbatsov commented Dec 20, 2019

pirj commented Dec 20, 2019

pirj commented Dec 20, 2019

bbatsov commented Dec 24, 2019 •

edited

pirj commented Dec 24, 2019 •

edited

[Fix #5979] Introduce cops with special status #7567

[Fix #5979] Introduce cops with special status #7567

Conversation

pirj commented Dec 12, 2019 • edited

pirj Dec 12, 2019

Choose a reason for hiding this comment

pirj Dec 12, 2019

Choose a reason for hiding this comment

bbatsov commented Dec 13, 2019

bbatsov commented Dec 13, 2019

bbatsov commented Dec 18, 2019

pirj commented Dec 18, 2019 • edited

bbatsov commented Dec 20, 2019

pirj commented Dec 20, 2019

pirj commented Dec 20, 2019

bbatsov commented Dec 24, 2019 • edited

pirj commented Dec 24, 2019 • edited

pirj commented Dec 12, 2019 •

edited

pirj commented Dec 18, 2019 •

edited

bbatsov commented Dec 24, 2019 •

edited

pirj commented Dec 24, 2019 •

edited