Skip to content
代码片段 群组 项目
提交 cfc8067c 编辑于 作者: Luke Duncalfe's avatar Luke Duncalfe
浏览文件

Add cop to ban using GraphQL ID type for IIDs

This enforces some of the guidelines added around use of `ID` type
added in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/152002.
上级 d48bba82
No related branches found
No related tags found
无相关合并请求
......@@ -9,7 +9,7 @@ class Base < ::Mutations::BaseMutation
required: true,
description: "Project where the issue is to upload designs for."
argument :iid, GraphQL::Types::ID,
argument :iid, GraphQL::Types::ID, # rubocop:disable Graphql/IDType -- Legacy argument using ID type kept for backwards compatibility
required: true,
description: "IID of the issue to modify designs for."
......
......@@ -14,7 +14,7 @@ class ProjectPipelineResolver < BaseResolver
description: 'Global ID of the Pipeline. For example, "gid://gitlab/Ci::Pipeline/314".',
prepare: ->(pipeline_id, _ctx) { pipeline_id.model_id }
argument :iid, GraphQL::Types::ID,
argument :iid, GraphQL::Types::ID, # rubocop:disable Graphql/IDType -- Legacy argument using ID type kept for backwards compatibility
required: false,
description: 'IID of the Pipeline. For example, "1".'
......
......@@ -2,7 +2,7 @@
module Resolvers
class DeploymentResolver < BaseResolver
argument :iid,
argument :iid, # rubocop:disable Graphql/IDType -- Legacy argument using ID type kept for backwards compatibility
GraphQL::Types::ID,
required: true,
description: 'Project-level internal ID of the Deployment.'
......
......@@ -14,7 +14,7 @@ class MilestoneType < BaseObject
field :id, GraphQL::Types::ID, null: false,
description: 'ID of the milestone.'
field :iid, GraphQL::Types::ID, null: false,
field :iid, GraphQL::Types::ID, null: false, # rubocop:disable Graphql/IDType -- Legacy argument using ID type kept for backwards compatibility
description: "Internal ID of the milestone."
field :title, GraphQL::Types::String, null: false,
......
......@@ -5,7 +5,7 @@ module Epics
class Base < ::Mutations::BaseMutation
include Mutations::ResolvesIssuable
argument :iid, GraphQL::Types::ID,
argument :iid, GraphQL::Types::ID, # rubocop:disable Graphql/IDType -- Legacy argument using ID type kept for backwards compatibility
required: true,
description: "IID of the epic to mutate."
......
......@@ -6,7 +6,7 @@ class EpicsResolver < BaseResolver
include SearchArguments
include LooksAhead
argument :iid, GraphQL::Types::ID,
argument :iid, GraphQL::Types::ID, # rubocop:disable Graphql/IDType -- Legacy argument using ID type kept for backwards compatibility
required: false,
description: 'IID of the epic, e.g., "1".'
......
......@@ -29,9 +29,10 @@ class IterationsResolver < BaseResolver
description: 'Global ID of the Iteration to look up.'
# rubocop:enable Graphql/IDType
argument :iid, GraphQL::Types::ID,
argument :iid, GraphQL::Types::ID, # rubocop:disable Graphql/IDType -- Legacy argument using ID type kept for backwards compatibility
required: false,
description: 'Internal ID of the Iteration to look up.'
argument :include_ancestors, GraphQL::Types::Boolean,
required: false,
description: 'Whether to include ancestor iterations. Defaults to true.'
......
......@@ -8,7 +8,7 @@ class RequirementsResolver < BaseResolver
type ::Types::RequirementsManagement::RequirementType.connection_type, null: true
argument :iid, GraphQL::Types::ID,
argument :iid, GraphQL::Types::ID, # rubocop:disable Graphql/IDType -- Legacy argument using ID type kept for backwards compatibility
required: false,
deprecated: { reason: 'Use work_item_iid instead', milestone: '15.8' },
description: 'IID of the requirement, for example, "1".'
......
......@@ -4,27 +4,40 @@ module RuboCop
module Cop
module Graphql
class IDType < RuboCop::Cop::Base
MSG = 'Do not use GraphQL::Types::ID, use a specific GlobalIDType instead'
MSG_USE_STRING_FOR_IID = 'Do not use GraphQL::Types::ID for IIDs, use GraphQL::Types::String instead'
MSG_USE_GLOBAL_ID = 'Do not use GraphQL::Types::ID, use a specific GlobalIDType instead'
ALLOWLISTED_ARGUMENTS = %i[
iid full_path project_path group_path target_project_path target_group_path target_path namespace_path
full_path project_path group_path target_project_path target_group_path target_path namespace_path
context_namespace_path
].freeze
def_node_search :graphql_id_type?, <<~PATTERN
def_node_matcher :iid_with_id?, <<~PATTERN
(send nil? {:field :argument}
(sym #iid?)
(const (const (const nil? :GraphQL) :Types) :ID)
(...)?)
PATTERN
def_node_search :graphql_id_allowed?, <<~PATTERN
(send nil? :argument (_ #does_not_match?) (const (const (const nil? :GraphQL) :Types) :ID) ...)
PATTERN
def on_send(node)
return unless graphql_id_type?(node)
return add_offense(node, message: MSG_USE_STRING_FOR_IID) if iid_with_id?(node)
return unless graphql_id_allowed?(node)
add_offense(node)
add_offense(node, message: MSG_USE_GLOBAL_ID)
end
private
def does_not_match?(arg)
!ALLOWLISTED_ARGUMENTS.include?(arg) # rubocop:disable Rails/NegateInclude
def does_not_match?(arg_name)
ALLOWLISTED_ARGUMENTS.exclude?(arg_name)
end
def iid?(arg_name)
arg_name == :iid || arg_name.end_with?('_iid')
end
end
end
......
......@@ -4,7 +4,7 @@
require_relative '../../../../rubocop/cop/graphql/id_type'
RSpec.describe RuboCop::Cop::Graphql::IDType do
RSpec.describe RuboCop::Cop::Graphql::IDType, feature_category: :api do
it 'adds an offense when GraphQL::Types::ID is used as a param to #argument' do
expect_offense(<<~TYPE)
argument :some_arg, GraphQL::Types::ID, some: other, params: do_not_matter
......@@ -27,4 +27,56 @@
argument :some_arg, ::Types::GlobalIDType[::Awardable], some: other, params: do_not_matter
TYPE
end
it 'adds an offense when GraphQL::Types::ID is used with an IID field' do
expect_offense(<<~TYPE)
field :iid, GraphQL::Types::ID, some: other, params: do_not_matter
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use GraphQL::Types::ID for IIDs, use GraphQL::Types::String instead
TYPE
end
it 'adds an offense when GraphQL::Types::ID is used with an IID-like field' do
expect_offense(<<~TYPE)
field :issue_iid, GraphQL::Types::ID, some: other, params: do_not_matter
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use GraphQL::Types::ID for IIDs, use GraphQL::Types::String instead
TYPE
end
it 'adds an offense when GraphQL::Types::ID is used with an IID argument' do
expect_offense(<<~TYPE)
argument :iid, GraphQL::Types::ID, some: other, params: do_not_matter
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use GraphQL::Types::ID for IIDs, use GraphQL::Types::String instead
TYPE
end
it 'adds an offense when GraphQL::Types::ID is used with an IID-like argument' do
expect_offense(<<~TYPE)
argument :merge_request_iid, GraphQL::Types::ID, some: other, params: do_not_matter
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use GraphQL::Types::ID for IIDs, use GraphQL::Types::String instead
TYPE
end
it 'does not add an offense when GraphQL::Types::String is used with an IID field' do
expect_no_offenses(<<~TYPE)
field :iid, GraphQL::Types::String, some: other, params: do_not_matter
TYPE
end
it 'does not add an offense when GraphQL::Types::String is used with an IID-like field' do
expect_no_offenses(<<~TYPE)
field :issue_iid, GraphQL::Types::String, some: other, params: do_not_matter
TYPE
end
it 'does not add an offense when GraphQL::Types::String is used with an IID argument' do
expect_no_offenses(<<~TYPE)
argument :iid, GraphQL::Types::String, some: other, params: do_not_matter
TYPE
end
it 'does not add an offense when GraphQL::Types::String is used with an IID-like argument' do
expect_no_offenses(<<~TYPE)
argument :merge_request_iid, GraphQL::Types::String, some: other, params: do_not_matter
TYPE
end
end
0% 加载中 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册