[Fix #10893] Fix loading behavior on running without bundle exec

[r7kamura]

This is an additional fix for #10893 and #10894.

The error in normal usage was fixed at #10894, but there is one more Kernel.require so there is a possible error in the case where users try to use the new feature.

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

[r7kamura]

This takes a bit of work to reproduce, but can be done as follows.

First, put a config file like this:

# .rubocop.yml
require:
  - rubocop-rspec

and locally install slightly modified rubocop-rspec gem like this:
(I changed the file path from lib/rubocop-rspec.rb to lib/rubocop/rspec.rb, then removed "rubocop/" from require_relative target)

diff --git a/lib/rubocop-rspec.rb b/lib/rubocop-rspec.rb
deleted file mode 100644
index ffc3c0f..0000000
--- a/lib/rubocop-rspec.rb
+++ /dev/null
@@ -1,58 +0,0 @@
-# frozen_string_literal: true
-
-require 'pathname'
-require 'yaml'
-
-require 'rubocop'
-
-require_relative 'rubocop/rspec/version'
-require_relative 'rubocop/rspec/inject'
-require_relative 'rubocop/rspec/node'
-require_relative 'rubocop/rspec/wording'
-require_relative 'rubocop/rspec/language/node_pattern'
-require_relative 'rubocop/rspec/language'
-
-require_relative 'rubocop/rspec/factory_bot/language'
-
-require_relative 'rubocop/cop/rspec/mixin/top_level_group'
-require_relative 'rubocop/cop/rspec/mixin/variable'
-require_relative 'rubocop/cop/rspec/mixin/final_end_location'
-require_relative 'rubocop/cop/rspec/mixin/comments_help'
-require_relative 'rubocop/cop/rspec/mixin/empty_line_separation'
-require_relative 'rubocop/cop/rspec/mixin/inside_example_group'
-require_relative 'rubocop/cop/rspec/mixin/namespace'
-
-require_relative 'rubocop/rspec/concept'
-require_relative 'rubocop/rspec/example_group'
-require_relative 'rubocop/rspec/example'
-require_relative 'rubocop/rspec/hook'
-require_relative 'rubocop/cop/rspec/base'
-require_relative 'rubocop/rspec/align_let_brace'
-require_relative 'rubocop/rspec/factory_bot'
-require_relative 'rubocop/rspec/corrector/move_node'
-
-RuboCop::RSpec::Inject.defaults!
-
-require_relative 'rubocop/cop/rspec_cops'
-
-# We have to register our autocorrect incompatibilities in RuboCop's cops
-# as well so we do not hit infinite loops
-
-RuboCop::Cop::Layout::ExtraSpacing.singleton_class.prepend(
-  Module.new do
-    def autocorrect_incompatible_with
-      super.push(RuboCop::Cop::RSpec::AlignLeftLetBrace)
-      .push(RuboCop::Cop::RSpec::AlignRightLetBrace)
-    end
-  end
-)
-
-RuboCop::Cop::Style::TrailingCommaInArguments.singleton_class.prepend(
-  Module.new do
-    def autocorrect_incompatible_with
-      super.push(RuboCop::Cop::RSpec::Capybara::CurrentPathExpectation)
-    end
-  end
-)
-
-RuboCop::AST::Node.include(RuboCop::RSpec::Node)
diff --git a/lib/rubocop/rspec.rb b/lib/rubocop/rspec.rb
new file mode 100644
index 0000000..1410c45
--- /dev/null
+++ b/lib/rubocop/rspec.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+require 'pathname'
+require 'yaml'
+
+require 'rubocop'
+
+require_relative 'rspec/version'
+require_relative 'rspec/inject'
+require_relative 'rspec/node'
+require_relative 'rspec/wording'
+require_relative 'rspec/language/node_pattern'
+require_relative 'rspec/language'
+
+require_relative 'rspec/factory_bot/language'
+
+require_relative 'cop/rspec/mixin/top_level_group'
+require_relative 'cop/rspec/mixin/variable'
+require_relative 'cop/rspec/mixin/final_end_location'
+require_relative 'cop/rspec/mixin/comments_help'
+require_relative 'cop/rspec/mixin/empty_line_separation'
+require_relative 'cop/rspec/mixin/inside_example_group'
+require_relative 'cop/rspec/mixin/namespace'
+
+require_relative 'rspec/concept'
+require_relative 'rspec/example_group'
+require_relative 'rspec/example'
+require_relative 'rspec/hook'
+require_relative 'cop/rspec/base'
+require_relative 'rspec/align_let_brace'
+require_relative 'rspec/factory_bot'
+require_relative 'rspec/corrector/move_node'
+
+RuboCop::RSpec::Inject.defaults!
+
+require_relative 'cop/rspec_cops'
+
+# We have to register our autocorrect incompatibilities in RuboCop's cops
+# as well so we do not hit infinite loops
+
+RuboCop::Cop::Layout::ExtraSpacing.singleton_class.prepend(
+  Module.new do
+    def autocorrect_incompatible_with
+      super.push(RuboCop::Cop::RSpec::AlignLeftLetBrace)
+      .push(RuboCop::Cop::RSpec::AlignRightLetBrace)
+    end
+  end
+)
+
+RuboCop::Cop::Style::TrailingCommaInArguments.singleton_class.prepend(
+  Module.new do
+    def autocorrect_incompatible_with
+      super.push(RuboCop::Cop::RSpec::Capybara::CurrentPathExpectation)
+    end
+  end
+)
+
+RuboCop::AST::Node.include(RuboCop::RSpec::Node)

[koic]

Implementations don't want to be complicated for stub/mock testing. Can you update it to use require and testing?

Suggested change

	self.class.call_require(namespaced_target)
	require(namespaced_target)

[r7kamura]

I agree with you on that point, but we are using RSpec/AnyInstance. If we use require directly, how should we test without any_instance_of?

[r7kamura]

Just to be clear, this singleton method was not necessarily prepared only for stub. This pull request was made because I used Kernel.require in two places and forgot to change one of them. If so, I thought it would be good to keep one caller as a preventive measure.

[koic]

rubocop:disable comment could be used. There may be a better way, but I won't know soon.

diff --git a/spec/rubocop/feature_loader_spec.rb b/spec/rubocop/feature_loader_spec.rb
index 4ac7cd102..0a7a00d68 100644
--- a/spec/rubocop/feature_loader_spec.rb
+++ b/spec/rubocop/feature_loader_spec.rb
@@ -1,6 +1,10 @@
 # frozen_string_literal: true

 RSpec.describe RuboCop::FeatureLoader do
+  # rubocop:disable RSpec/AnyInstance
+  let(:feature_loader) { allow_any_instance_of(described_class) }
+  # rubocop:enable RSpec/AnyInstance
+
   describe '.load' do
     subject(:load) do
       described_class.load(config_directory_path: config_directory_path, feature: feature)
@@ -16,18 +20,18 @@ RSpec.describe RuboCop::FeatureLoader do

     context 'with normally lodable feature' do
       before do
-        allow(Kernel).to receive(:require)
+        feature_loader.to receive(:require)
       end

[r7kamura]

Does that mean we should change it like this?

diff --git a/lib/rubocop/feature_loader.rb b/lib/rubocop/feature_loader.rb
index 39e40b57c..af22a2ab8 100644
--- a/lib/rubocop/feature_loader.rb
+++ b/lib/rubocop/feature_loader.rb
@@ -20,15 +20,6 @@ module RuboCop
       def load(config_directory_path:, feature:)
         new(config_directory_path: config_directory_path, feature: feature).load
       end
-
-      # @note Don't use `::Kernel.require(target)` here.
-      #   To prevent the following error:
-      #   https://github.com/rubocop/rubocop/issues/10893
-      # @api private
-      # @param [String] target
-      def call_require(target)
-        require(target)
-      end
     end
 
     # @param [String] config_directory_path
@@ -39,12 +30,16 @@ module RuboCop
     end
 
     def load
-      self.class.call_require(target)
+      # Don't use `::Kernel.require(target)` to prevent the following error:
+      # https://github.com/rubocop/rubocop/issues/10893
+      require(target)
     rescue ::LoadError => e
       raise if e.path != target
 
       begin
-        self.class.call_require(namespaced_target)
+        # Don't use `::Kernel.require(target)` to prevent the following error:
+        # https://github.com/rubocop/rubocop/issues/10893
+        require(namespaced_target)
       rescue ::LoadError => error_for_namespaced_target
         raise e if error_for_namespaced_target.path == namespaced_target
 
diff --git a/spec/rubocop/feature_loader_spec.rb b/spec/rubocop/feature_loader_spec.rb
index 1436aee62..c247bd37a 100644
--- a/spec/rubocop/feature_loader_spec.rb
+++ b/spec/rubocop/feature_loader_spec.rb
@@ -14,20 +14,28 @@ RSpec.describe RuboCop::FeatureLoader do
       'feature'
     end
 
+    let(:allow_feature_loader) do
+      allow_any_instance_of(described_class) # rubocop:disable RSpec/AnyInstance
+    end
+
+    let(:expect_feature_loader) do
+      expect_any_instance_of(described_class) # rubocop:disable RSpec/AnyInstance
+    end
+
     context 'with normally lodable feature' do
       before do
-        allow(described_class).to receive(:call_require)
+        allow_feature_loader.to receive(:require)
       end
 
       it 'loads it normally' do
-        expect(described_class).to receive(:call_require).with('feature')
+        expect_feature_loader.to receive(:require).with('feature')
         load
       end
     end
 
     context 'with dot-prefixed lodable feature' do
       before do
-        allow(described_class).to receive(:call_require)
+        allow_feature_loader.to receive(:require)
       end
 
       let(:feature) do
@@ -35,15 +43,15 @@ RSpec.describe RuboCop::FeatureLoader do
       end
 
       it 'loads it as relative path' do
-        expect(described_class).to receive(:call_require).with('path-to-config/./path/to/feature')
+        expect_feature_loader.to receive(:require).with('path-to-config/./path/to/feature')
         load
       end
     end
 
     context 'with namespaced feature' do
       before do
-        allow(described_class).to receive(:call_require).with('feature-foo').and_call_original
-        allow(described_class).to receive(:call_require).with('feature/foo')
+        allow_feature_loader.to receive(:require).with('feature-foo').and_call_original
+        allow_feature_loader.to receive(:require).with('feature/foo')
       end
 
       let(:feature) do
@@ -51,16 +59,16 @@ RSpec.describe RuboCop::FeatureLoader do
       end
 
       it 'loads it as namespaced feature' do
-        expect(described_class).to receive(:call_require).with('feature/foo')
+        expect_feature_loader.to receive(:require).with('feature/foo')
         load
       end
     end
 
     context 'with dot-prefixed namespaced feature' do
       before do
-        allow(described_class).to receive(:call_require).with('path-to-config/./feature-foo')
-                                                        .and_call_original
-        allow(described_class).to receive(:call_require).with('path-to-config/./feature/foo')
+        allow_feature_loader.to receive(:require).with('path-to-config/./feature-foo')
+                                                 .and_call_original
+        allow_feature_loader.to receive(:require).with('path-to-config/./feature/foo')
       end
 
       let(:feature) do
@@ -68,14 +76,14 @@ RSpec.describe RuboCop::FeatureLoader do
       end
 
       it 'loads it as namespaced feature' do
-        expect(described_class).to receive(:call_require).with('path-to-config/./feature/foo')
+        expect_feature_loader.to receive(:require).with('path-to-config/./feature/foo')
         load
       end
     end
 
     context 'with unexpected LoadError from require' do
       before do
-        allow(described_class).to receive(:call_require).and_raise(LoadError)
+        allow_feature_loader.to receive(:require).and_raise(LoadError)
       end
 
       it 'raises LoadError' do

[koic]

Can you apply it to this PR?

[koic]

Thanks!