Spec compliance: coercion of Int values

src/Executor/Values.php

@@ -183,7 +183,7 @@ class Values
      * @param InputType $type
      * @return array
      */
-    private static function isValidPHPValue($value, InputType $type)
+    public static function isValidPHPValue($value, InputType $type)
     {
         // A value must be provided if the type is non-null.
         if ($type instanceof NonNull) {

src/Type/Definition/IntType.php

@@ -1,9 +1,9 @@
 <?php
 namespace GraphQL\Type\Definition;
 
+use GraphQL\Error\InvariantViolation;
 use GraphQL\Error\UserError;
 use GraphQL\Language\AST\IntValueNode;
-use GraphQL\Language\AST\ValueNode;
 use GraphQL\Utils;
 
 /**
@@ -47,7 +47,11 @@ values. Int can represent values between -(2^31) and 2^31 - 1. ';
      */
     public function parseValue($value)
     {
-        return $this->coerceInt($value);
+        try {
+            return $this->coerceInt($value);
+        } catch (InvariantViolation $e) {
+            throw new UserError($e->getMessage(), $e->getCode(), $e);
+        }
     }
 
     /**
@@ -57,19 +61,33 @@ values. Int can represent values between -(2^31) and 2^31 - 1. ';
     private function coerceInt($value)
     {
         if ($value === '') {
-            throw new UserError(
+            throw new InvariantViolation(
                 'Int cannot represent non 32-bit signed integer value: (empty string)'
             );
         }
         if (false === $value || true === $value) {
             return (int) $value;
         }
-        if (is_numeric($value) && $value <= self::MAX_INT && $value >= self::MIN_INT) {
+        if (!is_numeric($value) || $value > self::MAX_INT || $value < self::MIN_INT) {
-            return (int) $value;
+            throw new InvariantViolation(
+                sprintf('Int cannot represent non 32-bit signed integer value: %s', Utils::printSafe($value))
+            );
         }
-        throw new UserError(
+        $num = (float) $value;
-            sprintf('Int cannot represent non 32-bit signed integer value: %s', Utils::printSafe($value))
+
-        );
+        // The GraphQL specification does not allow serializing non-integer values
+        // as Int to avoid accidental data loss.
+        // Examples: 1.0 == 1; 1.1 != 1, etc
+        if ($num != (int)$value) {
+            // Additionally account for scientific notation (i.e. 1e3), because (float)'1e3' is 1000, but (int)'1e3' is 1
+            $trimmed = floor($num);
+            if ($trimmed !== $num) {
+                throw new InvariantViolation(
+                    'Int cannot represent non-integer value: ' . Utils::printSafe($value)
+                );
+            }
+        }
+        return (int) $value;
     }
 
     /**

tests/Type/ScalarSerializationTest.php

@@ -1,6 +1,7 @@
 <?php
 namespace GraphQL\Tests\Type;
 
+use GraphQL\Error\InvariantViolation;
 use GraphQL\Error\UserError;
 use GraphQL\Type\Definition\Type;
 
@@ -19,53 +20,77 @@ class ScalarSerializationTest extends \PHPUnit_Framework_TestCase
         $this->assertSame(123, $intType->serialize('123'));
         $this->assertSame(0, $intType->serialize(0));
         $this->assertSame(-1, $intType->serialize(-1));
-        $this->assertSame(0, $intType->serialize(0.1));
-        $this->assertSame(1, $intType->serialize(1.1));
-        $this->assertSame(-1, $intType->serialize(-1.1));
         $this->assertSame(100000, $intType->serialize(1e5));
+        $this->assertSame(0, $intType->serialize(0e5));
+
+        // The GraphQL specification does not allow serializing non-integer values
+        // as Int to avoid accidental data loss.
+        try {
+            $intType->serialize(0.1);
+            $this->fail('Expected exception not thrown');
+        } catch (InvariantViolation $e) {
+            $this->assertEquals('Int cannot represent non-integer value: 0.1', $e->getMessage());
+        }
+        try {
+            $intType->serialize(1.1);
+            $this->fail('Expected exception not thrown');
+        } catch (InvariantViolation $e) {
+            $this->assertEquals('Int cannot represent non-integer value: 1.1', $e->getMessage());
+        }
+        try {
+            $intType->serialize(-1.1);
+            $this->fail('Expected exception not thrown');
+        } catch (InvariantViolation $e) {
+            $this->assertEquals('Int cannot represent non-integer value: -1.1', $e->getMessage());
+        }
+        try {
+            $intType->serialize('-1.1');
+            $this->fail('Expected exception not thrown');
+        } catch (InvariantViolation $e) {
+            $this->assertEquals('Int cannot represent non-integer value: "-1.1"', $e->getMessage());
+        }
+
         // Maybe a safe PHP int, but bigger than 2^32, so not
         // representable as a GraphQL Int
         try {
             $intType->serialize(9876504321);
             $this->fail('Expected exception was not thrown');
-        } catch (UserError $e) {
+        } catch (InvariantViolation $e) {
             $this->assertEquals('Int cannot represent non 32-bit signed integer value: 9876504321', $e->getMessage());
         }
 
         try {
             $intType->serialize(-9876504321);
             $this->fail('Expected exception was not thrown');
-        } catch (UserError $e) {
+        } catch (InvariantViolation $e) {
             $this->assertEquals('Int cannot represent non 32-bit signed integer value: -9876504321', $e->getMessage());
         }
 
         try {
             $intType->serialize(1e100);
             $this->fail('Expected exception was not thrown');
-        } catch (UserError $e) {
+        } catch (InvariantViolation $e) {
             $this->assertEquals('Int cannot represent non 32-bit signed integer value: 1.0E+100', $e->getMessage());
         }
 
         try {
             $intType->serialize(-1e100);
             $this->fail('Expected exception was not thrown');
-        } catch (UserError $e) {
+        } catch (InvariantViolation $e) {
             $this->assertEquals('Int cannot represent non 32-bit signed integer value: -1.0E+100', $e->getMessage());
         }
 
-        $this->assertSame(-1, $intType->serialize('-1.1'));
-
         try {
             $intType->serialize('one');
             $this->fail('Expected exception was not thrown');
-        } catch (UserError $e) {
+        } catch (InvariantViolation $e) {
             $this->assertEquals('Int cannot represent non 32-bit signed integer value: "one"', $e->getMessage());
         }
 
         try {
             $intType->serialize('');
             $this->fail('Expected exception was not thrown');
-        } catch (UserError $e) {
+        } catch (InvariantViolation $e) {
             $this->assertEquals('Int cannot represent non 32-bit signed integer value: (empty string)', $e->getMessage());
         }
 

tests/Utils/AstFromValueTest.php

@@ -40,8 +40,17 @@ class ASTFromValueTest extends \PHPUnit_Framework_TestCase
     public function testConvertsIntValuesToASTs()
     {
         $this->assertEquals(new IntValueNode(['value' => '123']), AST::astFromValue(123.0, Type::int()));
-        $this->assertEquals(new IntValueNode(['value' => '123']), AST::astFromValue(123.5, Type::int()));
         $this->assertEquals(new IntValueNode(['value' => '10000']), AST::astFromValue(1e4, Type::int()));
+        $this->assertEquals(new IntValueNode(['value' => '0']), AST::astFromValue(0e4, Type::int()));
+
+        // GraphQL spec does not allow coercing non-integer values to Int to avoid
+        // accidental data loss.
+        try {
+            AST::astFromValue(123.5, Type::int());
+            $this->fail('Expected exception not thrown');
+        } catch (\Exception $e) {
+            $this->assertEquals('Int cannot represent non-integer value: 123.5', $e->getMessage());
+        }
 
         try {
             AST::astFromValue(1e40, Type::int()); // Note: js version will produce 1e+40, both values are valid GraphQL floats
@@ -61,6 +70,7 @@ class ASTFromValueTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(new FloatValueNode(['value' => '123.5']), AST::astFromValue(123.5, Type::float()));
         $this->assertEquals(new IntValueNode(['value' => '10000']), AST::astFromValue(1e4, Type::float()));
         $this->assertEquals(new FloatValueNode(['value' => '1e+40']), AST::astFromValue(1e40, Type::float()));
+        $this->assertEquals(new IntValueNode(['value' => '0']), AST::astFromValue(0e40, Type::float()));
     }
 
     /**

tests/Utils/IsValidPHPValueTest.php

@@ -0,0 +1,90 @@
+<?php
+namespace GraphQL\Tests\Utils;
+
+
+use GraphQL\Executor\Values;
+use GraphQL\Type\Definition\Type;
+
+class IsValidPHPValueTest extends \PHPUnit_Framework_TestCase
+{
+    public function testValidIntValue()
+    {
+        // returns no error for int input
+        $result = Values::isValidPHPValue('1', Type::int());
+        $this->expectNoErrors($result);
+
+        // returns no error for negative int input:
+        $result = Values::isValidPHPValue('-1', Type::int());
+        $this->expectNoErrors($result);
+
+        // returns no error for exponent input:
+        $result = Values::isValidPHPValue('1e3', Type::int());
+        $this->expectNoErrors($result);
+        $result = Values::isValidPHPValue('0e3', Type::int());
+        $this->expectNoErrors($result);
+
+        // returns no error for null value:
+        $result = Values::isValidPHPValue(null, Type::int());
+        $this->expectNoErrors($result);
+
+        // returns a single error for empty value
+        $result = Values::isValidPHPValue('', Type::int());
+        $this->expectErrorResult($result, 1);
+
+        // returns error for float input as int
+        $result = Values::isValidPHPValue('1.5', Type::int());
+        $this->expectErrorResult($result, 1);
+
+        // returns a single error for char input
+        $result = Values::isValidPHPValue('a', Type::int());
+        $this->expectErrorResult($result, 1);
+
+        // returns a single error for char input
+        $result = Values::isValidPHPValue('meow', Type::int());
+        $this->expectErrorResult($result, 1);
+    }
+
+    public function testValidFloatValue()
+    {
+        // returns no error for int input
+        $result = Values::isValidPHPValue('1', Type::float());
+        $this->expectNoErrors($result);
+
+        // returns no error for exponent input
+        $result = Values::isValidPHPValue('1e3', Type::float());
+        $this->expectNoErrors($result);
+        $result = Values::isValidPHPValue('0e3', Type::float());
+        $this->expectNoErrors($result);
+
+        // returns no error for float input
+        $result = Values::isValidPHPValue('1.5', Type::float());
+        $this->expectNoErrors($result);
+
+        // returns no error for null value:
+        $result = Values::isValidPHPValue(null, Type::float());
+        $this->expectNoErrors($result);
+
+        // returns a single error for empty value
+        $result = Values::isValidPHPValue('', Type::float());
+        $this->expectErrorResult($result, 1);
+
+        // returns a single error for char input
+        $result = Values::isValidPHPValue('a', Type::float());
+        $this->expectErrorResult($result, 1);
+
+        // returns a single error for char input
+        $result = Values::isValidPHPValue('meow', Type::float());
+        $this->expectErrorResult($result, 1);
+    }
+
+    private function expectNoErrors($result)
+    {
+        $this->assertInternalType('array', $result);
+        $this->assertEquals([], $result);
+    }
+
+    private function expectErrorResult($result, $size) {
+        $this->assertInternalType('array', $result);
+        $this->assertEquals($size, count($result));
+    }
+}