diff --git a/src/Error/Warning.php b/src/Error/Warning.php index 2ec7631..5ccf04b 100644 --- a/src/Error/Warning.php +++ b/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; diff --git a/src/Type/Definition/FieldArgument.php b/src/Type/Definition/FieldArgument.php index 4a2199e..3385822 100644 --- a/src/Type/Definition/FieldArgument.php +++ b/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) { - $this->resolvedType = Type::resolve($this->type); - } - return $this->resolvedType; + return $this->type; } /** diff --git a/src/Type/Definition/FieldDefinition.php b/src/Type/Definition/FieldDefinition.php index d3721b9..e3b0ced 100644 --- a/src/Type/Definition/FieldDefinition.php +++ b/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) { - // TODO: deprecate types as callbacks - instead just allow field definitions to be callbacks - $this->resolvedType = Type::resolve($this->type); - } - return $this->resolvedType; + return $this->type; } /** diff --git a/src/Type/Definition/InputObjectField.php b/src/Type/Definition/InputObjectField.php index f782196..732baee 100644 --- a/src/Type/Definition/InputObjectField.php +++ b/src/Type/Definition/InputObjectField.php @@ -65,7 +65,7 @@ class InputObjectField */ public function getType() { - return Type::resolve($this->type); + return $this->type; } /** diff --git a/src/Type/Definition/ListOfType.php b/src/Type/Definition/ListOfType.php index 95fd9d9..49119b1 100644 --- a/src/Type/Definition/ListOfType.php +++ b/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; } } diff --git a/src/Type/Definition/NonNull.php b/src/Type/Definition/NonNull.php index efef27d..9be6f06 100644 --- a/src/Type/Definition/NonNull.php +++ b/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), diff --git a/src/Type/Definition/ObjectType.php b/src/Type/Definition/ObjectType.php index 4ec71bf..f2b9d70 100644 --- a/src/Type/Definition/ObjectType.php +++ b/src/Type/Definition/ObjectType.php @@ -151,20 +151,7 @@ class ObjectType extends Type implements OutputType, CompositeType ); } - $this->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; - } + $this->interfaces = $interfaces; } return $this->interfaces; } @@ -186,9 +173,8 @@ class ObjectType extends Type implements OutputType, CompositeType */ public function implementsInterface($iface) { - $iface = Type::resolve($iface); - $this->getInterfaces(); - return isset($this->interfaceMap[$iface->name]); + $map = $this->getInterfaceMap(); + return isset($map[$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( diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 70c516f..c42ba46 100644 --- a/src/Type/Definition/Type.php +++ b/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; } diff --git a/src/Type/Definition/UnionType.php b/src/Type/Definition/UnionType.php index fd1a79e..ebec1b6 100644 --- a/src/Type/Definition/UnionType.php +++ b/src/Type/Definition/UnionType.php @@ -83,10 +83,7 @@ class UnionType extends Type implements AbstractType, OutputType, CompositeType ); } - $this->types = []; - foreach ($types as $type) { - $this->types[] = Type::resolve($type); - } + $this->types = $types; } return $this->types; } diff --git a/src/Utils/TypeInfo.php b/src/Utils/TypeInfo.php index b1fc809..4edfc00 100644 --- a/src/Utils/TypeInfo.php +++ b/src/Utils/TypeInfo.php @@ -1,6 +1,7 @@ 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( diff --git a/tests/Type/ValidationTest.php b/tests/Type/ValidationTest.php index 90a308e..b2a16aa 100644 --- a/tests/Type/ValidationTest.php +++ b/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 - if ($type !== 'TestString') { - $this->assertEquals('BadObject.badField field type must be Output Type but got: ' . $type, $e->getMessage()); - } else { - $this->assertEquals('Expecting instance of GraphQL\Type\Definition\Type, got "string"', $e->getMessage()); - } + $this->assertEquals( + 'BadObject.badField field type must be Output Type but got: ' . Utils::printSafe($type), + $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( - 'BadInterface.badField field type must be Output Type but got: ' . $type, - $e->getMessage() - ); - } else { - // FIXME - $this->assertEquals( - 'Expecting instance of GraphQL\Type\Definition\Type, got "string"', - $e->getMessage() - ); - } + $this->assertEquals( + 'BadInterface.badField field type must be Output Type but got: ' . Utils::printSafe($type), + $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( - 'BadObject.badField(badArg): argument type must be Input Type but got: ' . $type, - $e->getMessage() - ); - } else { - $this->assertEquals( - 'Expecting instance of GraphQL\Type\Definition\Type, got "string"', - $e->getMessage() - ); - } + $this->assertEquals( + 'BadObject.badField(badArg): argument type must be Input Type but got: ' . Utils::printSafe($type), + $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( - "BadInputObject.badField field type must be Input Type but got: $type.", - $e->getMessage() - ); - } else { - $this->assertEquals( - 'Expecting instance of GraphQL\Type\Definition\Type, got "string"', - $e->getMessage() - ); - } + $this->assertEquals( + "BadInputObject.badField field type must be Input Type but got: " . Utils::printSafe($type) . ".", + $e->getMessage() + ); } } }