Merge branch '34997-replace-any-instance-of-with-next-instance-of' into 'master'

Rubocop Cop to auto-correct all *_any_instance_of spec helpers

See merge request gitlab-org/gitlab!19164

.rubocop.yml

@@ -297,3 +297,6 @@ Graphql/Descriptions:
   Include:
     - 'app/graphql/**/*'
     - 'ee/app/graphql/**/*'
+
+RSpec/AnyInstanceOf:
+  Enabled: false

rubocop/cop/rspec/any_instance_of.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # This cop checks for `allow_any_instance_of` or `expect_any_instance_of`
+      # usage in specs.
+      #
+      # @example
+      #
+      #   # bad
+      #   allow_any_instance_of(User).to receive(:invalidate_issue_cache_counts)
+      #
+      #   # bad
+      #   expect_any_instance_of(User).to receive(:invalidate_issue_cache_counts)
+      #
+      #   # good
+      #   allow_next_instance_of(User) do |instance|
+      #     allow(instance).to receive(:invalidate_issue_cache_counts)
+      #   end
+      #
+      #   # good
+      #   expect_next_instance_of(User) do |instance|
+      #     expect(instance).to receive(:invalidate_issue_cache_counts)
+      #   end
+      #
+      class AnyInstanceOf < RuboCop::Cop::Cop
+        MESSAGE_EXPECT = 'Do not use `expect_any_instance_of` method, use `expect_next_instance_of` instead.'
+        MESSAGE_ALLOW = 'Do not use `allow_any_instance_of` method, use `allow_next_instance_of` instead.'
+
+        def_node_search :expect_any_instance_of?, <<~PATTERN
+          (send (send nil? :expect_any_instance_of ...) ...)
+        PATTERN
+        def_node_search :allow_any_instance_of?, <<~PATTERN
+          (send (send nil? :allow_any_instance_of ...) ...)
+        PATTERN
+
+        def on_send(node)
+          if expect_any_instance_of?(node)
+            add_offense(node, location: :expression, message: MESSAGE_EXPECT)
+          elsif allow_any_instance_of?(node)
+            add_offense(node, location: :expression, message: MESSAGE_ALLOW)
+          end
+        end
+
+        def autocorrect(node)
+          replacement =
+            if expect_any_instance_of?(node)
+              replacement_any_instance_of(node, 'expect')
+            elsif allow_any_instance_of?(node)
+              replacement_any_instance_of(node, 'allow')
+            end
+
+          lambda do |corrector|
+            corrector.replace(node.loc.expression, replacement)
+          end
+        end
+
+        private
+
+        def replacement_any_instance_of(node, rspec_prefix)
+          method_call =
+            node.receiver.source.sub(
+              "#{rspec_prefix}_any_instance_of",
+              "#{rspec_prefix}_next_instance_of")
+
+          block = <<~RUBY.chomp
+            do |instance|
+              #{rspec_prefix}(instance).#{node.method_name} #{node.children.last.source}
+            end
+          RUBY
+
+          "#{method_call} #{block}"
+        end
+      end
+    end
+  end
+end

rubocop/rubocop.rb

@@ -32,6 +32,7 @@ require_relative 'cop/migration/timestamps'
 require_relative 'cop/migration/update_column_in_batches'
 require_relative 'cop/migration/update_large_table'
 require_relative 'cop/project_path_helper'
+require_relative 'cop/rspec/any_instance_of'
 require_relative 'cop/rspec/be_success_matcher'
 require_relative 'cop/rspec/env_assignment'
 require_relative 'cop/rspec/factories_in_migration_specs'

spec/controllers/abuse_reports_controller_spec.rb

@@ -49,7 +49,9 @@ describe AbuseReportsController do
       end
 
       it 'calls notify' do
-        expect_any_instance_of(AbuseReport).to receive(:notify)
+        expect_next_instance_of(AbuseReport) do |instance|
+          expect(instance).to receive(:notify)
+        end
 
         post :create, params: { abuse_report: attrs }
       end

spec/controllers/google_api/authorizations_controller_spec.rb

@@ -13,8 +13,9 @@ describe GoogleApi::AuthorizationsController do
     before do
       sign_in(user)
 
-      allow_any_instance_of(GoogleApi::CloudPlatform::Client)
-        .to receive(:get_token).and_return([token, expires_at])
+      allow_next_instance_of(GoogleApi::CloudPlatform::Client) do |instance|
+        allow(instance).to receive(:get_token).and_return([token, expires_at])
+      end
     end
 
     shared_examples_for 'access denied' do

spec/controllers/metrics_controller_spec.rb

@@ -23,7 +23,9 @@ describe MetricsController do
     allow(Prometheus::Client.configuration).to receive(:multiprocess_files_dir).and_return(metrics_multiproc_dir)
     allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(true)
     allow(Settings.monitoring).to receive(:ip_whitelist).and_return([whitelisted_ip, whitelisted_ip_range])
-    allow_any_instance_of(MetricsService).to receive(:metrics_text).and_return("prometheus_counter 1")
+    allow_next_instance_of(MetricsService) do |instance|
+      allow(instance).to receive(:metrics_text).and_return("prometheus_counter 1")
+    end
   end
 
   describe '#index' do

spec/controllers/snippets_controller_spec.rb

@@ -251,7 +251,9 @@ describe SnippetsController do
 
     context 'when the snippet is spam' do
       before do
-        allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true)
+        allow_next_instance_of(AkismetService) do |instance|
+          allow(instance).to receive(:spam?).and_return(true)
+        end
       end
 
       context 'when the snippet is private' do
@@ -323,7 +325,9 @@ describe SnippetsController do
 
     context 'when the snippet is spam' do
       before do
-        allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true)
+        allow_next_instance_of(AkismetService) do |instance|
+          allow(instance).to receive(:spam?).and_return(true)
+        end
       end
 
       context 'when the snippet is private' do
@@ -431,7 +435,9 @@ describe SnippetsController do
     let(:snippet) { create(:personal_snippet, :public, author: user) }
 
     before do
-      allow_any_instance_of(AkismetService).to receive_messages(submit_spam: true)
+      allow_next_instance_of(AkismetService) do |instance|
+        allow(instance).to receive_messages(submit_spam: true)
+      end
       stub_application_setting(akismet_enabled: true)
     end
 

spec/dependencies/omniauth_saml_spec.rb

@@ -14,7 +14,9 @@ describe 'processing of SAMLResponse in dependencies' do
 
   before do
     allow(saml_strategy).to receive(:session).and_return(session_mock)
-    allow_any_instance_of(OneLogin::RubySaml::Response).to receive(:is_valid?).and_return(true)
+    allow_next_instance_of(OneLogin::RubySaml::Response) do |instance|
+      allow(instance).to receive(:is_valid?).and_return(true)
+    end
     saml_strategy.send(:handle_response, mock_saml_response, {}, settings ) { }
   end
 

spec/helpers/application_helper_spec.rb

@@ -210,7 +210,9 @@ describe ApplicationHelper do
       let(:user) { create(:user, static_object_token: 'hunter1') }
 
       before do
-        allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_url).and_return('https://cdn.gitlab.com')
+        allow_next_instance_of(ApplicationSetting) do |instance|
+          allow(instance).to receive(:static_objects_external_storage_url).and_return('https://cdn.gitlab.com')
+        end
         allow(helper).to receive(:current_user).and_return(user)
       end
 

spec/rubocop/cop/rspec/any_instance_of_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require_relative '../../../../rubocop/cop/rspec/any_instance_of'
+
+describe RuboCop::Cop::RSpec::AnyInstanceOf do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'when calling allow_any_instance_of' do
+    let(:source) do
+      <<~SRC
+      allow_any_instance_of(User).to receive(:invalidate_issue_cache_counts)
+      SRC
+    end
+    let(:corrected_source) do
+      <<~SRC
+      allow_next_instance_of(User) do |instance|
+        allow(instance).to receive(:invalidate_issue_cache_counts)
+      end
+      SRC
+    end
+
+    it 'registers an offence' do
+      inspect_source(source)
+
+      expect(cop.offenses.size).to eq(1)
+    end
+
+    it 'can autocorrect the source' do
+      expect(autocorrect_source(source)).to eq(corrected_source)
+    end
+  end
+
+  context 'when calling expect_any_instance_of' do
+    let(:source) do
+      <<~SRC
+      expect_any_instance_of(User).to receive(:invalidate_issue_cache_counts).with(args).and_return(double)
+      SRC
+    end
+    let(:corrected_source) do
+      <<~SRC
+      expect_next_instance_of(User) do |instance|
+        expect(instance).to receive(:invalidate_issue_cache_counts).with(args).and_return(double)
+      end
+      SRC
+    end
+
+    it 'registers an offence' do
+      inspect_source(source)
+
+      expect(cop.offenses.size).to eq(1)
+    end
+
+    it 'can autocorrect the source' do
+      expect(autocorrect_source(source)).to eq(corrected_source)
+    end
+  end
+end