Spec compliance: coercion of Int values

This fixes an issue with coercing Int values when provided a floating point value which disagrees with the spec definition.

Updates test cases which were along the same incorrect line.

Note: this is technically a breaking change

Fixes #827

Author: leebyron

src/type/__tests__/serialization-test.js

@@ -33,17 +33,30 @@ describe('Type System: Scalar coercion', () => {
       GraphQLInt.serialize(-1)
     ).to.equal(-1);
     expect(
+      GraphQLInt.serialize(1e5)
+    ).to.equal(100000);
+    // The GraphQL specification does not allow serializing non-integer values
+    // as Int to avoid accidental data loss.
+    expect(() =>
       GraphQLInt.serialize(0.1)
-    ).to.equal(0);
-    expect(
+    ).to.throw(
+      'Int cannot represent non-integer value: 0.1'
+    );
+    expect(() =>
       GraphQLInt.serialize(1.1)
-    ).to.equal(1);
-    expect(
+    ).to.throw(
+      'Int cannot represent non-integer value: 1.1'
+    );
+    expect(() =>
       GraphQLInt.serialize(-1.1)
-    ).to.equal(-1);
-    expect(
-      GraphQLInt.serialize(1e5)
-    ).to.equal(100000);
+    ).to.throw(
+      'Int cannot represent non-integer value: -1.1'
+    );
+    expect(() =>
+      GraphQLInt.serialize('-1.1')
+    ).to.throw(
+      'Int cannot represent non-integer value: -1.1'
+    );
     // Maybe a safe JavaScript int, but bigger than 2^32, so not
     // representable as a GraphQL Int
     expect(() =>
@@ -67,9 +80,6 @@ describe('Type System: Scalar coercion', () => {
     ).to.throw(
       'Int cannot represent non 32-bit signed integer value: -1e+100'
     );
-    expect(
-      GraphQLInt.serialize('-1.1')
-    ).to.equal(-1);
     expect(() =>
       GraphQLInt.serialize('one')
     ).to.throw(
@@ -86,6 +96,11 @@ describe('Type System: Scalar coercion', () => {
     ).to.throw(
       'Int cannot represent non 32-bit signed integer value: (empty string)'
     );
+    expect(() =>
+      GraphQLInt.serialize(NaN)
+    ).to.throw(
+      'Int cannot represent non 32-bit signed integer value: NaN'
+    );
   });
 
   it('serializes output float', () => {

src/type/scalars.js

@@ -26,12 +26,18 @@ function coerceInt(value: mixed): ?number {
     );
   }
   const num = Number(value);
-  if (num === num && num <= MAX_INT && num >= MIN_INT) {
-    return (num < 0 ? Math.ceil : Math.floor)(num);
+  if (num !== num || num > MAX_INT || num < MIN_INT) {
+    throw new TypeError(
+      'Int cannot represent non 32-bit signed integer value: ' + String(value)
+    );
   }
-  throw new TypeError(
-    'Int cannot represent non 32-bit signed integer value: ' + String(value)
-  );
+  const int = Math.floor(num);
+  if (int !== num) {
+    throw new TypeError(
+      'Int cannot represent non-integer value: ' + String(value)
+    );
+  }
+  return int;
 }
 
 export const GraphQLInt = new GraphQLScalarType({

src/utilities/__tests__/astFromValue-test.js

@@ -38,6 +38,8 @@ describe('astFromValue', () => {
       null
     );
 
+    expect(astFromValue(NaN, GraphQLInt)).to.deep.equal(null);
+
     expect(astFromValue(null, GraphQLBoolean)).to.deep.equal(
       { kind: 'NullValue' }
     );
@@ -61,14 +63,16 @@ describe('astFromValue', () => {
       { kind: 'IntValue', value: '123' }
     );
 
-    expect(astFromValue(123.5, GraphQLInt)).to.deep.equal(
-      { kind: 'IntValue', value: '123' }
-    );
-
     expect(astFromValue(1e4, GraphQLInt)).to.deep.equal(
       { kind: 'IntValue', value: '10000' }
     );
 
+    // GraphQL spec does not allow coercing non-integer values to Int to avoid
+    // accidental data loss.
+    expect(() => astFromValue(123.5, GraphQLInt)).to.throw(
+      'Int cannot represent non-integer value: 123.5'
+    );
+
     // Note: outside the bounds of 32bit signed int.
     expect(() => astFromValue(1e40, GraphQLInt)).to.throw(
       'Int cannot represent non 32-bit signed integer value: 1e+40'

src/utilities/__tests__/isValidJSValue-test.js

@@ -31,13 +31,13 @@ describe('isValidJSValue for GraphQLInt', () => {
     expectNoErrors(result);
   });
 
-  it('returns no error for exponent input', () => {
-    const result = isValidJSValue('1e3', GraphQLInt);
+  it('returns no error for negative int input', () => {
+    const result = isValidJSValue('-1', GraphQLInt);
     expectNoErrors(result);
   });
 
-  it('returns no error for float input', () => {
-    const result = isValidJSValue('1.5', GraphQLInt);
+  it('returns no error for exponent input', () => {
+    const result = isValidJSValue('1e3', GraphQLInt);
     expectNoErrors(result);
   });
 
@@ -51,6 +51,11 @@ describe('isValidJSValue for GraphQLInt', () => {
     expectErrorResult(result, 1);
   });
 
+  it('returns error for float input as int', () => {
+    const result = isValidJSValue('1.5', GraphQLInt);
+    expectErrorResult(result, 1);
+  });
+
   it('returns a single error for char input', () => {
     const result = isValidJSValue('a', GraphQLInt);
     expectErrorResult(result, 1);