Add suggestions for invalid values (#1153)

leebyron · web-flow · commit b283d9b5b629 · 2017-12-15T15:21:46.000-08:00
For misspelled enums or field names, these suggestions can be helpful.

This also changes the suggestions algorithm to better detect case-sensitivity mistakes, which are common
diff --git a/src/execution/__tests__/variables-test.js b/src/execution/__tests__/variables-test.js
@@ -296,7 +296,7 @@ describe('Execute: Handles inputs', () => {
               message:
                 'Variable "$input" got invalid value ' +
                 '{"a":"foo","b":"bar","c":null}; ' +
-                'Expected non-nullable type String! at value.c.',
+                'Expected non-nullable type String! not to be null at value.c.',
               locations: [{ line: 2, column: 17 }],
               path: undefined,
             },
@@ -313,7 +313,7 @@ describe('Execute: Handles inputs', () => {
             {
               message:
                 'Variable "$input" got invalid value "foo bar"; ' +
-                'Expected object type TestInputObject.',
+                'Expected type TestInputObject to be an object.',
               locations: [{ line: 2, column: 17 }],
               path: undefined,
             },
@@ -535,7 +535,7 @@ describe('Execute: Handles inputs', () => {
           {
             message:
               'Variable "$value" got invalid value null; ' +
-              'Expected non-nullable type String!.',
+              'Expected non-nullable type String! not to be null.',
             locations: [{ line: 2, column: 31 }],
             path: undefined,
           },
@@ -734,7 +734,7 @@ describe('Execute: Handles inputs', () => {
           {
             message:
               'Variable "$input" got invalid value null; ' +
-              'Expected non-nullable type [String]!.',
+              'Expected non-nullable type [String]! not to be null.',
             locations: [{ line: 2, column: 17 }],
             path: undefined,
           },
@@ -825,7 +825,7 @@ describe('Execute: Handles inputs', () => {
           {
             message:
               'Variable "$input" got invalid value ["A",null,"B"]; ' +
-              'Expected non-nullable type String! at value[1].',
+              'Expected non-nullable type String! not to be null at value[1].',
             locations: [{ line: 2, column: 17 }],
             path: undefined,
           },
@@ -847,7 +847,7 @@ describe('Execute: Handles inputs', () => {
           {
             message:
               'Variable "$input" got invalid value null; ' +
-              'Expected non-nullable type [String!]!.',
+              'Expected non-nullable type [String!]! not to be null.',
             locations: [{ line: 2, column: 17 }],
             path: undefined,
           },
@@ -887,7 +887,7 @@ describe('Execute: Handles inputs', () => {
           {
             message:
               'Variable "$input" got invalid value ["A",null,"B"]; ' +
-              'Expected non-nullable type String! at value[1].',
+              'Expected non-nullable type String! not to be null at value[1].',
             locations: [{ line: 2, column: 17 }],
             path: undefined,
           },
diff --git a/src/jsutils/suggestionList.js b/src/jsutils/suggestionList.js
@@ -38,19 +38,34 @@ export default function suggestionList(
  * insertion, deletion, or substitution of a single character, or a swap of two
  * adjacent characters.
  *
+ * Includes a custom alteration from Damerau-Levenshtein to treat case changes
+ * as a single edit which helps identify mis-cased values with an edit distance
+ * of 1.
+ *
  * This distance can be useful for detecting typos in input or sorting
  *
  * @param {string} a
  * @param {string} b
  * @return {int} distance in number of edits
  */
-function lexicalDistance(a, b) {
+function lexicalDistance(aStr, bStr) {
+  if (aStr === bStr) {
+    return 0;
+  }
+
   let i;
   let j;
   const d = [];
+  const a = aStr.toLowerCase();
+  const b = bStr.toLowerCase();
   const aLength = a.length;
   const bLength = b.length;
 
+  // Any case change counts as a single edit
+  if (a === b) {
+    return 1;
+  }
+
   for (i = 0; i <= aLength; i++) {
     d[i] = [i];
   }
diff --git a/src/type/__tests__/enumType-test.js b/src/type/__tests__/enumType-test.js
@@ -161,7 +161,7 @@ describe('Type System: Enum Values', () => {
       errors: [
         {
           message:
-            'Expected type Color, found "GREEN"; Did you mean the enum value: GREEN?',
+            'Expected type Color, found "GREEN"; Did you mean the enum value GREEN?',
           locations: [{ line: 1, column: 23 }],
         },
       ],
@@ -175,7 +175,21 @@ describe('Type System: Enum Values', () => {
       errors: [
         {
           message:
-            'Expected type Color, found GREENISH; Did you mean the enum value: GREEN?',
+            'Expected type Color, found GREENISH; Did you mean the enum value GREEN?',
+          locations: [{ line: 1, column: 23 }],
+        },
+      ],
+    });
+  });
+
+  it('does not accept values with incorrect casing', async () => {
+    expect(
+      await graphql(schema, '{ colorEnum(fromEnum: green) }'),
+    ).to.jsonEqual({
+      errors: [
+        {
+          message:
+            'Expected type Color, found green; Did you mean the enum value GREEN?',
           locations: [{ line: 1, column: 23 }],
         },
       ],
diff --git a/src/utilities/__tests__/coerceValue-test.js b/src/utilities/__tests__/coerceValue-test.js
@@ -8,15 +8,24 @@
 import { describe, it } from 'mocha';
 import { expect } from 'chai';
 import { coerceValue } from '../coerceValue';
-import { GraphQLInt, GraphQLFloat, GraphQLString } from '../../type';
+import {
+  GraphQLInt,
+  GraphQLFloat,
+  GraphQLString,
+  GraphQLEnumType,
+  GraphQLInputObjectType,
+  GraphQLNonNull,
+} from '../../type';
 
 function expectNoErrors(result) {
   expect(result.errors).to.equal(undefined);
+  expect(result.value).not.to.equal(undefined);
 }
 
 function expectError(result, expected) {
   const messages = result.errors && result.errors.map(error => error.message);
   expect(messages).to.deep.equal([expected]);
+  expect(result.value).to.equal(undefined);
 }
 
 describe('coerceValue', () => {
@@ -130,4 +139,103 @@ describe('coerceValue', () => {
       );
     });
   });
+
+  describe('for GraphQLEnum', () => {
+    const TestEnum = new GraphQLEnumType({
+      name: 'TestEnum',
+      values: {
+        FOO: { value: 'InternalFoo' },
+        BAR: { value: 123456789 },
+      },
+    });
+
+    it('returns no error for a known enum name', () => {
+      const fooResult = coerceValue('FOO', TestEnum);
+      expectNoErrors(fooResult);
+      expect(fooResult.value).to.equal('InternalFoo');
+
+      const barResult = coerceValue('BAR', TestEnum);
+      expectNoErrors(barResult);
+      expect(barResult.value).to.equal(123456789);
+    });
+
+    it('results error for misspelled enum value', () => {
+      const result = coerceValue('foo', TestEnum);
+      expectError(result, 'Expected type TestEnum; did you mean FOO?');
+    });
+
+    it('results error for incorrect value type', () => {
+      const result1 = coerceValue(123, TestEnum);
+      expectError(result1, 'Expected type TestEnum.');
+
+      const result2 = coerceValue({ field: 'value' }, TestEnum);
+      expectError(result2, 'Expected type TestEnum.');
+    });
+  });
+
+  describe('for GraphQLInputObject', () => {
+    const TestInputObject = new GraphQLInputObjectType({
+      name: 'TestInputObject',
+      fields: {
+        foo: { type: GraphQLNonNull(GraphQLInt) },
+        bar: { type: GraphQLInt },
+      },
+    });
+
+    it('returns no error for a valid input', () => {
+      const result = coerceValue({ foo: 123 }, TestInputObject);
+      expectNoErrors(result);
+      expect(result.value).to.deep.equal({ foo: 123 });
+    });
+
+    it('returns no error for a non-object type', () => {
+      const result = coerceValue(123, TestInputObject);
+      expectError(result, 'Expected type TestInputObject to be an object.');
+    });
+
+    it('returns no error for an invalid field', () => {
+      const result = coerceValue({ foo: 'abc' }, TestInputObject);
+      expectError(
+        result,
+        'Expected type Int at value.foo; Int cannot represent non 32-bit signed integer value: abc',
+      );
+    });
+
+    it('returns multiple errors for multiple invalid fields', () => {
+      const result = coerceValue({ foo: 'abc', bar: 'def' }, TestInputObject);
+      expect(
+        result.errors && result.errors.map(error => error.message),
+      ).to.deep.equal([
+        'Expected type Int at value.foo; Int cannot represent non 32-bit signed integer value: abc',
+        'Expected type Int at value.bar; Int cannot represent non 32-bit signed integer value: def',
+      ]);
+    });
+
+    it('returns error for a missing required field', () => {
+      const result = coerceValue({ bar: 123 }, TestInputObject);
+      expectError(
+        result,
+        'Field value.foo of required type Int! was not provided.',
+      );
+    });
+
+    it('returns error for an unknown field', () => {
+      const result = coerceValue(
+        { foo: 123, unknownField: 123 },
+        TestInputObject,
+      );
+      expectError(
+        result,
+        'Field "unknownField" is not defined by type TestInputObject.',
+      );
+    });
+
+    it('returns error for a misspelled field', () => {
+      const result = coerceValue({ foo: 123, bart: 123 }, TestInputObject);
+      expectError(
+        result,
+        'Field "bart" is not defined by type TestInputObject; did you mean bar?',
+      );
+    });
+  });
 });
diff --git a/src/utilities/coerceValue.js b/src/utilities/coerceValue.js
@@ -10,6 +10,8 @@
 import { forEach, isCollection } from 'iterall';
 import isInvalid from '../jsutils/isInvalid';
 import isNullish from '../jsutils/isNullish';
+import orList from '../jsutils/orList';
+import suggestionList from '../jsutils/suggestionList';
 import { GraphQLError } from '../error';
 import type { ASTNode } from '../language/ast';
 import {
@@ -46,7 +48,7 @@ export function coerceValue(
     if (isNullish(value)) {
       return ofErrors([
         coercionError(
-          `Expected non-nullable type ${String(type)}`,
+          `Expected non-nullable type ${String(type)} not to be null`,
           blameNode,
           path,
         ),
@@ -74,7 +76,13 @@ export function coerceValue(
       return ofValue(parseResult);
     } catch (error) {
       return ofErrors([
-        coercionError(`Expected type ${type.name}`, blameNode, path, error),
+        coercionError(
+          `Expected type ${type.name}`,
+          blameNode,
+          path,
+          error.message,
+          error,
+        ),
       ]);
     }
   }
@@ -86,8 +94,16 @@ export function coerceValue(
         return ofValue(enumValue.value);
       }
     }
+    const suggestions = suggestionList(
+      String(value),
+      type.getValues().map(enumValue => enumValue.name),
+    );
+    const didYouMean =
+      suggestions.length !== 0
+        ? `did you mean ${orList(suggestions)}?`
+        : undefined;
     return ofErrors([
-      coercionError(`Expected type ${type.name}`, blameNode, path),
+      coercionError(`Expected type ${type.name}`, blameNode, path, didYouMean),
     ]);
   }
 
@@ -119,7 +135,11 @@ export function coerceValue(
   if (isInputObjectType(type)) {
     if (typeof value !== 'object') {
       return ofErrors([
-        coercionError(`Expected object type ${type.name}`, blameNode, path),
+        coercionError(
+          `Expected type ${type.name} to be an object`,
+          blameNode,
+          path,
+        ),
       ]);
     }
     let errors;
@@ -164,12 +184,18 @@ export function coerceValue(
     for (const fieldName in value) {
       if (hasOwnProperty.call(value, fieldName)) {
         if (!fields[fieldName]) {
+          const suggestions = suggestionList(fieldName, Object.keys(fields));
+          const didYouMean =
+            suggestions.length !== 0
+              ? `did you mean ${orList(suggestions)}?`
+              : undefined;
           errors = add(
             errors,
             coercionError(
               `Field "${fieldName}" is not defined by type ${type.name}`,
               blameNode,
               path,
+              didYouMean,
             ),
           );
         }
@@ -199,15 +225,13 @@ function atPath(prev, key) {
   return { prev, key };
 }
 
-function coercionError(message, blameNode, path, originalError) {
+function coercionError(message, blameNode, path, subMessage, originalError) {
   const pathStr = printPath(path);
   // Return a GraphQLError instance
   return new GraphQLError(
     message +
       (pathStr ? ' at ' + pathStr : '') +
-      (originalError && originalError.message
-        ? '; ' + originalError.message
-        : '.'),
+      (subMessage ? '; ' + subMessage : '.'),
     blameNode,
     undefined,
     undefined,
diff --git a/src/validation/__tests__/ValuesOfCorrectType-test.js b/src/validation/__tests__/ValuesOfCorrectType-test.js
diff --git a/src/validation/rules/ValuesOfCorrectType.js b/src/validation/rules/ValuesOfCorrectType.js
