From 9387548aa1f297f4dbda8acbcf07d66b323abaa8 Mon Sep 17 00:00:00 2001 From: Daniel Tschinder Date: Tue, 13 Feb 2018 18:04:03 +0100 Subject: [PATCH] Better Predicates Introduces new assertion functions for each kind of type mirroring the existing ones for the higher order types. ref: graphql/graphql-js#1137 --- docs/reference.md | 2 +- src/Type/Definition/InterfaceType.php | 14 +++ src/Type/Definition/ListOfType.php | 7 +- src/Type/Definition/NonNull.php | 54 ++++++----- src/Type/Definition/ObjectType.php | 14 +++ src/Type/Definition/Type.php | 18 +++- src/Utils/AST.php | 123 +++++++++++++------------- src/Utils/ASTDefinitionBuilder.php | 9 +- src/Utils/FindBreakingChanges.php | 13 ++- src/Utils/SchemaPrinter.php | 7 +- src/Validator/DocumentValidator.php | 35 ++++---- tests/Type/DefinitionTest.php | 9 -- tests/Type/ValidationTest.php | 10 +-- 13 files changed, 181 insertions(+), 134 deletions(-) diff --git a/docs/reference.md b/docs/reference.md index 7ee6f21..70f5614 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -2115,7 +2115,7 @@ static function valueFromASTUntyped($valueNode, array $variables = null) * @param Schema $schema * @param NamedTypeNode|ListTypeNode|NonNullTypeNode $inputTypeNode * @return Type - * @throws InvariantViolation + * @throws \Exception */ static function typeFromAST(GraphQL\Type\Schema $schema, $inputTypeNode) ``` diff --git a/src/Type/Definition/InterfaceType.php b/src/Type/Definition/InterfaceType.php index a92be48..33bcb67 100644 --- a/src/Type/Definition/InterfaceType.php +++ b/src/Type/Definition/InterfaceType.php @@ -12,6 +12,20 @@ use GraphQL\Utils\Utils; */ class InterfaceType extends Type implements AbstractType, OutputType, CompositeType { + /** + * @param mixed $type + * @return self + */ + public static function assertInterfaceType($type) + { + Utils::invariant( + $type instanceof self, + 'Expected ' . Utils::printSafe($type) . ' to be a GraphQL Interface type.' + ); + + return $type; + } + /** * @var FieldDefinition[] */ diff --git a/src/Type/Definition/ListOfType.php b/src/Type/Definition/ListOfType.php index 6c45466..5d61d1e 100644 --- a/src/Type/Definition/ListOfType.php +++ b/src/Type/Definition/ListOfType.php @@ -20,12 +20,7 @@ class ListOfType extends Type implements WrappingType, OutputType, InputType */ public function __construct($type) { - if (!$type instanceof Type && !is_callable($type)) { - throw new InvariantViolation( - 'Can only create List of a GraphQLType but got: ' . Utils::printSafe($type) - ); - } - $this->ofType = $type; + $this->ofType = Type::assertType($type); } /** diff --git a/src/Type/Definition/NonNull.php b/src/Type/Definition/NonNull.php index 4499c74..2f3912d 100644 --- a/src/Type/Definition/NonNull.php +++ b/src/Type/Definition/NonNull.php @@ -11,7 +11,35 @@ use GraphQL\Utils\Utils; class NonNull extends Type implements WrappingType, OutputType, InputType { /** - * @var ObjectType|InterfaceType|UnionType|ScalarType|InputObjectType|EnumType + * @param mixed $type + * @return self + */ + public static function assertNullType($type) + { + Utils::invariant( + $type instanceof self, + 'Expected ' . Utils::printSafe($type) . ' to be a GraphQL Non-Null type.' + ); + + return $type; + } + + /** + * @param mixed $type + * @return ObjectType|InterfaceType|UnionType|ScalarType|InputObjectType|EnumType|ListOfType + */ + public static function assertNullableType($type) + { + Utils::invariant( + Type::isType($type) && !$type instanceof self, + 'Expected ' . Utils::printSafe($type) . ' to be a GraphQL nullable type.' + ); + + return $type; + } + + /** + * @var ObjectType|InterfaceType|UnionType|ScalarType|InputObjectType|EnumType|ListOfType */ private $ofType; @@ -21,37 +49,17 @@ class NonNull extends Type implements WrappingType, OutputType, InputType */ public function __construct($type) { - if (!$type instanceof Type && !is_callable($type)) { - throw new InvariantViolation( - 'Can only create NonNull of a Nullable GraphQLType but got: ' . Utils::printSafe($type) - ); - } - if ($type instanceof NonNull) { - throw new InvariantViolation( - 'Can only create NonNull of a Nullable GraphQLType but got: ' . Utils::printSafe($type) - ); - } - Utils::invariant( - !($type instanceof NonNull), - 'Cannot nest NonNull inside NonNull' - ); - $this->ofType = $type; + $this->ofType = self::assertNullableType($type); } /** * @param bool $recurse - * @return ObjectType|InterfaceType|UnionType|ScalarType|InputObjectType|EnumType + * @return ObjectType|InterfaceType|UnionType|ScalarType|InputObjectType|EnumType|ListOfType * @throws InvariantViolation */ public function getWrappedType($recurse = false) { $type = $this->ofType; - - Utils::invariant( - !($type instanceof NonNull), - 'Cannot nest NonNull inside NonNull' - ); - return ($recurse && $type instanceof WrappingType) ? $type->getWrappedType($recurse) : $type; } diff --git a/src/Type/Definition/ObjectType.php b/src/Type/Definition/ObjectType.php index 5f2d3e1..5c5d0eb 100644 --- a/src/Type/Definition/ObjectType.php +++ b/src/Type/Definition/ObjectType.php @@ -49,6 +49,20 @@ use GraphQL\Utils\Utils; */ class ObjectType extends Type implements OutputType, CompositeType { + /** + * @param mixed $type + * @return self + */ + public static function assertObjectType($type) + { + Utils::invariant( + $type instanceof self, + 'Expected ' . Utils::printSafe($type) . ' to be a GraphQL Object type.' + ); + + return $type; + } + /** * @var FieldDefinition[] */ diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 5afceb8..ba797bc 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -4,8 +4,10 @@ namespace GraphQL\Type\Definition; use GraphQL\Error\InvariantViolation; use GraphQL\Language\AST\ListType; use GraphQL\Language\AST\NamedType; +use GraphQL\Language\AST\NonNullType; use GraphQL\Language\AST\TypeDefinitionNode; use GraphQL\Type\Introspection; +use GraphQL\Utils\Utils; /** * Registry of standard GraphQL types @@ -218,11 +220,25 @@ abstract class Type implements \JsonSerializable $type instanceof UnionType || $type instanceof EnumType || $type instanceof InputObjectType || - $type instanceof ListType || + $type instanceof ListOfType || $type instanceof NonNull ); } + /** + * @param mixed $type + * @return mixed + */ + public static function assertType($type) + { + Utils::invariant( + self::isType($type), + 'Expected ' . Utils::printSafe($type) . ' to be a GraphQL type.' + ); + + return $type; + } + /** * @api * @param Type $type diff --git a/src/Utils/AST.php b/src/Utils/AST.php index d5c867e..9a18915 100644 --- a/src/Utils/AST.php +++ b/src/Utils/AST.php @@ -1,6 +1,7 @@ $fieldNodes]); } - Utils::invariant( - $type instanceof ScalarType || $type instanceof EnumType, - "Must provide Input Type, cannot use: " . Utils::printSafe($type) - ); + if ($type instanceof ScalarType || $type instanceof EnumType) { + // Since value is an internally represented value, it must be serialized + // to an externally represented value before converting into an AST. + $serialized = $type->serialize($value); + if (null === $serialized || Utils::isInvalid($serialized)) { + return null; + } - // Since value is an internally represented value, it must be serialized - // to an externally represented value before converting into an AST. - $serialized = $type->serialize($value); - if (null === $serialized || Utils::isInvalid($serialized)) { - return null; - } - - // Others serialize based on their corresponding PHP scalar types. - if (is_bool($serialized)) { - return new BooleanValueNode(['value' => $serialized]); - } - if (is_int($serialized)) { - return new IntValueNode(['value' => $serialized]); - } - if (is_float($serialized)) { - if ((int) $serialized == $serialized) { + // Others serialize based on their corresponding PHP scalar types. + if (is_bool($serialized)) { + return new BooleanValueNode(['value' => $serialized]); + } + if (is_int($serialized)) { return new IntValueNode(['value' => $serialized]); } - return new FloatValueNode(['value' => $serialized]); - } - if (is_string($serialized)) { - // Enum types use Enum literals. - if ($type instanceof EnumType) { - return new EnumValueNode(['value' => $serialized]); + if (is_float($serialized)) { + if ((int) $serialized == $serialized) { + return new IntValueNode(['value' => $serialized]); + } + return new FloatValueNode(['value' => $serialized]); + } + if (is_string($serialized)) { + // Enum types use Enum literals. + if ($type instanceof EnumType) { + return new EnumValueNode(['value' => $serialized]); + } + + // ID types can use Int literals. + $asInt = (int) $serialized; + if ($type instanceof IDType && (string) $asInt === $serialized) { + return new IntValueNode(['value' => $serialized]); + } + + // Use json_encode, which uses the same string encoding as GraphQL, + // then remove the quotes. + return new StringValueNode([ + 'value' => substr(json_encode($serialized), 1, -1) + ]); } - // ID types can use Int literals. - $asInt = (int) $serialized; - if ($type instanceof IDType && (string) $asInt === $serialized) { - return new IntValueNode(['value' => $serialized]); - } - - // Use json_encode, which uses the same string encoding as GraphQL, - // then remove the quotes. - return new StringValueNode([ - 'value' => substr(json_encode($serialized), 1, -1) - ]); + throw new InvariantViolation('Cannot convert value to AST: ' . Utils::printSafe($serialized)); } - throw new InvariantViolation('Cannot convert value to AST: ' . Utils::printSafe($serialized)); + throw new Error('Unknown type: ' . Utils::printSafe($type) . '.'); } /** @@ -395,25 +395,26 @@ class AST return $enumValue->value; } - Utils::invariant($type instanceof ScalarType, 'Must be scalar type'); - /** @var ScalarType $type */ + if ($type instanceof ScalarType) { + // Scalars fulfill parsing a literal value via parseLiteral(). + // Invalid values represent a failure to parse correctly, in which case + // no value is returned. + try { + $result = $type->parseLiteral($valueNode, $variables); + } catch (\Exception $error) { + return $undefined; + } catch (\Throwable $error) { + return $undefined; + } - // Scalars fulfill parsing a literal value via parseLiteral(). - // Invalid values represent a failure to parse correctly, in which case - // no value is returned. - try { - $result = $type->parseLiteral($valueNode, $variables); - } catch (\Exception $error) { - return $undefined; - } catch (\Throwable $error) { - return $undefined; + if (Utils::isInvalid($result)) { + return $undefined; + } + + return $result; } - if (Utils::isInvalid($result)) { - return $undefined; - } - - return $result; + throw new Error('Unknown type: ' . Utils::printSafe($type) . '.'); } /** @@ -473,9 +474,9 @@ class AST return ($variables && isset($variables[$variableName]) && !Utils::isInvalid($variables[$variableName])) ? $variables[$variableName] : null; - default: - throw new InvariantViolation('Unexpected value kind: ' . $valueNode->kind); - } + } + + throw new Error('Unexpected value kind: ' . $valueNode->kind . '.'); } /** @@ -485,7 +486,7 @@ class AST * @param Schema $schema * @param NamedTypeNode|ListTypeNode|NonNullTypeNode $inputTypeNode * @return Type - * @throws InvariantViolation + * @throws \Exception */ public static function typeFromAST(Schema $schema, $inputTypeNode) { @@ -497,9 +498,11 @@ class AST $innerType = self::typeFromAST($schema, $inputTypeNode->type); return $innerType ? new NonNull($innerType) : null; } + if ($inputTypeNode instanceof NamedTypeNode) { + return $schema->getType($inputTypeNode->name->value); + } - Utils::invariant($inputTypeNode && $inputTypeNode instanceof NamedTypeNode, 'Must be a named type'); - return $schema->getType($inputTypeNode->name->value); + throw new Error('Unexpected type kind: ' . $inputTypeNode->kind . '.'); } /** diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index 073e773..c5516dd 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -75,8 +75,7 @@ class ASTDefinitionBuilder } if ($inputTypeNode->kind == NodeKind::NON_NULL_TYPE) { $wrappedType = $this->buildWrappedType($innerType, $inputTypeNode->type); - Utils::invariant(!($wrappedType instanceof NonNull), 'No nesting nonnull.'); - return Type::nonNull($wrappedType); + return Type::nonNull(NonNull::assertNullableType($wrappedType)); } return $innerType; } @@ -159,8 +158,7 @@ class ASTDefinitionBuilder public function buildObjectType($typeNode) { $type = $this->buildType($typeNode); - Utils::invariant($type instanceof ObjectType, 'Expected Object type.' . get_class($type)); - return $type; + return ObjectType::assertObjectType($type); } /** @@ -171,8 +169,7 @@ class ASTDefinitionBuilder public function buildInterfaceType($typeNode) { $type = $this->buildType($typeNode); - Utils::invariant($type instanceof InterfaceType, 'Expected Interface type.'); - return $type; + return InterfaceType::assertInterfaceType($type); } /** diff --git a/src/Utils/FindBreakingChanges.php b/src/Utils/FindBreakingChanges.php index 63acbef..6c67aa1 100644 --- a/src/Utils/FindBreakingChanges.php +++ b/src/Utils/FindBreakingChanges.php @@ -148,8 +148,11 @@ class FindBreakingChanges $dangerousChanges = []; foreach ($oldTypeMap as $oldTypeName => $oldTypeDefinition) { $newTypeDefinition = isset($newTypeMap[$oldTypeName]) ? $newTypeMap[$oldTypeName] : null; - if (!($oldTypeDefinition instanceof ObjectType || $oldTypeDefinition instanceof InterfaceType) || - !($newTypeDefinition instanceof $oldTypeDefinition)) { + if ( + !($oldTypeDefinition instanceof ObjectType || $oldTypeDefinition instanceof InterfaceType) || + !($newTypeDefinition instanceof ObjectType || $newTypeDefinition instanceof InterfaceType) || + !($newTypeDefinition instanceof $oldTypeDefinition) + ) { continue; } @@ -262,7 +265,11 @@ class FindBreakingChanges $breakingChanges = []; foreach ($oldTypeMap as $typeName => $oldType) { $newType = isset($newTypeMap[$typeName]) ? $newTypeMap[$typeName] : null; - if (!($oldType instanceof ObjectType || $oldType instanceof InterfaceType) || !($newType instanceof $oldType)) { + if ( + !($oldType instanceof ObjectType || $oldType instanceof InterfaceType) || + !($newType instanceof ObjectType || $newType instanceof InterfaceType) || + !($newType instanceof $oldType) + ) { continue; } $oldTypeFieldsDef = $oldType->getFields(); diff --git a/src/Utils/SchemaPrinter.php b/src/Utils/SchemaPrinter.php index d80e104..e0a74b7 100644 --- a/src/Utils/SchemaPrinter.php +++ b/src/Utils/SchemaPrinter.php @@ -1,6 +1,7 @@ parseLiteral($valueNode); - if (Utils::isInvalid($parseResult)) { + if ($type instanceof ScalarType) { + // Scalars determine if a literal values is valid via parseLiteral(). + try { + $parseResult = $type->parseLiteral($valueNode); + if (Utils::isInvalid($parseResult)) { + $printed = Printer::doPrint($valueNode); + return ["Expected type \"{$type->name}\", found $printed."]; + } + } catch (\Exception $error) { $printed = Printer::doPrint($valueNode); - return ["Expected type \"{$type->name}\", found $printed."]; + $message = $error->getMessage(); + return ["Expected type \"{$type->name}\", found $printed; $message"]; + } catch (\Throwable $error) { + $printed = Printer::doPrint($valueNode); + $message = $error->getMessage(); + return ["Expected type \"{$type->name}\", found $printed; $message"]; } - } catch (\Exception $error) { - $printed = Printer::doPrint($valueNode); - $message = $error->getMessage(); - return ["Expected type \"{$type->name}\", found $printed; $message"]; - } catch (\Throwable $error) { - $printed = Printer::doPrint($valueNode); - $message = $error->getMessage(); - return ["Expected type \"{$type->name}\", found $printed; $message"]; + + return []; } - return []; + throw new Error('Unknown type: ' . Utils::printSafe($type) . '.'); } /** diff --git a/tests/Type/DefinitionTest.php b/tests/Type/DefinitionTest.php index 1622220..9f1b8ce 100644 --- a/tests/Type/DefinitionTest.php +++ b/tests/Type/DefinitionTest.php @@ -468,15 +468,6 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase } } - /** - * @it prohibits nesting NonNull inside NonNull - */ - public function testProhibitsNonNullNesting() - { - $this->setExpectedException('\Exception'); - new NonNull(new NonNull(Type::int())); - } - /** * @it prohibits putting non-Object types in unions */ diff --git a/tests/Type/ValidationTest.php b/tests/Type/ValidationTest.php index 373180d..1409da4 100644 --- a/tests/Type/ValidationTest.php +++ b/tests/Type/ValidationTest.php @@ -1975,7 +1975,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase } - // DESCRIBE: Type System: List must accept GraphQL types + // DESCRIBE: Type System: List must accept only types /** * @it accepts an type as item type of list @@ -2022,7 +2022,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase $this->fail("Expected exception not thrown for: " . Utils::printSafe($type)); } catch (InvariantViolation $e) { $this->assertEquals( - 'Can only create List of a GraphQLType but got: ' . Utils::printSafe($type), + 'Expected '. Utils::printSafe($type) . ' to be a GraphQL type.', $e->getMessage() ); } @@ -2030,7 +2030,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase } - // DESCRIBE: Type System: NonNull must accept GraphQL types + // DESCRIBE: Type System: NonNull must only accept non-nullable types /** * @it accepts an type as nullable type of non-null @@ -2057,8 +2057,6 @@ class ValidationTest extends \PHPUnit_Framework_TestCase } } - // TODO: rejects a non-type as nullable type of non-null: ${type} - /** * @it rejects a non-type as nullable type of non-null */ @@ -2079,7 +2077,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase $this->fail("Expected exception not thrown for: " . Utils::printSafe($type)); } catch (InvariantViolation $e) { $this->assertEquals( - 'Can only create NonNull of a Nullable GraphQLType but got: ' . Utils::printSafe($type), + 'Expected ' . Utils::printSafe($type) . ' to be a GraphQL nullable type.', $e->getMessage() ); }