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

[Fix #9752] Improve error message for top level department #9936

Merged
merged 1 commit into from Jul 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
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