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

Make pending cops warning slightly more useful #8414

Merged
merged 2 commits into from Aug 27, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -28,6 +28,7 @@

### Changes

* [#8413](https://github.com/rubocop-hq/rubocop/issues/8413): Pending cops warning now contains snippet that can be directly copied into `.rubocop.yml` as well as a notice about `NewCops: enable` config option. ([@colszowka][])
* [#8376](https://github.com/rubocop-hq/rubocop/pull/8376): `Style/MethodMissingSuper` cop is removed in favor of new `Lint/MissingSuper` cop. ([@fatkodima][])
* [#8350](https://github.com/rubocop-hq/rubocop/pull/8350): Set default max line length to 120 for `Style/MultilineMethodSignature`. ([@koic][])
* [#8338](https://github.com/rubocop-hq/rubocop/pull/8338): **potentially breaking**. Config#for_department now returns only the config specified for that department; the 'Enabled' attribute is no longer calculated. ([@marcandre][])
Expand Down Expand Up @@ -4724,3 +4725,4 @@
[@knejad]: https://github.com/knejad
[@iamravitejag]: https://github.com/iamravitejag
[@volfgox]: https://github.com/volfgox
[@colszowka]: https://github.com/colszowka
23 changes: 17 additions & 6 deletions lib/rubocop/config_loader.rb
Expand Up @@ -140,22 +140,33 @@ def project_root
@project_root ||= find_project_root
end

PENDING_BANNER = <<~BANNER
The following cops were added to RuboCop, but are not configured. Please set Enabled to either `true` or `false` in your `.rubocop.yml` file.

Please also note that can also opt-in to new cops by default by adding this to your config:
AllCops:
NewCops: enable
BANNER

def warn_on_pending_cops(pending_cops)
return if pending_cops.empty?

warn Rainbow('The following cops were added to RuboCop, but are not ' \
'configured. Please set Enabled to either `true` or ' \
'`false` in your `.rubocop.yml` file:').yellow
warn Rainbow(PENDING_BANNER).yellow

pending_cops.each do |cop|
version = cop.metadata['VersionAdded'] || 'N/A'

warn Rainbow(" - #{cop.name} (#{version})").yellow
warn_pending_cop cop
end

warn Rainbow('For more information: https://docs.rubocop.org/rubocop/versioning.html').yellow
end

def warn_pending_cop(cop)
version = cop.metadata['VersionAdded'] || 'N/A'

warn Rainbow("#{cop.name}: # (new in #{version})").yellow
warn Rainbow(' Enabled: true').yellow
end

# Merges the given configuration with the default one.
def merge_with_default(config, config_file, unset_nil: true)
resolver.merge_with_default(config, config_file, unset_nil: unset_nil)
Expand Down
10 changes: 5 additions & 5 deletions spec/rubocop/cli/cli_options_spec.rb
Expand Up @@ -272,7 +272,7 @@ class SomeCop < Cop
end

let(:pending_cop_warning) { <<~PENDING_COP_WARNING }
The following cops were added to RuboCop, but are not configured. Please set Enabled to either `true` or `false` in your `.rubocop.yml` file:
The following cops were added to RuboCop, but are not configured. Please set Enabled to either `true` or `false` in your `.rubocop.yml` file.
PENDING_COP_WARNING

let(:inspected_output) { <<~INSPECTED_OUTPUT }
Expand Down Expand Up @@ -327,9 +327,9 @@ class SomeCop < Cop

remaining_range =
pending_cop_warning.length..-(inspected_output.length + 1)
pending_cops = output[remaining_range].split("\n")
pending_cops = output[remaining_range]

expect(pending_cops).to include(' - Style/SomeCop (0.80)')
expect(pending_cops).to include("Style/SomeCop: # (new in 0.80)\n Enabled: true")

manual_url = output[remaining_range].split("\n").last

Expand All @@ -346,9 +346,9 @@ class SomeCop < Cop

remaining_range =
pending_cop_warning.length..-(inspected_output.length + 1)
pending_cops = output[remaining_range].split("\n")
pending_cops = output[remaining_range]

expect(pending_cops).to include(' - Style/SomeCop (N/A)')
expect(pending_cops).to include("Style/SomeCop: # (new in N/A)\n Enabled: true")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"new in N/A" sound quite odd, maybe remove it altogether?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, was thinking the same


manual_url = output[remaining_range].split("\n").last

Expand Down