Merge branch 'ajk-rubocop-resolver-type' into 'master'

hAdd a rule to enforce resolver type annotations

See merge request gitlab-org/gitlab!47375

.rubocop.yml

@@ -355,6 +355,12 @@ Cop/SidekiqOptionsQueue:
     - 'spec/**/*.rb'
     - 'ee/spec/**/*.rb'
 
+Graphql/ResolverType:
+  Enabled: true
+  Include:
+    - 'app/graphql/resolvers/**/*'
+    - 'ee/app/graphql/resolvers/**/*'
+
 Graphql/AuthorizeTypes:
   Enabled: true
   Include:

.rubocop_manual_todo.yml

@@ -45,6 +45,35 @@ Graphql/IDType:
     - 'app/graphql/resolvers/snippets_resolver.rb'
     - 'app/graphql/resolvers/user_merge_requests_resolver.rb'
 
+Graphql/ResolverType:
+  Exclude:
+    - 'app/graphql/resolvers/assigned_merge_requests_resolver.rb'
+    - 'app/graphql/resolvers/authored_merge_requests_resolver.rb'
+    - 'app/graphql/resolvers/base_resolver.rb'
+    - 'app/graphql/resolvers/ci/pipeline_stages_resolver.rb'
+    - 'app/graphql/resolvers/commit_pipelines_resolver.rb'
+    - 'app/graphql/resolvers/error_tracking/sentry_error_stack_trace_resolver.rb'
+    - 'app/graphql/resolvers/group_issues_resolver.rb'
+    - 'app/graphql/resolvers/group_merge_requests_resolver.rb'
+    - 'app/graphql/resolvers/group_milestones_resolver.rb'
+    - 'app/graphql/resolvers/members_resolver.rb'
+    - 'app/graphql/resolvers/merge_request_pipelines_resolver.rb'
+    - 'app/graphql/resolvers/merge_request_resolver.rb'
+    - 'app/graphql/resolvers/merge_requests_resolver.rb'
+    - 'app/graphql/resolvers/project_merge_requests_resolver.rb'
+    - 'app/graphql/resolvers/project_milestones_resolver.rb'
+    - 'app/graphql/resolvers/project_pipelines_resolver.rb'
+    - 'app/graphql/resolvers/projects/snippets_resolver.rb'
+    - 'app/graphql/resolvers/snippets_resolver.rb'
+    - 'app/graphql/resolvers/user_merge_requests_resolver.rb'
+    - 'app/graphql/resolvers/users/group_count_resolver.rb'
+    - 'app/graphql/resolvers/users/snippets_resolver.rb'
+    - 'ee/app/graphql/resolvers/ci/jobs_resolver.rb'
+    - 'ee/app/graphql/resolvers/geo/merge_request_diff_registries_resolver.rb'
+    - 'ee/app/graphql/resolvers/geo/package_file_registries_resolver.rb'
+    - 'ee/app/graphql/resolvers/geo/terraform_state_version_registries_resolver.rb'
+    - 'ee/app/graphql/resolvers/vulnerabilities_base_resolver.rb'
+
 Rails/SaveBang:
   Exclude:
     - 'ee/spec/controllers/projects/merge_requests_controller_spec.rb'

rubocop/cop/graphql/resolver_type.rb

+# frozen_string_literal: true
+
+# This cop checks for missing GraphQL type annotations on resolvers
+#
+# @example
+#
+#   # bad
+#   module Resolvers
+#     class NoTypeResolver < BaseResolver
+#       field :some_field, GraphQL::STRING_TYPE
+#     end
+#   end
+#
+#   # good
+#   module Resolvers
+#     class WithTypeResolver < BaseResolver
+#       type MyType, null: true
+#
+#       field :some_field, GraphQL::STRING_TYPE
+#     end
+#   end
+
+module RuboCop
+  module Cop
+    module Graphql
+      class ResolverType < RuboCop::Cop::Cop
+        MSG = 'Missing type annotation: Please add `type` DSL method call. ' \
+          'e.g: type UserType.connection_type, null: true'
+
+        def_node_matcher :typed?, <<~PATTERN
+          (... (begin <(send nil? :type ...) ...>))
+        PATTERN
+
+        def on_class(node)
+          add_offense(node, location: :expression) if resolver?(node) && !typed?(node)
+        end
+
+        private
+
+        def resolver?(node)
+          node.loc.name.source.end_with?('Resolver')
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/graphql/resolver_type_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rubocop'
+
+require_relative '../../../../rubocop/cop/graphql/resolver_type'
+
+RSpec.describe RuboCop::Cop::Graphql::ResolverType, type: :rubocop do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  it 'adds an offense when there is no type annotaion' do
+    lacks_type = <<-SRC
+      module Resolvers
+        class FooResolver < BaseResolver
+          def resolve(**args)
+            [:thing]
+          end
+        end
+      end
+    SRC
+
+    inspect_source(lacks_type)
+
+    expect(cop.offenses.size).to eq 1
+  end
+
+  it 'does not add an offense for resolvers that have a type call' do
+    expect_no_offenses(<<-SRC)
+      module Resolvers
+        class FooResolver < BaseResolver
+          type [SomeEnum], null: true
+
+          def resolve(**args)
+            [:thing]
+          end
+        end
+      end
+    SRC
+  end
+
+  it 'ignores type calls on other objects' do
+    lacks_type = <<-SRC
+      module Resolvers
+        class FooResolver < BaseResolver
+          class FalsePositive < BaseObject
+            type RedHerringType, null: true
+          end
+
+          def resolve(**args)
+            [:thing]
+          end
+        end
+      end
+    SRC
+
+    inspect_source(lacks_type)
+
+    expect(cop.offenses.size).to eq 1
+  end
+
+  it 'does not add an offense unless the class is named using the Resolver convention' do
+    expect_no_offenses(<<-TYPE)
+      module Resolvers
+        class FooThingy
+          def something_other_than_resolve(**args)
+            [:thing]
+          end
+        end
+      end
+    TYPE
+  end
+end