Scalar coercion cleanup (#1414)

* Cleanup + restrict Boolean serialize coercion to numbers

* Make coercion code more explicit.

* Address review comments

* fixing const num that's not const

Author: IvanGoncharov

src/execution/__tests__/schema-test.js

@@ -91,7 +91,7 @@ describe('Execute: Handles execution with a complex schema', () => {
     function article(id) {
       return {
         id,
-        isPublished: 'true',
+        isPublished: true,
         author: johnSmith,
         title: 'My Article ' + id,
         body: 'This is a post',

src/type/__tests__/serialization-test.js

@@ -69,7 +69,7 @@ describe('Type System: Scalar coercion', () => {
       'Int cannot represent non-integer value: Infinity',
     );
     expect(() => GraphQLInt.serialize([5])).to.throw(
-      'Int cannot represent an array value: [5]',
+      'Int cannot represent non-integer value: [5]',
     );
   });
 
@@ -98,7 +98,7 @@ describe('Type System: Scalar coercion', () => {
       'Float cannot represent non numeric value: ""',
     );
     expect(() => GraphQLFloat.serialize([5])).to.throw(
-      'Float cannot represent an array value: [5]',
+      'Float cannot represent non numeric value: [5]',
     );
   });
 
@@ -109,15 +109,6 @@ describe('Type System: Scalar coercion', () => {
     expect(GraphQLString.serialize(true)).to.equal('true');
     expect(GraphQLString.serialize(false)).to.equal('false');
 
-    expect(() => GraphQLString.serialize([1])).to.throw(
-      'String cannot represent value: [1]',
-    );
-
-    const badObjValue = {};
-    expect(() => GraphQLString.serialize(badObjValue)).to.throw(
-      'String cannot represent value: {}',
-    );
-
     const stringableObjValue = {
       valueOf() {
         return 'something useful';
@@ -126,19 +117,41 @@ describe('Type System: Scalar coercion', () => {
     expect(GraphQLString.serialize(stringableObjValue)).to.equal(
       'something useful',
     );
+
+    expect(() => GraphQLString.serialize(NaN)).to.throw(
+      'String cannot represent value: NaN',
+    );
+
+    expect(() => GraphQLString.serialize([1])).to.throw(
+      'String cannot represent value: [1]',
+    );
+
+    const badObjValue = {};
+    expect(() => GraphQLString.serialize(badObjValue)).to.throw(
+      'String cannot represent value: {}',
+    );
   });
 
   it('serializes output as Boolean', () => {
-    expect(GraphQLBoolean.serialize('string')).to.equal(true);
-    expect(GraphQLBoolean.serialize('false')).to.equal(true);
-    expect(GraphQLBoolean.serialize('')).to.equal(false);
     expect(GraphQLBoolean.serialize(1)).to.equal(true);
     expect(GraphQLBoolean.serialize(0)).to.equal(false);
     expect(GraphQLBoolean.serialize(true)).to.equal(true);
     expect(GraphQLBoolean.serialize(false)).to.equal(false);
 
+    expect(() => GraphQLBoolean.serialize(NaN)).to.throw(
+      'Boolean cannot represent a non boolean value: NaN',
+    );
+    expect(() => GraphQLBoolean.serialize('')).to.throw(
+      'Boolean cannot represent a non boolean value: ""',
+    );
+    expect(() => GraphQLBoolean.serialize('true')).to.throw(
+      'Boolean cannot represent a non boolean value: "true"',
+    );
     expect(() => GraphQLBoolean.serialize([false])).to.throw(
-      'Boolean cannot represent an array value: [false]',
+      'Boolean cannot represent a non boolean value: [false]',
+    );
+    expect(() => GraphQLBoolean.serialize({})).to.throw(
+      'Boolean cannot represent a non boolean value: {}',
     );
   });
 
@@ -148,6 +161,7 @@ describe('Type System: Scalar coercion', () => {
     expect(GraphQLID.serialize('')).to.equal('');
     expect(GraphQLID.serialize(123)).to.equal('123');
     expect(GraphQLID.serialize(0)).to.equal('0');
+    expect(GraphQLID.serialize(-1)).to.equal('-1');
 
     const objValue = {
       _id: 123,
@@ -171,8 +185,8 @@ describe('Type System: Scalar coercion', () => {
       'ID cannot represent value: true',
     );
 
-    expect(() => GraphQLID.serialize(-1.1)).to.throw(
-      'ID cannot represent value: -1.1',
+    expect(() => GraphQLID.serialize(3.14)).to.throw(
+      'ID cannot represent value: 3.14',
     );
 
     expect(() => GraphQLID.serialize({})).to.throw(

src/type/scalars.js

@@ -22,13 +22,16 @@ const MAX_INT = 2147483647;
 const MIN_INT = -2147483648;
 
 function serializeInt(value: mixed): number {
-  if (Array.isArray(value)) {
-    throw new TypeError(
-      `Int cannot represent an array value: ${inspect(value)}`,
-    );
+  if (typeof value === 'boolean') {
+    return value ? 1 : 0;
   }
-  const num = Number(value);
-  if (value === '' || !isInteger(num)) {
+
+  let num = value;
+  if (typeof value === 'string' && value !== '') {
+    num = Number(value);
+  }
+
+  if (!isInteger(num)) {
     throw new TypeError(
       `Int cannot represent non-integer value: ${inspect(value)}`,
     );
@@ -74,13 +77,15 @@ export const GraphQLInt = new GraphQLScalarType({
 });
 
 function serializeFloat(value: mixed): number {
-  if (Array.isArray(value)) {
-    throw new TypeError(
-      `Float cannot represent an array value: ${inspect(value)}`,
-    );
+  if (typeof value === 'boolean') {
+    return value ? 1 : 0;
+  }
+
+  let num = value;
+  if (typeof value === 'string' && value !== '') {
+    num = Number(value);
   }
-  const num = Number(value);
-  if (value === '' || !isFinite(num)) {
+  if (!isFinite(num)) {
     throw new TypeError(
       `Float cannot represent non numeric value: ${inspect(value)}`,
     );
@@ -118,16 +123,18 @@ function serializeString(value: mixed): string {
   // (ex: MongoDB id objects).
   const result =
     value && typeof value.valueOf === 'function' ? value.valueOf() : value;
-  // Serialize string, number, and boolean values to a string, but do not
+  // Serialize string, boolean and number values to a string, but do not
   // attempt to coerce object, function, symbol, or other types as strings.
-  if (
-    typeof result !== 'string' &&
-    typeof result !== 'number' &&
-    typeof result !== 'boolean'
-  ) {
-    throw new TypeError(`String cannot represent value: ${inspect(result)}`);
-  }
-  return String(result);
+  if (typeof result === 'string') {
+    return result;
+  }
+  if (typeof result === 'boolean') {
+    return result ? 'true' : 'false';
+  }
+  if (isFinite(result)) {
+    return result.toString();
+  }
+  throw new TypeError(`String cannot represent value: ${inspect(value)}`);
 }
 
 function coerceString(value: mixed): string {
@@ -153,12 +160,15 @@ export const GraphQLString = new GraphQLScalarType({
 });
 
 function serializeBoolean(value: mixed): boolean {
-  if (Array.isArray(value)) {
-    throw new TypeError(
-      `Boolean cannot represent an array value: ${inspect(value)}`,
-    );
+  if (typeof value === 'boolean') {
+    return value;
   }
-  return Boolean(value);
+  if (isFinite(value)) {
+    return value !== 0;
+  }
+  throw new TypeError(
+    `Boolean cannot represent a non boolean value: ${inspect(value)}`,
+  );
 }
 
 function coerceBoolean(value: mixed): boolean {
@@ -185,23 +195,23 @@ function serializeID(value: mixed): string {
   // to represent an object identifier (ex. MongoDB).
   const result =
     value && typeof value.valueOf === 'function' ? value.valueOf() : value;
-  if (
-    typeof result !== 'string' &&
-    (typeof result !== 'number' || !isInteger(result))
-  ) {
-    throw new TypeError(`ID cannot represent value: ${inspect(value)}`);
+  if (typeof result === 'string') {
+    return result;
   }
-  return String(result);
+  if (isInteger(result)) {
+    return String(result);
+  }
+  throw new TypeError(`ID cannot represent value: ${inspect(value)}`);
 }
 
 function coerceID(value: mixed): string {
-  if (
-    typeof value !== 'string' &&
-    (typeof value !== 'number' || !isInteger(value))
-  ) {
-    throw new TypeError(`ID cannot represent value: ${inspect(value)}`);
+  if (typeof value === 'string') {
+    return value;
+  }
+  if (isInteger(value)) {
+    return value.toString();
   }
-  return String(value);
+  throw new TypeError(`ID cannot represent value: ${inspect(value)}`);
 }
 
 export const GraphQLID = new GraphQLScalarType({