Removed callbacks in field types (previously deprecated in #35)

src/Error/Warning.php

@@ -8,8 +8,9 @@ final class Warning
     const CONFIG_WARNING = 4;
     const RESOLVE_TYPE_WARNING = 8;
     const CONFIG_DEPRECATION_WARNING = 16;
+    const NOT_A_TYPE = 32;
 
-    const ALL = 23;
+    const ALL = 63;
 
     static $enableWarnings = self::ALL;
 

src/Type/Definition/FieldArgument.php

@@ -33,15 +33,10 @@ class FieldArgument
      */
     public $config;
 
-    /**
-     * @var InputType|callable
-     */
-    private $type;
-
     /**
      * @var InputType
      */
-    private $resolvedType;
+    private $type;
 
     /**
      * @var bool
@@ -95,10 +90,7 @@ class FieldArgument
      */
     public function getType()
     {
-        if (null === $this->resolvedType) {
+        return $this->type;
-            $this->resolvedType = Type::resolve($this->type);
-        }
-        return $this->resolvedType;
     }
 
     /**

src/Type/Definition/FieldDefinition.php

@@ -55,15 +55,10 @@ class FieldDefinition
      */
     public $config;
 
-    /**
-     * @var OutputType|callable
-     */
-    private $type;
-
     /**
      * @var OutputType
      */
-    private $resolvedType;
+    private $type;
 
     private static $def;
 
@@ -216,11 +211,7 @@ class FieldDefinition
      */
     public function getType()
     {
-        if (null === $this->resolvedType) {
+        return $this->type;
-            // TODO: deprecate types as callbacks - instead just allow field definitions to be callbacks
-            $this->resolvedType = Type::resolve($this->type);
-        }
-        return $this->resolvedType;
     }
 
     /**

src/Type/Definition/InputObjectField.php

@@ -65,7 +65,7 @@ class InputObjectField
      */
     public function getType()
     {
-        return Type::resolve($this->type);
+        return $this->type;
     }
 
     /**

src/Type/Definition/ListOfType.php

@@ -33,7 +33,7 @@ class ListOfType extends Type implements WrappingType, OutputType, InputType
      */
     public function toString()
     {
-        $type = Type::resolve($this->ofType);
+        $type = $this->ofType;
         $str = $type instanceof Type ? $type->toString() : (string) $type;
         return '[' . $str . ']';
     }
@@ -44,7 +44,7 @@ class ListOfType extends Type implements WrappingType, OutputType, InputType
      */
     public function getWrappedType($recurse = false)
     {
-        $type = Type::resolve($this->ofType);
+        $type = $this->ofType;
         return ($recurse && $type instanceof WrappingType) ? $type->getWrappedType($recurse) : $type;
     }
 }

src/Type/Definition/NonNull.php

@@ -45,7 +45,7 @@ class NonNull extends Type implements WrappingType, OutputType, InputType
      */
     public function getWrappedType($recurse = false)
     {
-        $type = Type::resolve($this->ofType);
+        $type = $this->ofType;
 
         Utils::invariant(
             !($type instanceof NonNull),

src/Type/Definition/ObjectType.php

@@ -151,20 +151,7 @@ class ObjectType extends Type implements OutputType, CompositeType
                 );
             }
 
-            $this->interfaces = [];
+            $this->interfaces = $interfaces;
-            foreach ($interfaces as $iface) {
-                $iface = Type::resolve($iface);
-                if (!$iface instanceof InterfaceType) {
-                    throw new InvariantViolation(sprintf(
-                        '%s may only implement Interface types, it cannot implement %s',
-                        $this->name,
-                        Utils::printSafe($iface)
-                    ));
-                }
-                // TODO: return interfaceMap vs interfaces. Possibly breaking change?
-                $this->interfaces[] = $iface;
-                $this->interfaceMap[$iface->name] = $iface;
-            }
         }
         return $this->interfaces;
     }
@@ -186,9 +173,8 @@ class ObjectType extends Type implements OutputType, CompositeType
      */
     public function implementsInterface($iface)
     {
-        $iface = Type::resolve($iface);
+        $map = $this->getInterfaceMap();
-        $this->getInterfaces();
+        return isset($map[$iface->name]);
-        return isset($this->interfaceMap[$iface->name]);
     }
 
     /**
@@ -239,7 +225,7 @@ class ObjectType extends Type implements OutputType, CompositeType
         foreach ($this->getInterfaces() as $iface) {
             Utils::invariant(
                 $iface instanceof InterfaceType,
-                "{$this->name} may only implement Interface types, it cannot implement: %s.",
+                "{$this->name} may only implement Interface types, it cannot implement %s.",
                 Utils::printSafe($iface)
             );
             Utils::invariant(

src/Type/Definition/Type.php

@@ -180,29 +180,6 @@ abstract class Type implements \JsonSerializable
         while ($type instanceof WrappingType) {
             $type = $type->getWrappedType();
         }
-        return self::resolve($type);
-    }
-
-    /**
-     * @param $type
-     * @return mixed
-     */
-    public static function resolve($type)
-    {
-        if (is_callable($type)) {
-            trigger_error(
-                'Passing type as closure is deprecated (see https://github.com/webonyx/graphql-php/issues/35 for alternatives)',
-                E_USER_DEPRECATED
-            );
-            $type = $type();
-        }
-
-        if (!$type instanceof Type) {
-            throw new InvariantViolation(sprintf(
-                'Expecting instance of ' . __CLASS__ . ', got "%s"',
-                Utils::getVariableType($type)
-            ));
-        }
         return $type;
     }
 

src/Type/Definition/UnionType.php

@@ -83,10 +83,7 @@ class UnionType extends Type implements AbstractType, OutputType, CompositeType
                 );
             }
 
-            $this->types = [];
+            $this->types = $types;
-            foreach ($types as $type) {
-                $this->types[] = Type::resolve($type);
-            }
         }
         return $this->types;
     }

src/Utils/TypeInfo.php

@@ -1,6 +1,7 @@
 <?php
 namespace GraphQL\Utils;
 
+use GraphQL\Error\Warning;
 use GraphQL\Language\AST\FieldNode;
 use GraphQL\Language\AST\ListTypeNode;
 use GraphQL\Language\AST\NamedTypeNode;
@@ -103,6 +104,14 @@ class TypeInfo
         if ($type instanceof WrappingType) {
             return self::extractTypes($type->getWrappedType(true), $typeMap);
         }
+        if (!$type instanceof Type) {
+            Warning::warnOnce(
+                'One of schema types is not a valid type definition instance. Ignoring it. '.
+                'Try running $schema->assertValid() to find out the cause of this warning.',
+                Warning::NOT_A_TYPE
+            );
+            return $typeMap;
+        }
 
         if (!empty($typeMap[$type->name])) {
             Utils::invariant(

tests/Type/ValidationTest.php

@@ -121,6 +121,14 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
         ]);
 
         $this->notInputTypes[] = $this->String;
+
+        Warning::suppress(Warning::NOT_A_TYPE);
+    }
+
+    public function tearDown()
+    {
+        parent::tearDown();
+        Warning::enable(Warning::NOT_A_TYPE);
     }
 
     public function testRejectsTypesWithoutNames()
@@ -572,11 +580,9 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
             ]
         ]));
 
-        // FIXME: this exception message is caused by old mechanism of type resolution (see #35),
-        // this has to be changed as soon as we drop this mechanism
         $this->setExpectedException(
             InvariantViolation::class,
-            'Expecting instance of GraphQL\Type\Definition\Type, got "stdClass'
+            'SomeObject.field field type must be Output Type but got: instance of stdClass'
         );
         $schema->assertValid();
     }
@@ -719,10 +725,9 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
             ]
         ]));
 
-        // FIXME
         $this->setExpectedException(
             InvariantViolation::class,
-            'Expecting instance of GraphQL\Type\Definition\Type, got "NULL"'
+            'SomeObject.badField(0:) Must be named. Unexpected name: 0'
         );
 
         $schema->assertValid();
@@ -1023,7 +1028,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
 
         $this->setExpectedException(
             InvariantViolation::class,
-            'Expecting instance of GraphQL\Type\Definition\Type, got "NULL"'
+            'SomeInputObject.0: Must be named. Unexpected name: 0'
         );
         $schema->assertValid();
     }
@@ -1737,7 +1742,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
 
         $this->setExpectedException(
             InvariantViolation::class,
-            'Expecting instance of GraphQL\Type\Definition\Type, got "NULL"'
+            'BadObject.badField field type must be Output Type but got: null'
         );
 
         $schema->assertValid();
@@ -1755,12 +1760,10 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
                 $schema->assertValid();
                 $this->fail('Expected exception not thrown for ' . Utils::printSafe($type));
             } catch (InvariantViolation $e) {
-                // FIXME
+                $this->assertEquals(
-                if ($type !== 'TestString') {
+                    'BadObject.badField field type must be Output Type but got: ' . Utils::printSafe($type),
-                    $this->assertEquals('BadObject.badField field type must be Output Type but got: ' . $type, $e->getMessage());
+                    $e->getMessage()
-                } else {
+                );
-                    $this->assertEquals('Expecting instance of GraphQL\Type\Definition\Type, got "string"', $e->getMessage());
-                }
             }
         }
     }
@@ -1844,7 +1847,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
                 $this->fail('Exepected exception not thrown for type ' . $type);
             } catch (InvariantViolation $e) {
                 $this->assertEquals(
-                    'BadObject may only implement Interface types, it cannot implement ' . $type,
+                    'BadObject may only implement Interface types, it cannot implement ' . $type . '.',
                     $e->getMessage()
                 );
             }
@@ -1914,7 +1917,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
 
         $this->setExpectedException(
             InvariantViolation::class,
-            'Expecting instance of GraphQL\Type\Definition\Type, got "NULL"' // FIXME
+            'BadInterface.badField field type must be Output Type but got: null'
         );
 
         $schema->assertValid();
@@ -1932,18 +1935,10 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
                 $schema->assertValid();
                 $this->fail('Expected exception not thrown for type ' . $type);
             } catch (InvariantViolation $e) {
-                if ($type !== 'TestString') {
+                $this->assertEquals(
-                    $this->assertEquals(
+                    'BadInterface.badField field type must be Output Type but got: ' . Utils::printSafe($type),
-                        'BadInterface.badField field type must be Output Type but got: ' . $type,
+                    $e->getMessage()
-                        $e->getMessage()
+                );
-                    );
-                } else {
-                    // FIXME
-                    $this->assertEquals(
-                        'Expecting instance of GraphQL\Type\Definition\Type, got "string"',
-                        $e->getMessage()
-                    );
-                }
             }
         }
     }
@@ -1974,7 +1969,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
             $this->fail('Expected exception not thrown');
         } catch (InvariantViolation $e) {
             $this->assertEquals(
-                'Expecting instance of GraphQL\Type\Definition\Type, got "NULL"',
+                'BadObject.badField(badArg): argument type must be Input Type but got: null',
                 $e->getMessage()
             );
         }
@@ -1991,17 +1986,10 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
                 $schema->assertValid();
                 $this->fail('Expected exception not thrown for type ' . $type);
             } catch (InvariantViolation $e) {
-                if ($type !== 'TestString') {
+                $this->assertEquals(
-                    $this->assertEquals(
+                    'BadObject.badField(badArg): argument type must be Input Type but got: ' . Utils::printSafe($type),
-                        'BadObject.badField(badArg): argument type must be Input Type but got: ' . $type,
+                    $e->getMessage()
-                        $e->getMessage()
+                );
-                    );
-                } else {
-                    $this->assertEquals(
-                        'Expecting instance of GraphQL\Type\Definition\Type, got "string"',
-                        $e->getMessage()
-                    );
-                }
             }
         }
     }
@@ -2027,10 +2015,9 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
     {
         $schema = $this->schemaWithInputFieldOfType(null);
 
-        // FIXME
         $this->setExpectedException(
             InvariantViolation::class,
-            'Expecting instance of GraphQL\Type\Definition\Type, got "NULL"'
+            'BadInputObject.badField field type must be Input Type but got: null.'
         );
         $schema->assertValid();
     }
@@ -2046,17 +2033,10 @@ class ValidationTest extends \PHPUnit_Framework_TestCase
                 $schema->assertValid();
                 $this->fail('Expected exception not thrown for type ' . $type);
             } catch (InvariantViolation $e) {
-                if ($type !== 'TestString') {
+                $this->assertEquals(
-                    $this->assertEquals(
+                    "BadInputObject.badField field type must be Input Type but got: " . Utils::printSafe($type) . ".",
-                        "BadInputObject.badField field type must be Input Type but got: $type.",
+                    $e->getMessage()
-                        $e->getMessage()
+                );
-                    );
-                } else {
-                    $this->assertEquals(
-                        'Expecting instance of GraphQL\Type\Definition\Type, got "string"',
-                        $e->getMessage()
-                    );
-                }
             }
         }
     }