From fd9d34ee340e0876090bfdb48d754d6fb9db7d09 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sun, 2 Jun 2019 04:43:42 +0300 Subject: [PATCH] Extract "didYouMean" util function --- src/execution/__tests__/variables-test.js | 2 +- src/jsutils/__tests__/didYouMean-test.js | 36 +++++++++++++ src/jsutils/__tests__/quotedOrList-test.js | 36 ------------- src/jsutils/didYouMean.js | 46 ++++++++++++++++ src/jsutils/orList.js | 30 ----------- src/jsutils/quotedOrList.js | 17 ------ src/subscription/__tests__/subscribe-test.js | 2 +- src/type/__tests__/enumType-test.js | 6 +-- src/utilities/__tests__/coerceValue-test.js | 46 ++++++++-------- src/utilities/coerceValue.js | 32 +++++------ .../__tests__/ValuesOfCorrectType-test.js | 44 ++++++--------- src/validation/rules/FieldsOnCorrectType.js | 17 +++--- src/validation/rules/KnownArgumentNames.js | 22 ++++---- src/validation/rules/KnownTypeNames.js | 11 ++-- .../rules/PossibleTypeExtensions.js | 11 ++-- src/validation/rules/ScalarLeafs.js | 4 +- src/validation/rules/ValuesOfCorrectType.js | 53 +++++++++---------- 17 files changed, 198 insertions(+), 217 deletions(-) create mode 100644 src/jsutils/__tests__/didYouMean-test.js delete mode 100644 src/jsutils/__tests__/quotedOrList-test.js create mode 100644 src/jsutils/didYouMean.js delete mode 100644 src/jsutils/orList.js delete mode 100644 src/jsutils/quotedOrList.js diff --git a/src/execution/__tests__/variables-test.js b/src/execution/__tests__/variables-test.js index f833194328..e7afe731e6 100644 --- a/src/execution/__tests__/variables-test.js +++ b/src/execution/__tests__/variables-test.js @@ -687,7 +687,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$value" got invalid value [1, 2, 3]; Expected type String; String cannot represent a non string value: [1, 2, 3]', + 'Variable "$value" got invalid value [1, 2, 3]; Expected type String. String cannot represent a non string value: [1, 2, 3]', locations: [{ line: 2, column: 16 }], }, ], diff --git a/src/jsutils/__tests__/didYouMean-test.js b/src/jsutils/__tests__/didYouMean-test.js new file mode 100644 index 0000000000..37dc0ce41f --- /dev/null +++ b/src/jsutils/__tests__/didYouMean-test.js @@ -0,0 +1,36 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict + */ + +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import didYouMean from '../didYouMean'; + +describe('didYouMean', () => { + it('Does accept an empty list', () => { + expect(didYouMean([])).to.equal(''); + }); + + it('Handle single suggestion', () => { + expect(didYouMean(['A'])).to.equal(' Did you mean A?'); + }); + + it('Handle two suggestions', () => { + expect(didYouMean(['A', 'B'])).to.equal(' Did you mean A or B?'); + }); + + it('Handle multiple suggestions', () => { + expect(didYouMean(['A', 'B', 'C'])).to.equal(' Did you mean A, B, or C?'); + }); + + it('Limits to five suggestions', () => { + expect(didYouMean(['A', 'B', 'C', 'D', 'E', 'F'])).to.equal( + ' Did you mean A, B, C, D, or E?', + ); + }); +}); diff --git a/src/jsutils/__tests__/quotedOrList-test.js b/src/jsutils/__tests__/quotedOrList-test.js deleted file mode 100644 index 4490ef5893..0000000000 --- a/src/jsutils/__tests__/quotedOrList-test.js +++ /dev/null @@ -1,36 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow strict - */ - -import { expect } from 'chai'; -import { describe, it } from 'mocha'; -import quotedOrList from '../quotedOrList'; - -describe('quotedOrList', () => { - it('Does not accept an empty list', () => { - expect(() => quotedOrList([])).to.throw(Error); - }); - - it('Returns single quoted item', () => { - expect(quotedOrList(['A'])).to.equal('"A"'); - }); - - it('Returns two item list', () => { - expect(quotedOrList(['A', 'B'])).to.equal('"A" or "B"'); - }); - - it('Returns comma separated many item list', () => { - expect(quotedOrList(['A', 'B', 'C'])).to.equal('"A", "B", or "C"'); - }); - - it('Limits to five items', () => { - expect(quotedOrList(['A', 'B', 'C', 'D', 'E', 'F'])).to.equal( - '"A", "B", "C", "D", or "E"', - ); - }); -}); diff --git a/src/jsutils/didYouMean.js b/src/jsutils/didYouMean.js new file mode 100644 index 0000000000..5a9da1b84a --- /dev/null +++ b/src/jsutils/didYouMean.js @@ -0,0 +1,46 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict + */ + +const MAX_SUGGESTIONS = 5; + +/** + * Given [ A, B, C ] return ' Did you mean A, B, or C?'. + */ +declare function didYouMean(suggestions: $ReadOnlyArray): string; +// eslint-disable-next-line no-redeclare +declare function didYouMean( + subMessage: string, + suggestions: $ReadOnlyArray, +): string; + +// eslint-disable-next-line no-redeclare +export default function didYouMean(firstArg, secondArg) { + const [subMessage, suggestions] = + typeof firstArg === 'string' + ? [firstArg, secondArg] + : [undefined, firstArg]; + + let message = ' Did you mean '; + if (subMessage) { + message += subMessage + ' '; + } + + switch (suggestions.length) { + case 0: + return ''; + case 1: + return message + suggestions[0] + '?'; + case 2: + return message + suggestions[0] + ' or ' + suggestions[1] + '?'; + } + + const selected = suggestions.slice(0, MAX_SUGGESTIONS); + const lastItem = selected.pop(); + return message + selected.join(', ') + ', or ' + lastItem + '?'; +} diff --git a/src/jsutils/orList.js b/src/jsutils/orList.js deleted file mode 100644 index 892306a2a3..0000000000 --- a/src/jsutils/orList.js +++ /dev/null @@ -1,30 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow strict - */ - -import invariant from './invariant'; - -const MAX_LENGTH = 5; - -/** - * Given [ A, B, C ] return 'A, B, or C'. - */ -export default function orList(items: $ReadOnlyArray): string { - invariant(items.length !== 0); - - if (items.length === 1) { - return items[0]; - } - if (items.length === 2) { - return items[0] + ' or ' + items[1]; - } - - const selected = items.slice(0, MAX_LENGTH); - const lastItem = selected.pop(); - return selected.join(', ') + ', or ' + lastItem; -} diff --git a/src/jsutils/quotedOrList.js b/src/jsutils/quotedOrList.js deleted file mode 100644 index 0779c071a6..0000000000 --- a/src/jsutils/quotedOrList.js +++ /dev/null @@ -1,17 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow strict - */ - -import orList from './orList'; - -/** - * Given [ A, B, C ] return '"A", "B", or "C"'. - */ -export default function quotedOrList(items: $ReadOnlyArray): string { - return orList(items.map(item => `"${item}"`)); -} diff --git a/src/subscription/__tests__/subscribe-test.js b/src/subscription/__tests__/subscribe-test.js index 1dc04586ed..acf4ecec7e 100644 --- a/src/subscription/__tests__/subscribe-test.js +++ b/src/subscription/__tests__/subscribe-test.js @@ -483,7 +483,7 @@ describe('Subscription Initialization Phase', () => { errors: [ { message: - 'Variable "$priority" got invalid value "meow"; Expected type Int; Int cannot represent non-integer value: "meow"', + 'Variable "$priority" got invalid value "meow"; Expected type Int. Int cannot represent non-integer value: "meow"', locations: [{ line: 2, column: 21 }], }, ], diff --git a/src/type/__tests__/enumType-test.js b/src/type/__tests__/enumType-test.js index 9114fb253e..4bcb6f013e 100644 --- a/src/type/__tests__/enumType-test.js +++ b/src/type/__tests__/enumType-test.js @@ -165,7 +165,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 }], }, ], @@ -179,7 +179,7 @@ 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 }], }, ], @@ -193,7 +193,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 }], }, ], diff --git a/src/utilities/__tests__/coerceValue-test.js b/src/utilities/__tests__/coerceValue-test.js index 3b4ca712aa..2f85aa5590 100644 --- a/src/utilities/__tests__/coerceValue-test.js +++ b/src/utilities/__tests__/coerceValue-test.js @@ -37,7 +37,7 @@ describe('coerceValue', () => { it('returns error for array input as string', () => { const result = coerceValue([1, 2, 3], GraphQLString); expectErrors(result).to.deep.equal([ - 'Expected type String; String cannot represent a non string value: [1, 2, 3]', + 'Expected type String. String cannot represent a non string value: [1, 2, 3]', ]); }); }); @@ -46,7 +46,7 @@ describe('coerceValue', () => { it('returns error for array input as ID', () => { const result = coerceValue([1, 2, 3], GraphQLID); expectErrors(result).to.deep.equal([ - 'Expected type ID; ID cannot represent value: [1, 2, 3]', + 'Expected type ID. ID cannot represent value: [1, 2, 3]', ]); }); }); @@ -60,7 +60,7 @@ describe('coerceValue', () => { it('returns error for numeric looking string', () => { const result = coerceValue('1', GraphQLInt); expectErrors(result).to.deep.equal([ - 'Expected type Int; Int cannot represent non-integer value: "1"', + 'Expected type Int. Int cannot represent non-integer value: "1"', ]); }); @@ -82,49 +82,49 @@ describe('coerceValue', () => { it('returns a single error for empty string as value', () => { const result = coerceValue('', GraphQLInt); expectErrors(result).to.deep.equal([ - 'Expected type Int; Int cannot represent non-integer value: ""', + 'Expected type Int. Int cannot represent non-integer value: ""', ]); }); it('returns a single error for 2^32 input as int', () => { const result = coerceValue(Math.pow(2, 32), GraphQLInt); expectErrors(result).to.deep.equal([ - 'Expected type Int; Int cannot represent non 32-bit signed integer value: 4294967296', + 'Expected type Int. Int cannot represent non 32-bit signed integer value: 4294967296', ]); }); it('returns a single error for float input as int', () => { const result = coerceValue(1.5, GraphQLInt); expectErrors(result).to.deep.equal([ - 'Expected type Int; Int cannot represent non-integer value: 1.5', + 'Expected type Int. Int cannot represent non-integer value: 1.5', ]); }); it('returns a single error for NaN input as int', () => { const result = coerceValue(NaN, GraphQLInt); expectErrors(result).to.deep.equal([ - 'Expected type Int; Int cannot represent non-integer value: NaN', + 'Expected type Int. Int cannot represent non-integer value: NaN', ]); }); it('returns a single error for Infinity input as int', () => { const result = coerceValue(Infinity, GraphQLInt); expectErrors(result).to.deep.equal([ - 'Expected type Int; Int cannot represent non-integer value: Infinity', + 'Expected type Int. Int cannot represent non-integer value: Infinity', ]); }); it('returns a single error for char input', () => { const result = coerceValue('a', GraphQLInt); expectErrors(result).to.deep.equal([ - 'Expected type Int; Int cannot represent non-integer value: "a"', + 'Expected type Int. Int cannot represent non-integer value: "a"', ]); }); it('returns a single error for string input', () => { const result = coerceValue('meow', GraphQLInt); expectErrors(result).to.deep.equal([ - 'Expected type Int; Int cannot represent non-integer value: "meow"', + 'Expected type Int. Int cannot represent non-integer value: "meow"', ]); }); }); @@ -148,7 +148,7 @@ describe('coerceValue', () => { it('returns error for numeric looking string', () => { const result = coerceValue('1', GraphQLFloat); expectErrors(result).to.deep.equal([ - 'Expected type Float; Float cannot represent non numeric value: "1"', + 'Expected type Float. Float cannot represent non numeric value: "1"', ]); }); @@ -160,35 +160,35 @@ describe('coerceValue', () => { it('returns a single error for empty string input', () => { const result = coerceValue('', GraphQLFloat); expectErrors(result).to.deep.equal([ - 'Expected type Float; Float cannot represent non numeric value: ""', + 'Expected type Float. Float cannot represent non numeric value: ""', ]); }); it('returns a single error for NaN input', () => { const result = coerceValue(NaN, GraphQLFloat); expectErrors(result).to.deep.equal([ - 'Expected type Float; Float cannot represent non numeric value: NaN', + 'Expected type Float. Float cannot represent non numeric value: NaN', ]); }); it('returns a single error for Infinity input', () => { const result = coerceValue(Infinity, GraphQLFloat); expectErrors(result).to.deep.equal([ - 'Expected type Float; Float cannot represent non numeric value: Infinity', + 'Expected type Float. Float cannot represent non numeric value: Infinity', ]); }); it('returns a single error for char input', () => { const result = coerceValue('a', GraphQLFloat); expectErrors(result).to.deep.equal([ - 'Expected type Float; Float cannot represent non numeric value: "a"', + 'Expected type Float. Float cannot represent non numeric value: "a"', ]); }); it('returns a single error for char input', () => { const result = coerceValue('meow', GraphQLFloat); expectErrors(result).to.deep.equal([ - 'Expected type Float; Float cannot represent non numeric value: "meow"', + 'Expected type Float. Float cannot represent non numeric value: "meow"', ]); }); }); @@ -213,7 +213,7 @@ describe('coerceValue', () => { it('results error for misspelled enum value', () => { const result = coerceValue('foo', TestEnum); expectErrors(result).to.deep.equal([ - 'Expected type TestEnum; did you mean FOO?', + 'Expected type TestEnum. Did you mean FOO?', ]); }); @@ -250,15 +250,15 @@ describe('coerceValue', () => { it('returns an error for an invalid field', () => { const result = coerceValue({ foo: 'abc' }, TestInputObject); expectErrors(result).to.deep.equal([ - 'Expected type Int at value.foo; Int cannot represent non-integer value: "abc"', + 'Expected type Int at value.foo. Int cannot represent non-integer value: "abc"', ]); }); it('returns multiple errors for multiple invalid fields', () => { const result = coerceValue({ foo: 'abc', bar: 'def' }, TestInputObject); expectErrors(result).to.deep.equal([ - 'Expected type Int at value.foo; Int cannot represent non-integer value: "abc"', - 'Expected type Int at value.bar; Int cannot represent non-integer value: "def"', + 'Expected type Int at value.foo. Int cannot represent non-integer value: "abc"', + 'Expected type Int at value.bar. Int cannot represent non-integer value: "def"', ]); }); @@ -282,7 +282,7 @@ describe('coerceValue', () => { it('returns error for a misspelled field', () => { const result = coerceValue({ foo: 123, bart: 123 }, TestInputObject); expectErrors(result).to.deep.equal([ - 'Field "bart" is not defined by type TestInputObject; did you mean bar?', + 'Field "bart" is not defined by type TestInputObject. Did you mean bar?', ]); }); }); @@ -298,8 +298,8 @@ describe('coerceValue', () => { it('returns an error for an invalid input', () => { const result = coerceValue([1, 'b', true], TestList); expectErrors(result).to.deep.equal([ - 'Expected type Int at value[1]; Int cannot represent non-integer value: "b"', - 'Expected type Int at value[2]; Int cannot represent non-integer value: true', + 'Expected type Int at value[1]. Int cannot represent non-integer value: "b"', + 'Expected type Int at value[2]. Int cannot represent non-integer value: true', ]); }); diff --git a/src/utilities/coerceValue.js b/src/utilities/coerceValue.js index a0ccd1ab4b..101e95c0f1 100644 --- a/src/utilities/coerceValue.js +++ b/src/utilities/coerceValue.js @@ -11,7 +11,7 @@ import { forEach, isCollection } from 'iterall'; import objectValues from '../polyfills/objectValues'; import inspect from '../jsutils/inspect'; import isInvalid from '../jsutils/isInvalid'; -import orList from '../jsutils/orList'; +import didYouMean from '../jsutils/didYouMean'; import suggestionList from '../jsutils/suggestionList'; import { GraphQLError } from '../error/GraphQLError'; import { type ASTNode } from '../language/ast'; @@ -81,7 +81,7 @@ export function coerceValue( `Expected type ${type.name}`, blameNode, path, - error.message, + ' ' + error.message, error, ), ]); @@ -99,12 +99,13 @@ export function coerceValue( 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, didYouMean), + coercionError( + `Expected type ${type.name}`, + blameNode, + path, + didYouMean(suggestions), + ), ]); } @@ -182,17 +183,13 @@ export function coerceValue( for (const fieldName of Object.keys(value)) { 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, + didYouMean(suggestions), ), ); } @@ -224,11 +221,16 @@ function atPath(prev, key) { function coercionError(message, blameNode, path, subMessage, originalError) { const pathStr = printPath(path); + let fullMessage = message; + + if (pathStr) { + fullMessage += ' at ' + pathStr; + } + fullMessage += subMessage ? '.' + subMessage : '.'; + // Return a GraphQLError instance return new GraphQLError( - message + - (pathStr ? ' at ' + pathStr : '') + - (subMessage ? '; ' + subMessage : '.'), + fullMessage, blameNode, undefined, undefined, diff --git a/src/validation/__tests__/ValuesOfCorrectType-test.js b/src/validation/__tests__/ValuesOfCorrectType-test.js index ebfc042895..13a4726fda 100644 --- a/src/validation/__tests__/ValuesOfCorrectType-test.js +++ b/src/validation/__tests__/ValuesOfCorrectType-test.js @@ -12,6 +12,7 @@ import { expectValidationErrors } from './harness'; import { ValuesOfCorrectType, badValueMessage, + badEnumValueMessage, requiredFieldMessage, unknownFieldMessage, } from '../rules/ValuesOfCorrectType'; @@ -31,6 +32,13 @@ function badValue(typeName, value, line, column, message) { }; } +function badEnumValue(typeName, value, line, column, message) { + return { + message: badEnumValueMessage(typeName, value, message), + locations: [{ line, column }], + }; +} + function requiredField(typeName, fieldName, fieldTypeName, line, column) { return { message: requiredFieldMessage(typeName, fieldName, fieldTypeName), @@ -38,9 +46,9 @@ function requiredField(typeName, fieldName, fieldTypeName, line, column) { }; } -function unknownField(typeName, fieldName, line, column, message) { +function unknownField(typeName, fieldName, line, column, suggestedFields) { return { - message: unknownFieldMessage(typeName, fieldName, message), + message: unknownFieldMessage(typeName, fieldName, suggestedFields), locations: [{ line, column }], }; } @@ -414,15 +422,7 @@ describe('Validate: Values of correct type', () => { doesKnowCommand(dogCommand: "SIT") } } - `).to.deep.equal([ - badValue( - 'DogCommand', - '"SIT"', - 4, - 41, - 'Did you mean the enum value SIT?', - ), - ]); + `).to.deep.equal([badEnumValue('DogCommand', '"SIT"', 4, 41, ['SIT'])]); }); it('Boolean into Enum', () => { @@ -452,15 +452,7 @@ describe('Validate: Values of correct type', () => { doesKnowCommand(dogCommand: sit) } } - `).to.deep.equal([ - badValue( - 'DogCommand', - 'sit', - 4, - 41, - 'Did you mean the enum value SIT?', - ), - ]); + `).to.deep.equal([badEnumValue('DogCommand', 'sit', 4, 41, ['SIT'])]); }); }); @@ -789,13 +781,11 @@ describe('Validate: Values of correct type', () => { } } `).to.deep.equal([ - unknownField( - 'ComplexInput', - 'unknownField', - 6, - 15, - 'Did you mean nonNullField, intField, or booleanField?', - ), + unknownField('ComplexInput', 'unknownField', 6, 15, [ + 'nonNullField', + 'intField', + 'booleanField', + ]), ]); }); diff --git a/src/validation/rules/FieldsOnCorrectType.js b/src/validation/rules/FieldsOnCorrectType.js index abcb020aa6..9b35378f77 100644 --- a/src/validation/rules/FieldsOnCorrectType.js +++ b/src/validation/rules/FieldsOnCorrectType.js @@ -10,7 +10,7 @@ import { type ValidationContext } from '../ValidationContext'; import { GraphQLError } from '../../error/GraphQLError'; import suggestionList from '../../jsutils/suggestionList'; -import quotedOrList from '../../jsutils/quotedOrList'; +import didYouMean from '../../jsutils/didYouMean'; import { type FieldNode } from '../../language/ast'; import { type ASTVisitor } from '../../language/visitor'; import { type GraphQLSchema } from '../../type/schema'; @@ -27,14 +27,13 @@ export function undefinedFieldMessage( suggestedTypeNames: Array, suggestedFieldNames: Array, ): string { - let message = `Cannot query field "${fieldName}" on type "${type}".`; - if (suggestedTypeNames.length !== 0) { - const suggestions = quotedOrList(suggestedTypeNames); - message += ` Did you mean to use an inline fragment on ${suggestions}?`; - } else if (suggestedFieldNames.length !== 0) { - message += ` Did you mean ${quotedOrList(suggestedFieldNames)}?`; - } - return message; + const quotedTypeNames = suggestedTypeNames.map(x => `"${x}"`); + const quotedFieldNames = suggestedFieldNames.map(x => `"${x}"`); + return ( + `Cannot query field "${fieldName}" on type "${type}".` + + (didYouMean('to use an inline fragment on', quotedTypeNames) || + didYouMean(quotedFieldNames)) + ); } /** diff --git a/src/validation/rules/KnownArgumentNames.js b/src/validation/rules/KnownArgumentNames.js index 1f97f2a6d7..f474f59233 100644 --- a/src/validation/rules/KnownArgumentNames.js +++ b/src/validation/rules/KnownArgumentNames.js @@ -14,7 +14,7 @@ import { import { GraphQLError } from '../../error/GraphQLError'; import { type ASTVisitor } from '../../language/visitor'; import suggestionList from '../../jsutils/suggestionList'; -import quotedOrList from '../../jsutils/quotedOrList'; +import didYouMean from '../../jsutils/didYouMean'; import { Kind } from '../../language/kinds'; import { specifiedDirectives } from '../../type/directives'; @@ -24,13 +24,10 @@ export function unknownArgMessage( typeName: string, suggestedArgs: Array, ): string { - let message = - `Unknown argument "${argName}" on field "${fieldName}" of ` + - `type "${typeName}".`; - if (suggestedArgs.length) { - message += ` Did you mean ${quotedOrList(suggestedArgs)}?`; - } - return message; + return ( + `Unknown argument "${argName}" on field "${fieldName}" of type "${typeName}".` + + didYouMean(suggestedArgs.map(x => `"${x}"`)) + ); } export function unknownDirectiveArgMessage( @@ -38,11 +35,10 @@ export function unknownDirectiveArgMessage( directiveName: string, suggestedArgs: Array, ): string { - let message = `Unknown argument "${argName}" on directive "@${directiveName}".`; - if (suggestedArgs.length) { - message += ` Did you mean ${quotedOrList(suggestedArgs)}?`; - } - return message; + return ( + `Unknown argument "${argName}" on directive "@${directiveName}".` + + didYouMean(suggestedArgs.map(x => `"${x}"`)) + ); } /** diff --git a/src/validation/rules/KnownTypeNames.js b/src/validation/rules/KnownTypeNames.js index 2b378573ca..e6203180d8 100644 --- a/src/validation/rules/KnownTypeNames.js +++ b/src/validation/rules/KnownTypeNames.js @@ -13,7 +13,7 @@ import { } from '../ValidationContext'; import { GraphQLError } from '../../error/GraphQLError'; import suggestionList from '../../jsutils/suggestionList'; -import quotedOrList from '../../jsutils/quotedOrList'; +import didYouMean from '../../jsutils/didYouMean'; import { type ASTNode } from '../../language/ast'; import { type ASTVisitor } from '../../language/visitor'; import { @@ -27,11 +27,10 @@ export function unknownTypeMessage( typeName: string, suggestedTypes: Array, ): string { - let message = `Unknown type "${typeName}".`; - if (suggestedTypes.length) { - message += ` Did you mean ${quotedOrList(suggestedTypes)}?`; - } - return message; + return ( + `Unknown type "${typeName}".` + + didYouMean(suggestedTypes.map(x => `"${x}"`)) + ); } /** diff --git a/src/validation/rules/PossibleTypeExtensions.js b/src/validation/rules/PossibleTypeExtensions.js index 988169599d..4d70b20c07 100644 --- a/src/validation/rules/PossibleTypeExtensions.js +++ b/src/validation/rules/PossibleTypeExtensions.js @@ -7,7 +7,7 @@ * @flow strict */ -import quotedOrList from '../../jsutils/quotedOrList'; +import didYouMean from '../../jsutils/didYouMean'; import suggestionList from '../../jsutils/suggestionList'; import { type SDLValidationContext } from '../ValidationContext'; import { GraphQLError } from '../../error/GraphQLError'; @@ -27,11 +27,10 @@ export function extendingUnknownTypeMessage( typeName: string, suggestedTypes: Array, ): string { - let message = `Cannot extend type "${typeName}" because it is not defined.`; - if (suggestedTypes.length) { - message += ` Did you mean ${quotedOrList(suggestedTypes)}?`; - } - return message; + return ( + `Cannot extend type "${typeName}" because it is not defined.` + + didYouMean(suggestedTypes.map(x => `"${x}"`)) + ); } export function extendingDifferentTypeKindMessage( diff --git a/src/validation/rules/ScalarLeafs.js b/src/validation/rules/ScalarLeafs.js index b729e1a746..c3f376c9c8 100644 --- a/src/validation/rules/ScalarLeafs.js +++ b/src/validation/rules/ScalarLeafs.js @@ -29,8 +29,8 @@ export function requiredSubselectionMessage( type: string, ): string { return ( - `Field "${fieldName}" of type "${type}" must have a ` + - `selection of subfields. Did you mean "${fieldName} { ... }"?` + `Field "${fieldName}" of type "${type}" must have a selection of subfields.` + + ` Did you mean "${fieldName} { ... }"?` ); } diff --git a/src/validation/rules/ValuesOfCorrectType.js b/src/validation/rules/ValuesOfCorrectType.js index 2042a44f60..95484ab39c 100644 --- a/src/validation/rules/ValuesOfCorrectType.js +++ b/src/validation/rules/ValuesOfCorrectType.js @@ -14,7 +14,6 @@ import { type ValueNode } from '../../language/ast'; import { print } from '../../language/printer'; import { type ASTVisitor } from '../../language/visitor'; import { - type GraphQLType, isScalarType, isEnumType, isInputObjectType, @@ -27,7 +26,7 @@ import { import inspect from '../../jsutils/inspect'; import isInvalid from '../../jsutils/isInvalid'; import keyMap from '../../jsutils/keyMap'; -import orList from '../../jsutils/orList'; +import didYouMean from '../../jsutils/didYouMean'; import suggestionList from '../../jsutils/suggestionList'; export function badValueMessage( @@ -41,6 +40,17 @@ export function badValueMessage( ); } +export function badEnumValueMessage( + typeName: string, + valueName: string, + suggestedValues: $ReadOnlyArray, +) { + return ( + `Expected type ${typeName}, found ${valueName}.` + + didYouMean('the enum value', suggestedValues) + ); +} + export function requiredFieldMessage( typeName: string, fieldName: string, @@ -55,11 +65,11 @@ export function requiredFieldMessage( export function unknownFieldMessage( typeName: string, fieldName: string, - message?: string, + suggestedFields: $ReadOnlyArray, ): string { return ( - `Field "${fieldName}" is not defined by type ${typeName}` + - (message ? `; ${message}` : '.') + `Field "${fieldName}" is not defined by type ${typeName}.` + + didYouMean(suggestedFields) ); } @@ -117,13 +127,9 @@ export function ValuesOfCorrectType(context: ValidationContext): ASTVisitor { node.name.value, Object.keys(parentType.getFields()), ); - const didYouMean = - suggestions.length !== 0 - ? `Did you mean ${orList(suggestions)}?` - : undefined; context.reportError( new GraphQLError( - unknownFieldMessage(parentType.name, node.name.value, didYouMean), + unknownFieldMessage(parentType.name, node.name.value, suggestions), node, ), ); @@ -136,7 +142,7 @@ export function ValuesOfCorrectType(context: ValidationContext): ASTVisitor { } else if (!type.getValue(node.value)) { context.reportError( new GraphQLError( - badValueMessage( + badEnumValueMessage( type.name, print(node), enumTypeSuggestion(type, node), @@ -167,16 +173,14 @@ function isValidScalar(context: ValidationContext, node: ValueNode): void { const type = getNamedType(locationType); if (!isScalarType(type)) { - context.reportError( - new GraphQLError( - badValueMessage( + const message = isEnumType(type) + ? badEnumValueMessage( inspect(locationType), print(node), enumTypeSuggestion(type, node), - ), - node, - ), - ); + ) + : badValueMessage(inspect(locationType), print(node)); + context.reportError(new GraphQLError(message, node)); return; } @@ -207,14 +211,7 @@ function isValidScalar(context: ValidationContext, node: ValueNode): void { } } -function enumTypeSuggestion(type: GraphQLType, node: ValueNode): string | void { - if (isEnumType(type)) { - const suggestions = suggestionList( - print(node), - type.getValues().map(value => value.name), - ); - if (suggestions.length !== 0) { - return `Did you mean the enum value ${orList(suggestions)}?`; - } - } +function enumTypeSuggestion(type, node) { + const allNames = type.getValues().map(value => value.name); + return suggestionList(print(node), allNames); }