Merge pull request #5491 from rmosolgo/better-method-docs

Improve documentation for `field` and `argument` DSL methods

Author: rmosolgo

Gemfile

@@ -14,6 +14,7 @@ if RUBY_VERSION >= "3.0"
 end
 
 if RUBY_VERSION >= "3.2.0"
+  gem "minitest-mock"
   gem "async", "~>2.0"
   gem "minitest-mock"
 end

lib/graphql/schema/argument.rb

@@ -39,9 +39,14 @@ def from_resolver?
       # @param arg_name [Symbol]
       # @param type_expr
       # @param desc [String]
+      # @param type [Class, Array<Class>] Input type; positional argument also accepted
+      # @param name [Symbol] positional argument also accepted        # @param loads [Class, Array<Class>] A GraphQL type to load for the given ID when one is present
+      # @param definition_block [Proc] Called with the newly-created {Argument}
+      # @param owner [Class] Private, used by GraphQL-Ruby during schema definition
       # @param required [Boolean, :nullable] if true, this argument is non-null; if false, this argument is nullable. If `:nullable`, then the argument must be provided, though it may be `null`.
       # @param description [String]
       # @param default_value [Object]
+      # @param loads [Class, Array<Class>] A GraphQL type to load for the given ID when one is present
       # @param as [Symbol] Override the keyword name when passed to a method
       # @param prepare [Symbol] A method to call to transform this argument's valuebefore sending it to field resolution
       # @param camelize [Boolean] if true, the name will be camelized when building the schema
@@ -50,6 +55,8 @@ def from_resolver?
       # @param deprecation_reason [String]
       # @param validates [Hash, nil] Options for building validators, if any should be applied
       # @param replace_null_with_default [Boolean] if `true`, incoming values of `null` will be replaced with the configured `default_value`
+      # @param comment [String] Private, used by GraphQL-Ruby when parsing GraphQL schema files
+      # @param ast_node [GraphQL::Language::Nodes::InputValueDefinition] Private, used by GraphQL-Ruby when parsing schema files
       def initialize(arg_name = nil, type_expr = nil, desc = nil, required: true, type: nil, name: nil, loads: nil, description: nil, comment: nil, ast_node: nil, default_value: NOT_CONFIGURED, as: nil, from_resolver: false, camelize: true, prepare: nil, owner:, validates: nil, directives: nil, deprecation_reason: nil, replace_null_with_default: false, &definition_block)
         arg_name ||= name
         @name = -(camelize ? Member::BuildType.camelize(arg_name.to_s) : arg_name.to_s)

lib/graphql/schema/field.rb

@@ -209,6 +209,11 @@ def method_conflict_warning?
       # @param method_conflict_warning [Boolean] If false, skip the warning if this field's method conflicts with a built-in method
       # @param validates [Array<Hash>] Configurations for validating this field
       # @param fallback_value [Object] A fallback value if the method is not defined
+      # @param dynamic_introspection [Boolean] (Private, used by GraphQL-Ruby)
+      # @param relay_node_field [Boolean] (Private, used by GraphQL-Ruby)
+      # @param relay_nodes_field [Boolean] (Private, used by GraphQL-Ruby)
+      # @param extras [Array<:ast_node, :parent, :lookahead, :owner, :execution_errors, :graphql_name, :argument_details, Symbol>] Extra arguments to be injected into the resolver for this field
+      # @param definition_block [Proc] an additional block for configuring the field. Receive the field as a block param, or, if no block params are defined, then the block is `instance_eval`'d on the new {Field}.
       def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CONFIGURED, comment: NOT_CONFIGURED, deprecation_reason: nil, method: nil, hash_key: nil, dig: nil, resolver_method: nil, connection: nil, max_page_size: NOT_CONFIGURED, default_page_size: NOT_CONFIGURED, scope: nil, introspection: false, camelize: true, trace: nil, complexity: nil, ast_node: nil, extras: EMPTY_ARRAY, extensions: EMPTY_ARRAY, connection_extension: self.class.connection_extension, resolver_class: nil, subscription_scope: nil, relay_node_field: false, relay_nodes_field: false, method_conflict_warning: true, broadcastable: NOT_CONFIGURED, arguments: EMPTY_HASH, directives: EMPTY_HASH, validates: EMPTY_ARRAY, fallback_value: NOT_CONFIGURED, dynamic_introspection: false, &definition_block)
         if name.nil?
           raise ArgumentError, "missing first `name` argument or keyword `name:`"
@@ -301,7 +306,7 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CON
 
         @extensions = EMPTY_ARRAY
         @call_after_define = false
-        set_pagination_extensions(connection_extension: connection_extension)
+        set_pagination_extensions(connection_extension: NOT_CONFIGURED.equal?(connection_extension) ? self.class.connection_extension : connection_extension)
         # Do this last so we have as much context as possible when initializing them:
         if !extensions.empty?
           self.extensions(extensions)

lib/graphql/schema/member/has_arguments.rb

@@ -14,29 +14,52 @@ def self.extended(cls)
           cls.extend(ClassConfigured)
         end
 
-        # @see {GraphQL::Schema::Argument#initialize} for parameters
-        # @return [GraphQL::Schema::Argument] An instance of {argument_class}, created from `*args`
-        def argument(*args, **kwargs, &block)
-          kwargs[:owner] = self
-          loads = kwargs[:loads]
-          if loads
-            name = args[0]
-            name_as_string = name.to_s
-
-            inferred_arg_name = case name_as_string
+        # @param arg_name [Symbol] The underscore-cased name of this argument, `name:` keyword also accepted
+        # @param type_expr The GraphQL type of this argument; `type:` keyword also accepted
+        # @param desc [String] Argument description, `description:` keyword also accepted
+        # @option kwargs [Boolean, :nullable] :required if true, this argument is non-null; if false, this argument is nullable. If `:nullable`, then the argument must be provided, though it may be `null`.
+        # @option kwargs [String] :description Positional argument also accepted
+        # @option kwargs [Class, Array<Class>] :type Input type; positional argument also accepted
+        # @option kwargs [Symbol] :name positional argument also accepted
+        # @option kwargs [Object] :default_value
+        # @option kwargs [Class, Array<Class>] :loads A GraphQL type to load for the given ID when one is present
+        # @option kwargs [Symbol] :as Override the keyword name when passed to a method
+        # @option kwargs [Symbol] :prepare A method to call to transform this argument's valuebefore sending it to field resolution
+        # @option kwargs [Boolean] :camelize if true, the name will be camelized when building the schema
+        # @option kwargs [Boolean] :from_resolver if true, a Resolver class defined this argument
+        # @option kwargs [Hash{Class => Hash}] :directives
+        # @option kwargs [String] :deprecation_reason
+        # @option kwargs [String] :comment Private, used by GraphQL-Ruby when parsing GraphQL schema files
+        # @option kwargs [GraphQL::Language::Nodes::InputValueDefinition] :ast_node Private, used by GraphQL-Ruby when parsing schema files
+        # @option kwargs [Hash, nil] :validates Options for building validators, if any should be applied
+        # @option kwargs [Boolean] :replace_null_with_default if `true`, incoming values of `null` will be replaced with the configured `default_value`
+        # @param definition_block [Proc] Called with the newly-created {Argument}
+        # @param kwargs [Hash] Keywords for defining an argument. Any keywords not documented here must be handled by your base Argument class.
+        # @return [GraphQL::Schema::Argument] An instance of {argument_class} created from these arguments
+        def argument(arg_name = nil, type_expr = nil, desc = nil, **kwargs, &definition_block)
+          if kwargs[:loads]
+            loads_name = arg_name || kwargs[:name]
+            loads_name_as_string = loads_name.to_s
+
+            inferred_arg_name = case loads_name_as_string
             when /_id$/
-              name_as_string.sub(/_id$/, "").to_sym
+              loads_name_as_string.sub(/_id$/, "").to_sym
             when /_ids$/
-              name_as_string.sub(/_ids$/, "")
+              loads_name_as_string.sub(/_ids$/, "")
                 .sub(/([^s])$/, "\\1s")
                 .to_sym
             else
-              name
+              loads_name
             end
 
             kwargs[:as] ||= inferred_arg_name
           end
-          arg_defn = self.argument_class.new(*args, **kwargs, &block)
+          kwargs[:owner] = self
+          arg_defn = self.argument_class.new(
+            arg_name, type_expr, desc,
+            **kwargs,
+            &definition_block
+          )
           add_argument(arg_defn)
           arg_defn
         end

lib/graphql/schema/member/has_fields.rb

@@ -5,13 +5,52 @@ class Schema
     class Member
       # Shared code for Objects, Interfaces, Mutations, Subscriptions
       module HasFields
+        include EmptyObjects
         # Add a field to this object or interface with the given definition
-        # @param name_positional [Symbol] Keyword `name:` also supported
-        # @param type_positional [Class, Array<Class>] Keyword `type:` also supported
-        # @param desc_positional [String] Keyword `description:` also supported
-        # @see {GraphQL::Schema::Field#initialize} for keywords
+        # @param name_positional [Symbol] The underscore-cased version of this field name (will be camelized for the GraphQL API); `name:` keyword is also accepted
+        # @param type_positional [Class, GraphQL::BaseType, Array] The return type of this field; `type:` keyword is also accepted
+        # @param desc_positional [String] Field description; `description:` keyword is also accepted
+        # @option kwargs [Symbol] :name The underscore-cased version of this field name (will be camelized for the GraphQL API); positional argument also accepted
+        # @option kwargs [Class, GraphQL::BaseType, Array] :type The return type of this field; positional argument is also accepted
+        # @option kwargs [Boolean] :null (defaults to `true`) `true` if this field may return `null`, `false` if it is never `null`
+        # @option kwargs [String] :description Field description; positional argument also accepted
+        # @option kwargs [String] :comment Field comment
+        # @option kwargs [String] :deprecation_reason If present, the field is marked "deprecated" with this message
+        # @option kwargs [Symbol] :method The method to call on the underlying object to resolve this field (defaults to `name`)
+        # @option kwargs [String, Symbol] :hash_key The hash key to lookup on the underlying object (if its a Hash) to resolve this field (defaults to `name` or `name.to_s`)
+        # @option kwargs [Array<String, Symbol>] :dig The nested hash keys to lookup on the underlying hash to resolve this field using dig
+        # @option kwargs [Symbol] :resolver_method The method on the type to call to resolve this field (defaults to `name`)
+        # @option kwargs [Boolean] :connection `true` if this field should get automagic connection behavior; default is to infer by `*Connection` in the return type name
+        # @option kwargs [Class] :connection_extension The extension to add, to implement connections. If `nil`, no extension is added.
+        # @option kwargs [Integer, nil] :max_page_size For connections, the maximum number of items to return from this field, or `nil` to allow unlimited results.
+        # @option kwargs [Integer, nil] :default_page_size For connections, the default number of items to return from this field, or `nil` to return unlimited results.
+        # @option kwargs [Boolean] :introspection If true, this field will be marked as `#introspection?` and the name may begin with `__`
+        # @option kwargs [{String=>GraphQL::Schema::Argument, Hash}] :arguments Arguments for this field (may be added in the block, also)
+        # @option kwargs [Boolean] :camelize If true, the field name will be camelized when building the schema
+        # @option kwargs [Numeric] :complexity When provided, set the complexity for this field
+        # @option kwargs [Boolean] :scope If true, the return type's `.scope_items` method will be called on the return value
+        # @option kwargs [Symbol, String] :subscription_scope A key in `context` which will be used to scope subscription payloads
+        # @option kwargs [Array<Class, Hash<Class => Object>>] :extensions Named extensions to apply to this field (see also {#extension})
+        # @option kwargs [Hash{Class => Hash}] :directives Directives to apply to this field
+        # @option kwargs [Boolean] :trace If true, a {GraphQL::Tracing} tracer will measure this scalar field
+        # @option kwargs [Boolean] :broadcastable Whether or not this field can be distributed in subscription broadcasts
+        # @option kwargs [Language::Nodes::FieldDefinition, nil] :ast_node If this schema was parsed from definition, this AST node defined the field
+        # @option kwargs [Boolean] :method_conflict_warning If false, skip the warning if this field's method conflicts with a built-in method
+        # @option kwargs [Array<Hash>] :validates Configurations for validating this field
+        # @option kwargs [Object] :fallback_value A fallback value if the method is not defined
+        # @option kwargs [Class<GraphQL::Schema::Mutation>] :mutation
+        # @option kwargs [Class<GraphQL::Schema::Resolver>] :resolver
+        # @option kwargs [Class<GraphQL::Schema::Subscription>] :subscription
+        # @option kwargs [Boolean] :dynamic_introspection (Private, used by GraphQL-Ruby)
+        # @option kwargs [Boolean] :relay_node_field (Private, used by GraphQL-Ruby)
+        # @option kwargs [Boolean] :relay_nodes_field (Private, used by GraphQL-Ruby)
+        # @option kwargs [Array<:ast_node, :parent, :lookahead, :owner, :execution_errors, :graphql_name, :argument_details, Symbol>] :extras Extra arguments to be injected into the resolver for this field
+        # @param kwargs [Hash] Keywords for defining the field. Any not documented here will be passed to your base field class where they must be handled.
+        # @param definition_block [Proc] an additional block for configuring the field. Receive the field as a block param, or, if no block params are defined, then the block is `instance_eval`'d on the new {Field}.
+        # @yieldparam field [GraphQL::Schema::Field] The newly-created field instance
+        # @yieldreturn [void]
         # @return [GraphQL::Schema::Field]
-        def field(name_positional = nil, type_positional = nil, desc_positional = nil, **kwargs, &block)
+        def field(name_positional = nil, type_positional = nil, desc_positional = nil, **kwargs, &definition_block)
           resolver = kwargs.delete(:resolver)
           mutation = kwargs.delete(:mutation)
           subscription = kwargs.delete(:subscription)
@@ -41,7 +80,8 @@ def field(name_positional = nil, type_positional = nil, desc_positional = nil, *
             end
           end
 
-          field_defn = field_class.new(owner: self, **kwargs, &block)
+          kwargs[:owner] = self
+          field_defn = field_class.new(**kwargs, &definition_block)
           add_field(field_defn)
           field_defn
         end
@@ -264,7 +304,7 @@ def visible_interface_implementation?(type, context, warden)
           end
         end
 
-        # @param [GraphQL::Schema::Field]
+        # @param field_defn [GraphQL::Schema::Field]
         # @return [String] A warning to give when this field definition might conflict with a built-in method
         def conflict_field_name_warning(field_defn)
           "#{self.graphql_name}'s `field :#{field_defn.original_name}` conflicts with a built-in method, use `resolver_method:` to pick a different resolver method for this field (for example, `resolver_method: :resolve_#{field_defn.resolver_method}` and `def resolve_#{field_defn.resolver_method}`). Or use `method_conflict_warning: false` to suppress this warning."

spec/graphql/schema/argument_spec.rb

@@ -798,4 +798,21 @@ class Query < GraphQL::Schema::Object
       assert_equal 15, res2["data"]["add"]
     end
   end
+
+  describe "argument definitions" do
+    it "HasArguments::argument documents each argument" do
+      has_arguments_argument_comment = File.read("./lib/graphql/schema/member/has_arguments.rb")[/(\s+#[^\n]*\n)+\s+def argument\(/m]
+      has_arguments_argument_doc_param_names = has_arguments_argument_comment.split("\n").map { |line| (line[/@param (\S+)/] || line[/@option kwargs \[.*\] :(\S+)/]); $1 }.compact
+      argument_initialize_argument_names = GraphQL::Schema::Argument.instance_method(:initialize).parameters.map { |param| param[1].to_s }
+      assert_equal ["kwargs"], has_arguments_argument_doc_param_names - argument_initialize_argument_names
+      assert_equal ["owner"], argument_initialize_argument_names - has_arguments_argument_doc_param_names
+    end
+
+    it "Argument::initialize documents each argument" do
+      argument_initialize_comment = File.read("./lib/graphql/schema/argument.rb")[/(\s+#[^\n]*\n)+ {6}def initialize\(/m]
+      argument_initialize_doc_param_names = argument_initialize_comment.split("\n").map { |line| line[/@param (\S+)/]; $1 }.compact
+      argument_initialize_argument_names = GraphQL::Schema::Argument.instance_method(:initialize).parameters.map { |param| param[1].to_s }
+      assert_equal argument_initialize_doc_param_names.sort, argument_initialize_argument_names.sort
+    end
+  end
 end

spec/graphql/schema/field_spec.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 require "spec_helper"
+
 describe GraphQL::Schema::Field do
   describe "graphql definition" do
     let(:object_class) { Jazz::Query }
@@ -925,4 +926,35 @@ class Connection < GraphQL::Schema::Object; end
     field = GraphQL::Schema::Field.new(name: "blah", owner: nil, type: FieldConnectionTest::Connection, connection: true)
     assert field.connection?
   end
+
+  describe "argument documentation" do
+    it "HasFields::field documents each argument" do
+      has_fields_field_comment = File.read("./lib/graphql/schema/member/has_fields.rb")[/(\s+#[^\n]*\n)+\s+def field\(/m]
+      has_field_field_doc_param_names = has_fields_field_comment.split("\n").map do |line|
+        line[/@param (\S+)/] || line[/@option kwargs \[.*\] :(\S+)/]
+        $1
+      end.compact
+
+      field_initialize_argument_names = GraphQL::Schema::Field.instance_method(:initialize).parameters.map { |param| param[1].to_s }
+
+      expected_differences = [
+        "name_positional",
+        "type_positional",
+        "desc_positional",
+        "mutation",
+        "resolver",
+        "subscription",
+        "kwargs",
+      ]
+      assert_equal expected_differences, has_field_field_doc_param_names - field_initialize_argument_names
+      assert_equal ["owner", "resolver_class"], field_initialize_argument_names - has_field_field_doc_param_names
+    end
+
+    it "Field::initialize documents each argument" do
+      field_initialize_comment = File.read("./lib/graphql/schema/field.rb")[/(\s+#[^\n]*\n)+ {6}def initialize\(/m]
+      field_initialize_doc_param_names = field_initialize_comment.split("\n").map { |line| line[/@param (\S+)/]; $1 }.compact
+      field_initialize_argument_names = GraphQL::Schema::Field.instance_method(:initialize).parameters.map { |param| param[1].to_s }
+      assert_equal field_initialize_doc_param_names.sort, field_initialize_argument_names.sort
+    end
+  end
 end

spec/spec_helper.rb

@@ -51,7 +51,6 @@
 require "minitest/autorun"
 require "minitest/focus"
 require "minitest/reporters"
-require "minitest/mock"
 require "graphql/batch"
 
 running_in_rubymine = ENV["RM_INFO"]