Merge branch 'bw-graphql-name-position-cop' into 'master'

[graphql] Cop to ensure graphql_name is first line in class

See merge request gitlab-org/gitlab!80522

app/graphql/types/base_enum.rb

 # frozen_string_literal: true
 
+# rubocop:disable Graphql/GraphqlNamePosition
 module Types
   class BaseEnum < GraphQL::Schema::Enum
     class CustomValue < GraphQL::Schema::EnumValue

doc/development/api_graphql_styleguide.md

@@ -442,10 +442,10 @@ booleans:
 
 ```ruby
 class MergeRequestPermissionsType < BasePermissionType
-  present_using MergeRequestPresenter
-
   graphql_name 'MergeRequestPermissions'
 
+  present_using MergeRequestPresenter
+
   abilities :admin_merge_request, :update_merge_request, :create_note
 
   ability_field :resolve_note,
@@ -1329,6 +1329,10 @@ class UserUpdateMutation < BaseMutation
 end
 ```
 
+Due to changes in the `1.13` version of the `graphql-ruby` gem, `graphql_name` should be the first
+line of the class to ensure that type names are generated correctly. The `Graphql::GraphqlNamePosition` cop enforces this.
+See [issue #27536](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/27536#note_840245581) for further context.
+
 Our GraphQL mutation names are historically inconsistent, but new mutation names should follow the
 convention `'{Resource}{Action}'` or `'{Resource}{Action}{Attribute}'`.
 
@@ -1511,9 +1515,9 @@ GraphQL-name of the mutation:
 ```ruby
 module Types
   class MutationType < BaseObject
-    include Gitlab::Graphql::MountMutation
+    graphql_name 'Mutation'
 
-    graphql_name "Mutation"
+    include Gitlab::Graphql::MountMutation
 
     mount_mutation Mutations::MergeRequests::SetDraft
   end

ee/app/graphql/types/geo/job_artifact_registry_type.rb

@@ -4,11 +4,11 @@ module Types
   module Geo
     # rubocop:disable Graphql/AuthorizeTypes because it is included
     class JobArtifactRegistryType < BaseObject
-      include ::Types::Geo::RegistryType
-
       graphql_name 'JobArtifactRegistry'
       description 'Represents the Geo replication and verification state of a job_artifact.'
 
+      include ::Types::Geo::RegistryType
+
       field :artifact_id, GraphQL::Types::ID, null: false, description: 'ID of the Job Artifact.'
     end
   end

rubocop/cop/graphql/graphql_name_position.rb

+# frozen_string_literal: true
+
+# This cop ensures that if a class uses `graphql_name`, then
+# it's the first line of the class
+#
+# @example
+#
+#   # bad
+#   class AwfulClass
+#     field :some_field, GraphQL::Types::JSON
+#     graphql_name 'AwfulClass'
+#   end
+#
+#   # good
+#   class GreatClass
+#     graphql_name 'AwfulClass'
+#     field :some_field, GraphQL::Types::String
+#   end
+
+module RuboCop
+  module Cop
+    module Graphql
+      class GraphqlNamePosition < RuboCop::Cop::Cop
+        MSG = '`graphql_name` should be the first line of the class: '\
+              'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#naming-conventions'
+
+        def_node_search :graphql_name?, <<~PATTERN
+          (send nil? :graphql_name ...)
+        PATTERN
+
+        def on_class(node)
+          return unless graphql_name?(node)
+          return if node.body.single_line?
+
+          add_offense(node, location: :expression) unless graphql_name?(node.body.children.first)
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/graphql/graphql_name_position_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+require_relative '../../../../rubocop/cop/graphql/graphql_name_position'
+
+RSpec.describe RuboCop::Cop::Graphql::GraphqlNamePosition do
+  subject(:cop) { described_class.new }
+
+  it 'adds an offense when graphql_name is not on the first line' do
+    expect_offense(<<~TYPE)
+      module Types
+        class AType < BaseObject
+        ^^^^^^^^^^^^^^^^^^^^^^^^ `graphql_name` should be the first line of the class: https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#naming-conventions
+          field :a_thing
+          field :another_thing
+          graphql_name 'ATypeName'
+        end
+      end
+    TYPE
+  end
+
+  it 'does not add an offense for classes that have no call to graphql_name' do
+    expect_no_offenses(<<~TYPE.strip)
+      module Types
+        class AType < BaseObject
+          authorize :an_ability, :second_ability
+
+          field :a_thing
+        end
+      end
+    TYPE
+  end
+
+  it 'does not add an offense for classes that only call graphql_name' do
+    expect_no_offenses(<<~TYPE.strip)
+      module Types
+        class AType < BaseObject
+          graphql_name 'ATypeName'
+        end
+      end
+    TYPE
+  end
+end