From 7e6f9683211d675dae9fd1117b59ad28232a77b8 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Fri, 21 Aug 2020 13:31:07 +0300 Subject: [PATCH] Deep dup default confing before letting specs modify it All cops config: ```ruby RuboCop::ConfigLoader.default_configuration.for_all_cops.dig('RSpec', 'Language') ``` specifically `for_all_cops` is memoized in `Config`: ```ruby # rubocop-0.89.1/lib/rubocop/config.rb:132: def for_all_cops @for_all_cops ||= self['AllCops'] || {} end ``` It's not so obvious to notice, since in this cop the example that is modifying the language is in its own context, and it runs last. Even if you add an example after it, due to how RSpec orders examples, it will still be executed last. However, if you add a `context` with an example, you'll see that the language changes leaked. Regular `freeze` and `dup` won't work: { 'Language' => # <= this is dupped { 'Includes' => # <= this is the same Hash in the original and dup { 'Example' => ['it_behaves_like, 'include_examples', ..., 'it_has_special_behavior'] So, we need to deep dup the default config to avoid config modification leakages. --- .../cop/rspec/empty_example_group_spec.rb | 16 +++----------- .../default_rspec_language_config_context.rb | 22 +++++++++++++------ 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/spec/rubocop/cop/rspec/empty_example_group_spec.rb b/spec/rubocop/cop/rspec/empty_example_group_spec.rb index 3232c25fc..5a3e8ebfc 100644 --- a/spec/rubocop/cop/rspec/empty_example_group_spec.rb +++ b/spec/rubocop/cop/rspec/empty_example_group_spec.rb @@ -212,20 +212,10 @@ end context 'when a custom include method is specified' do - let(:language) do - RuboCop::ConfigLoader - .default_configuration - .for_all_cops - .dig('RSpec', 'Language') - end - let(:all_cops_config) do - cfg = { 'TargetRubyVersion' => ruby_version } - cfg['TargetRailsVersion'] = rails_version if rails_version - cfg.merge({ 'RSpec' => { 'Language' => language } }) - end - before do - language['Includes']['Example'] << 'it_has_special_behavior' + all_cops_config + .dig('RSpec', 'Language', 'Includes', 'Example') + .push('it_has_special_behavior') end it 'ignores an empty example group with a custom include' do diff --git a/spec/shared/default_rspec_language_config_context.rb b/spec/shared/default_rspec_language_config_context.rb index d88deb850..e1fa2cb1e 100644 --- a/spec/shared/default_rspec_language_config_context.rb +++ b/spec/shared/default_rspec_language_config_context.rb @@ -4,12 +4,20 @@ let(:all_cops_config) do cfg = { 'TargetRubyVersion' => ruby_version } cfg['TargetRailsVersion'] = rails_version if rails_version - cfg.merge({ - 'RSpec' => { - 'Language' => RuboCop::ConfigLoader - .default_configuration - .for_all_cops.dig('RSpec', 'Language') - } - }) + default_language = RuboCop::ConfigLoader + .default_configuration + .for_all_cops.dig('RSpec', 'Language') + cfg.merge('RSpec' => { 'Language' => deep_dup(default_language) }) + end + + def deep_dup(object) + case object + when Array + object.map { |item| deep_dup(item) } + when Hash + object.transform_values(&method(:deep_dup)) + else + object # only collections undergo modifications and need duping + end end end