Skip to content

Commit

Permalink
Add unset_nil option to ConfigLoader.merge_with_default
Browse files Browse the repository at this point in the history
### 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.
  • Loading branch information
koic authored and bbatsov committed May 15, 2019
1 parent 0eb812b commit 72402c0
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
4 changes: 2 additions & 2 deletions lib/rubocop/config_loader.rb
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions lib/rubocop/config_loader_resolver.rb
Expand Up @@ -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']
Expand All @@ -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

Expand Down

0 comments on commit 72402c0

Please sign in to comment.