From 61090b15d99c0c4b174d9c50c92ccab039f1af85 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 7 Apr 2020 01:07:46 +0900 Subject: [PATCH] [Fix #7850] Make it possible to enable/disable pending cops Resolves #7850. This PR will add `--enable-pending-cops` and `--disable-pending-cops` command-line options and `NewCops` will be provided in .rubocop.yml. When `NewCops` is `enable`, pending cops are enabled in bulk. ```yaml AllCops: NewCops: enable ``` Can be overridden by the `--enable-pending-cops` command-line option. When `NewCops` is `disable`, pending cops are disabled in bulk. ```yaml AllCops: NewCops: disable ``` Can be overridden by the `--disable-pending-cops` command-line option. The default value of `NewCops` is `pending`. --- .rubocop_todo.yml | 2 +- CHANGELOG.md | 4 ++ config/default.yml | 8 +++ lib/rubocop/cli.rb | 14 ++-- lib/rubocop/config.rb | 8 +++ lib/rubocop/config_loader.rb | 20 ++++-- lib/rubocop/config_validator.rb | 16 +++++ lib/rubocop/cop/registry.rb | 15 +++-- lib/rubocop/options.rb | 6 ++ lib/rubocop/runner.rb | 2 +- manual/basic_usage.md | 2 + manual/versioning.md | 20 +++++- spec/rubocop/cli/cli_options_spec.rb | 70 +++++++++++++++++++- spec/rubocop/config_loader_spec.rb | 18 +++++ spec/rubocop/cop/registry_spec.rb | 98 +++++++++++++++++++++++++++- spec/rubocop/options_spec.rb | 2 + 16 files changed, 284 insertions(+), 21 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 219a2b47a57..f404c1fa8ed 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -13,7 +13,7 @@ InternalAffairs/NodeDestructuring: # Offense count: 50 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 183 + Max: 186 # Offense count: 209 # Configuration parameters: CountComments, ExcludedMethods. diff --git a/CHANGELOG.md b/CHANGELOG.md index faf8b748a8d..71a25ceefa3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### New features + +* [#7850](https://github.com/rubocop-hq/rubocop/issues/7850): Make it possible to enable/disable pending cops. ([@koic][]) + ### Bug fixes * [#7842](https://github.com/rubocop-hq/rubocop/issues/7842): Fix a false positive for `Lint/RaiseException` when raising Exception with explicit namespace. ([@koic][]) diff --git a/config/default.yml b/config/default.yml index 3544924dfb8..a4559adc8fb 100644 --- a/config/default.yml +++ b/config/default.yml @@ -97,6 +97,14 @@ AllCops: # to true in the same configuration. EnabledByDefault: false DisabledByDefault: false + # New cops introduced between major versions are set to a special pending status + # and are not enabled by default with warning message. + # Change this behavior by overriding either `NewCops: enable` or `NewCops: disable`. + # When `NewCops` is `enable`, pending cops are enabled in bulk. Can be overridden by + # the `--enable-pending-cops` command-line option. + # When `NewCops` is `disable`, pending cops are disabled in bulk. Can be overridden by + # the `--disable-pending-cops` command-line option. + NewCops: pending # Enables the result cache if `true`. Can be overridden by the `--cache` command # line option. UseCache: true diff --git a/lib/rubocop/cli.rb b/lib/rubocop/cli.rb index 92080cf352f..107b2b4fb59 100644 --- a/lib/rubocop/cli.rb +++ b/lib/rubocop/cli.rb @@ -83,10 +83,7 @@ def validate_options_vs_config end def act_on_options - ConfigLoader.debug = @options[:debug] - ConfigLoader.auto_gen_config = @options[:auto_gen_config] - ConfigLoader.ignore_parent_exclusion = @options[:ignore_parent_exclusion] - ConfigLoader.options_config = @options[:config] + set_options_to_config_loader @config_store.options_config = @options[:config] if @options[:config] @config_store.force_default_config! if @options[:force_default_config] @@ -102,6 +99,15 @@ def act_on_options end end + def set_options_to_config_loader + ConfigLoader.debug = @options[:debug] + ConfigLoader.auto_gen_config = @options[:auto_gen_config] + ConfigLoader.disable_pending_cops = @options[:disable_pending_cops] + ConfigLoader.enable_pending_cops = @options[:enable_pending_cops] + ConfigLoader.ignore_parent_exclusion = @options[:ignore_parent_exclusion] + ConfigLoader.options_config = @options[:config] + end + def handle_exiting_options return unless Options::EXITING_OPTIONS.any? { |o| @options.key? o } diff --git a/lib/rubocop/config.rb b/lib/rubocop/config.rb index 3f69e4e27a1..a423d194b97 100644 --- a/lib/rubocop/config.rb +++ b/lib/rubocop/config.rb @@ -116,6 +116,14 @@ def for_all_cops @for_all_cops ||= self['AllCops'] || {} end + def disabled_new_cops? + for_all_cops['NewCops'] == 'disable' + end + + def enabled_new_cops? + for_all_cops['NewCops'] == 'enable' + end + def file_to_include?(file) relative_file_path = path_relative_to_config(file) diff --git a/lib/rubocop/config_loader.rb b/lib/rubocop/config_loader.rb index f64c5aa0af9..b1f15c28730 100644 --- a/lib/rubocop/config_loader.rb +++ b/lib/rubocop/config_loader.rb @@ -24,7 +24,7 @@ class << self include FileFinder attr_accessor :debug, :auto_gen_config, :ignore_parent_exclusion, - :options_config + :options_config, :disable_pending_cops, :enable_pending_cops attr_writer :default_configuration alias debug? debug @@ -91,15 +91,22 @@ def configuration_from_file(config_file) else add_excludes_from_files(config, config_file) end + merge_with_default(config, config_file).tap do |merged_config| - warn_on_pending_cops(merged_config.pending_cops) + unless possible_new_cops?(config) + warn_on_pending_cops(merged_config.pending_cops) + end end end + def possible_new_cops?(config) + disable_pending_cops || enable_pending_cops || + config.disabled_new_cops? || config.enabled_new_cops? + end + def add_excludes_from_files(config, config_file) - found_files = - find_files_upwards(DOTFILE, config_file) + - [find_user_dotfile, find_user_xdg_config].compact + found_files = find_files_upwards(DOTFILE, config_file) + + [find_user_dotfile, find_user_xdg_config].compact return if found_files.empty? return if PathUtil.relative_path(found_files.last) == @@ -250,8 +257,7 @@ def read_file(absolute_path) def yaml_safe_load(yaml_code, filename) if defined?(SafeYAML) && SafeYAML.respond_to?(:load) - SafeYAML.load(yaml_code, filename, - whitelisted_tags: %w[!ruby/regexp]) + SafeYAML.load(yaml_code, filename, whitelisted_tags: %w[!ruby/regexp]) # Ruby 2.6+ elsif Gem::Version.new(Psych::VERSION) >= Gem::Version.new('3.1.0') YAML.safe_load( diff --git a/lib/rubocop/config_validator.rb b/lib/rubocop/config_validator.rb index e7f3e342218..1f03c011db0 100644 --- a/lib/rubocop/config_validator.rb +++ b/lib/rubocop/config_validator.rb @@ -13,6 +13,7 @@ class ConfigValidator INTERNAL_PARAMS = %w[Description StyleGuide VersionAdded VersionChanged VersionRemoved Reference Safe SafeAutoCorrect].freeze + NEW_COPS_VALUES = %w[pending disable enable].freeze def_delegators :@config, :smart_loaded_path, :for_all_cops @@ -22,6 +23,7 @@ def initialize(config) @target_ruby = TargetRuby.new(config) end + # rubocop:disable Metrics/AbcSize def validate check_cop_config_value(@config) reject_conflicting_safe_settings @@ -37,11 +39,13 @@ def validate alert_about_unrecognized_cops(invalid_cop_names) check_target_ruby + validate_new_cops_parameter validate_parameter_names(valid_cop_names) validate_enforced_styles(valid_cop_names) validate_syntax_cop reject_mutually_exclusive_defaults end + # rubocop:enable Metrics/AbcSize def target_ruby_version target_ruby.version @@ -107,6 +111,18 @@ def validate_syntax_cop 'It\'s not possible to disable this cop.' end + def validate_new_cops_parameter + new_cop_parameter = @config.for_all_cops['NewCops'] + return if new_cop_parameter.nil? || + NEW_COPS_VALUES.include?(new_cop_parameter) + + message = "invalid #{new_cop_parameter} for `NewCops` found in" \ + "#{smart_loaded_path}\n" \ + "Valid choices are: #{NEW_COPS_VALUES.join(', ')}" + + raise ValidationError, message + end + def validate_parameter_names(valid_cop_names) valid_cop_names.each do |name| validate_section_presence(name) diff --git a/lib/rubocop/cop/registry.rb b/lib/rubocop/cop/registry.rb index 6c82983226a..8cf55460b3f 100644 --- a/lib/rubocop/cop/registry.rb +++ b/lib/rubocop/cop/registry.rb @@ -22,12 +22,13 @@ def initialize(name, origin, badges) # Registry that tracks all cops by their badge and department. class Registry - def initialize(cops = []) + def initialize(cops = [], options = {}) @registry = {} @departments = {} @cops_by_cop_name = Hash.new { |hash, key| hash[key] = [] } cops.each { |cop| enlist(cop) } + @options = options end def enlist(cop) @@ -147,9 +148,8 @@ def enabled(config, only, only_safe = false) def enabled?(cop, config, only_safe) cfg = config.for_cop(cop) - # cfg['Enabled'] might be a string `pending`, which is considered - # disabled - cop_enabled = cfg.fetch('Enabled') == true + cop_enabled = cfg.fetch('Enabled') == true || + enabled_pending_cop?(cfg, config) if only_safe cop_enabled && cfg.fetch('Safe', true) @@ -158,6 +158,13 @@ def enabled?(cop, config, only_safe) end end + def enabled_pending_cop?(cop_cfg, config) + return false if @options[:disable_pending_cops] + + cop_cfg.fetch('Enabled') == 'pending' && + (@options[:enable_pending_cops] || config.enabled_new_cops?) + end + def names cops.map(&:cop_name) end diff --git a/lib/rubocop/options.rb b/lib/rubocop/options.rb index a0b3bd62ad0..2db38382d18 100644 --- a/lib/rubocop/options.rb +++ b/lib/rubocop/options.rb @@ -154,6 +154,7 @@ def add_flags_with_optional_args(opts) end end + # rubocop:disable Metrics/MethodLength def add_boolean_flags(opts) option(opts, '-F', '--fail-fast') option(opts, '-C', '--cache FLAG') @@ -162,6 +163,8 @@ def add_boolean_flags(opts) option(opts, '-E', '--extra-details') option(opts, '-S', '--display-style-guide') option(opts, '-a', '--auto-correct') + option(opts, '--disable-pending-cops') + option(opts, '--enable-pending-cops') option(opts, '--ignore-disable-comments') option(opts, '--safe') @@ -172,6 +175,7 @@ def add_boolean_flags(opts) option(opts, '-V', '--verbose-version') option(opts, '-P', '--parallel') end + # rubocop:enable Metrics/MethodLength def add_aliases(opts) option(opts, '-l', '--lint') do @@ -438,7 +442,9 @@ module OptionsHelp debug: 'Display debug info.', display_cop_names: ['Display cop names in offense messages.', 'Default is true.'], + disable_pending_cops: 'Run without pending cops.', display_style_guide: 'Display style guide URLs in offense messages.', + enable_pending_cops: 'Run with pending cops.', extra_details: 'Display extra details in offense messages.', lint: 'Run only lint cops.', safe: 'Run only safe cops.', diff --git a/lib/rubocop/runner.rb b/lib/rubocop/runner.rb index 5114a1e9de2..12c0dc91a50 100644 --- a/lib/rubocop/runner.rb +++ b/lib/rubocop/runner.rb @@ -314,7 +314,7 @@ def mobilized_cop_classes(config) cop_classes.reject! { |c| c.match?(@options[:except]) } - Cop::Registry.new(cop_classes) + Cop::Registry.new(cop_classes, @options) end end diff --git a/manual/basic_usage.md b/manual/basic_usage.md index b3ed954ad30..3df1fbdd2f0 100644 --- a/manual/basic_usage.md +++ b/manual/basic_usage.md @@ -116,9 +116,11 @@ Command flag | Description `-c/--config` | Run with specified config file. `-C/--cache` | Store and reuse results for faster operation. `-d/--debug` | Displays some extra debug output. + --disable-pending-cops | Run without pending cops. ` --disable-uncorrectable` | Used with --auto-correct to annotate any offenses that do not support autocorrect with `rubocop:todo` comments. `-D/--[no-]display-cop-names` | Displays cop names in offense messages. Default is true. ` --display-only-fail-level-offenses` | Only output offense messages at the specified `--fail-level` or above + --enable-pending-cops | Run with pending cops. ` --except` | Run all cops enabled by configuration except the specified cop(s) and/or departments. ` --exclude-limit` | Limit how many individual files `--auto-gen-config` can list in `Exclude` parameters, default is 15. `-E/--extra-details` | Displays extra details in offense messages. diff --git a/manual/versioning.md b/manual/versioning.md index eb1d26ee106..3a83b6f1b59 100644 --- a/manual/versioning.md +++ b/manual/versioning.md @@ -54,7 +54,25 @@ For more information: https://docs.rubocop.org/en/latest/versioning/ You can see that 3 new cops were added in RuboCop 0.80 and it's up to you to decide if you want to enable or disable them. -To suppress this message set `Enabled` to either `true` or `false` in your `.rubocop.yml` file. +To suppress this message set `NewCops` to either `enable` or `disable` in your `.rubocop.yml` file. + +The following setting or using `rubocop --enable-pending-cops` command-line option, pending cops are enabled in bulk. + +```yaml +AllCops: + NewCops: enable +``` + +The following setting or using `rubocop --disable-pending-cops` command-line option, pending cops are disabled in bulk. + +```yaml +AllCops: + NewCops: disable +``` + +The command-line options takes precedence over `.rubocop.yml` file. + +Or set `Enabled` to either `true` or `false` in your `.rubocop.yml` file. `Style/ANewCop` is an example of a newly added pending cop: diff --git a/spec/rubocop/cli/cli_options_spec.rb b/spec/rubocop/cli/cli_options_spec.rb index 511ada6cbc0..ce4a895bb7c 100644 --- a/spec/rubocop/cli/cli_options_spec.rb +++ b/spec/rubocop/cli/cli_options_spec.rb @@ -5,6 +5,8 @@ subject(:cli) { described_class.new } + let(:rubocop) { "#{RuboCop::ConfigLoader::RUBOCOP_HOME}/exe/rubocop" } + before do RuboCop::ConfigLoader.default_configuration = nil end @@ -221,8 +223,6 @@ class SomeCop < Cop end context 'when specifying a pending cop' do - let(:rubocop) { "#{RuboCop::ConfigLoader::RUBOCOP_HOME}/exe/rubocop" } - # Since we define a new cop class, we have to do this in a separate # process. Otherwise, the extra cop will affect other specs. let(:output) do @@ -260,10 +260,16 @@ class SomeCop < Cop end context 'when Style department is enabled' do + let(:new_cops_option) { '' } + before do create_file('.rubocop.yml', <<~YAML) require: rubocop_ext + AllCops: + DefaultFormatter: progress + #{new_cops_option} + Style/SomeCop: Description: Something Enabled: pending @@ -287,6 +293,66 @@ class SomeCop < Cop expect(manual_url).to eq(versioning_manual_url) end + + context 'when using `--disable-pending-cops` command-line option' do + let(:option) { '--disable-pending-cops' } + + let(:output) do + `ruby -I . "#{rubocop}" --require redirect.rb #{option}` + end + + it 'does not display a pending cop warning' do + expect(output).not_to start_with(pending_cop_warning) + end + end + + context 'when using `--enable-pending-cops` command-line option' do + let(:option) { '--enable-pending-cops' } + + let(:output) do + `ruby -I . "#{rubocop}" --require redirect.rb #{option}` + end + + it 'does not display a pending cop warning' do + expect(output).not_to start_with(pending_cop_warning) + end + end + + context 'when specifying `NewCops: pending` in .rubocop.yml' do + let(:new_cops_option) { 'NewCops: pending' } + + let(:output) do + `ruby -I . "#{rubocop}" --require redirect.rb` + end + + it 'displays a pending cop warning' do + expect(output).to start_with(pending_cop_warning) + end + end + + context 'when specifying `NewCops: disable` in .rubocop.yml' do + let(:new_cops_option) { 'NewCops: disable' } + + let(:output) do + `ruby -I . "#{rubocop}" --require redirect.rb` + end + + it 'does not display a pending cop warning' do + expect(output).not_to start_with(pending_cop_warning) + end + end + + context 'when specifying `NewCops: enable` in .rubocop.yml' do + let(:new_cops_option) { 'NewCops: enable' } + + let(:output) do + `ruby -I . "#{rubocop}" --require redirect.rb` + end + + it 'does not display a pending cop warning' do + expect(output).not_to start_with(pending_cop_warning) + end + end end context 'when Style department is disabled' do diff --git a/spec/rubocop/config_loader_spec.rb b/spec/rubocop/config_loader_spec.rb index 0c8b5c3bf14..57ab2220fc5 100644 --- a/spec/rubocop/config_loader_spec.rb +++ b/spec/rubocop/config_loader_spec.rb @@ -1093,6 +1093,24 @@ def cop_enabled?(cop_class) end end + context 'does not set `pending`, `disable`, or `enable` to `NewCops`' do + before do + create_file(configuration_path, <<~YAML) + AllCops: + NewCops: true + YAML + end + + it 'gets a warning message' do + expect do + load_file + end.to raise_error( + RuboCop::ValidationError, + /invalid true for `NewCops` found in/ + ) + end + end + context 'when the file does not exist' do let(:configuration_path) { 'file_that_does_not_exist.yml' } diff --git a/spec/rubocop/cop/registry_spec.rb b/spec/rubocop/cop/registry_spec.rb index fc23b5123af..e4d5297acb3 100644 --- a/spec/rubocop/cop/registry_spec.rb +++ b/spec/rubocop/cop/registry_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Registry do - subject(:registry) { described_class.new(cops) } + subject(:registry) { described_class.new(cops, options) } let(:cops) do stub_const('RuboCop::Cop::Test', Module.new) @@ -33,6 +33,8 @@ class Foo < Cop ] end + let(:options) { {} } + # `RuboCop::Cop::Cop` mutates its `registry` when inherited from. # This can introduce nondeterministic failures in other parts of the # specs if this mutation occurs before code that depends on this global cop @@ -203,6 +205,100 @@ class Foo < Cop result = registry.enabled(config, ['Lint/BooleanSymbol']) expect(result).to eql(cops) end + + context 'when specifying `--disable-pending-cops` command-line option' do + let(:options) do + { disable_pending_cops: true } + end + + it 'does not include them' do + result = registry.enabled(config, []) + expect(result).not_to include(RuboCop::Cop::Lint::BooleanSymbol) + end + + context 'when specifying `NewCops: enable` option in .rubocop.yml' do + let(:config) do + RuboCop::Config.new( + 'AllCops' => { 'NewCops' => 'enable' }, + 'Lint/BooleanSymbol' => { 'Enabled' => 'pending' } + ) + end + + it 'does not include them because command-line option takes ' \ + 'precedence over .rubocop.yml' do + result = registry.enabled(config, []) + expect(result).not_to include(RuboCop::Cop::Lint::BooleanSymbol) + end + end + end + + context 'when specifying `--enable-pending-cops` command-line option' do + let(:options) do + { enable_pending_cops: true } + end + + it 'includes them' do + result = registry.enabled(config, []) + expect(result).to include(RuboCop::Cop::Lint::BooleanSymbol) + end + + context 'when specifying `NewCops: disable` option in .rubocop.yml' do + let(:config) do + RuboCop::Config.new( + 'AllCops' => { 'NewCops' => 'disable' }, + 'Lint/BooleanSymbol' => { 'Enabled' => 'pending' } + ) + end + + it 'includes them because command-line option takes ' \ + 'precedence over .rubocop.yml' do + result = registry.enabled(config, []) + expect(result).to include(RuboCop::Cop::Lint::BooleanSymbol) + end + end + end + + context 'when specifying `NewCops: pending` option in .rubocop.yml' do + let(:config) do + RuboCop::Config.new( + 'AllCops' => { 'NewCops' => 'pending' }, + 'Lint/BooleanSymbol' => { 'Enabled' => 'pending' } + ) + end + + it 'does not include them' do + result = registry.enabled(config, []) + expect(result).not_to include(RuboCop::Cop::Lint::BooleanSymbol) + end + end + + context 'when specifying `NewCops: disable` option in .rubocop.yml' do + let(:config) do + RuboCop::Config.new( + 'AllCops' => { 'NewCops' => 'disable' }, + 'Lint/BooleanSymbol' => { 'Enabled' => 'pending' } + ) + end + + it 'does not include them' do + result = registry.enabled(config, []) + expect(result).not_to include(RuboCop::Cop::Lint::BooleanSymbol) + end + end + + context 'when specifying `NewCops: enable` option in .rubocop.yml' do + let(:config) do + RuboCop::Config.new( + 'AllCops' => { 'NewCops' => 'enable' }, + 'Lint/BooleanSymbol' => { 'Enabled' => 'pending' } + ) + end + + it 'includes them' do + result = registry.enabled(config, []) + expect(result).to include(RuboCop::Cop::Lint::BooleanSymbol) + end + end end end diff --git a/spec/rubocop/options_spec.rb b/spec/rubocop/options_spec.rb index 04f96de1da5..94c59c2329c 100644 --- a/spec/rubocop/options_spec.rb +++ b/spec/rubocop/options_spec.rb @@ -110,6 +110,8 @@ def abs(path) -E, --extra-details Display extra details in offense messages. -S, --display-style-guide Display style guide URLs in offense messages. -a, --auto-correct Auto-correct offenses. + --disable-pending-cops Run without pending cops. + --enable-pending-cops Run with pending cops. --ignore-disable-comments Run cops even when they are disabled locally with a comment. --safe Run only safe cops.