From f140149127a7040b6661676b205a37e54be49b0d Mon Sep 17 00:00:00 2001 From: Daniel Tschinder Date: Tue, 24 Apr 2018 15:14:31 +0200 Subject: [PATCH] Make Types throw instead of returning Utils::undefined() --- examples/01-blog/Blog/Type/Scalar/UrlType.php | 3 +- src/Executor/Executor.php | 20 ++++++++----- src/Type/Definition/BooleanType.php | 16 ++++++++-- src/Type/Definition/CustomScalarType.php | 9 +++--- src/Type/Definition/EnumType.php | 30 ++++++++++++------- src/Type/Definition/FloatType.php | 5 +++- src/Type/Definition/IDType.php | 16 ++++++++-- src/Type/Definition/IntType.php | 5 +++- src/Type/Definition/LeafType.php | 8 +++-- src/Type/Definition/StringType.php | 5 +++- src/Utils/AST.php | 27 +++++++++-------- src/Utils/Value.php | 9 +----- src/Validator/Rules/ValuesOfCorrectType.php | 15 ++-------- tests/Executor/TestClasses.php | 19 ++++++++++-- 14 files changed, 117 insertions(+), 70 deletions(-) diff --git a/examples/01-blog/Blog/Type/Scalar/UrlType.php b/examples/01-blog/Blog/Type/Scalar/UrlType.php index c539121..0590546 100644 --- a/examples/01-blog/Blog/Type/Scalar/UrlType.php +++ b/examples/01-blog/Blog/Type/Scalar/UrlType.php @@ -30,11 +30,12 @@ class UrlType extends ScalarType * * @param mixed $value * @return mixed + * @throws Error */ public function parseValue($value) { if (!is_string($value) || !filter_var($value, FILTER_VALIDATE_URL)) { // quite naive, but after all this is example - throw new \UnexpectedValueException("Cannot represent value as URL: " . Utils::printSafe($value)); + throw new Error("Cannot represent value as URL: " . Utils::printSafe($value)); } return $value; } diff --git a/src/Executor/Executor.php b/src/Executor/Executor.php index fac9553..15098e0 100644 --- a/src/Executor/Executor.php +++ b/src/Executor/Executor.php @@ -1197,8 +1197,7 @@ class Executor } /** - * Complete a Scalar or Enum by serializing to a valid value, returning - * null if serialization is not possible. + * Complete a Scalar or Enum by serializing to a valid value, throwing if serialization is not possible. * * @param LeafType $returnType * @param $result @@ -1207,14 +1206,21 @@ class Executor */ private function completeLeafValue(LeafType $returnType, &$result) { - $serializedResult = $returnType->serialize($result); - - if (Utils::isInvalid($serializedResult)) { + try { + return $returnType->serialize($result); + } catch (\Exception $error) { throw new InvariantViolation( - 'Expected a value of type "'. Utils::printSafe($returnType) . '" but received: ' . Utils::printSafe($result) + 'Expected a value of type "'. Utils::printSafe($returnType) . '" but received: ' . Utils::printSafe($result), + 0, + $error + ); + } catch (\Throwable $error) { + throw new InvariantViolation( + 'Expected a value of type "'. Utils::printSafe($returnType) . '" but received: ' . Utils::printSafe($result), + 0, + $error ); } - return $serializedResult; } /** diff --git a/src/Type/Definition/BooleanType.php b/src/Type/Definition/BooleanType.php index 2a1adc7..723990c 100644 --- a/src/Type/Definition/BooleanType.php +++ b/src/Type/Definition/BooleanType.php @@ -1,7 +1,9 @@ value; } - return Utils::undefined(); + + // Intentionally without message, as all information already in wrapped Exception + throw new \Exception(); } } diff --git a/src/Type/Definition/CustomScalarType.php b/src/Type/Definition/CustomScalarType.php index 14e1d53..d801644 100644 --- a/src/Type/Definition/CustomScalarType.php +++ b/src/Type/Definition/CustomScalarType.php @@ -1,6 +1,8 @@ config['parseValue'])) { return call_user_func($this->config['parseValue'], $value); } else { @@ -37,9 +35,10 @@ class CustomScalarType extends ScalarType } /** - * @param $valueNode + * @param Node $valueNode * @param array|null $variables * @return mixed + * @throws \Exception */ public function parseLiteral(/* GraphQL\Language\AST\ValueNode */ $valueNode, array $variables = null) { diff --git a/src/Type/Definition/EnumType.php b/src/Type/Definition/EnumType.php index dbb7e33..08c02f1 100644 --- a/src/Type/Definition/EnumType.php +++ b/src/Type/Definition/EnumType.php @@ -1,6 +1,7 @@ name; } - return Utils::undefined(); + throw new Error("Cannot serialize value as enum: " . Utils::printSafe($value)); } /** * @param $value - * @return null + * @return mixed + * @throws Error */ public function parseValue($value) { $lookup = $this->getNameLookup(); - return isset($lookup[$value]) ? $lookup[$value]->value : Utils::undefined(); + if (isset($lookup[$value])) { + return $lookup[$value]->value; + } + + throw new Error("Cannot represent value as enum: " . Utils::printSafe($value)); } /** - * @param $value + * @param $valueNode * @param array|null $variables * @return null + * @throws \Exception */ - public function parseLiteral($value, array $variables = null) + public function parseLiteral($valueNode, array $variables = null) { - if ($value instanceof EnumValueNode) { + if ($valueNode instanceof EnumValueNode) { $lookup = $this->getNameLookup(); - if (isset($lookup[$value->value])) { - $enumValue = $lookup[$value->value]; + if (isset($lookup[$valueNode->value])) { + $enumValue = $lookup[$valueNode->value]; if ($enumValue) { return $enumValue->value; } } } - return null; + + // Intentionally without message, as all information already in wrapped Exception + throw new \Exception(); } /** diff --git a/src/Type/Definition/FloatType.php b/src/Type/Definition/FloatType.php index e6391de..e04cd5a 100644 --- a/src/Type/Definition/FloatType.php +++ b/src/Type/Definition/FloatType.php @@ -49,13 +49,16 @@ values as specified by * @param $valueNode * @param array|null $variables * @return float|null + * @throws \Exception */ public function parseLiteral($valueNode, array $variables = null) { if ($valueNode instanceof FloatValueNode || $valueNode instanceof IntValueNode) { return (float) $valueNode->value; } - return Utils::undefined(); + + // Intentionally without message, as all information already in wrapped Exception + throw new \Exception(); } private function coerceFloat($value) { diff --git a/src/Type/Definition/IDType.php b/src/Type/Definition/IDType.php index d60a3a1..a35cf09 100644 --- a/src/Type/Definition/IDType.php +++ b/src/Type/Definition/IDType.php @@ -3,6 +3,7 @@ namespace GraphQL\Type\Definition; use GraphQL\Error\Error; use GraphQL\Language\AST\IntValueNode; +use GraphQL\Language\AST\Node; use GraphQL\Language\AST\StringValueNode; use GraphQL\Utils\Utils; @@ -30,6 +31,7 @@ When expected as an input type, any string (such as `"4"`) or integer /** * @param mixed $value * @return string + * @throws Error */ public function serialize($value) { @@ -51,22 +53,30 @@ When expected as an input type, any string (such as `"4"`) or integer /** * @param mixed $value * @return string + * @throws Error */ public function parseValue($value) { - return (is_string($value) || is_int($value)) ? (string) $value : Utils::undefined(); + if (is_string($value) || is_int($value)) { + return (string) $value; + } + + throw new Error("Cannot represent value as ID: " . Utils::printSafe($value)); } /** - * @param $ast + * @param Node $valueNode * @param array|null $variables * @return null|string + * @throws \Exception */ public function parseLiteral($valueNode, array $variables = null) { if ($valueNode instanceof StringValueNode || $valueNode instanceof IntValueNode) { return $valueNode->value; } - return Utils::undefined(); + + // Intentionally without message, as all information already in wrapped Exception + throw new \Exception(); } } diff --git a/src/Type/Definition/IntType.php b/src/Type/Definition/IntType.php index 0e401cf..3133aa2 100644 --- a/src/Type/Definition/IntType.php +++ b/src/Type/Definition/IntType.php @@ -55,6 +55,7 @@ values. Int can represent values between -(2^31) and 2^31 - 1. '; * @param $valueNode * @param array|null $variables * @return int|null + * @throws \Exception */ public function parseLiteral($valueNode, array $variables = null) { @@ -64,7 +65,9 @@ values. Int can represent values between -(2^31) and 2^31 - 1. '; return $val; } } - return Utils::undefined(); + + // Intentionally without message, as all information already in wrapped Exception + throw new \Exception(); } private function coerceInt($value) { diff --git a/src/Type/Definition/LeafType.php b/src/Type/Definition/LeafType.php index 9569b59..14ded04 100644 --- a/src/Type/Definition/LeafType.php +++ b/src/Type/Definition/LeafType.php @@ -1,6 +1,7 @@ value; } - return Utils::undefined(); + + // Intentionally without message, as all information already in wrapped Exception + throw new \Exception(); } private function coerceString($value) { diff --git a/src/Utils/AST.php b/src/Utils/AST.php index 9a18915..7462cd8 100644 --- a/src/Utils/AST.php +++ b/src/Utils/AST.php @@ -209,9 +209,18 @@ class AST 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; + try { + $serialized = $type->serialize($value); + } catch (\Exception $error) { + if ($error instanceof Error && $type instanceof EnumType) { + return null; + } + throw $error; + } catch (\Throwable $error) { + if ($error instanceof Error && $type instanceof EnumType) { + return null; + } + throw $error; } // Others serialize based on their corresponding PHP scalar types. @@ -400,18 +409,12 @@ class AST // Invalid values represent a failure to parse correctly, in which case // no value is returned. try { - $result = $type->parseLiteral($valueNode, $variables); + return $type->parseLiteral($valueNode, $variables); } catch (\Exception $error) { return $undefined; } catch (\Throwable $error) { return $undefined; } - - if (Utils::isInvalid($result)) { - return $undefined; - } - - return $result; } throw new Error('Unknown type: ' . Utils::printSafe($type) . '.'); @@ -420,7 +423,7 @@ class AST /** * Produces a PHP value given a GraphQL Value AST. * - * Unlike `valueFromAST()`, no type is provided. The resulting JavaScript value + * Unlike `valueFromAST()`, no type is provided. The resulting PHP value * will reflect the provided GraphQL value AST. * * | GraphQL Value | PHP Value | @@ -471,7 +474,7 @@ class AST ); case $valueNode instanceof VariableNode: $variableName = $valueNode->name->value; - return ($variables && isset($variables[$variableName]) && !Utils::isInvalid($variables[$variableName])) + return ($variables && isset($variables[$variableName])) ? $variables[$variableName] : null; } diff --git a/src/Utils/Value.php b/src/Utils/Value.php index 5606091..bf01018 100644 --- a/src/Utils/Value.php +++ b/src/Utils/Value.php @@ -46,14 +46,7 @@ class Value // throw to indicate failure. If it throws, maintain a reference to // the original error. try { - $parseResult = $type->parseValue($value); - if (Utils::isInvalid($parseResult)) { - return self::ofErrors([ - self::coercionError("Expected type {$type->name}", $blameNode, $path), - ]); - } - - return self::ofValue($parseResult); + return self::ofValue($type->parseValue($value)); } catch (\Exception $error) { return self::ofErrors([ self::coercionError( diff --git a/src/Validator/Rules/ValuesOfCorrectType.php b/src/Validator/Rules/ValuesOfCorrectType.php index c77c93b..a64d034 100644 --- a/src/Validator/Rules/ValuesOfCorrectType.php +++ b/src/Validator/Rules/ValuesOfCorrectType.php @@ -173,20 +173,9 @@ class ValuesOfCorrectType extends AbstractValidationRule } // Scalars determine if a literal value is valid via parseLiteral() which - // may throw or return an invalid value to indicate failure. + // may throw to indicate failure. try { - $parseResult = $type->parseLiteral($node); - if (Utils::isInvalid($parseResult)) { - $context->reportError( - new Error( - self::badValueMessage( - (string) $locationType, - Printer::doPrint($node) - ), - $node - ) - ); - } + $type->parseLiteral($node); } catch (\Exception $error) { // Ensure a reference to the original error is maintained. $context->reportError( diff --git a/tests/Executor/TestClasses.php b/tests/Executor/TestClasses.php index ef86938..0ae6ca6 100644 --- a/tests/Executor/TestClasses.php +++ b/tests/Executor/TestClasses.php @@ -1,6 +1,7 @@ value === 'SerializedValue') { return 'DeserializedValue'; } - return Utils::undefined(); + + throw new Error("Cannot represent literal as ComplexScalar: " . Utils::printSafe($valueNode->value)); } }