Skip to content

Commit

Permalink
[Fix rubocop#9752] Improve error message for top level department
Browse files Browse the repository at this point in the history
When there are cop departments that seem to have a structure, like in
the cookstyle gem where departments are named `Chef/Style`,
`Chef/Correctness`, etc., it's easy to assume that there is a `Chef`
department that can disable all departments under it. This is not the
case, but the error message
`Error: unrecognized cop Chef found in .cookstyle.yml` is not
informative enough.

If an unrecognized cop or department is the first part of existing
departments, we list the existing departments. This should help the user
realize what needs to be changed.

Also, we change the wording of `unrecognized cop` to
`unrecognized cop or department`, and extend the did-you-mean
functionality to also look for similar department names.
  • Loading branch information
jonas054 authored and thearjunmdas committed Aug 20, 2021
1 parent 5129edd commit 72529a9
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 16 deletions.
1 change: 1 addition & 0 deletions changelog/fix_error_message_for_dept.md
@@ -0,0 +1 @@
* [#9752](https://github.com/rubocop/rubocop/issues/9752): Improve error message for top level department used in configuration. ([@jonas054][])
23 changes: 18 additions & 5 deletions lib/rubocop/config_validator.rb
Expand Up @@ -104,19 +104,32 @@ def alert_about_unrecognized_cops(invalid_cop_names)
# to do so than to pass the value around to various methods.
next if name == 'inherit_mode'

suggestions = NameSimilarity.find_similar_names(name, Cop::Registry.global.map(&:cop_name))
suggestion = "Did you mean `#{suggestions.join('`, `')}`?" if suggestions.any?

message = <<~MESSAGE.rstrip
unrecognized cop #{name} found in #{smart_loaded_path}
#{suggestion}
unrecognized cop or department #{name} found in #{smart_loaded_path}
#{suggestion(name)}
MESSAGE

unknown_cops << message
end
raise ValidationError, unknown_cops.join("\n") if unknown_cops.any?
end

def suggestion(name)
registry = Cop::Registry.global
departments = registry.departments.map(&:to_s)
suggestions = NameSimilarity.find_similar_names(name, departments + registry.map(&:cop_name))
if suggestions.any?
"Did you mean `#{suggestions.join('`, `')}`?"
else
# Department names can contain slashes, e.g. Chef/Correctness, but there's no support for
# the concept of higher level departments in RuboCop. It's a flat structure. So if the user
# tries to configure a "top level department", we hint that it's the bottom level
# departments that should be configured.
suggestions = departments.select { |department| department.start_with?("#{name}/") }
"#{name} is not a department. Use `#{suggestions.join('`, `')}`." if suggestions.any?
end
end

def validate_syntax_cop
syntax_config = @config['Lint/Syntax']
default_config = ConfigLoader.default_configuration['Lint/Syntax']
Expand Down
12 changes: 8 additions & 4 deletions spec/rubocop/cli_spec.rb
Expand Up @@ -1380,6 +1380,8 @@ def meow_at_4am?
Layout/LyneLenth:
Enabled: true
Max: 100
Linth:
Enabled: false
Lint/LiteralInCondition:
Enabled: true
Style/AlignHash:
Expand All @@ -1389,11 +1391,13 @@ def meow_at_4am?
expect(cli.run(%w[--format simple example])).to eq(2)
expect($stderr.string)
.to eq(<<~OUTPUT)
Error: unrecognized cop Layout/LyneLenth found in example/.rubocop.yml
Error: unrecognized cop or department Layout/LyneLenth found in example/.rubocop.yml
Did you mean `Layout/LineLength`?
unrecognized cop Lint/LiteralInCondition found in example/.rubocop.yml
unrecognized cop or department Linth found in example/.rubocop.yml
Did you mean `Lint`?
unrecognized cop or department Lint/LiteralInCondition found in example/.rubocop.yml
Did you mean `Lint/LiteralAsCondition`?
unrecognized cop Style/AlignHash found in example/.rubocop.yml
unrecognized cop or department Style/AlignHash found in example/.rubocop.yml
Did you mean `Style/Alias`, `Style/OptionHash`?
OUTPUT
end
Expand Down Expand Up @@ -1688,7 +1692,7 @@ def method(foo, bar, qux, fred, arg5, f) end #{'#' * 85}
YAML
expect(cli.run(['example1.rb'])).to eq(2)
expect($stderr.string.strip).to eq(
'Error: unrecognized cop Syntax/Whatever found in .rubocop.yml'
'Error: unrecognized cop or department Syntax/Whatever found in .rubocop.yml'
)
end
end
Expand Down
23 changes: 17 additions & 6 deletions spec/rubocop/config_loader_spec.rb
Expand Up @@ -540,11 +540,12 @@
context 'when a department is disabled', :restore_registry do
let(:file_path) { '.rubocop.yml' }

shared_examples 'resolves enabled/disabled for all cops' do |enabled_by_default, disabled_by_default|
shared_examples 'resolves enabled/disabled for all ' \
'cops' do |enabled_by_default, disabled_by_default, custom_dept_to_disable|
before { stub_cop_class('RuboCop::Cop::Foo::Bar::Baz') }

it "handles EnabledByDefault: #{enabled_by_default}, " \
"DisabledByDefault: #{disabled_by_default}" do
"DisabledByDefault: #{disabled_by_default} with disabled #{custom_dept_to_disable}" do
create_file('grandparent_rubocop.yml', <<~YAML)
Naming/FileName:
Enabled: pending
Expand Down Expand Up @@ -573,7 +574,7 @@
Naming:
Enabled: false
Foo/Bar:
#{custom_dept_to_disable}:
Enabled: false
YAML
create_file(file_path, <<~YAML)
Expand Down Expand Up @@ -603,6 +604,15 @@ def enabled?(cop)
configuration_from_file.for_cop(cop)['Enabled']
end

if custom_dept_to_disable == 'Foo'
message = <<~'OUTPUT'.chomp
unrecognized cop or department Foo found in parent_rubocop.yml
Foo is not a department. Use `Foo/Bar`.
OUTPUT
expect { enabled?('Foo/Bar/Baz') }.to raise_error(RuboCop::ValidationError, message)
next
end

# Department disabled in parent config, cop enabled in child.
expect(enabled?('Metrics/MethodLength')).to be(true)

Expand Down Expand Up @@ -641,9 +651,10 @@ def enabled?(cop)
end
end

include_examples 'resolves enabled/disabled for all cops', false, false
include_examples 'resolves enabled/disabled for all cops', false, true
include_examples 'resolves enabled/disabled for all cops', true, false
include_examples 'resolves enabled/disabled for all cops', false, false, 'Foo/Bar'
include_examples 'resolves enabled/disabled for all cops', false, true, 'Foo/Bar'
include_examples 'resolves enabled/disabled for all cops', true, false, 'Foo/Bar'
include_examples 'resolves enabled/disabled for all cops', false, false, 'Foo'
end

context 'when a third party require defines a new gem', :restore_registry do
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/config_spec.rb
Expand Up @@ -31,7 +31,7 @@
it 'raises an validation error' do
expect { configuration }.to raise_error(
RuboCop::ValidationError,
'unrecognized cop LyneLenth found in .rubocop.yml'
'unrecognized cop or department LyneLenth found in .rubocop.yml'
)
end
end
Expand Down

0 comments on commit 72529a9

Please sign in to comment.