From 9e967ed28605e0296e71452210154f8bc5fc98ab Mon Sep 17 00:00:00 2001
From: Peter Leitzen <pleitzen@gitlab.com>
Date: Mon, 15 May 2023 07:26:28 +0200
Subject: [PATCH] Add and enable cop Graphql/ResourceNotAvailable
Current offenses are excludes via
rake rubocop:todo:generate[Graphql/ResourceNotAvailableError]
Put this cop in grace period.
---
.rubocop.yml | 5 ++
.../graphql/resource_not_available_error.yml | 42 +++++++++++++++++
.../graphql/resource_not_available_error.rb | 47 +++++++++++++++++++
rubocop/node_pattern_helper.rb | 14 ++++++
.../resource_not_available_error_spec.rb | 37 +++++++++++++++
spec/rubocop/node_pattern_helper_spec.rb | 20 ++++++++
6 files changed, 165 insertions(+)
create mode 100644 .rubocop_todo/graphql/resource_not_available_error.yml
create mode 100644 rubocop/cop/graphql/resource_not_available_error.rb
create mode 100644 rubocop/node_pattern_helper.rb
create mode 100644 spec/rubocop/cop/graphql/resource_not_available_error_spec.rb
create mode 100644 spec/rubocop/node_pattern_helper_spec.rb
diff --git a/.rubocop.yml b/.rubocop.yml
index 81ad4cd31f354..9ceb846627056 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -1007,3 +1007,8 @@ SidekiqLoadBalancing/WorkerDataConsistency:
# This cop is disabled for Ruby 3.0+ anyway.
Lint/NonDeterministicRequireOrder:
Enabled: false
+
+Graphql/ResourceNotAvailableError:
+ Exclude:
+ # Definition of `raise_resource_not_available_error!`
+ - 'lib/gitlab/graphql/authorize/authorize_resource.rb'
diff --git a/.rubocop_todo/graphql/resource_not_available_error.yml b/.rubocop_todo/graphql/resource_not_available_error.yml
new file mode 100644
index 0000000000000..68e68c51277c7
--- /dev/null
+++ b/.rubocop_todo/graphql/resource_not_available_error.yml
@@ -0,0 +1,42 @@
+---
+# Cop supports --autocorrect.
+Graphql/ResourceNotAvailableError:
+ Details: grace period
+ Exclude:
+ - 'app/graphql/mutations/achievements/create.rb'
+ - 'app/graphql/mutations/admin/sidekiq_queues/delete_jobs.rb'
+ - 'app/graphql/mutations/ci/ci_cd_settings_update.rb'
+ - 'app/graphql/mutations/ci/job_artifact/bulk_destroy.rb'
+ - 'app/graphql/mutations/ci/runner/create.rb'
+ - 'app/graphql/mutations/custom_emoji/create.rb'
+ - 'app/graphql/mutations/custom_emoji/destroy.rb'
+ - 'app/graphql/mutations/design_management/move.rb'
+ - 'app/graphql/mutations/issues/bulk_update.rb'
+ - 'app/graphql/mutations/issues/set_crm_contacts.rb'
+ - 'app/graphql/mutations/issues/set_escalation_status.rb'
+ - 'app/graphql/mutations/notes/create/base.rb'
+ - 'app/graphql/mutations/notes/create/note.rb'
+ - 'app/graphql/mutations/notes/reposition_image_diff_note.rb'
+ - 'app/graphql/mutations/notes/update/image_diff_note.rb'
+ - 'app/graphql/mutations/saved_replies/create.rb'
+ - 'app/graphql/mutations/saved_replies/destroy.rb'
+ - 'app/graphql/mutations/saved_replies/update.rb'
+ - 'app/graphql/mutations/todos/mark_all_done.rb'
+ - 'app/graphql/mutations/work_items/export.rb'
+ - 'app/graphql/resolvers/ci/runner_setup_resolver.rb'
+ - 'app/graphql/resolvers/concerns/search_arguments.rb'
+ - 'app/graphql/resolvers/container_repository_tags_resolver.rb'
+ - 'app/graphql/resolvers/design_management/versions_resolver.rb'
+ - 'app/graphql/resolvers/kas/agent_configurations_resolver.rb'
+ - 'app/graphql/resolvers/kas/agent_connections_resolver.rb'
+ - 'app/graphql/resolvers/projects/snippets_resolver.rb'
+ - 'app/graphql/types/container_repository_details_type.rb'
+ - 'app/graphql/types/container_repository_type.rb'
+ - 'ee/app/graphql/ee/types/query_type.rb'
+ - 'ee/app/graphql/mutations/ai/action.rb'
+ - 'ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/base.rb'
+ - 'ee/app/graphql/mutations/issues/set_escalation_policy.rb'
+ - 'ee/app/graphql/mutations/projects/set_locked.rb'
+ - 'ee/app/graphql/resolvers/incident_management/oncall_shifts_resolver.rb'
+ - 'ee/app/graphql/resolvers/product_analytics/visualization_resolver.rb'
+ - 'ee/app/graphql/resolvers/remote_development/workspaces_resolver.rb'
diff --git a/rubocop/cop/graphql/resource_not_available_error.rb b/rubocop/cop/graphql/resource_not_available_error.rb
new file mode 100644
index 0000000000000..d759e14500874
--- /dev/null
+++ b/rubocop/cop/graphql/resource_not_available_error.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require_relative '../../node_pattern_helper'
+
+module RuboCop
+ module Cop
+ module Graphql
+ # Encourages the use of `raise_resource_not_available_error!` method
+ # instead of `raise Gitlab::Graphql::Errors::ResourceNotAvailable`.
+ #
+ # @example
+ #
+ # # bad
+ # raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'message'
+ #
+ # # good
+ # raise_resource_not_available_error! 'message'
+ class ResourceNotAvailableError < Base
+ extend NodePatternHelper
+ extend AutoCorrector
+
+ MSG = 'Prefer using `raise_resource_not_available_error!` instead.'
+
+ EXCEPTION = 'Gitlab::Graphql::Errors::ResourceNotAvailable'
+
+ RESTRICT_ON_SEND = %i[raise].freeze
+
+ def_node_matcher :error, const_pattern(EXCEPTION)
+
+ def_node_matcher :raise_error, <<~PATTERN
+ (send nil? :raise #error $...)
+ PATTERN
+
+ def on_send(node)
+ raise_error(node) do |args|
+ add_offense(node) do |corrector|
+ replacement = +'raise_resource_not_available_error!'
+ replacement << " #{args.map(&:source).join(', ')}" if args.any?
+
+ corrector.replace(node, replacement)
+ end
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/rubocop/node_pattern_helper.rb b/rubocop/node_pattern_helper.rb
new file mode 100644
index 0000000000000..ecd4560763c5b
--- /dev/null
+++ b/rubocop/node_pattern_helper.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module RuboCop
+ module NodePatternHelper
+ # Returns a nested `(const ...)` node pattern for a full qualified +name+.
+ #
+ # @examples
+ # const_pattern 'Foo::Bar' # => (const (const {nil? cbase} :Foo) :Bar)
+ # const_pattern 'Foo::Bar', parent: ':Baz' # => (const (const :Baz :Foo) :Bar)
+ def const_pattern(name, parent: '{nil? cbase}')
+ name.split('::').inject(parent) { |memo, name_part| "(const #{memo} :#{name_part})" }
+ end
+ end
+end
diff --git a/spec/rubocop/cop/graphql/resource_not_available_error_spec.rb b/spec/rubocop/cop/graphql/resource_not_available_error_spec.rb
new file mode 100644
index 0000000000000..6003b9f39546b
--- /dev/null
+++ b/spec/rubocop/cop/graphql/resource_not_available_error_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'rubocop_spec_helper'
+
+require_relative '../../../../rubocop/cop/graphql/resource_not_available_error'
+
+RSpec.describe RuboCop::Cop::Graphql::ResourceNotAvailableError, feature_category: :shared do
+ shared_examples 'flagging and auto-correction' do |exception|
+ it "flags and auto-corrects `raise #{exception}`" do
+ expect_offense(<<~'RUBY', exception: exception)
+ raise %{exception}
+ ^^^^^^^{exception} Prefer using `raise_resource_not_available_error!` instead.
+
+ raise %{exception}, 'message ' \
+ ^^^^^^^{exception}^^^^^^^^^^^^^^ Prefer using `raise_resource_not_available_error!` instead.
+ 'with new lines'
+ RUBY
+
+ expect_correction(<<~'RUBY')
+ raise_resource_not_available_error!
+
+ raise_resource_not_available_error! 'message ' \
+ 'with new lines'
+ RUBY
+ end
+ end
+
+ it_behaves_like 'flagging and auto-correction', 'Gitlab::Graphql::Errors::ResourceNotAvailable'
+ it_behaves_like 'flagging and auto-correction', '::Gitlab::Graphql::Errors::ResourceNotAvailable'
+
+ it 'does not flag unrelated exceptions' do
+ expect_no_offenses(<<~RUBY)
+ raise Gitlab::Graphql::Errors::ResourceVeryAvailable
+ raise ::Gitlab::Graphql::Errors::ResourceVeryAvailable
+ RUBY
+ end
+end
diff --git a/spec/rubocop/node_pattern_helper_spec.rb b/spec/rubocop/node_pattern_helper_spec.rb
new file mode 100644
index 0000000000000..a141e81b61868
--- /dev/null
+++ b/spec/rubocop/node_pattern_helper_spec.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+require 'rubocop_spec_helper'
+
+require_relative '../../rubocop/node_pattern_helper'
+
+RSpec.describe RuboCop::NodePatternHelper, feature_category: :tooling do
+ include described_class
+
+ describe '#const_pattern' do
+ it 'returns nested const node patterns' do
+ expect(const_pattern('Foo')).to eq('(const {nil? cbase} :Foo)')
+ expect(const_pattern('Foo::Bar')).to eq('(const (const {nil? cbase} :Foo) :Bar)')
+ end
+
+ it 'returns nested const node patterns with custom parent' do
+ expect(const_pattern('Foo::Bar', parent: 'nil?')).to eq('(const (const nil? :Foo) :Bar)')
+ end
+ end
+end
--
GitLab