diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e48f76c5513..1d3e29c49cb 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2022-01-30 21:43:14 UTC using RuboCop version 1.25.0. +# on 2022-02-13 08:32:10 UTC using RuboCop version 1.25.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -13,9 +13,9 @@ InternalAffairs/NodeDestructuring: # Offense count: 55 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 191 + Max: 192 -# Offense count: 235 +# Offense count: 238 # Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods. Metrics/MethodLength: Max: 14 @@ -39,10 +39,10 @@ RSpec/AnyInstance: RSpec/ContextWording: Enabled: false -# Offense count: 2414 +# Offense count: 2424 # Configuration parameters: CountAsOne. RSpec/ExampleLength: - Enabled: false + Max: 126 # Offense count: 39 RSpec/ExpectOutput: @@ -58,7 +58,7 @@ RSpec/ExpectOutput: - 'spec/rubocop/result_cache_spec.rb' - 'spec/rubocop/target_finder_spec.rb' -# Offense count: 404 +# Offense count: 407 RSpec/MultipleExpectations: Max: 25 @@ -72,4 +72,5 @@ RSpec/SubjectStub: # Offense count: 12 Rake/MethodDefinitionInTask: - Enabled: false + Exclude: + - 'tasks/cut_release.rake' diff --git a/lib/rubocop/config.rb b/lib/rubocop/config.rb index 9bdcad1ec95..03d9b756598 100644 --- a/lib/rubocop/config.rb +++ b/lib/rubocop/config.rb @@ -279,10 +279,12 @@ def enable_cop?(qualified_cop_name, cop_options) # If the cop is explicitly enabled, the other checks can be skipped. return true if cop_options['Enabled'] == true - department = department_of(qualified_cop_name) cop_enabled = cop_options.fetch('Enabled') { !for_all_cops['DisabledByDefault'] } return true if cop_enabled == 'override_department' - return false if department && department['Enabled'] == false + + department_enabled = department_of(qualified_cop_name)&.dig('Enabled') + return false if department_enabled == false + return department_enabled == 'override_department' if cop_enabled == 'disabled_by_department' cop_enabled end diff --git a/lib/rubocop/config_loader_resolver.rb b/lib/rubocop/config_loader_resolver.rb index 72d979634c5..27db7c1d41a 100644 --- a/lib/rubocop/config_loader_resolver.rb +++ b/lib/rubocop/config_loader_resolver.rb @@ -127,10 +127,9 @@ def override_department_setting_for_cops(base_hash, derived_hash) department = Regexp.last_match(1) next unless disabled?(derived_hash, department) || disabled?(base_hash, department) - # The `override_department` setting for the `Enabled` parameter is an - # internal setting that's not documented in the manual. It will cause a - # cop to be enabled later, when logic surrounding enabled/disabled it - # run, even though its department is disabled. + # The 'override_department' setting for the `Enabled` parameter is an internal setting + # that's not documented in the manual. It will cause a cop to be enabled later, when logic + # surrounding enabled/disabled is run, even though its department is disabled. derived_hash[key]['Enabled'] = 'override_department' if derived_hash[key]['Enabled'] end end @@ -147,7 +146,8 @@ def override_enabled_for_disabled_departments(base_hash, derived_hash) cops_to_disable.each do |cop_name| next unless base_hash.dig(cop_name, 'Enabled') == true - derived_hash.replace(merge({ cop_name => { 'Enabled' => false } }, derived_hash)) + derived_hash.replace(merge({ cop_name => { 'Enabled' => 'disabled_by_department' } }, + derived_hash)) end end diff --git a/lib/rubocop/config_validator.rb b/lib/rubocop/config_validator.rb index 8d4868350bc..b25fbb3aaf6 100644 --- a/lib/rubocop/config_validator.rb +++ b/lib/rubocop/config_validator.rb @@ -19,7 +19,7 @@ class ConfigValidator # @api private CONFIG_CHECK_KEYS = %w[Enabled Safe SafeAutoCorrect AutoCorrect].to_set.freeze - CONFIG_CHECK_DEPARTMENTS = %w[pending override_department].freeze + CONFIG_CHECK_DEPARTMENTS = %w[pending override_department disabled_by_department].freeze private_constant :CONFIG_CHECK_KEYS, :CONFIG_CHECK_DEPARTMENTS def_delegators :@config, :smart_loaded_path, :for_all_cops @@ -105,13 +105,15 @@ def alert_about_unrecognized_cops(invalid_cop_names) unknown_cops = [] invalid_cop_names.each do |name| # There could be a custom cop with this name. If so, don't warn - next if Cop::Registry.global.contains_cop_matching?([name]) + next if registry.contains_cop_matching?([name]) # Special case for inherit_mode, which is a directive that we keep in # the configuration (even though it's not a cop), because it's easier # to do so than to pass the value around to various methods. next if name == 'inherit_mode' + next if departments.any? { |department| department.start_with?("#{name}/") } + message = <<~MESSAGE.rstrip unrecognized cop or department #{name} found in #{smart_loaded_path} #{suggestion(name)} @@ -123,19 +125,16 @@ def alert_about_unrecognized_cops(invalid_cop_names) 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 + "Did you mean `#{suggestions.join('`, `')}`?" if suggestions.any? + end + + def departments + registry.departments.map(&:to_s) + end + + def registry + Cop::Registry.global end def validate_syntax_cop diff --git a/spec/rubocop/cli_spec.rb b/spec/rubocop/cli_spec.rb index 7db28135fd8..1b93335d4db 100644 --- a/spec/rubocop/cli_spec.rb +++ b/spec/rubocop/cli_spec.rb @@ -1281,6 +1281,139 @@ def meow_at_4am? RESULT end + context 'with a custom cop' do + before do + create_file('dept_config.yml', <<~YAML) + Dept: + Enabled: true + Dept/Subdept: + Enabled: true + Dept/Subdept/#{cop_name}: + Enabled: true + YAML + create_file('dept.rb', "require_relative 'lib/dept/subdept/cop'") + create_file('lib/dept/subdept/cop.rb', <<~RUBY) + module RuboCop + module Cop + module Dept + module Subdept + class #{cop_name} < Base + def on_str(node) + add_offense(node, message: "I don't like strings.") + end + end + end + end + end + end + RUBY + create_file('example.rb', <<~RUBY) + # frozen_string_literal: true + + puts 'hello' + RUBY + RuboCop::ConfigLoader.default_configuration["Dept/Subdept/#{cop_name}"] = { + 'Enabled' => true + } + end + + around { |example| RuboCop::Cop::Registry.with_temporary_global { example.run } } + + after { RuboCop::ConfigLoader.default_configuration.delete("Dept/Subdept/#{cop_name}") } + + context 'with its top level department disabed' do + let(:cop_name) { 'CopA' } + + it 'disables the cop' do + create_file('.rubocop.yml', <<~YAML) + inherit_from: dept_config.yml + Dept: + Enabled: false + YAML + expect(cli.run(%w[--require ./dept example.rb])).to eq(0) + end + end + + context 'with its top level department disabed and its immediate department enabled' do + let(:cop_name) { 'CopB' } + + it 'enables the cop and gets severity from the immediate department' do + create_file('.parent_rubocop.yml', <<~YAML) + inherit_from: dept_config.yml + Dept: + Enabled: false + Severity: error + YAML + create_file('.rubocop.yml', <<~YAML) + inherit_from: .parent_rubocop.yml + Dept/Subdept: + Enabled: true + Severity: warning + YAML + expect(cli.run(%w[--require ./dept --format simple example.rb])).to eq(1) + expect($stdout.string.strip).to eq(<<~OUTPUT.strip) + == example.rb == + W: 3: 6: Dept/Subdept/CopB: I don't like strings. + + 1 file inspected, 1 offense detected + OUTPUT + end + end + + context 'with its top level department disabed and its immediate department enabled and ' \ + 'the cop explicitly disabled' do + let(:cop_name) { 'CopE' } + + it 'disables the cop' do + create_file('.rubocop.yml', <<~YAML) + Dept: + Enabled: false + Dept/Subdept: + Enabled: true + Dept/Subdept/CopE: + Enabled: false + YAML + expect(cli.run(%w[--require ./dept example.rb])).to eq(0) + end + end + + context 'with its top level department enabed and its immediate department disabled' do + let(:cop_name) { 'CopC' } + + it 'disables the cop' do + create_file('.parent_rubocop.yml', <<~YAML) + inherit_from: dept_config.yml + YAML + create_file('.rubocop.yml', <<~YAML) + inherit_from: .parent_rubocop.yml + Dept/Subdept: + Enabled: false + YAML + expect(cli.run(%w[--require ./dept example.rb])).to eq(0) + end + end + + context 'with its top level department disabed and the cop explicitly enabled' do + let(:cop_name) { 'CopD' } + + it 'disables the cop' do + create_file('.rubocop.yml', <<~YAML) + inherit_from: dept_config.yml + Dept: + Enabled: false + Dept/Subdept/CopD: + Enabled: true + YAML + expect(cli.run(%w[--require ./dept --format offenses example.rb])).to eq(1) + expect($stdout.string.strip).to eq(<<~OUTPUT.strip) + 1 Dept/Subdept/CopD + -- + 1 Total + OUTPUT + end + end + end + it 'can use an alternative max line length from a config file' do create_file('example_src/example1.rb', <<~RUBY) # frozen_string_literal: true diff --git a/spec/rubocop/config_loader_spec.rb b/spec/rubocop/config_loader_spec.rb index 8eb77cc8ea4..2eae8c6516a 100644 --- a/spec/rubocop/config_loader_spec.rb +++ b/spec/rubocop/config_loader_spec.rb @@ -657,15 +657,6 @@ 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) @@ -690,11 +681,13 @@ def enabled?(cop) # Department disabled in grandparent, cop enabled in child config. expect(enabled?('Lint/RaiseException')).to be(true) - # Cop enabled in grandparent, nested department disabled in parent. + # Cop enabled in grandparent, nested department (either Foo or Foo/Bar) disabled in + # parent. expect(enabled?('Foo/Bar/Baz')).to be(false) # Cop with similar prefix to disabled nested department. - expect(enabled?('Foo/BarBaz')).to eq(!disabled_by_default) + expect(enabled?('Foo/BarBaz')).to eq(!disabled_by_default && + custom_dept_to_disable != 'Foo') # Cop enabled in default config, department disabled in grandparent. expect(enabled?('Lint/StructNewOverride')).to be(false)