Skip to content

Commit

Permalink
Revert "BREAKING/BUGFIX: Strict coercion of scalar types (webonyx#278)"
Browse files Browse the repository at this point in the history
This reverts commit 975c9fe
  • Loading branch information
mdio committed Sep 30, 2022
1 parent 04a4869 commit 08abff0
Show file tree
Hide file tree
Showing 12 changed files with 393 additions and 291 deletions.
12 changes: 10 additions & 2 deletions src/Type/Definition/BooleanType.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,23 @@ class BooleanType extends ScalarType
public $description = 'The `Boolean` scalar type represents `true` or `false`.';

/**
* Serialize the given value to a boolean.
* Coerce the given value to a boolean.
*
* The GraphQL spec leaves this up to the implementations, so we just do what
* PHP does natively to make this intuitive for developers.
*
* @param mixed $value
*
* @throws Error
*/
public function serialize($value) : bool
{
if (is_array($value)) {
throw new Error(
'Boolean cannot represent an array value: ' . Utils::printSafe($value)
);
}

return (bool) $value;
}

Expand All @@ -45,7 +53,7 @@ public function parseValue($value)
return $value;
}

throw new Error('Boolean cannot represent a non boolean value: ' . Utils::printSafe($value));
throw new Error('Cannot represent value as boolean: ' . Utils::printSafe($value));
}

/**
Expand Down
55 changes: 32 additions & 23 deletions src/Type/Definition/FloatType.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
use GraphQL\Language\AST\IntValueNode;
use GraphQL\Language\AST\Node;
use GraphQL\Utils\Utils;
use function floatval;
use function is_array;
use function is_bool;
use function is_finite;
use function is_float;
use function is_int;
use function is_nan;
use function is_numeric;
use function sprintf;

class FloatType extends ScalarType
{
Expand All @@ -31,15 +31,32 @@ class FloatType extends ScalarType
/**
* @param mixed $value
*
* @return float|null
*
* @throws Error
*/
public function serialize($value) : float
public function serialize($value)
{
$float = is_numeric($value) || is_bool($value)
? (float) $value
: null;
return $this->coerceFloat($value);
}

if ($float === null || ! is_finite($float)) {
private function coerceFloat($value)
{
if (is_array($value)) {
throw new Error(
sprintf('Float cannot represent an array value: %s', Utils::printSafe($value))
);
}

if ($value === '') {
throw new Error(
'Float cannot represent non numeric value: (empty string)'
);
}

$float = is_numeric($value) || is_bool($value) ? (float) $value : null;

if ($float === null || ! is_finite($float) || is_nan($float)) {
throw new Error(
'Float cannot represent non numeric value: ' .
Utils::printSafe($value)
Expand All @@ -52,28 +69,20 @@ public function serialize($value) : float
/**
* @param mixed $value
*
* @return float|null
*
* @throws Error
*/
public function parseValue($value) : float
public function parseValue($value)
{
$float = is_float($value) || is_int($value)
? (float) $value
: null;

if ($float === null || ! is_finite($float)) {
throw new Error(
'Float cannot represent non numeric value: ' .
Utils::printSafe($value)
);
}

return $float;
return $this->coerceFloat($value);
}

/**
* @param Node $valueNode
* @param mixed[]|null $variables
*
* @return float
* @return float|null
*
* @throws Exception
*/
Expand All @@ -84,6 +93,6 @@ public function parseLiteral(Node $valueNode, ?array $variables = null)
}

// Intentionally without message, as all information already in wrapped Exception
throw new Error();
throw new Exception();
}
}
30 changes: 23 additions & 7 deletions src/Type/Definition/IDType.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,22 @@ class IDType extends ScalarType
*/
public function serialize($value)
{
$canCast = is_string($value)
|| is_int($value)
|| (is_object($value) && method_exists($value, '__toString'));

if (! $canCast) {
throw new Error('ID cannot represent value: ' . Utils::printSafe($value));
if ($value === true) {
return 'true';
}
if ($value === false) {
return 'false';
}
if ($value === null) {
return 'null';
}
if (is_array($value)) {
throw new Error(
'ID cannot represent an array value: ' . Utils::printSafe($value)
);
}
if (! is_scalar($value) && (! is_object($value) || ! method_exists($value, '__toString'))) {
throw new Error('ID cannot represent non scalar value: ' . Utils::printSafe($value));
}

return (string) $value;
Expand All @@ -58,7 +68,13 @@ public function parseValue($value) : string
if (is_string($value) || is_int($value)) {
return (string) $value;
}
throw new Error('ID cannot represent value: ' . Utils::printSafe($value));
if (is_array($value)) {
throw new Error(
'ID cannot represent an array value: ' . Utils::printSafe($value)
);
}

throw new Error('Cannot represent value as ID: ' . Utils::printSafe($value));
}

/**
Expand Down
73 changes: 41 additions & 32 deletions src/Type/Definition/IntType.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@
use GraphQL\Language\AST\Node;
use GraphQL\Utils\Utils;
use function floatval;
use function floor;
use function intval;
use function is_array;
use function is_bool;
use function is_float;
use function is_int;
use function is_numeric;
use function sprintf;

class IntType extends ScalarType
{
Expand Down Expand Up @@ -44,62 +43,72 @@ class IntType extends ScalarType
*/
public function serialize($value)
{
// Fast path for 90+% of cases:
if (is_int($value) && $value <= self::MAX_INT && $value >= self::MIN_INT) {
return $value;
return $this->coerceInt($value);
}

/**
* @param mixed $value
*
* @return int
*/
private function coerceInt($value)
{
if (is_array($value)) {
throw new Error(
sprintf('Int cannot represent an array value: %s', Utils::printSafe($value))
);
}

$float = is_numeric($value) || is_bool($value)
? (float) $value
: null;
if ($value === '') {
throw new Error(
'Int cannot represent non-integer value: (empty string)'
);
}

if ($float === null || floor($float) !== $float) {
if (! is_numeric($value) && ! is_bool($value)) {
throw new Error(
'Int cannot represent non-integer value: ' .
Utils::printSafe($value)
);
}

if ($float > self::MAX_INT || $float < self::MIN_INT) {
$num = floatval($value);
if ($num > self::MAX_INT || $num < self::MIN_INT) {
throw new Error(
'Int cannot represent non 32-bit signed integer value: ' .
Utils::printSafe($value)
);
}
$int = intval($num);
// int cast with == used for performance reasons
// phpcs:ignore
if ($int != $num) {
throw new Error(
'Int cannot represent non-integer value: ' .
Utils::printSafe($value)
);
}

return (int) $float;
return $int;
}

/**
* @param mixed $value
*
* @return int|null
*
* @throws Error
*/
public function parseValue($value) : int
public function parseValue($value)
{
$isInt = is_int($value) || (is_float($value) && floor($value) === $value);

if (! $isInt) {
throw new Error(
'Int cannot represent non-integer value: ' .
Utils::printSafe($value)
);
}

if ($value > self::MAX_INT || $value < self::MIN_INT) {
throw new Error(
'Int cannot represent non 32-bit signed integer value: ' .
Utils::printSafe($value)
);
}

return (int) $value;
return $this->coerceInt($value);
}

/**
* @param Node $valueNode
* @param mixed[]|null $variables
*
* @return int
* @return int|null
*
* @throws Exception
*/
Expand All @@ -113,6 +122,6 @@ public function parseLiteral(Node $valueNode, ?array $variables = null)
}

// Intentionally without message, as all information already in wrapped Exception
throw new Error();
throw new Exception();
}
}
37 changes: 24 additions & 13 deletions src/Type/Definition/StringType.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use GraphQL\Utils\Utils;
use function is_object;
use function is_scalar;
use function is_string;
use function method_exists;

class StringType extends ScalarType
Expand All @@ -34,13 +33,31 @@ class StringType extends ScalarType
*/
public function serialize($value)
{
$canCast = is_scalar($value)
|| (is_object($value) && method_exists($value, '__toString'))
|| $value === null;
return $this->coerceString($value);
}

if (! $canCast) {
private function coerceString($value)
{
if ($value === true) {
return 'true';
}
if ($value === false) {
return 'false';
}
if ($value === null) {
return 'null';
}
if (is_array($value)) {
throw new Error(
'String cannot represent value: ' . Utils::printSafe($value)
'String cannot represent an array value: ' . Utils::printSafe($value)
);
}
if (is_object($value) && method_exists($value, '__toString')) {
return (string) $value;
}
if (! is_scalar($value)) {
throw new Error(
'String cannot represent non scalar value: ' . Utils::printSafe($value)
);
}

Expand All @@ -56,13 +73,7 @@ public function serialize($value)
*/
public function parseValue($value)
{
if (! is_string($value)) {
throw new Error(
'String cannot represent a non string value: ' . Utils::printSafe($value)
);
}

return $value;
return $this->coerceString($value);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/Executor/ExecutorSchemaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public function testExecutesUsingASchema() : void
'isPublished' => true,
'title' => 'My Article 1',
'body' => 'This is a post',
'keywords' => ['foo', 'bar', '1', '1', null],
'keywords' => ['foo', 'bar', '1', 'true', null],
],
],
'meta' => [ 'title' => 'My Article 1 | My Blog' ],
Expand Down
2 changes: 1 addition & 1 deletion tests/Executor/VariablesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ public function testReportsErrorForArrayPassedIntoStringInput() : void
'errors' => [[
'message' =>
'Variable "$value" got invalid value [1,2,3]; Expected type ' .
'String; String cannot represent a non string value: [1,2,3]',
'String; String cannot represent an array value: [1,2,3]',
'locations' => [
['line' => 2, 'column' => 31],
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

declare(strict_types=1);

namespace GraphQL\Tests\Type\TestClasses;
namespace GraphQL\Tests\Type;

class ObjectIdStub
{
Expand Down
Loading

0 comments on commit 08abff0

Please sign in to comment.