From e4147c88363abb9b95a3696f86d2686e30d06274 Mon Sep 17 00:00:00 2001 From: Jonas Arvidsson Date: Fri, 16 Jul 2021 10:42:27 +0200 Subject: [PATCH] [Fix #9752] Improve error message for top level department 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. --- changelog/fix_error_message_for_dept.md | 1 + lib/rubocop/config_validator.rb | 23 ++++++++++++++++++----- spec/rubocop/cli_spec.rb | 12 ++++++++---- spec/rubocop/config_loader_spec.rb | 23 +++++++++++++++++------ spec/rubocop/config_spec.rb | 2 +- 5 files changed, 45 insertions(+), 16 deletions(-) create mode 100644 changelog/fix_error_message_for_dept.md diff --git a/changelog/fix_error_message_for_dept.md b/changelog/fix_error_message_for_dept.md new file mode 100644 index 00000000000..8c50df131a8 --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/config_validator.rb b/lib/rubocop/config_validator.rb index f8c7d3ecd1a..87656273736 100644 --- a/lib/rubocop/config_validator.rb +++ b/lib/rubocop/config_validator.rb @@ -104,12 +104,9 @@ 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 @@ -117,6 +114,22 @@ def alert_about_unrecognized_cops(invalid_cop_names) 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'] diff --git a/spec/rubocop/cli_spec.rb b/spec/rubocop/cli_spec.rb index c84bc7c5cb4..75a4e43bfd9 100644 --- a/spec/rubocop/cli_spec.rb +++ b/spec/rubocop/cli_spec.rb @@ -1380,6 +1380,8 @@ def meow_at_4am? Layout/LyneLenth: Enabled: true Max: 100 + Linth: + Enabled: false Lint/LiteralInCondition: Enabled: true Style/AlignHash: @@ -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 @@ -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 diff --git a/spec/rubocop/config_loader_spec.rb b/spec/rubocop/config_loader_spec.rb index a9af310322b..9f363d134f7 100644 --- a/spec/rubocop/config_loader_spec.rb +++ b/spec/rubocop/config_loader_spec.rb @@ -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 @@ -573,7 +574,7 @@ Naming: Enabled: false - Foo/Bar: + #{custom_dept_to_disable}: Enabled: false YAML create_file(file_path, <<~YAML) @@ -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) @@ -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 diff --git a/spec/rubocop/config_spec.rb b/spec/rubocop/config_spec.rb index c01b0ee886d..f17a08d77ea 100644 --- a/spec/rubocop/config_spec.rb +++ b/spec/rubocop/config_spec.rb @@ -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