Merge pull request #282 from graphql/errormessage

[validator] Add suggested types to incorrect field message

Author: dschafer

src/validation/__tests__/FieldsOnCorrectType.js

@@ -7,6 +7,7 @@
  *  of patent rights can be found in the PATENTS file in the same directory.
  */
 
+import { expect } from 'chai';
 import { describe, it } from 'mocha';
 import { expectPassesRule, expectFailsRule } from './harness';
 import {
@@ -15,9 +16,9 @@ import {
 } from '../rules/FieldsOnCorrectType';
 
 
-function undefinedField(field, type, line, column) {
+function undefinedField(field, type, suggestions, line, column) {
   return {
-    message: undefinedFieldMessage(field, type),
+    message: undefinedFieldMessage(field, type, suggestions),
     locations: [ { line, column } ],
   };
 }
@@ -84,8 +85,8 @@ describe('Validate: Fields on correct type', () => {
           }
         }
       }`,
-      [ undefinedField('unknown_pet_field', 'Pet', 3, 9),
-        undefinedField('unknown_cat_field', 'Cat', 5, 13) ]
+      [ undefinedField('unknown_pet_field', 'Pet', [], 3, 9),
+        undefinedField('unknown_cat_field', 'Cat', [], 5, 13) ]
     );
   });
 
@@ -94,7 +95,7 @@ describe('Validate: Fields on correct type', () => {
       fragment fieldNotDefined on Dog {
         meowVolume
       }`,
-      [ undefinedField('meowVolume', 'Dog', 3, 9) ]
+      [ undefinedField('meowVolume', 'Dog', [], 3, 9) ]
     );
   });
 
@@ -105,7 +106,7 @@ describe('Validate: Fields on correct type', () => {
           deeper_unknown_field
         }
       }`,
-      [ undefinedField('unknown_field', 'Dog', 3, 9) ]
+      [ undefinedField('unknown_field', 'Dog', [], 3, 9) ]
     );
   });
 
@@ -116,7 +117,7 @@ describe('Validate: Fields on correct type', () => {
           unknown_field
         }
       }`,
-      [ undefinedField('unknown_field', 'Pet', 4, 11) ]
+      [ undefinedField('unknown_field', 'Pet', [], 4, 11) ]
     );
   });
 
@@ -127,7 +128,7 @@ describe('Validate: Fields on correct type', () => {
           meowVolume
         }
       }`,
-      [ undefinedField('meowVolume', 'Dog', 4, 11) ]
+      [ undefinedField('meowVolume', 'Dog', [], 4, 11) ]
     );
   });
 
@@ -136,7 +137,7 @@ describe('Validate: Fields on correct type', () => {
       fragment aliasedFieldTargetNotDefined on Dog {
         volume : mooVolume
       }`,
-      [ undefinedField('mooVolume', 'Dog', 3, 9) ]
+      [ undefinedField('mooVolume', 'Dog', [], 3, 9) ]
     );
   });
 
@@ -145,7 +146,7 @@ describe('Validate: Fields on correct type', () => {
       fragment aliasedLyingFieldTargetNotDefined on Dog {
         barkVolume : kawVolume
       }`,
-      [ undefinedField('kawVolume', 'Dog', 3, 9) ]
+      [ undefinedField('kawVolume', 'Dog', [], 3, 9) ]
     );
   });
 
@@ -154,16 +155,16 @@ describe('Validate: Fields on correct type', () => {
       fragment notDefinedOnInterface on Pet {
         tailLength
       }`,
-      [ undefinedField('tailLength', 'Pet', 3, 9) ]
+      [ undefinedField('tailLength', 'Pet', [], 3, 9) ]
     );
   });
 
-  it('Defined on implmentors but not on interface', () => {
+  it('Defined on implementors but not on interface', () => {
     expectFailsRule(FieldsOnCorrectType, `
       fragment definedOnImplementorsButNotInterface on Pet {
         nickname
       }`,
-      [ undefinedField('nickname', 'Pet', 3, 9) ]
+      [ undefinedField('nickname', 'Pet', [ 'Cat', 'Dog' ], 3, 9) ]
     );
   });
 
@@ -180,7 +181,7 @@ describe('Validate: Fields on correct type', () => {
       fragment directFieldSelectionOnUnion on CatOrDog {
         directField
       }`,
-      [ undefinedField('directField', 'CatOrDog', 3, 9) ]
+      [ undefinedField('directField', 'CatOrDog', [], 3, 9) ]
     );
   });
 
@@ -189,7 +190,15 @@ describe('Validate: Fields on correct type', () => {
       fragment definedOnImplementorsQueriedOnUnion on CatOrDog {
         name
       }`,
-      [ undefinedField('name', 'CatOrDog', 3, 9) ]
+      [
+        undefinedField(
+          'name',
+          'CatOrDog',
+          [ 'Being', 'Pet', 'Canine', 'Cat', 'Dog' ],
+          3,
+          9
+        )
+      ]
     );
   });
 
@@ -206,4 +215,29 @@ describe('Validate: Fields on correct type', () => {
     `);
   });
 
+  describe('Fields on correct type error message', () => {
+    it('Works with no suggestions', () => {
+      expect(
+        undefinedFieldMessage('T', 'f', [])
+      ).to.equal('Cannot query field "T" on type "f".');
+    });
+
+    it('Works with no small numbers of suggestions', () => {
+      expect(
+        undefinedFieldMessage('T', 'f', [ 'A', 'B' ])
+      ).to.equal('Cannot query field "T" on type "f". ' +
+        'However, this field exists on "A", "B". ' +
+        'Perhaps you meant to use an inline fragment?');
+    });
+
+    it('Works with lots of suggestions', () => {
+      expect(
+        undefinedFieldMessage('T', 'f', [ 'A', 'B', 'C', 'D', 'E', 'F' ])
+      ).to.equal('Cannot query field "T" on type "f". ' +
+        'However, this field exists on "A", "B", "C", "D", "E", ' +
+        'and 1 other types. ' +
+        'Perhaps you meant to use an inline fragment?');
+    });
+  });
 });
+

src/validation/__tests__/harness.js

@@ -53,6 +53,16 @@ const Pet = new GraphQLInterfaceType({
   }),
 });
 
+const Canine = new GraphQLInterfaceType({
+  name: 'Canine',
+  fields: () => ({
+    name: {
+      type: GraphQLString,
+      args: { surname: { type: GraphQLBoolean } },
+    }
+  }),
+});
+
 const DogCommand = new GraphQLEnumType({
   name: 'DogCommand',
   values: {
@@ -93,7 +103,7 @@ const Dog = new GraphQLObjectType({
       args: { x: { type: GraphQLInt }, y: { type: GraphQLInt } },
     },
   }),
-  interfaces: [ Being, Pet ],
+  interfaces: [ Being, Pet, Canine ],
 });
 
 const Cat = new GraphQLObjectType({

src/validation/__tests__/validation.js

@@ -65,9 +65,9 @@ describe('Validate: Supports full validation', () => {
     );
 
     expect(errors).to.deep.equal([
-      { message: 'Cannot query field "catOrDog" on "QueryRoot".' },
-      { message: 'Cannot query field "furColor" on "Cat".' },
-      { message: 'Cannot query field "isHousetrained" on "Dog".' }
+      { message: 'Cannot query field "catOrDog" on type "QueryRoot".' },
+      { message: 'Cannot query field "furColor" on type "Cat".' },
+      { message: 'Cannot query field "isHousetrained" on type "Dog".' }
     ]);
   });
 

src/validation/rules/FieldsOnCorrectType.js

@@ -11,14 +11,31 @@
 import type { ValidationContext } from '../index';
 import { GraphQLError } from '../../error';
 import type { Field } from '../../language/ast';
-import type { GraphQLType } from '../../type/definition';
-
+import {
+  isAbstractType,
+  GraphQLAbstractType,
+  GraphQLObjectType,
+} from '../../type/definition';
 
 export function undefinedFieldMessage(
   fieldName: string,
-  type: GraphQLType
+  type: string,
+  suggestedTypes: Array<string>
 ): string {
-  return `Cannot query field "${fieldName}" on "${type}".`;
+  let message = `Cannot query field "${fieldName}" on type "${type}".`;
+  const MAX_LENGTH = 5;
+  if (suggestedTypes.length !== 0) {
+    let suggestions = suggestedTypes
+      .slice(0, MAX_LENGTH)
+      .map(t => `"${t}"`)
+      .join(', ');
+    if (suggestedTypes.length > MAX_LENGTH) {
+      suggestions += `, and ${suggestedTypes.length - MAX_LENGTH} other types`;
+    }
+    message += ` However, this field exists on ${suggestions}.`;
+    message += ` Perhaps you meant to use an inline fragment?`;
+  }
+  return message;
 }
 
 /**
@@ -34,12 +51,64 @@ export function FieldsOnCorrectType(context: ValidationContext): any {
       if (type) {
         const fieldDef = context.getFieldDef();
         if (!fieldDef) {
+          // This isn't valid. Let's find suggestions, if any.
+          let suggestedTypes = [];
+          if (isAbstractType(type)) {
+            suggestedTypes =
+              getSiblingInterfacesIncludingField(type, node.name.value);
+            suggestedTypes = suggestedTypes.concat(
+              getImplementationsIncludingField(type, node.name.value)
+            );
+          }
           context.reportError(new GraphQLError(
-            undefinedFieldMessage(node.name.value, type.name),
+            undefinedFieldMessage(node.name.value, type.name, suggestedTypes),
             [ node ]
           ));
         }
       }
     }
   };
 }
+
+/**
+ * Return implementations of `type` that include `fieldName` as a valid field.
+ */
+function getImplementationsIncludingField(
+  type: GraphQLAbstractType,
+  fieldName: string
+): Array<string> {
+  return type.getPossibleTypes()
+    .filter(t => t.getFields()[fieldName] !== undefined)
+    .map(t => t.name)
+    .sort();
+}
+
+/**
+ * Go through all of the implementations of type, and find other interaces
+ * that they implement. If those interfaces include `field` as a valid field,
+ * return them, sorted by how often the implementations include the other
+ * interface.
+ */
+function getSiblingInterfacesIncludingField(
+  type: GraphQLAbstractType,
+  fieldName: string
+): Array<string> {
+  const implementingObjects = type.getPossibleTypes()
+    .filter(t => t instanceof GraphQLObjectType);
+
+  const suggestedInterfaces = implementingObjects.reduce((acc, t) => {
+    t.getInterfaces().forEach(i => {
+      if (i.getFields()[fieldName] === undefined) {
+        return;
+      }
+      if (acc[i.name] === undefined) {
+        acc[i.name] = 0;
+      }
+      acc[i.name] += 1;
+    });
+    return acc;
+  }, {});
+  return Object.keys(suggestedInterfaces)
+    .sort((a,b) => suggestedInterfaces[b] - suggestedInterfaces[a]);
+}
+