From 72402c03f7581f0496a28536e6a36769629f3628 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 14 May 2019 18:02:58 +0900 Subject: [PATCH] Add `unset_nil` option to `ConfigLoader.merge_with_default` ### Summary Follow up #6942. `unset_nil` was introduced to RuboCop core at #6942. This causes the following warning to be displayed for `TargetRailsVersion` and `Database` where `null` (or `~`) is specified in the RuboCop Rails `config/default.yml` configuration. - https://github.com/rubocop-hq/rubocop-rails/blob/5180fa2c25cef467eececd9c80739f7cad6adbbf/config/default.yml#L11 - https://github.com/rubocop-hq/rubocop-rails/blob/5180fa2c25cef467eececd9c80739f7cad6adbbf/config/default.yml#L89 ```console % cd path/to/rubocop-rails % bundle exec rake (snip) Warning: AllCops does not support TargetRailsVersion parameter. Supported parameters are: - RubyInterpreters - Include - Exclude - DefaultFormatter - DisplayCopNames - DisplayStyleGuide - StyleGuideBaseURL - ExtraDetails - StyleGuideCopsOnly - EnabledByDefault - DisabledByDefault - UseCache - MaxFilesInCache - CacheRootDirectory - AllowSymlinksInCacheRootDirectory - TargetRubyVersion Warning: Rails/BulkChangeTable does not support Database parameter. Supported parameters are: - Enabled - SupportedDatabases - Include ``` This PR aims to prevent this issue by the following work around to RuboCop Rails. I will open a PR to RuboCop Rails if this PR is accepted. ```diff diff --git a/lib/rubocop/rails/inject.rb b/lib/rubocop/rails/inject.rb index abb715ea7..8d1ffa371 100644 --- a/lib/rubocop/rails/inject.rb +++ b/lib/rubocop/rails/inject.rb @@ -10,7 +10,7 @@ module RuboCop hash = ConfigLoader.send(:load_yaml_configuration, path) config = Config.new(hash, path) puts "configuration from #{path}" if ConfigLoader.debug? - config = ConfigLoader.merge_with_default(config, path) + config = ConfigLoader.merge_with_default(config, path, unset_nil: false) ConfigLoader.instance_variable_set(:@default_configuration, config) end end ``` ### Other Information The following example app code reproduces this issue using RuboCop Rails master code. ```ruby # Gemfile source "https://rubygems.org" git_source(:github) {|repo_name| "https://github.com/#{repo_name}" } gem 'rubocop', github: 'rubocop-hq/rubocop' gem 'rubocop-rails', github: 'rubocop-hq/rubocop-rails' ``` ```ruby # app/models/user.rb class User < ActiveRecord::Base end ``` ```yaml # .rubocop.yml AllCops: TargetRailsVersion: 5.2 ``` ``` % bunlde install % bundle exec rubocop --only Rails/ApplicationRecord --require rubocop-rails Warning: AllCops does not support TargetRailsVersion parameter. Supported parameters are: - RubyInterpreters - Include - Exclude - DefaultFormatter - DisplayCopNames - DisplayStyleGuide - StyleGuideBaseURL - ExtraDetails - StyleGuideCopsOnly - EnabledByDefault - DisabledByDefault - UseCache - MaxFilesInCache - CacheRootDirectory - AllowSymlinksInCacheRootDirectory - TargetRubyVersion Inspecting 2 files .C Offenses: app/models/user.rb:1:14: C: Rails/ApplicationRecord: Models should subclass ApplicationRecord. class User < ActiveRecord::Base ^^^^^^^^^^^^^^^^^^ 2 files inspected, 1 offense detected ``` This means that an unexpected warning is displayed during a migration period until RuboCop Rails cops is removed from RuboCop core. --- lib/rubocop/config_loader.rb | 4 ++-- lib/rubocop/config_loader_resolver.rb | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/rubocop/config_loader.rb b/lib/rubocop/config_loader.rb index ba08b271f92..3070d7626e7 100644 --- a/lib/rubocop/config_loader.rb +++ b/lib/rubocop/config_loader.rb @@ -120,8 +120,8 @@ def default_configuration # If AllCops::EnabledByDefault is true, it changes the Enabled params # so that only cops explicitly disabled in user configuration are # disabled. - def merge_with_default(config, config_file) - resolver.merge_with_default(config, config_file) + def merge_with_default(config, config_file, unset_nil: true) + resolver.merge_with_default(config, config_file, unset_nil: unset_nil) end def add_inheritance_from_auto_generated_file diff --git a/lib/rubocop/config_loader_resolver.rb b/lib/rubocop/config_loader_resolver.rb index 85be7da02ff..18a0da415dd 100644 --- a/lib/rubocop/config_loader_resolver.rb +++ b/lib/rubocop/config_loader_resolver.rb @@ -55,7 +55,7 @@ def resolve_inheritance_from_gems(hash, gems) # only cops from user configuration are enabled. If # AllCops::EnabledByDefault is true, it changes the Enabled params so that # only cops explicitly disabled in user configuration are disabled. - def merge_with_default(config, config_file) + def merge_with_default(config, config_file, unset_nil:) default_configuration = ConfigLoader.default_configuration disabled_by_default = config.for_all_cops['DisabledByDefault'] @@ -71,7 +71,8 @@ def merge_with_default(config, config_file) config = handle_disabled_by_default(config, default_configuration) end - opts = { inherit_mode: config['inherit_mode'] || {}, unset_nil: true } + opts = { inherit_mode: config['inherit_mode'] || {}, + unset_nil: unset_nil } Config.new(merge(default_configuration, config, opts), config_file) end