diff --git a/.rubocop.yml b/.rubocop.yml index a3f1f57f1401c46f7ce42fa8341e727040591a78..1d5cf7642c2c03c23e4ad001eaf539f5e0c65073 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -297,3 +297,6 @@ Graphql/Descriptions: Include: - 'app/graphql/**/*' - 'ee/app/graphql/**/*' + +RSpec/AnyInstanceOf: + Enabled: false diff --git a/rubocop/cop/rspec/any_instance_of.rb b/rubocop/cop/rspec/any_instance_of.rb new file mode 100644 index 0000000000000000000000000000000000000000..a939af36c13054a5b65ddbe74c44e34bd7fc12e5 --- /dev/null +++ b/rubocop/cop/rspec/any_instance_of.rb @@ -0,0 +1,78 @@ +# 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 diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 70679aa1e780b93983314ef1845c92c6523feb39..159892ae0c1ea510f47c939337a9e532a65fab50 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -32,6 +32,7 @@ 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' diff --git a/spec/controllers/abuse_reports_controller_spec.rb b/spec/controllers/abuse_reports_controller_spec.rb index e360ab68cf26b9a071142d1c3263be9244a37b00..e573ef4be49b04c9d2d45e96a4a93b88cf7d5c37 100644 --- a/spec/controllers/abuse_reports_controller_spec.rb +++ b/spec/controllers/abuse_reports_controller_spec.rb @@ -49,7 +49,9 @@ 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 diff --git a/spec/controllers/google_api/authorizations_controller_spec.rb b/spec/controllers/google_api/authorizations_controller_spec.rb index 940bf9c68286a7c443b978aa43b132855a80603d..4d200140f164b5919c6992e0a05f9dbf6952c501 100644 --- a/spec/controllers/google_api/authorizations_controller_spec.rb +++ b/spec/controllers/google_api/authorizations_controller_spec.rb @@ -13,8 +13,9 @@ 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 diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 7fb3578cd0ad0bb04eb3d1659a8ac18623c20422..1d378b9b9dc498309e952e17cdc7eff7864a8c3d 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -23,7 +23,9 @@ 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 diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index e892c736c69f6698775975b7ec7d3b88e150faf6..054d448c28deef929dba606f7a9f6c519e7de4ad 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -251,7 +251,9 @@ def create_snippet(snippet_params = {}, additional_params = {}) 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 @@ def update_snippet(snippet_params = {}, additional_params = {}) 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 @@ def update_snippet(snippet_params = {}, additional_params = {}) 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 diff --git a/spec/dependencies/omniauth_saml_spec.rb b/spec/dependencies/omniauth_saml_spec.rb index 8a685648c71596a25f5df679ad299eb65f96486d..e0ea9c38e697eca0beb9fd0b5cf758f6c6297b57 100644 --- a/spec/dependencies/omniauth_saml_spec.rb +++ b/spec/dependencies/omniauth_saml_spec.rb @@ -14,7 +14,9 @@ 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 diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index e8c438e459b60d4b4b7ed374aa12730b0f4a8a4b..d3d25d3cb74e0d1cd218fe22a27bd230d6e93bfd 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -210,7 +210,9 @@ def element(*arguments) 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 diff --git a/spec/rubocop/cop/rspec/any_instance_of_spec.rb b/spec/rubocop/cop/rspec/any_instance_of_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b16f8ac189ce44b7439bf28b71f25bf441a67658 --- /dev/null +++ b/spec/rubocop/cop/rspec/any_instance_of_spec.rb @@ -0,0 +1,61 @@ +# 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