Merge pull request #5137 from rmosolgo/dont-allow-empty-types

Require object types to have fields

benchmark/run.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 require "graphql"
+ADD_WARDEN = false
 require "jazz"
 require "benchmark/ips"
 require "stackprof"

lib/graphql/schema/has_single_input_argument.rb

@@ -136,6 +136,8 @@ def description(new_desc = nil)
                 super || "Autogenerated input type of #{self.mutation.graphql_name}"
               end
             end
+            # For compatibility, in case no arguments are defined:
+            has_no_arguments(true)
             mutation(mutation_class)
             # these might be inherited:
             mutation_args.each do |arg|

lib/graphql/schema/input_object.rb

@@ -10,6 +10,14 @@ class InputObject < GraphQL::Schema::Member
 
       include GraphQL::Dig
 
+      # Raised when an InputObject doesn't have any arguments defined and hasn't explicitly opted out of this requirement
+      class ArgumentsAreRequiredError < GraphQL::Error
+        def initialize(input_object_type)
+          message = "Input Object types must have arguments, but #{input_object_type.graphql_name} doesn't have any. Define an argument for this type, remove it from your schema, or add `has_no_arguments(true)` to its definition."
+          super(message)
+        end
+      end
+
       # @return [GraphQL::Query::Context] The context for this query
       attr_reader :context
       # @return [GraphQL::Execution::Interpereter::Arguments] The underlying arguments instance
@@ -56,33 +64,6 @@ def prepare
         end
       end
 
-      def self.authorized?(obj, value, ctx)
-        # Authorize each argument (but this doesn't apply if `prepare` is implemented):
-        if value.respond_to?(:key?)
-          ctx.types.arguments(self).each do |input_obj_arg|
-            if value.key?(input_obj_arg.keyword) &&
-              !input_obj_arg.authorized?(obj, value[input_obj_arg.keyword], ctx)
-              return false
-            end
-          end
-        end
-        # It didn't early-return false:
-        true
-      end
-
-      def self.one_of
-        if !one_of?
-          if all_argument_definitions.any? { |arg| arg.type.non_null? }
-            raise ArgumentError, "`one_of` may not be used with required arguments -- add `required: false` to argument definitions to use `one_of`"
-          end
-          directive(GraphQL::Schema::Directive::OneOf)
-        end
-      end
-
-      def self.one_of?
-        false # Re-defined when `OneOf` is added
-      end
-
       def unwrap_value(value)
         case value
         when Array
@@ -121,6 +102,33 @@ def to_kwargs
       end
 
       class << self
+        def authorized?(obj, value, ctx)
+          # Authorize each argument (but this doesn't apply if `prepare` is implemented):
+          if value.respond_to?(:key?)
+            ctx.types.arguments(self).each do |input_obj_arg|
+              if value.key?(input_obj_arg.keyword) &&
+                !input_obj_arg.authorized?(obj, value[input_obj_arg.keyword], ctx)
+                return false
+              end
+            end
+          end
+          # It didn't early-return false:
+          true
+        end
+
+        def one_of
+          if !one_of?
+            if all_argument_definitions.any? { |arg| arg.type.non_null? }
+              raise ArgumentError, "`one_of` may not be used with required arguments -- add `required: false` to argument definitions to use `one_of`"
+            end
+            directive(GraphQL::Schema::Directive::OneOf)
+          end
+        end
+
+        def one_of?
+          false # Re-defined when `OneOf` is added
+        end
+
         def argument(*args, **kwargs, &block)
           argument_defn = super(*args, **kwargs, &block)
           if one_of?
@@ -246,6 +254,25 @@ def coerce_result(value, ctx)
           result
         end
 
+        # @param new_has_no_arguments [Boolean] Call with `true` to make this InputObject type ignore the requirement to have any defined arguments.
+        # @return [void]
+        def has_no_arguments(new_has_no_arguments)
+          @has_no_arguments = new_has_no_arguments
+          nil
+        end
+
+        # @return [Boolean] `true` if `has_no_arguments(true)` was configued
+        def has_no_arguments?
+          @has_no_arguments
+        end
+
+        def arguments(context = GraphQL::Query::NullContext.instance, require_defined_arguments = true)
+          if require_defined_arguments && !has_no_arguments? && !any_arguments?
+            warn(GraphQL::Schema::InputObject::ArgumentsAreRequiredError.new(self).message + "\n\nThis will raise an error in a future GraphQL-Ruby version.")
+          end
+          super(context, false)
+        end
+
         private
 
         # Suppress redefinition warning for objectId arguments

lib/graphql/schema/member/has_arguments.rb

@@ -76,7 +76,7 @@ def remove_argument(arg_defn)
         end
 
         # @return [Hash<String => GraphQL::Schema::Argument] Arguments defined on this thing, keyed by name. Includes inherited definitions
-        def arguments(context = GraphQL::Query::NullContext.instance)
+        def arguments(context = GraphQL::Query::NullContext.instance, _require_defined_arguments = nil)
           if own_arguments.any?
             own_arguments_that_apply = {}
             own_arguments.each do |name, args_entry|
@@ -100,9 +100,9 @@ def inherited(child_class)
           end
 
           module InheritedArguments
-            def arguments(context = GraphQL::Query::NullContext.instance)
-              own_arguments = super
-              inherited_arguments = superclass.arguments(context)
+            def arguments(context = GraphQL::Query::NullContext.instance, require_defined_arguments = true)
+              own_arguments = super(context, require_defined_arguments)
+              inherited_arguments = superclass.arguments(context, false)
 
               if own_arguments.any?
                 if inherited_arguments.any?
@@ -149,7 +149,7 @@ def get_argument(argument_name, context = GraphQL::Query::NullContext.instance)
         end
 
         module FieldConfigured
-          def arguments(context = GraphQL::Query::NullContext.instance)
+          def arguments(context = GraphQL::Query::NullContext.instance, _require_defined_arguments = nil)
             own_arguments = super
             if @resolver_class
               inherited_arguments = @resolver_class.field_arguments(context)

lib/graphql/schema/member/has_fields.rb

@@ -79,6 +79,18 @@ def global_id_field(field_name, **kwargs)
           end
         end
 
+        # @param new_has_no_fields [Boolean] Call with `true` to make this Object type ignore the requirement to have any defined fields.
+        # @return [void]
+        def has_no_fields(new_has_no_fields)
+          @has_no_fields = new_has_no_fields
+          nil
+        end
+
+        # @return [Boolean] `true` if `has_no_fields(true)` was configued
+        def has_no_fields?
+          @has_no_fields
+        end
+
         # @return [Hash<String => GraphQL::Schema::Field, Array<GraphQL::Schema::Field>>] Fields defined on this class _specifically_, not parent classes
         def own_fields
           @own_fields ||= {}
@@ -155,9 +167,11 @@ def fields(context = GraphQL::Query::NullContext.instance)
             warden = Warden.from_context(context)
             # Local overrides take precedence over inherited fields
             visible_fields = {}
+            had_any_fields_at_all = false
             for ancestor in ancestors
               if ancestor.respond_to?(:own_fields) && visible_interface_implementation?(ancestor, context, warden)
                 ancestor.own_fields.each do |field_name, fields_entry|
+                  had_any_fields_at_all = true
                   # Choose the most local definition that passes `.visible?` --
                   # stop checking for fields by name once one has been found.
                   if !visible_fields.key?(field_name) && (f = Warden.visible_entry?(:visible_field?, fields_entry, context, warden))
@@ -166,6 +180,9 @@ def fields(context = GraphQL::Query::NullContext.instance)
                 end
               end
             end
+            if !had_any_fields_at_all && !has_no_fields?
+              warn(GraphQL::Schema::Object::FieldsAreRequiredError.new(self).message + "\n\nThis will raise an error in a future GraphQL-Ruby version.")
+            end
             visible_fields
           end
         end
@@ -188,6 +205,7 @@ def inherited(subclass)
           subclass.class_eval do
             @own_fields ||= nil
             @field_class ||= nil
+            @has_no_fields ||= false
           end
         end
 

lib/graphql/schema/object.rb

@@ -8,6 +8,14 @@ class Object < GraphQL::Schema::Member
       extend GraphQL::Schema::Member::HasFields
       extend GraphQL::Schema::Member::HasInterfaces
 
+      # Raised when an Object doesn't have any field defined and hasn't explicitly opted out of this requirement
+      class FieldsAreRequiredError < GraphQL::Error
+        def initialize(object_type)
+          message = "Object types must have fields, but #{object_type.graphql_name} doesn't have any. Define a field for this type, remove it from your schema, or add `has_no_fields(true)` to its definition."
+          super(message)
+        end
+      end
+
       # @return [Object] the application object this type is wrapping
       attr_reader :object
 

spec/graphql/logger_spec.rb

@@ -78,6 +78,8 @@ class Query < GraphQL::Schema::Object
 
           def node(id:)
           end
+
+          field :something_else, String
         end
         query(Query)
       end

spec/graphql/schema/build_from_definition_spec.rb

@@ -18,15 +18,12 @@ def assert_schema_and_compare_output(definition)
   query: HelloScalars
 }
 
-type EmptyType
-
 type HelloScalars {
   bool: Boolean
   float: Float
   id: ID
   int: Int
   str: String!
-  t: EmptyType
 }
       SCHEMA
 

spec/graphql/schema/directive/flagged_spec.rb

@@ -41,6 +41,8 @@ def antarctica; true; end
       field :santas_workshop, Boolean, null: false,
         directives: { GraphQL::Schema::Directive::Flagged => { by: ["northPole"] } }
       def santas_workshop; true; end
+
+      field :something_not_flagged, String
     end
 
     query(Query)

spec/graphql/schema/has_single_input_argument_spec.rb

@@ -124,7 +124,6 @@ def resolve
             name: 'name',
           }
         end
-
       end
       class Mutation < GraphQL::Schema::Object
         field_class GraphQL::Schema::Field

spec/graphql/schema/input_object_spec.rb

@@ -1419,4 +1419,58 @@ def result(values:)
       end
     end
   end
+
+  describe "when no arguments are defined" do
+    describe "when defined with no fields" do
+      class NoArgumentsSchema < GraphQL::Schema
+        class NoArgumentsInput < GraphQL::Schema::InputObject
+        end
+
+        class NoArgumentsCompatInput < GraphQL::Schema::InputObject
+          has_no_arguments(true)
+        end
+
+        class Query < GraphQL::Schema::Object
+          field :no_arguments, String, fallback_value: "NO_ARGS" do
+            argument :input, NoArgumentsInput
+          end
+
+          field :no_arguments_compat, String, fallback_value: "OK" do
+            argument :input, NoArgumentsCompatInput
+          end
+        end
+
+        query(Query)
+      end
+
+      it "raises an error at runtime and printing" do
+        refute NoArgumentsSchema::NoArgumentsInput.has_no_arguments?
+
+        expected_message = "Input Object types must have arguments, but NoArgumentsInput doesn't have any. Define an argument for this type, remove it from your schema, or add `has_no_arguments(true)` to its definition.
+
+This will raise an error in a future GraphQL-Ruby version.
+"
+        res = assert_warns(expected_message) do
+          NoArgumentsSchema.execute("{ noArguments(input: {}) }")
+        end
+        assert_equal "NO_ARGS", res["data"]["noArguments"]
+
+        assert_warns(expected_message) do
+          NoArgumentsSchema.to_definition
+        end
+
+        assert_warns(expected_message) do
+          NoArgumentsSchema.to_json
+        end
+      end
+
+      it "doesn't raise an error if has_no_arguments(true)" do
+        assert NoArgumentsSchema::NoArgumentsCompatInput.has_no_arguments?
+        res = assert_warns("") do
+          NoArgumentsSchema.execute("{ noArgumentsCompat(input: {}) }")
+        end
+        assert_equal "OK", res["data"]["noArgumentsCompat"]
+      end
+    end
+  end
 end

spec/graphql/schema/object_spec.rb

@@ -512,4 +512,52 @@ def self.wrap(obj, ctx)
       assert_nil obj1.comment
     end
   end
+
+  describe "when defined with no fields" do
+    class NoFieldsSchema < GraphQL::Schema
+      class NoFieldsThing < GraphQL::Schema::Object
+      end
+
+      class NoFieldsCompatThing < GraphQL::Schema::Object
+        has_no_fields(true)
+      end
+
+      class Query < GraphQL::Schema::Object
+        field :no_fields_thing, NoFieldsThing
+        field :no_fields_compat_thing, NoFieldsCompatThing
+      end
+
+      query(Query)
+    end
+
+    it "raises an error at runtime and printing" do
+      refute NoFieldsSchema::NoFieldsThing.has_no_fields?
+
+      expected_message = "Object types must have fields, but NoFieldsThing doesn't have any. Define a field for this type, remove it from your schema, or add `has_no_fields(true)` to its definition.
+
+This will raise an error in a future GraphQL-Ruby version.
+"
+      res = assert_warns(expected_message) do
+        NoFieldsSchema.execute("{ noFieldsThing { blah } }")
+      end
+      assert_equal ["Field 'blah' doesn't exist on type 'NoFieldsThing'"], res["errors"].map { |err| err["message"] }
+
+      assert_warns(expected_message) do
+        NoFieldsSchema.to_definition
+      end
+
+      assert_warns(expected_message) do
+        NoFieldsSchema.to_json
+      end
+    end
+
+    it "doesn't raise an error if has_no_fields(true)" do
+      assert NoFieldsSchema::NoFieldsCompatThing.has_no_fields?
+
+      res = assert_warns "" do
+        NoFieldsSchema.execute("{ noFieldsCompatThing { blah } }")
+      end
+      assert_equal ["Field 'blah' doesn't exist on type 'NoFieldsCompatThing'"], res["errors"].map { |e| e["message"] }
+    end
+  end
 end

spec/graphql/schema/printer_spec.rb

@@ -73,10 +73,13 @@ class MediaRating < GraphQL::Schema::Enum
       value :BOO_HISS
     end
 
+
     class NoFields < GraphQL::Schema::Object
+      has_no_fields(true)
     end
 
     class NoArguments < GraphQL::Schema::InputObject
+      has_no_arguments(true)
     end
 
     class Query < GraphQL::Schema::Object
@@ -710,7 +713,6 @@ def self.visible?(member, ctx)
     assert_equal expected, custom_filter_schema.to_definition(context: context)
   end
 
-
   it "applies an `except` filter" do
     expected = <<SCHEMA
 type Audio {