RSpec/Language isn't merging properly

[dvandersluis]

It appears that if values for RSpec/Language are being set in .rubocop.yml, they are clobbering the existing setup and this is causing quite a few other cops to fail due to RSpec syntax not being recognized.

With this minimal .rubocop.yml:

require:
  - rubocop-rspec

AllCops:
  TargetRubyVersion: 2.6

RSpec:
  Language:
    ExampleGroups:
      Regular:
        - endpoint
    Helpers:
      - set

I get the following in RuboCop::RSpec::Language.config:

{
  "ExampleGroups" => {
    "Regular" => ["endpoint"],
    "Skipped" => ["xdescribe", "xcontext", "xfeature"],
    "Focused" => ["fdescribe", "fcontext", "ffeature"]
  },
  "Examples" => {
    "Regular" => ["it", "specify", "example", "scenario", "its"],
    "Focused" => ["fit", "fspecify", "fexample", "fscenario", "focus"],
    "Skipped" => ["xit", "xspecify", "xexample", "xscenario", "skip"],
    "Pending" => ["pending"]
  },
  "Expectations" => ["expect", "is_expected", "expect_any_instance_of"],
  "Helpers" => ["set"],
  "Hooks" => ["prepend_before", "before", "append_before", "around", "prepend_after", "after", "append_after"],
  "HookScopes" => ["each", "example", "context", "all", "suite"],
  "Includes" => {
    "Examples" => ["it_behaves_like", "it_should_behave_like", "include_examples"],
    "Context" => ["include_context"]
  },
  "Runners" => ["to", "to_not", "not_to"],
  "SharedGroups" => {
    "Examples" => ["shared_examples", "shared_examples_for"],
    "Context" => ["shared_context"]
  },
  "Subjects" => ["subject", "subject!"]
}

You'll see that ExampleGroups/Regular and Helpers have been overridden to only be the values I specified. Am I missing something?

[pirj]

I was under the impression that the default merging strategy for arrays in RuboCop configuration is merge, and only Include is the exception to this rule with its default override, and I was so wrong:

Arrays override because if they were merged, there would be no way to remove elements in child files.

Following the advice from https://docs.rubocop.org/rubocop/1.0/configuration.html#merging-arrays-using-inherit_mode, I've tried:

1. Adding inherit_mode to rubocop-rspec's config/default.yml in several variants:

RSpec:
  inherit_mode:
    merge:
      - Language/Includes/Examples
      - Language/Includes/Context

and

RSpec:
  Language:
    inherit_mode:
      merge:
        - Includes/Examples
        - Includes/Context

and

RSpec:
  Language:
    Includes:
      inherit_mode:
        merge:
          - Examples
          - Context

and none of this worked.

2. In the project that adds an alias, added the following:

require:
  - rubocop-rspec

RSpec:
  Language:
    Includes:
      inherit_mode:
        merge:
          - Examples
      Examples:
        - endpoint
    Helpers:
      - set

and still Includes/Examples is overridden instead of merged.

Adding

inherit_gem:
  rubocop-rspec: config/default.yml

makes no difference.

Symptom:

> RuboCop::RSpec::Language.config
=> {"inherit_mode"=>{"merge"=>["Includes/Examples", "Includes/Context"]},
 "ExampleGroups"=>
  {"Regular"=>["describe", "context", "feature", "example_group"],
   "Skipped"=>["xdescribe", "xcontext", "xfeature"],
   "Focused"=>["fdescribe", "fcontext", "ffeature"]},
 "Examples"=>
  {"Regular"=>["it", "specify", "example", "scenario", "its"],
   "Focused"=>["fit", "fspecify", "fexample", "fscenario", "focus"],
   "Skipped"=>["xit", "xspecify", "xexample", "xscenario", "skip"],
   "Pending"=>["pending"]},
 "Expectations"=>["expect", "is_expected", "expect_any_instance_of"],
 "Helpers"=>["set"],
 "Hooks"=>  ["prepend_before", "before", "append_before", "around", "prepend_after", "after", "append_after"],
 "HookScopes"=>["each", "example", "context", "all", "suite"],
 "Includes"=>
  {"inherit_mode"=>{"merge"=>["Examples"]},
   "Examples"=>["endpoint"],
   "Context"=>["include_context"]},
 "Runners"=>["to", "to_not", "not_to"],
 "SharedGroups"=>
  {"Examples"=>["shared_examples", "shared_examples_for"],
   "Context"=>["shared_context"]},
 "Subjects"=>["subject", "subject!"]}

The Language.config is set here:

        # Set the config for dynamic DSL configuration-aware helpers
        # that have no other means of accessing the configuration.
        def on_new_investigation
          super
          RuboCop::RSpec::Language.config = config['RSpec']['Language']
        end

From RuboCop's standpoint, what is wrong? Why aren't those arrays merged?

[dvandersluis]

Ok I was hoping that there was something specific to language config loading happening in rubocop-rspec, but yeah it looks like we have some stuff to fix in rubocop. There are a couple tangentially related rubocop issues (eg. rubocop/rubocop#9325).

I think it’s probably that merge_mode is overly complex and doesn’t cover edge cases properly.

We should take this issue to the rubocop repo but I don’t have access to transfer it from here 😅

[pirj]

Can't remember the exact methodology I used to test how config merging works when the language configuration feature was about to be merged. To my shame, it was probably the optimistic "yay, no offences" approach, including the testing done a bit later in palkan/action_policy#138 and sibling PRs.

I don't have permissions to transfer the issue over either. However, even though are some deficiencies on the RuboCop side, I believe we still have to address an issue with the missing inherit_mode/merge option here in rubocop-rspec, so I'd keep this ticket.

[pirj]

Cry for help!
I admit our shameful fiasco.

Asking for help with configuring rubocop-rspec's config/defaults.yml in such a way that deeply-nested keys of a list type (e.g. RSpec/Language/ExampleGroups/Regular) had inherit_mode: merge.

@bquorning @Darhazer
Summoning @dvandersluis @koic @marcandre as most recent committers to lib/rubocop/config_loader.rb.

[pirj]

Culprit

rubocop/rubocop@52d03b4#diff-a923ce94f1d49facbed6a8bad517fe80d9052d653a1210c51530d39de572dcc1R131:

The optional directive inherit_mode is used to specify which configuration keys that have array values should be merged together instead of overriding the inherited value.

One caveat is that this directive only works with local and inherited configuration files, it is unable to merge with the default.yml config.

Debug

Breakpoint in https://github.com/rubocop/rubocop/blob/b5f343842a14f18b08406458d20670d2b401c492/lib/rubocop/config_loader_resolver.rb#L107:

binding.pry if key == "Regular"

Introspect:

opts # => {:inherit_mode=>{}, :unset_nil=>true}
key # => "Regular"
base_hash # => {"inherit_mode"=>{"merge"=>["Regular"]}, "Regular"=>["it", "specify", "example", "scenario", "its"]}
derived_hash # => => {"Regular"=>["mycustomexamplealias"]}

config/default.yml:

RSpec:
  Language:
    Examples:
      inherit_mode:
        merge:
          - Regular
      Regular:
        - it
        - specify
        - example
        - scenario
        - its

.rubocop.yml:

require:
  - rubocop-rspec

RSpec:
  Language:
    Examples:
      Regular:
        - mycustomexamplealias

Fix(es)

Respect setting-local inherit_mode

Not only check the inherit_mode defined on the root config level, but also respect local base_hash['inherit_mode']

elsif should_union?(base_hash, key, opts[:inherit_mode])

Might have side effects if the inherit_mode is also set in the .rubocop.yml, say, when they want to override instead of merge. So, we'll have those three:

# .rubocop.yml
inherit_mode:
  override:
    - Regular

RSpec:
  Language:
    Examples:
      inherit_mode:
        override:
          - Regular
      Regular:
        - mycustomexamplealias

and potentially conflicting config/default.yml:

RSpec:
  Language:
    Examples:
      inherit_mode:
        merge:
          - Regular

Still, this may be the simplest to resolve the issue.

Respect nesting

opts has no notion of nesting or namespaces. They accept a key, and that's it.
merge method has no awareness of the context, e.g. it receives base_hash:

=> {"inherit_mode"=>{"merge"=>["Regular"]}, "Regular"=>["it", "specify", "example", "scenario", "its"]}

but doesn't know that it's nested inside RSpec/Language. This, if opts[:inherit_mode] was set to "Language/RSpec/Examples/Regular", it (should_union? to be precise) would be unable to compare "Regular" to "Language/RSpec/Examples/Regular".

It might not be the best approach to force defining inherit_mode globally, with namespaces, though.

Workaround

I thought of a brute force way of fixing this by putting

inherit_mode:
  merge:
    - Regular

to rubocop-rspec's config/default.yml, but it's not respected, and opts ({:inherit_mode=>{}, :unset_nil=>true}) are coming from .rubocop.yml, not config/default.yml.

This didn't work. And this could have potential side effects if users set inherit_mode in their configs for the same keys.

@bbatsov @jonas054

[pirj]

I intend to bump the minimum required version of rubocop once a version that includes the fix is released.