From 4e4d16e407990ab968fa157660195947318358ad Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Fri, 28 May 2021 16:35:44 -0700 Subject: [PATCH] Add coordinate field to schema element definitions * Defines a `GraphQLSchemaElement` base class which defines a `.coordinate` property and `toString`/`toJSON` methods. * Adds base class to types, fields, arguments, input fields, enum values, and directives. * Uses this in validation error printing string templates. --- src/execution/__tests__/nonnull-test.ts | 8 +- src/execution/__tests__/variables-test.ts | 21 +- src/execution/values.ts | 20 +- src/index.ts | 11 +- src/type/__tests__/definition-test.ts | 16 +- src/type/__tests__/directive-test.ts | 2 + src/type/__tests__/enumType-test.ts | 2 + src/type/__tests__/introspection-test.ts | 2 +- src/type/__tests__/predicate-test.ts | 28 +- src/type/__tests__/validation-test.ts | 8 +- src/type/definition.ts | 415 +++++++++--------- src/type/directives.ts | 44 +- src/type/index.ts | 9 +- src/type/introspection.ts | 45 +- src/type/validate.ts | 91 ++-- .../__tests__/buildClientSchema-test.ts | 3 + .../__tests__/findBreakingChanges-test.ts | 120 ++--- src/utilities/findBreakingChanges.ts | 65 ++- .../__tests__/NoDeprecatedCustomRule-test.ts | 6 +- .../ProvidedRequiredArgumentsRule-test.ts | 22 +- .../rules/FieldsOnCorrectTypeRule.ts | 2 +- .../rules/KnownArgumentNamesRule.ts | 5 +- .../rules/ProvidedRequiredArgumentsRule.ts | 5 +- .../rules/ValuesOfCorrectTypeRule.ts | 17 +- .../rules/VariablesInAllowedPositionRule.ts | 5 +- .../rules/custom/NoDeprecatedCustomRule.ts | 37 +- 26 files changed, 473 insertions(+), 536 deletions(-) diff --git a/src/execution/__tests__/nonnull-test.ts b/src/execution/__tests__/nonnull-test.ts index 223c552c33..dff67abd63 100644 --- a/src/execution/__tests__/nonnull-test.ts +++ b/src/execution/__tests__/nonnull-test.ts @@ -616,7 +616,7 @@ describe('Execute: handles non-nullable types', () => { errors: [ { message: - 'Argument "cannotBeNull" of required type "String!" was not provided.', + 'Argument Query.withNonNullArg(cannotBeNull:) of required type String! was not provided.', locations: [{ line: 3, column: 13 }], path: ['withNonNullArg'], }, @@ -643,7 +643,7 @@ describe('Execute: handles non-nullable types', () => { errors: [ { message: - 'Argument "cannotBeNull" of non-null type "String!" must not be null.', + 'Argument Query.withNonNullArg(cannotBeNull:) of non-null type String! must not be null.', locations: [{ line: 3, column: 42 }], path: ['withNonNullArg'], }, @@ -673,7 +673,7 @@ describe('Execute: handles non-nullable types', () => { errors: [ { message: - 'Argument "cannotBeNull" of required type "String!" was provided the variable "$testVar" which was not provided a runtime value.', + 'Argument Query.withNonNullArg(cannotBeNull:) of required type String! was provided the variable "$testVar" which was not provided a runtime value.', locations: [{ line: 3, column: 42 }], path: ['withNonNullArg'], }, @@ -701,7 +701,7 @@ describe('Execute: handles non-nullable types', () => { errors: [ { message: - 'Argument "cannotBeNull" of non-null type "String!" must not be null.', + 'Argument Query.withNonNullArg(cannotBeNull:) of non-null type String! must not be null.', locations: [{ line: 3, column: 43 }], path: ['withNonNullArg'], }, diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index ebf7d0b169..27404ea5a2 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -203,7 +203,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Argument "input" has invalid value ["foo", "bar", "baz"].', + 'Argument TestType.fieldWithObjectInput(input:) of type TestInputObject has invalid value ["foo", "bar", "baz"].', path: ['fieldWithObjectInput'], locations: [{ line: 3, column: 41 }], }, @@ -617,7 +617,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$value" of required type "String!" was not provided.', + 'Variable "$value" of required type String! was not provided.', locations: [{ line: 2, column: 16 }], }, ], @@ -636,7 +636,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$value" of non-null type "String!" must not be null.', + 'Variable "$value" of non-null type String! must not be null.', locations: [{ line: 2, column: 16 }], }, ], @@ -682,7 +682,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Argument "input" of required type "String!" was not provided.', + 'Argument TestType.fieldWithNonNullableStringInput(input:) of required type String! was not provided.', locations: [{ line: 1, column: 3 }], path: ['fieldWithNonNullableStringInput'], }, @@ -730,7 +730,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Argument "input" of required type "String!" was provided the variable "$foo" which was not provided a runtime value.', + 'Argument TestType.fieldWithNonNullableStringInput(input:) of required type String! was provided the variable "$foo" which was not provided a runtime value.', locations: [{ line: 3, column: 50 }], path: ['fieldWithNonNullableStringInput'], }, @@ -785,7 +785,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" of non-null type "[String]!" must not be null.', + 'Variable "$input" of non-null type [String]! must not be null.', locations: [{ line: 2, column: 16 }], }, ], @@ -867,7 +867,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" of non-null type "[String!]!" must not be null.', + 'Variable "$input" of non-null type [String!]! must not be null.', locations: [{ line: 2, column: 16 }], }, ], @@ -916,7 +916,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" expected value of type "TestType!" which cannot be used as an input type.', + 'Variable "$input" expected value of type TestType! which cannot be used as an input type.', locations: [{ line: 2, column: 24 }], }, ], @@ -935,7 +935,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" expected value of type "UnknownType!" which cannot be used as an input type.', + 'Variable "$input" expected value of type UnknownType! which cannot be used as an input type.', locations: [{ line: 2, column: 24 }], }, ], @@ -981,7 +981,8 @@ describe('Execute: Handles inputs', () => { }, errors: [ { - message: 'Argument "input" has invalid value WRONG_TYPE.', + message: + 'Argument TestType.fieldWithDefaultArgumentValue(input:) of type String has invalid value WRONG_TYPE.', locations: [{ line: 3, column: 48 }], path: ['fieldWithDefaultArgumentValue'], }, diff --git a/src/execution/values.ts b/src/execution/values.ts index 6d4c95b85a..02c2683094 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -87,7 +87,7 @@ function coerceVariableValues( const varTypeStr = print(varDefNode.type); onError( new GraphQLError( - `Variable "$${varName}" expected value of type "${varTypeStr}" which cannot be used as an input type.`, + `Variable "$${varName}" expected value of type ${varTypeStr} which cannot be used as an input type.`, varDefNode.type, ), ); @@ -98,10 +98,9 @@ function coerceVariableValues( if (varDefNode.defaultValue) { coercedValues[varName] = valueFromAST(varDefNode.defaultValue, varType); } else if (isNonNullType(varType)) { - const varTypeStr = inspect(varType); onError( new GraphQLError( - `Variable "$${varName}" of required type "${varTypeStr}" was not provided.`, + `Variable "$${varName}" of required type ${varType} was not provided.`, varDefNode, ), ); @@ -111,10 +110,9 @@ function coerceVariableValues( const value = inputs[varName]; if (value === null && isNonNullType(varType)) { - const varTypeStr = inspect(varType); onError( new GraphQLError( - `Variable "$${varName}" of non-null type "${varTypeStr}" must not be null.`, + `Variable "$${varName}" of non-null type ${varType} must not be null.`, varDefNode, ), ); @@ -178,8 +176,7 @@ export function getArgumentValues( coercedValues[name] = argDef.defaultValue; } else if (isNonNullType(argType)) { throw new GraphQLError( - `Argument "${name}" of required type "${inspect(argType)}" ` + - 'was not provided.', + `Argument ${argDef} of required type ${argType} was not provided.`, node, ); } @@ -199,7 +196,7 @@ export function getArgumentValues( coercedValues[name] = argDef.defaultValue; } else if (isNonNullType(argType)) { throw new GraphQLError( - `Argument "${name}" of required type "${inspect(argType)}" ` + + `Argument ${argDef} of required type ${argType} ` + `was provided the variable "$${variableName}" which was not provided a runtime value.`, valueNode, ); @@ -211,8 +208,7 @@ export function getArgumentValues( if (isNull && isNonNullType(argType)) { throw new GraphQLError( - `Argument "${name}" of non-null type "${inspect(argType)}" ` + - 'must not be null.', + `Argument ${argDef} of non-null type ${argType} must not be null.`, valueNode, ); } @@ -223,7 +219,9 @@ export function getArgumentValues( // execution. This is a runtime check to ensure execution does not // continue with an invalid argument value. throw new GraphQLError( - `Argument "${name}" has invalid value ${print(valueNode)}.`, + `Argument ${argDef} of type ${argType} has invalid value ${print( + valueNode, + )}.`, valueNode, ); } diff --git a/src/index.ts b/src/index.ts index 859bfe5744..5ed1c34dc8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -32,14 +32,17 @@ export { graphql, graphqlSync } from './graphql'; /** Create and operate on GraphQL type definitions and schema. */ export { /** Definitions */ - GraphQLSchema, - GraphQLDirective, + GraphQLSchemaElement, GraphQLScalarType, GraphQLObjectType, GraphQLInterfaceType, GraphQLUnionType, GraphQLEnumType, GraphQLInputObjectType, + GraphQLField, + GraphQLArgument, + GraphQLEnumValue, + GraphQLInputField, GraphQLList, GraphQLNonNull, /** Standard GraphQL Scalars */ @@ -144,23 +147,19 @@ export type { GraphQLSchemaExtensions, GraphQLDirectiveConfig, GraphQLDirectiveExtensions, - GraphQLArgument, GraphQLArgumentConfig, GraphQLArgumentExtensions, GraphQLEnumTypeConfig, GraphQLEnumTypeExtensions, - GraphQLEnumValue, GraphQLEnumValueConfig, GraphQLEnumValueConfigMap, GraphQLEnumValueExtensions, - GraphQLField, GraphQLFieldConfig, GraphQLFieldConfigArgumentMap, GraphQLFieldConfigMap, GraphQLFieldExtensions, GraphQLFieldMap, GraphQLFieldResolver, - GraphQLInputField, GraphQLInputFieldConfig, GraphQLInputFieldConfigMap, GraphQLInputFieldExtensions, diff --git a/src/type/__tests__/definition-test.ts b/src/type/__tests__/definition-test.ts index 1cf4e4f397..80c17c7f40 100644 --- a/src/type/__tests__/definition-test.ts +++ b/src/type/__tests__/definition-test.ts @@ -165,11 +165,11 @@ describe('Type System: Objects', () => { }, }; const testObject1 = new GraphQLObjectType({ - name: 'Test1', + name: 'Test', fields: outputFields, }); const testObject2 = new GraphQLObjectType({ - name: 'Test2', + name: 'Test', fields: outputFields, }); @@ -191,11 +191,11 @@ describe('Type System: Objects', () => { field2: { type: ScalarType }, }; const testInputObject1 = new GraphQLInputObjectType({ - name: 'Test1', + name: 'Test', fields: inputFields, }); const testInputObject2 = new GraphQLInputObjectType({ - name: 'Test2', + name: 'Test', fields: inputFields, }); @@ -243,6 +243,7 @@ describe('Type System: Objects', () => { }); expect(objType.getFields()).to.deep.equal({ f: { + coordinate: 'SomeObject.f', name: 'f', description: undefined, type: ScalarType, @@ -270,11 +271,13 @@ describe('Type System: Objects', () => { }); expect(objType.getFields()).to.deep.equal({ f: { + coordinate: 'SomeObject.f', name: 'f', description: undefined, type: ScalarType, args: [ { + coordinate: 'SomeObject.f(arg:)', name: 'arg', description: undefined, type: ScalarType, @@ -624,6 +627,7 @@ describe('Type System: Enums', () => { expect(EnumTypeWithNullishValue.getValues()).to.deep.equal([ { + coordinate: 'EnumWithNullishValue.NULL', name: 'NULL', description: undefined, value: null, @@ -632,6 +636,7 @@ describe('Type System: Enums', () => { astNode: undefined, }, { + coordinate: 'EnumWithNullishValue.NAN', name: 'NAN', description: undefined, value: NaN, @@ -640,6 +645,7 @@ describe('Type System: Enums', () => { astNode: undefined, }, { + coordinate: 'EnumWithNullishValue.NO_CUSTOM_VALUE', name: 'NO_CUSTOM_VALUE', description: undefined, value: 'NO_CUSTOM_VALUE', @@ -730,6 +736,7 @@ describe('Type System: Input Objects', () => { }); expect(inputObjType.getFields()).to.deep.equal({ f: { + coordinate: 'SomeInputObject.f', name: 'f', description: undefined, type: ScalarType, @@ -750,6 +757,7 @@ describe('Type System: Input Objects', () => { }); expect(inputObjType.getFields()).to.deep.equal({ f: { + coordinate: 'SomeInputObject.f', name: 'f', description: undefined, type: ScalarType, diff --git a/src/type/__tests__/directive-test.ts b/src/type/__tests__/directive-test.ts index 19249b3b14..d6b6979dc6 100644 --- a/src/type/__tests__/directive-test.ts +++ b/src/type/__tests__/directive-test.ts @@ -33,6 +33,7 @@ describe('Type System: Directive', () => { name: 'Foo', args: [ { + coordinate: '@Foo(foo:)', name: 'foo', description: undefined, type: GraphQLString, @@ -42,6 +43,7 @@ describe('Type System: Directive', () => { astNode: undefined, }, { + coordinate: '@Foo(bar:)', name: 'bar', description: undefined, type: GraphQLInt, diff --git a/src/type/__tests__/enumType-test.ts b/src/type/__tests__/enumType-test.ts index c3cf23cd1c..00a4ec18dc 100644 --- a/src/type/__tests__/enumType-test.ts +++ b/src/type/__tests__/enumType-test.ts @@ -342,6 +342,7 @@ describe('Type System: Enum Values', () => { const values = ComplexEnum.getValues(); expect(values).to.have.deep.ordered.members([ { + coordinate: 'Complex.ONE', name: 'ONE', description: undefined, value: Complex1, @@ -350,6 +351,7 @@ describe('Type System: Enum Values', () => { astNode: undefined, }, { + coordinate: 'Complex.TWO', name: 'TWO', description: undefined, value: Complex2, diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index 0a480c3e71..8f12cd0398 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -1480,7 +1480,7 @@ describe('Introspection', () => { errors: [ { message: - 'Field "__type" argument "name" of type "String!" is required, but it was not provided.', + 'Argument .__type(name:) of type "String!" is required, but it was not provided.', locations: [{ line: 3, column: 9 }], }, ], diff --git a/src/type/__tests__/predicate-test.ts b/src/type/__tests__/predicate-test.ts index 94e152e1aa..b15acac6f8 100644 --- a/src/type/__tests__/predicate-test.ts +++ b/src/type/__tests__/predicate-test.ts @@ -1,11 +1,7 @@ import { expect } from 'chai'; import { describe, it } from 'mocha'; -import type { - GraphQLArgument, - GraphQLInputField, - GraphQLInputType, -} from '../definition'; +import type { GraphQLInputType } from '../definition'; import { GraphQLDirective, GraphQLSkipDirective, @@ -29,8 +25,10 @@ import { GraphQLScalarType, GraphQLEnumType, GraphQLInputObjectType, + GraphQLInputField, GraphQLInterfaceType, GraphQLObjectType, + GraphQLArgument, GraphQLUnionType, isType, isScalarType, @@ -567,15 +565,7 @@ describe('Type predicates', () => { type: GraphQLInputType; defaultValue?: unknown; }): GraphQLArgument { - return { - name: 'someArg', - type: config.type, - description: undefined, - defaultValue: config.defaultValue, - deprecationReason: null, - extensions: undefined, - astNode: undefined, - }; + return new GraphQLArgument('SomeType.someField', 'someArg', config); } it('returns true for required arguments', () => { @@ -615,15 +605,7 @@ describe('Type predicates', () => { type: GraphQLInputType; defaultValue?: unknown; }): GraphQLInputField { - return { - name: 'someInputField', - type: config.type, - description: undefined, - defaultValue: config.defaultValue, - deprecationReason: null, - extensions: undefined, - astNode: undefined, - }; + return new GraphQLInputField('SomeType', 'someInputField', config); } it('returns true for required input field', () => { diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 6d8ef8f789..a3c8b20451 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -1974,7 +1974,7 @@ describe('Objects must adhere to Interface they implement', () => { expect(validateSchema(schema)).to.deep.equal([ { message: - 'Object field AnotherObject.field includes required argument requiredArg that is missing from the Interface field AnotherInterface.field.', + 'Argument AnotherObject.field(requiredArg:) must not be required type String! if not provided by the Interface field AnotherInterface.field.', locations: [ { line: 13, column: 11 }, { line: 7, column: 9 }, @@ -2169,11 +2169,11 @@ describe('Interfaces must adhere to Interface they implement', () => { } interface ParentInterface { - field(input: String): String + field(input: String!): String } interface ChildInterface implements ParentInterface { - field(input: String, anotherInput: String): String + field(input: String!, anotherInput: String): String } `); expect(validateSchema(schema)).to.deep.equal([]); @@ -2431,7 +2431,7 @@ describe('Interfaces must adhere to Interface they implement', () => { expect(validateSchema(schema)).to.deep.equal([ { message: - 'Object field ChildInterface.field includes required argument requiredArg that is missing from the Interface field ParentInterface.field.', + 'Argument ChildInterface.field(requiredArg:) must not be required type String! if not provided by the Interface field ParentInterface.field.', locations: [ { line: 13, column: 11 }, { line: 7, column: 9 }, diff --git a/src/type/definition.ts b/src/type/definition.ts index 70b3bbbe1b..09cb797413 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -522,6 +522,25 @@ export function getNamedType( } } +/** + * The base class for all Schema Elements. + */ +export class GraphQLSchemaElement { + readonly coordinate: string; + + constructor(coordinate: string) { + this.coordinate = coordinate; + } + + toString(): string { + return this.coordinate; + } + + toJSON(): string { + return this.toString(); + } +} + /** * Used while defining GraphQL types to allow for circular references in * otherwise immutable type definitions. @@ -574,7 +593,7 @@ export interface GraphQLScalarTypeExtensions { * }); * */ -export class GraphQLScalarType { +export class GraphQLScalarType extends GraphQLSchemaElement { name: string; description: Maybe; specifiedByURL: Maybe; @@ -586,6 +605,7 @@ export class GraphQLScalarType { extensionASTNodes: ReadonlyArray; constructor(config: Readonly>) { + super(config.name); const parseValue = config.parseValue ?? identityFunc; this.name = config.name; this.description = config.description; @@ -636,14 +656,6 @@ export class GraphQLScalarType { }; } - toString(): string { - return this.name; - } - - toJSON(): string { - return this.toString(); - } - get [Symbol.toStringTag]() { return 'GraphQLScalarType'; } @@ -739,7 +751,10 @@ export interface GraphQLObjectTypeExtensions<_TSource = any, _TContext = any> { * }); * */ -export class GraphQLObjectType { +export class GraphQLObjectType< + TSource = any, + TContext = any, +> extends GraphQLSchemaElement { name: string; description: Maybe; isTypeOf: Maybe>; @@ -751,6 +766,7 @@ export class GraphQLObjectType { private _interfaces: ThunkArray; constructor(config: Readonly>) { + super(config.name); this.name = config.name; this.description = config.description; this.isTypeOf = config.isTypeOf; @@ -787,7 +803,7 @@ export class GraphQLObjectType { name: this.name, description: this.description, interfaces: this.getInterfaces(), - fields: fieldsToFieldsConfig(this.getFields()), + fields: mapValue(this.getFields(), (field) => field.toConfig()), isTypeOf: this.isTypeOf, extensions: this.extensions, astNode: this.astNode, @@ -795,14 +811,6 @@ export class GraphQLObjectType { }; } - toString(): string { - return this.name; - } - - toJSON(): string { - return this.toString(); - } - get [Symbol.toStringTag]() { return 'GraphQLObjectType'; } @@ -833,90 +841,17 @@ function defineFieldMap( `${config.name} fields must be an object with field names as keys or a function which returns such an object.`, ); - return mapValue(fieldMap, (fieldConfig, fieldName) => { - devAssert( - isPlainObj(fieldConfig), - `${config.name}.${fieldName} field config must be an object.`, - ); - devAssert( - fieldConfig.resolve == null || typeof fieldConfig.resolve === 'function', - `${config.name}.${fieldName} field resolver must be a function if ` + - `provided, but got: ${inspect(fieldConfig.resolve)}.`, - ); - - const argsConfig = fieldConfig.args ?? {}; - devAssert( - isPlainObj(argsConfig), - `${config.name}.${fieldName} args must be an object with argument names as keys.`, - ); - - return { - name: fieldName, - description: fieldConfig.description, - type: fieldConfig.type, - args: defineArguments(argsConfig), - resolve: fieldConfig.resolve, - subscribe: fieldConfig.subscribe, - deprecationReason: fieldConfig.deprecationReason, - extensions: fieldConfig.extensions && toObjMap(fieldConfig.extensions), - astNode: fieldConfig.astNode, - }; - }); -} - -export function defineArguments( - config: GraphQLFieldConfigArgumentMap, -): ReadonlyArray { - return Object.entries(config).map(([argName, argConfig]) => ({ - name: argName, - description: argConfig.description, - type: argConfig.type, - defaultValue: argConfig.defaultValue, - deprecationReason: argConfig.deprecationReason, - extensions: argConfig.extensions && toObjMap(argConfig.extensions), - astNode: argConfig.astNode, - })); + return mapValue( + fieldMap, + (fieldConfig, fieldName) => + new GraphQLField(config.name, fieldName, fieldConfig), + ); } function isPlainObj(obj: unknown): boolean { return isObjectLike(obj) && !Array.isArray(obj); } -function fieldsToFieldsConfig( - fields: GraphQLFieldMap, -): GraphQLFieldConfigMap { - return mapValue(fields, (field) => ({ - description: field.description, - type: field.type, - args: argsToArgsConfig(field.args), - resolve: field.resolve, - subscribe: field.subscribe, - deprecationReason: field.deprecationReason, - extensions: field.extensions, - astNode: field.astNode, - })); -} - -/** - * @internal - */ -export function argsToArgsConfig( - args: ReadonlyArray, -): GraphQLFieldConfigArgumentMap { - return keyValMap( - args, - (arg) => arg.name, - (arg) => ({ - description: arg.description, - type: arg.type, - defaultValue: arg.defaultValue, - deprecationReason: arg.deprecationReason, - extensions: arg.extensions, - astNode: arg.astNode, - }), - ); -} - export interface GraphQLObjectTypeConfig { name: string; description?: Maybe; @@ -1038,11 +973,11 @@ export type GraphQLFieldConfigMap = ObjMap< GraphQLFieldConfig >; -export interface GraphQLField< +export class GraphQLField< TSource, TContext, TArgs = { [argument: string]: any }, -> { +> extends GraphQLSchemaElement { name: string; description: Maybe; type: GraphQLOutputType; @@ -1052,9 +987,65 @@ export interface GraphQLField< deprecationReason: Maybe; extensions: Maybe>>; astNode: Maybe; + + constructor( + parentCoordinate: string, + name: string, + config: GraphQLFieldConfig, + ) { + const coordinate = `${parentCoordinate}.${name}`; + + devAssert( + isPlainObj(config), + `${coordinate} field config must be an object.`, + ); + + devAssert( + config.resolve == null || typeof config.resolve === 'function', + `${coordinate} field resolver must be a function if ` + + `provided, but got: ${inspect(config.resolve)}.`, + ); + + const argsConfig = config.args ?? {}; + devAssert( + isPlainObj(argsConfig), + `${coordinate} args must be an object with argument names as keys.`, + ); + + super(coordinate); + this.name = name; + this.description = config.description; + this.type = config.type; + this.args = Object.entries(argsConfig).map( + ([argName, argConfig]) => + new GraphQLArgument(coordinate, argName, argConfig), + ); + this.resolve = config.resolve; + this.subscribe = config.subscribe; + this.deprecationReason = config.deprecationReason; + this.extensions = config.extensions && toObjMap(config.extensions); + this.astNode = config.astNode; + } + + toConfig(): GraphQLFieldConfig { + return { + description: this.description, + type: this.type, + args: keyValMap( + this.args, + (arg) => arg.name, + (arg) => arg.toConfig(), + ), + resolve: this.resolve, + subscribe: this.subscribe, + deprecationReason: this.deprecationReason, + extensions: this.extensions, + astNode: this.astNode, + }; + } } -export interface GraphQLArgument { +export class GraphQLArgument extends GraphQLSchemaElement { name: string; description: Maybe; type: GraphQLInputType; @@ -1062,6 +1053,33 @@ export interface GraphQLArgument { deprecationReason: Maybe; extensions: Maybe>; astNode: Maybe; + + constructor( + parentCoordinate: string, + name: string, + config: GraphQLArgumentConfig, + ) { + const coordinate = `${parentCoordinate}(${name}:)`; + super(coordinate); + this.name = name; + this.description = config.description; + this.type = config.type; + this.defaultValue = config.defaultValue; + this.deprecationReason = config.deprecationReason; + this.extensions = config.extensions && toObjMap(config.extensions); + this.astNode = config.astNode; + } + + toConfig(): GraphQLArgumentConfig { + return { + description: this.description, + type: this.type, + defaultValue: this.defaultValue, + deprecationReason: this.deprecationReason, + extensions: this.extensions, + astNode: this.astNode, + }; + } } export function isRequiredArgument(arg: GraphQLArgument): boolean { @@ -1103,7 +1121,7 @@ export interface GraphQLInterfaceTypeExtensions { * }); * */ -export class GraphQLInterfaceType { +export class GraphQLInterfaceType extends GraphQLSchemaElement { name: string; description: Maybe; resolveType: Maybe>; @@ -1115,6 +1133,7 @@ export class GraphQLInterfaceType { private _interfaces: ThunkArray; constructor(config: Readonly>) { + super(config.name); this.name = config.name; this.description = config.description; this.resolveType = config.resolveType; @@ -1151,7 +1170,7 @@ export class GraphQLInterfaceType { name: this.name, description: this.description, interfaces: this.getInterfaces(), - fields: fieldsToFieldsConfig(this.getFields()), + fields: mapValue(this.getFields(), (field) => field.toConfig()), resolveType: this.resolveType, extensions: this.extensions, astNode: this.astNode, @@ -1159,14 +1178,6 @@ export class GraphQLInterfaceType { }; } - toString(): string { - return this.name; - } - - toJSON(): string { - return this.toString(); - } - get [Symbol.toStringTag]() { return 'GraphQLInterfaceType'; } @@ -1232,7 +1243,7 @@ export interface GraphQLUnionTypeExtensions { * }); * */ -export class GraphQLUnionType { +export class GraphQLUnionType extends GraphQLSchemaElement { name: string; description: Maybe; resolveType: Maybe>; @@ -1243,6 +1254,7 @@ export class GraphQLUnionType { private _types: ThunkArray; constructor(config: Readonly>) { + super(config.name); this.name = config.name; this.description = config.description; this.resolveType = config.resolveType; @@ -1278,14 +1290,6 @@ export class GraphQLUnionType { }; } - toString(): string { - return this.name; - } - - toJSON(): string { - return this.toString(); - } - get [Symbol.toStringTag]() { return 'GraphQLUnionType'; } @@ -1358,7 +1362,7 @@ export interface GraphQLEnumTypeExtensions { * Note: If a value is not provided in a definition, the name of the enum value * will be used as its internal value. */ -export class GraphQLEnumType /* */ { +export class GraphQLEnumType /* */ extends GraphQLSchemaElement { name: string; description: Maybe; extensions: Maybe>; @@ -1370,13 +1374,21 @@ export class GraphQLEnumType /* */ { private _nameLookup: ObjMap; constructor(config: Readonly */>) { + super(config.name); this.name = config.name; this.description = config.description; this.extensions = config.extensions && toObjMap(config.extensions); this.astNode = config.astNode; this.extensionASTNodes = config.extensionASTNodes ?? []; - this._values = defineEnumValues(this.name, config.values); + devAssert( + isPlainObj(config.values), + `${this.name} values must be an object with value names as keys.`, + ); + this._values = Object.entries(config.values).map( + ([valueName, valueConfig]) => + new GraphQLEnumValue(config.name, valueName, valueConfig), + ); this._valueLookup = new Map( this._values.map((enumValue) => [enumValue.value, enumValue]), ); @@ -1449,36 +1461,20 @@ export class GraphQLEnumType /* */ { } toConfig(): GraphQLEnumTypeNormalizedConfig { - const values = keyValMap( - this.getValues(), - (value) => value.name, - (value) => ({ - description: value.description, - value: value.value, - deprecationReason: value.deprecationReason, - extensions: value.extensions, - astNode: value.astNode, - }), - ); - return { name: this.name, description: this.description, - values, + values: keyValMap( + this.getValues(), + (value) => value.name, + (value) => value.toConfig(), + ), extensions: this.extensions, astNode: this.astNode, extensionASTNodes: this.extensionASTNodes, }; } - toString(): string { - return this.name; - } - - toJSON(): string { - return this.toString(); - } - get [Symbol.toStringTag]() { return 'GraphQLEnumType'; } @@ -1494,31 +1490,6 @@ function didYouMeanEnumValue( return didYouMean('the enum value', suggestedValues); } -function defineEnumValues( - typeName: string, - valueMap: GraphQLEnumValueConfigMap /* */, -): Array */> { - devAssert( - isPlainObj(valueMap), - `${typeName} values must be an object with value names as keys.`, - ); - return Object.entries(valueMap).map(([valueName, valueConfig]) => { - devAssert( - isPlainObj(valueConfig), - `${typeName}.${valueName} must refer to an object with a "value" key ` + - `representing an internal value but got: ${inspect(valueConfig)}.`, - ); - return { - name: valueName, - description: valueConfig.description, - value: valueConfig.value !== undefined ? valueConfig.value : valueName, - deprecationReason: valueConfig.deprecationReason, - extensions: valueConfig.extensions && toObjMap(valueConfig.extensions), - astNode: valueConfig.astNode, - }; - }); -} - export interface GraphQLEnumTypeConfig { name: string; description?: Maybe; @@ -1557,13 +1528,45 @@ export interface GraphQLEnumValueConfig { astNode?: Maybe; } -export interface GraphQLEnumValue { +export class GraphQLEnumValue extends GraphQLSchemaElement { name: string; description: Maybe; value: any /* T */; deprecationReason: Maybe; extensions: Maybe>; astNode: Maybe; + + constructor( + parentCoordinate: string, + name: string, + config: GraphQLEnumValueConfig, + ) { + const coordinate = `${parentCoordinate}.${name}`; + + devAssert( + isPlainObj(config), + `${coordinate} must refer to an object with a "value" key ` + + `representing an internal value but got: ${inspect(config)}.`, + ); + + super(coordinate); + this.name = name; + this.description = config.description; + this.value = config.value !== undefined ? config.value : name; + this.deprecationReason = config.deprecationReason; + this.extensions = config.extensions && toObjMap(config.extensions); + this.astNode = config.astNode; + } + + toConfig(): GraphQLEnumValueConfig { + return { + description: this.description, + value: this.value, + deprecationReason: this.deprecationReason, + extensions: this.extensions, + astNode: this.astNode, + }; + } } /** @@ -1599,7 +1602,7 @@ export interface GraphQLInputObjectTypeExtensions { * }); * */ -export class GraphQLInputObjectType { +export class GraphQLInputObjectType extends GraphQLSchemaElement { name: string; description: Maybe; extensions: Maybe>; @@ -1609,6 +1612,7 @@ export class GraphQLInputObjectType { private _fields: ThunkObjMap; constructor(config: Readonly) { + super(config.name); this.name = config.name; this.description = config.description; this.extensions = config.extensions && toObjMap(config.extensions); @@ -1627,32 +1631,16 @@ export class GraphQLInputObjectType { } toConfig(): GraphQLInputObjectTypeNormalizedConfig { - const fields = mapValue(this.getFields(), (field) => ({ - description: field.description, - type: field.type, - defaultValue: field.defaultValue, - extensions: field.extensions, - astNode: field.astNode, - })); - return { name: this.name, description: this.description, - fields, + fields: mapValue(this.getFields(), (field) => field.toConfig()), extensions: this.extensions, astNode: this.astNode, extensionASTNodes: this.extensionASTNodes, }; } - toString(): string { - return this.name; - } - - toJSON(): string { - return this.toString(); - } - get [Symbol.toStringTag]() { return 'GraphQLInputObjectType'; } @@ -1666,22 +1654,11 @@ function defineInputFieldMap( isPlainObj(fieldMap), `${config.name} fields must be an object with field names as keys or a function which returns such an object.`, ); - return mapValue(fieldMap, (fieldConfig, fieldName) => { - devAssert( - !('resolve' in fieldConfig), - `${config.name}.${fieldName} field has a resolve property, but Input Types cannot define resolvers.`, - ); - - return { - name: fieldName, - description: fieldConfig.description, - type: fieldConfig.type, - defaultValue: fieldConfig.defaultValue, - deprecationReason: fieldConfig.deprecationReason, - extensions: fieldConfig.extensions && toObjMap(fieldConfig.extensions), - astNode: fieldConfig.astNode, - }; - }); + return mapValue( + fieldMap, + (fieldConfig, fieldName) => + new GraphQLInputField(config.name, fieldName, fieldConfig), + ); } export interface GraphQLInputObjectTypeConfig { @@ -1724,7 +1701,7 @@ export interface GraphQLInputFieldConfig { export type GraphQLInputFieldConfigMap = ObjMap; -export interface GraphQLInputField { +export class GraphQLInputField extends GraphQLSchemaElement { name: string; description: Maybe; type: GraphQLInputType; @@ -1732,6 +1709,38 @@ export interface GraphQLInputField { deprecationReason: Maybe; extensions: Maybe>; astNode: Maybe; + + constructor( + parentCoordinate: string, + name: string, + config: GraphQLInputFieldConfig, + ) { + const coordinate = `${parentCoordinate}.${name}`; + + devAssert( + !('resolve' in config), + `${coordinate} field has a resolve property, but Input Types cannot define resolvers.`, + ); + + super(coordinate); + this.name = name; + this.description = config.description; + this.type = config.type; + this.defaultValue = config.defaultValue; + this.deprecationReason = config.deprecationReason; + this.extensions = config.extensions && toObjMap(config.extensions); + this.astNode = config.astNode; + } + + toConfig(): GraphQLInputFieldConfig { + return { + description: this.description, + type: this.type, + defaultValue: this.defaultValue, + extensions: this.extensions, + astNode: this.astNode, + }; + } } export function isRequiredInputField(field: GraphQLInputField): boolean { diff --git a/src/type/directives.ts b/src/type/directives.ts index 3dff8298bb..5384895a63 100644 --- a/src/type/directives.ts +++ b/src/type/directives.ts @@ -1,5 +1,7 @@ +import type { ObjMap } from '../jsutils/ObjMap'; import { inspect } from '../jsutils/inspect'; import { toObjMap } from '../jsutils/toObjMap'; +import { keyValMap } from '../jsutils/keyValMap'; import { devAssert } from '../jsutils/devAssert'; import { instanceOf } from '../jsutils/instanceOf'; import { isObjectLike } from '../jsutils/isObjectLike'; @@ -9,16 +11,13 @@ import type { DirectiveDefinitionNode } from '../language/ast'; import type { DirectiveLocationEnum } from '../language/directiveLocation'; import { DirectiveLocation } from '../language/directiveLocation'; -import type { - GraphQLArgument, - GraphQLFieldConfigArgumentMap, -} from './definition'; -import { GraphQLString, GraphQLBoolean } from './scalars'; +import type { GraphQLArgumentConfig } from './definition'; import { - defineArguments, - argsToArgsConfig, + GraphQLArgument, + GraphQLSchemaElement, GraphQLNonNull, } from './definition'; +import { GraphQLString, GraphQLBoolean } from './scalars'; /** * Test if the given value is a GraphQL directive. @@ -53,7 +52,7 @@ export interface GraphQLDirectiveExtensions { * Directives are used by the GraphQL runtime as a way of modifying execution * behavior. Type system creators will usually not create these directly. */ -export class GraphQLDirective { +export class GraphQLDirective extends GraphQLSchemaElement { name: string; description: Maybe; locations: Array; @@ -63,6 +62,8 @@ export class GraphQLDirective { astNode: Maybe; constructor(config: Readonly) { + const coordinate = `@${config.name}`; + super(coordinate); this.name = config.name; this.description = config.description; this.locations = config.locations; @@ -73,16 +74,19 @@ export class GraphQLDirective { devAssert(config.name, 'Directive must be named.'); devAssert( Array.isArray(config.locations), - `@${config.name} locations must be an Array.`, + `${coordinate} locations must be an Array.`, ); const args = config.args ?? {}; devAssert( isObjectLike(args) && !Array.isArray(args), - `@${config.name} args must be an object with argument names as keys.`, + `${coordinate} args must be an object with argument names as keys.`, ); - this.args = defineArguments(args); + this.args = Object.entries(args).map( + ([argName, argConfig]) => + new GraphQLArgument(coordinate, argName, argConfig), + ); } toConfig(): GraphQLDirectiveNormalizedConfig { @@ -90,21 +94,17 @@ export class GraphQLDirective { name: this.name, description: this.description, locations: this.locations, - args: argsToArgsConfig(this.args), + args: keyValMap( + this.args, + (arg) => arg.name, + (arg) => arg.toConfig(), + ), isRepeatable: this.isRepeatable, extensions: this.extensions, astNode: this.astNode, }; } - toString(): string { - return '@' + this.name; - } - - toJSON(): string { - return this.toString(); - } - get [Symbol.toStringTag]() { return 'GraphQLDirective'; } @@ -114,14 +114,14 @@ export interface GraphQLDirectiveConfig { name: string; description?: Maybe; locations: Array; - args?: Maybe; + args?: Maybe>; isRepeatable?: Maybe; extensions?: Maybe>; astNode?: Maybe; } interface GraphQLDirectiveNormalizedConfig extends GraphQLDirectiveConfig { - args: GraphQLFieldConfigArgumentMap; + args: ObjMap; isRepeatable: boolean; extensions: Maybe>; } diff --git a/src/type/index.ts b/src/type/index.ts index fab8bb65dc..f4c419bb66 100644 --- a/src/type/index.ts +++ b/src/type/index.ts @@ -53,12 +53,17 @@ export { getNullableType, getNamedType, /** Definitions */ + GraphQLSchemaElement, GraphQLScalarType, GraphQLObjectType, GraphQLInterfaceType, GraphQLUnionType, GraphQLEnumType, GraphQLInputObjectType, + GraphQLField, + GraphQLArgument, + GraphQLEnumValue, + GraphQLInputField, /** Type Wrappers */ GraphQLList, GraphQLNonNull, @@ -78,23 +83,19 @@ export type { GraphQLNamedOutputType, ThunkArray, ThunkObjMap, - GraphQLArgument, GraphQLArgumentConfig, GraphQLArgumentExtensions, GraphQLEnumTypeConfig, GraphQLEnumTypeExtensions, - GraphQLEnumValue, GraphQLEnumValueConfig, GraphQLEnumValueConfigMap, GraphQLEnumValueExtensions, - GraphQLField, GraphQLFieldConfig, GraphQLFieldConfigArgumentMap, GraphQLFieldConfigMap, GraphQLFieldExtensions, GraphQLFieldMap, GraphQLFieldResolver, - GraphQLInputField, GraphQLInputFieldConfig, GraphQLInputFieldConfigMap, GraphQLInputFieldExtensions, diff --git a/src/type/introspection.ts b/src/type/introspection.ts index 5859aa27ed..6dfd193f5e 100644 --- a/src/type/introspection.ts +++ b/src/type/introspection.ts @@ -12,7 +12,6 @@ import type { GraphQLNamedType, GraphQLInputField, GraphQLEnumValue, - GraphQLField, GraphQLFieldConfigMap, } from './definition'; import { GraphQLString, GraphQLBoolean } from './scalars'; @@ -20,6 +19,7 @@ import { GraphQLList, GraphQLNonNull, GraphQLObjectType, + GraphQLField, GraphQLEnumType, isScalarType, isObjectType, @@ -480,53 +480,24 @@ export const __TypeKind: GraphQLEnumType = new GraphQLEnumType({ }, }); -/** - * Note that these are GraphQLField and not GraphQLFieldConfig, - * so the format for args is different. - */ - -export const SchemaMetaFieldDef: GraphQLField = { - name: '__schema', +export const SchemaMetaFieldDef = new GraphQLField('', '__schema', { type: new GraphQLNonNull(__Schema), description: 'Access the current type schema of this server.', - args: [], resolve: (_source, _args, _context, { schema }) => schema, - deprecationReason: undefined, - extensions: undefined, - astNode: undefined, -}; +}); -export const TypeMetaFieldDef: GraphQLField = { - name: '__type', +export const TypeMetaFieldDef = new GraphQLField('', '__type', { type: __Type, description: 'Request the type information of a single type.', - args: [ - { - name: 'name', - description: undefined, - type: new GraphQLNonNull(GraphQLString), - defaultValue: undefined, - deprecationReason: undefined, - extensions: undefined, - astNode: undefined, - }, - ], + args: { name: { type: new GraphQLNonNull(GraphQLString) } }, resolve: (_source, { name }, _context, { schema }) => schema.getType(name), - deprecationReason: undefined, - extensions: undefined, - astNode: undefined, -}; +}); -export const TypeNameMetaFieldDef: GraphQLField = { - name: '__typename', +export const TypeNameMetaFieldDef = new GraphQLField('', '__typename', { type: new GraphQLNonNull(GraphQLString), description: 'The name of the current Object type at runtime.', - args: [], resolve: (_source, _args, _context, { parentType }) => parentType.name, - deprecationReason: undefined, - extensions: undefined, - astNode: undefined, -}; +}); export const introspectionTypes: ReadonlyArray = Object.freeze([ diff --git a/src/type/validate.ts b/src/type/validate.ts index 77420d8bc5..a6de555816 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -184,17 +184,17 @@ function validateDirectives(context: SchemaValidationContext): void { // Ensure the type is an input type. if (!isInputType(arg.type)) { context.reportError( - `The type of @${directive.name}(${arg.name}:) must be Input Type ` + + `The type of ${arg} must be Input Type ` + `but got: ${inspect(arg.type)}.`, arg.astNode, ); } if (isRequiredArgument(arg) && arg.deprecationReason != null) { - context.reportError( - `Required argument @${directive.name}(${arg.name}:) cannot be deprecated.`, - [getDeprecatedDirectiveNode(arg.astNode), arg.astNode?.type], - ); + context.reportError(`Required argument ${arg} cannot be deprecated.`, [ + getDeprecatedDirectiveNode(arg.astNode), + arg.astNode?.type, + ]); } } } @@ -266,7 +266,7 @@ function validateFields( // Objects and Interfaces both must define one or more fields. if (fields.length === 0) { - context.reportError(`Type ${type.name} must define one or more fields.`, [ + context.reportError(`Type ${type} must define one or more fields.`, [ type.astNode, ...type.extensionASTNodes, ]); @@ -279,7 +279,7 @@ function validateFields( // Ensure the type is an output type if (!isOutputType(field.type)) { context.reportError( - `The type of ${type.name}.${field.name} must be Output Type ` + + `The type of ${field} must be Output Type ` + `but got: ${inspect(field.type)}.`, field.astNode?.type, ); @@ -287,25 +287,23 @@ function validateFields( // Ensure the arguments are valid for (const arg of field.args) { - const argName = arg.name; - // Ensure they are named correctly. validateName(context, arg); // Ensure the type is an input type if (!isInputType(arg.type)) { context.reportError( - `The type of ${type.name}.${field.name}(${argName}:) must be Input ` + + `The type of ${arg} must be Input ` + `Type but got: ${inspect(arg.type)}.`, arg.astNode?.type, ); } if (isRequiredArgument(arg) && arg.deprecationReason != null) { - context.reportError( - `Required argument ${type.name}.${field.name}(${argName}:) cannot be deprecated.`, - [getDeprecatedDirectiveNode(arg.astNode), arg.astNode?.type], - ); + context.reportError(`Required argument ${arg} cannot be deprecated.`, [ + getDeprecatedDirectiveNode(arg.astNode), + arg.astNode?.type, + ]); } } } @@ -319,7 +317,7 @@ function validateInterfaces( for (const iface of type.getInterfaces()) { if (!isInterfaceType(iface)) { context.reportError( - `Type ${inspect(type)} must only implement Interface types, ` + + `Type ${type} must only implement Interface types, ` + `it cannot implement ${inspect(iface)}.`, getAllImplementsInterfaceNodes(type, iface), ); @@ -328,7 +326,7 @@ function validateInterfaces( if (type === iface) { context.reportError( - `Type ${type.name} cannot implement itself because it would create a circular reference.`, + `Type ${type} cannot implement itself because it would create a circular reference.`, getAllImplementsInterfaceNodes(type, iface), ); continue; @@ -336,7 +334,7 @@ function validateInterfaces( if (ifaceTypeNames[iface.name]) { context.reportError( - `Type ${type.name} can only implement ${iface.name} once.`, + `Type ${type} can only implement ${iface} once.`, getAllImplementsInterfaceNodes(type, iface), ); continue; @@ -358,13 +356,12 @@ function validateTypeImplementsInterface( // Assert each interface field is implemented. for (const ifaceField of Object.values(iface.getFields())) { - const fieldName = ifaceField.name; - const typeField = typeFieldMap[fieldName]; + const typeField = typeFieldMap[ifaceField.name]; // Assert interface field exists on type. if (!typeField) { context.reportError( - `Interface field ${iface.name}.${fieldName} expected but ${type.name} does not provide it.`, + `Interface field ${ifaceField} expected but ${type} does not provide it.`, [ifaceField.astNode, type.astNode, ...type.extensionASTNodes], ); continue; @@ -374,22 +371,20 @@ function validateTypeImplementsInterface( // a valid subtype. (covariant) if (!isTypeSubTypeOf(context.schema, typeField.type, ifaceField.type)) { context.reportError( - `Interface field ${iface.name}.${fieldName} expects type ` + - `${inspect(ifaceField.type)} but ${type.name}.${fieldName} ` + - `is type ${inspect(typeField.type)}.`, + `Interface field ${ifaceField} expects type ${ifaceField.type} ` + + `but ${typeField} is type ${typeField.type}.`, [ifaceField.astNode?.type, typeField.astNode?.type], ); } // Assert each interface field arg is implemented. for (const ifaceArg of ifaceField.args) { - const argName = ifaceArg.name; - const typeArg = typeField.args.find((arg) => arg.name === argName); + const typeArg = typeField.args.find((arg) => arg.name === ifaceArg.name); // Assert interface field arg exists on object field. if (!typeArg) { context.reportError( - `Interface field argument ${iface.name}.${fieldName}(${argName}:) expected but ${type.name}.${fieldName} does not provide it.`, + `Interface field argument ${ifaceArg} expected but ${typeField} does not provide it.`, [ifaceArg.astNode, typeField.astNode], ); continue; @@ -400,10 +395,8 @@ function validateTypeImplementsInterface( // TODO: change to contravariant? if (!isEqualType(ifaceArg.type, typeArg.type)) { context.reportError( - `Interface field argument ${iface.name}.${fieldName}(${argName}:) ` + - `expects type ${inspect(ifaceArg.type)} but ` + - `${type.name}.${fieldName}(${argName}:) is type ` + - `${inspect(typeArg.type)}.`, + `Interface field argument ${ifaceArg} expects type ${ifaceArg.type} ` + + `but ${typeArg} is type ${typeArg.type}.`, [ // istanbul ignore next (TODO need to write coverage tests) ifaceArg.astNode?.type, @@ -418,13 +411,17 @@ function validateTypeImplementsInterface( // Assert additional arguments must not be required. for (const typeArg of typeField.args) { - const argName = typeArg.name; - const ifaceArg = ifaceField.args.find((arg) => arg.name === argName); - if (!ifaceArg && isRequiredArgument(typeArg)) { - context.reportError( - `Object field ${type.name}.${fieldName} includes required argument ${argName} that is missing from the Interface field ${iface.name}.${fieldName}.`, - [typeArg.astNode, ifaceField.astNode], + if (isRequiredArgument(typeArg)) { + const ifaceArg = ifaceField.args.find( + (arg) => arg.name === typeArg.name, ); + if (!ifaceArg) { + context.reportError( + `Argument ${typeArg} must not be required type ${typeArg.type} ` + + `if not provided by the Interface field ${ifaceField}.`, + [typeArg.astNode, ifaceField.astNode], + ); + } } } } @@ -440,8 +437,8 @@ function validateTypeImplementsAncestors( if (!ifaceInterfaces.includes(transitive)) { context.reportError( transitive === type - ? `Type ${type.name} cannot implement ${iface.name} because it would create a circular reference.` - : `Type ${type.name} must implement ${transitive.name} because it is implemented by ${iface.name}.`, + ? `Type ${type} cannot implement ${iface} because it would create a circular reference.` + : `Type ${type} must implement ${transitive} because it is implemented by ${iface}.`, [ ...getAllImplementsInterfaceNodes(iface, transitive), ...getAllImplementsInterfaceNodes(type, iface), @@ -459,7 +456,7 @@ function validateUnionMembers( if (memberTypes.length === 0) { context.reportError( - `Union type ${union.name} must define one or more member types.`, + `Union type ${union} must define one or more member types.`, [union.astNode, ...union.extensionASTNodes], ); } @@ -468,7 +465,7 @@ function validateUnionMembers( for (const memberType of memberTypes) { if (includedTypeNames[memberType.name]) { context.reportError( - `Union type ${union.name} can only include type ${memberType.name} once.`, + `Union type ${union} can only include type ${memberType} once.`, getUnionMemberTypeNodes(union, memberType.name), ); continue; @@ -476,7 +473,7 @@ function validateUnionMembers( includedTypeNames[memberType.name] = true; if (!isObjectType(memberType)) { context.reportError( - `Union type ${union.name} can only include Object types, ` + + `Union type ${union} can only include Object types, ` + `it cannot include ${inspect(memberType)}.`, getUnionMemberTypeNodes(union, String(memberType)), ); @@ -492,7 +489,7 @@ function validateEnumValues( if (enumValues.length === 0) { context.reportError( - `Enum type ${enumType.name} must define one or more values.`, + `Enum type ${enumType} must define one or more values.`, [enumType.astNode, ...enumType.extensionASTNodes], ); } @@ -504,7 +501,7 @@ function validateEnumValues( validateName(context, enumValue); if (valueName === 'true' || valueName === 'false' || valueName === 'null') { context.reportError( - `Enum type ${enumType.name} cannot include value: ${valueName}.`, + `Enum type ${enumType} cannot include value: ${valueName}.`, enumValue.astNode, ); } @@ -519,7 +516,7 @@ function validateInputFields( if (fields.length === 0) { context.reportError( - `Input Object type ${inputObj.name} must define one or more fields.`, + `Input Object type ${inputObj} must define one or more fields.`, [inputObj.astNode, ...inputObj.extensionASTNodes], ); } @@ -532,7 +529,7 @@ function validateInputFields( // Ensure the type is an input type if (!isInputType(field.type)) { context.reportError( - `The type of ${inputObj.name}.${field.name} must be Input Type ` + + `The type of ${field} must be Input Type ` + `but got: ${inspect(field.type)}.`, field.astNode?.type, ); @@ -540,7 +537,7 @@ function validateInputFields( if (isRequiredInputField(field) && field.deprecationReason != null) { context.reportError( - `Required input field ${inputObj.name}.${field.name} cannot be deprecated.`, + `Required input field ${field} cannot be deprecated.`, [ getDeprecatedDirectiveNode(field.astNode), // istanbul ignore next (TODO need to write coverage tests) @@ -591,7 +588,7 @@ function createInputObjectCircularRefsValidator( const cyclePath = fieldPath.slice(cycleIndex); const pathStr = cyclePath.map((fieldObj) => fieldObj.name).join('.'); context.reportError( - `Cannot reference Input Object "${fieldType.name}" within itself through a series of non-null fields: "${pathStr}".`, + `Cannot reference Input Object "${fieldType}" within itself through a series of non-null fields: "${pathStr}".`, cyclePath.map((fieldObj) => fieldObj.astNode), ); } diff --git a/src/utilities/__tests__/buildClientSchema-test.ts b/src/utilities/__tests__/buildClientSchema-test.ts index d65f69b54a..448ac2bcdb 100644 --- a/src/utilities/__tests__/buildClientSchema-test.ts +++ b/src/utilities/__tests__/buildClientSchema-test.ts @@ -381,6 +381,7 @@ describe('Type System: build schema from introspection', () => { // rather than using the integers defined in the "server" schema. expect(clientFoodEnum.getValues()).to.deep.equal([ { + coordinate: 'Food.VEGETABLES', name: 'VEGETABLES', description: 'Foods that are vegetables.', value: 'VEGETABLES', @@ -389,6 +390,7 @@ describe('Type System: build schema from introspection', () => { astNode: undefined, }, { + coordinate: 'Food.FRUITS', name: 'FRUITS', description: null, value: 'FRUITS', @@ -397,6 +399,7 @@ describe('Type System: build schema from introspection', () => { astNode: undefined, }, { + coordinate: 'Food.OILS', name: 'OILS', description: null, value: 'OILS', diff --git a/src/utilities/__tests__/findBreakingChanges-test.ts b/src/utilities/__tests__/findBreakingChanges-test.ts index a4ab722084..9a36089863 100644 --- a/src/utilities/__tests__/findBreakingChanges-test.ts +++ b/src/utilities/__tests__/findBreakingChanges-test.ts @@ -56,7 +56,7 @@ describe('findBreakingChanges', () => { }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Query.foo changed type from Float to String.', + description: 'Field Query.foo changed type from Float to String.', }, ]); expect(findBreakingChanges(oldSchema, oldSchema)).to.deep.equal([]); @@ -148,55 +148,56 @@ describe('findBreakingChanges', () => { expect(changes).to.deep.equal([ { type: BreakingChangeType.FIELD_REMOVED, - description: 'Type1.field2 was removed.', + description: 'Field Type1.field2 was removed.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field3 changed type from String to Boolean.', + description: 'Field Type1.field3 changed type from String to Boolean.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field4 changed type from TypeA to TypeB.', + description: 'Field Type1.field4 changed type from TypeA to TypeB.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field6 changed type from String to [String].', + description: 'Field Type1.field6 changed type from String to [String].', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field7 changed type from [String] to String.', + description: 'Field Type1.field7 changed type from [String] to String.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field9 changed type from Int! to Int.', + description: 'Field Type1.field9 changed type from Int! to Int.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field10 changed type from [Int]! to [Int].', + description: 'Field Type1.field10 changed type from [Int]! to [Int].', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field11 changed type from Int to [Int]!.', + description: 'Field Type1.field11 changed type from Int to [Int]!.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field13 changed type from [Int!] to [Int].', + description: 'Field Type1.field13 changed type from [Int!] to [Int].', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field14 changed type from [Int] to [[Int]].', + description: 'Field Type1.field14 changed type from [Int] to [[Int]].', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field15 changed type from [[Int]] to [Int].', + description: 'Field Type1.field15 changed type from [[Int]] to [Int].', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field16 changed type from Int! to [Int]!.', + description: 'Field Type1.field16 changed type from Int! to [Int]!.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'Type1.field18 changed type from [[Int!]!] to [[Int!]].', + description: + 'Field Type1.field18 changed type from [[Int!]!] to [[Int!]].', }, ]); }); @@ -309,8 +310,7 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.REQUIRED_INPUT_FIELD_ADDED, - description: - 'A required field requiredField on input type InputType1 was added.', + description: 'A required field InputType1.requiredField was added.', }, ]); }); @@ -359,7 +359,7 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.VALUE_REMOVED_FROM_ENUM, - description: 'VALUE1 was removed from enum type EnumType1.', + description: 'Enum value EnumType1.VALUE1 was removed.', }, ]); }); @@ -388,15 +388,15 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.ARG_REMOVED, - description: 'Interface1.field1 arg arg1 was removed.', + description: 'Argument Interface1.field1(arg1:) was removed.', }, { type: BreakingChangeType.ARG_REMOVED, - description: 'Interface1.field1 arg objectArg was removed.', + description: 'Argument Interface1.field1(objectArg:) was removed.', }, { type: BreakingChangeType.ARG_REMOVED, - description: 'Type1.field1 arg name was removed.', + description: 'Argument Type1.field1(name:) was removed.', }, ]); }); @@ -450,62 +450,62 @@ describe('findBreakingChanges', () => { { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg1 has changed type from String to Int.', + 'Argument Type1.field1(arg1:) has changed type from String to Int.', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg2 has changed type from String to [String].', + 'Argument Type1.field1(arg2:) has changed type from String to [String].', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg3 has changed type from [String] to String.', + 'Argument Type1.field1(arg3:) has changed type from [String] to String.', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg4 has changed type from String to String!.', + 'Argument Type1.field1(arg4:) has changed type from String to String!.', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg5 has changed type from String! to Int.', + 'Argument Type1.field1(arg5:) has changed type from String! to Int.', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg6 has changed type from String! to Int!.', + 'Argument Type1.field1(arg6:) has changed type from String! to Int!.', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg8 has changed type from Int to [Int]!.', + 'Argument Type1.field1(arg8:) has changed type from Int to [Int]!.', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg9 has changed type from [Int] to [Int!].', + 'Argument Type1.field1(arg9:) has changed type from [Int] to [Int!].', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg11 has changed type from [Int] to [[Int]].', + 'Argument Type1.field1(arg11:) has changed type from [Int] to [[Int]].', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg12 has changed type from [[Int]] to [Int].', + 'Argument Type1.field1(arg12:) has changed type from [[Int]] to [Int].', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg13 has changed type from Int! to [Int]!.', + 'Argument Type1.field1(arg13:) has changed type from Int! to [Int]!.', }, { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'Type1.field1 arg arg15 has changed type from [[Int]!] to [[Int!]!].', + 'Argument Type1.field1(arg15:) has changed type from [[Int]!] to [[Int!]!].', }, ]); }); @@ -531,7 +531,8 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.REQUIRED_ARG_ADDED, - description: 'A required arg newRequiredArg on Type1.field1 was added.', + description: + 'A required argument Type1.field1(newRequiredArg:) was added.', }, ]); }); @@ -720,12 +721,11 @@ describe('findBreakingChanges', () => { { type: BreakingChangeType.ARG_CHANGED_KIND, description: - 'ArgThatChanges.field1 arg id has changed type from Float to String.', + 'Argument ArgThatChanges.field1(id:) has changed type from Float to String.', }, { type: BreakingChangeType.VALUE_REMOVED_FROM_ENUM, - description: - 'VALUE0 was removed from enum type EnumTypeThatLosesAValue.', + description: 'Enum value EnumTypeThatLosesAValue.VALUE0 was removed.', }, { type: BreakingChangeType.IMPLEMENTED_INTERFACE_REMOVED, @@ -744,34 +744,35 @@ describe('findBreakingChanges', () => { }, { type: BreakingChangeType.FIELD_REMOVED, - description: 'TypeThatHasBreakingFieldChanges.field1 was removed.', + description: + 'Field TypeThatHasBreakingFieldChanges.field1 was removed.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, description: - 'TypeThatHasBreakingFieldChanges.field2 changed type from String to Boolean.', + 'Field TypeThatHasBreakingFieldChanges.field2 changed type from String to Boolean.', }, { type: BreakingChangeType.DIRECTIVE_REMOVED, - description: 'DirectiveThatIsRemoved was removed.', + description: 'Directive @DirectiveThatIsRemoved was removed.', }, { type: BreakingChangeType.DIRECTIVE_ARG_REMOVED, - description: 'arg1 was removed from DirectiveThatRemovesArg.', + description: 'Argument @DirectiveThatRemovesArg(arg1:) was removed.', }, { type: BreakingChangeType.REQUIRED_DIRECTIVE_ARG_ADDED, description: - 'A required arg arg1 on directive NonNullDirectiveAdded was added.', + 'A required argument @NonNullDirectiveAdded(arg1:) was added.', }, { type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED, description: - 'Repeatable flag was removed from DirectiveThatWasRepeatable.', + 'Repeatable flag was removed from @DirectiveThatWasRepeatable.', }, { type: BreakingChangeType.DIRECTIVE_LOCATION_REMOVED, - description: 'QUERY was removed from DirectiveName.', + description: 'QUERY was removed from @DirectiveName.', }, ]); }); @@ -789,7 +790,7 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.DIRECTIVE_REMOVED, - description: 'DirectiveThatIsRemoved was removed.', + description: 'Directive @DirectiveThatIsRemoved was removed.', }, ]); }); @@ -808,7 +809,7 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.DIRECTIVE_REMOVED, - description: `${GraphQLDeprecatedDirective.name} was removed.`, + description: `Directive ${GraphQLDeprecatedDirective} was removed.`, }, ]); }); @@ -825,7 +826,7 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.DIRECTIVE_ARG_REMOVED, - description: 'arg1 was removed from DirectiveWithArg.', + description: 'Argument @DirectiveWithArg(arg1:) was removed.', }, ]); }); @@ -847,7 +848,7 @@ describe('findBreakingChanges', () => { { type: BreakingChangeType.REQUIRED_DIRECTIVE_ARG_ADDED, description: - 'A required arg newRequiredArg on directive DirectiveName was added.', + 'A required argument @DirectiveName(newRequiredArg:) was added.', }, ]); }); @@ -864,7 +865,7 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED, - description: 'Repeatable flag was removed from DirectiveName.', + description: 'Repeatable flag was removed from @DirectiveName.', }, ]); }); @@ -881,7 +882,7 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.DIRECTIVE_LOCATION_REMOVED, - description: 'QUERY was removed from DirectiveName.', + description: 'QUERY was removed from @DirectiveName.', }, ]); }); @@ -941,27 +942,27 @@ describe('findDangerousChanges', () => { { type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, description: - 'Type1.field1 arg withDefaultValue defaultValue was removed.', + 'Type1.field1(withDefaultValue:) defaultValue was removed.', }, { type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, description: - 'Type1.field1 arg stringArg has changed defaultValue from "test" to "Test".', + 'Type1.field1(stringArg:) has changed defaultValue from "test" to "Test".', }, { type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, description: - 'Type1.field1 arg emptyArray has changed defaultValue from [] to [7].', + 'Type1.field1(emptyArray:) has changed defaultValue from [] to [7].', }, { type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, description: - 'Type1.field1 arg valueArray has changed defaultValue from [["a", "b"], ["c"]] to [["b", "a"], ["d"]].', + 'Type1.field1(valueArray:) has changed defaultValue from [["a", "b"], ["c"]] to [["b", "a"], ["d"]].', }, { type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, description: - 'Type1.field1 arg complexObject has changed defaultValue from {innerInputArray: [{arrayField: [1, 2, 3]}]} to {innerInputArray: [{arrayField: [3, 2, 1]}]}.', + 'Type1.field1(complexObject:) has changed defaultValue from {innerInputArray: [{arrayField: [1, 2, 3]}]} to {innerInputArray: [{arrayField: [3, 2, 1]}]}.', }, ]); }); @@ -1049,7 +1050,7 @@ describe('findDangerousChanges', () => { expect(findDangerousChanges(oldSchema, newSchema)).to.deep.equal([ { type: DangerousChangeType.VALUE_ADDED_TO_ENUM, - description: 'VALUE2 was added to enum type EnumType1.', + description: 'Enum value EnumType1.VALUE2 was added.', }, ]); }); @@ -1141,8 +1142,7 @@ describe('findDangerousChanges', () => { expect(findDangerousChanges(oldSchema, newSchema)).to.deep.equal([ { type: DangerousChangeType.OPTIONAL_INPUT_FIELD_ADDED, - description: - 'An optional field field2 on input type InputType1 was added.', + description: 'An optional field InputType1.field2 was added.', }, ]); }); @@ -1187,12 +1187,12 @@ describe('findDangerousChanges', () => { expect(findDangerousChanges(oldSchema, newSchema)).to.deep.equal([ { type: DangerousChangeType.VALUE_ADDED_TO_ENUM, - description: 'VALUE2 was added to enum type EnumType1.', + description: 'Enum value EnumType1.VALUE2 was added.', }, { type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, description: - 'Type1.field1 arg argThatChangesDefaultValue has changed defaultValue from "test" to "Test".', + 'Type1.field1(argThatChangesDefaultValue:) has changed defaultValue from "test" to "Test".', }, { type: DangerousChangeType.IMPLEMENTED_INTERFACE_ADDED, @@ -1223,7 +1223,7 @@ describe('findDangerousChanges', () => { expect(findDangerousChanges(oldSchema, newSchema)).to.deep.equal([ { type: DangerousChangeType.OPTIONAL_ARG_ADDED, - description: 'An optional arg arg2 on Type1.field1 was added.', + description: 'An optional argument Type1.field1(arg2:) was added.', }, ]); }); diff --git a/src/utilities/findBreakingChanges.ts b/src/utilities/findBreakingChanges.ts index 0da4cceaa7..e79fd8043a 100644 --- a/src/utilities/findBreakingChanges.ts +++ b/src/utilities/findBreakingChanges.ts @@ -125,7 +125,7 @@ function findDirectiveChanges( for (const oldDirective of directivesDiff.removed) { schemaChanges.push({ type: BreakingChangeType.DIRECTIVE_REMOVED, - description: `${oldDirective.name} was removed.`, + description: `Directive ${oldDirective} was removed.`, }); } @@ -136,7 +136,7 @@ function findDirectiveChanges( if (isRequiredArgument(newArg)) { schemaChanges.push({ type: BreakingChangeType.REQUIRED_DIRECTIVE_ARG_ADDED, - description: `A required arg ${newArg.name} on directive ${oldDirective.name} was added.`, + description: `A required argument ${newArg} was added.`, }); } } @@ -144,14 +144,14 @@ function findDirectiveChanges( for (const oldArg of argsDiff.removed) { schemaChanges.push({ type: BreakingChangeType.DIRECTIVE_ARG_REMOVED, - description: `${oldArg.name} was removed from ${oldDirective.name}.`, + description: `Argument ${oldArg} was removed.`, }); } if (oldDirective.isRepeatable && !newDirective.isRepeatable) { schemaChanges.push({ type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED, - description: `Repeatable flag was removed from ${oldDirective.name}.`, + description: `Repeatable flag was removed from ${oldDirective}.`, }); } @@ -159,7 +159,7 @@ function findDirectiveChanges( if (!newDirective.locations.includes(location)) { schemaChanges.push({ type: BreakingChangeType.DIRECTIVE_LOCATION_REMOVED, - description: `${location} was removed from ${oldDirective.name}.`, + description: `${location} was removed from ${oldDirective}.`, }); } } @@ -183,8 +183,8 @@ function findTypeChanges( schemaChanges.push({ type: BreakingChangeType.TYPE_REMOVED, description: isSpecifiedScalarType(oldType) - ? `Standard scalar ${oldType.name} was removed because it is not referenced anymore.` - : `${oldType.name} was removed.`, + ? `Standard scalar ${oldType} was removed because it is not referenced anymore.` + : `${oldType} was removed.`, }); } @@ -208,9 +208,9 @@ function findTypeChanges( } else if (oldType.constructor !== newType.constructor) { schemaChanges.push({ type: BreakingChangeType.TYPE_CHANGED_KIND, - description: - `${oldType.name} changed from ` + - `${typeKindName(oldType)} to ${typeKindName(newType)}.`, + description: `${oldType} changed from ${typeKindName( + oldType, + )} to ${typeKindName(newType)}.`, }); } } @@ -232,12 +232,12 @@ function findInputObjectTypeChanges( if (isRequiredInputField(newField)) { schemaChanges.push({ type: BreakingChangeType.REQUIRED_INPUT_FIELD_ADDED, - description: `A required field ${newField.name} on input type ${oldType.name} was added.`, + description: `A required field ${newField} was added.`, }); } else { schemaChanges.push({ type: DangerousChangeType.OPTIONAL_INPUT_FIELD_ADDED, - description: `An optional field ${newField.name} on input type ${oldType.name} was added.`, + description: `An optional field ${newField} was added.`, }); } } @@ -245,7 +245,7 @@ function findInputObjectTypeChanges( for (const oldField of fieldsDiff.removed) { schemaChanges.push({ type: BreakingChangeType.FIELD_REMOVED, - description: `${oldType.name}.${oldField.name} was removed.`, + description: `${oldField} was removed.`, }); } @@ -257,9 +257,7 @@ function findInputObjectTypeChanges( if (!isSafe) { schemaChanges.push({ type: BreakingChangeType.FIELD_CHANGED_KIND, - description: - `${oldType.name}.${oldField.name} changed type from ` + - `${String(oldField.type)} to ${String(newField.type)}.`, + description: `${newField} changed type from ${oldField.type} to ${newField.type}.`, }); } } @@ -277,14 +275,14 @@ function findUnionTypeChanges( for (const newPossibleType of possibleTypesDiff.added) { schemaChanges.push({ type: DangerousChangeType.TYPE_ADDED_TO_UNION, - description: `${newPossibleType.name} was added to union type ${oldType.name}.`, + description: `${newPossibleType} was added to union type ${oldType}.`, }); } for (const oldPossibleType of possibleTypesDiff.removed) { schemaChanges.push({ type: BreakingChangeType.TYPE_REMOVED_FROM_UNION, - description: `${oldPossibleType.name} was removed from union type ${oldType.name}.`, + description: `${oldPossibleType} was removed from union type ${oldType}.`, }); } @@ -301,14 +299,14 @@ function findEnumTypeChanges( for (const newValue of valuesDiff.added) { schemaChanges.push({ type: DangerousChangeType.VALUE_ADDED_TO_ENUM, - description: `${newValue.name} was added to enum type ${oldType.name}.`, + description: `Enum value ${newValue} was added.`, }); } for (const oldValue of valuesDiff.removed) { schemaChanges.push({ type: BreakingChangeType.VALUE_REMOVED_FROM_ENUM, - description: `${oldValue.name} was removed from enum type ${oldType.name}.`, + description: `Enum value ${oldValue} was removed.`, }); } @@ -325,14 +323,14 @@ function findImplementedInterfacesChanges( for (const newInterface of interfacesDiff.added) { schemaChanges.push({ type: DangerousChangeType.IMPLEMENTED_INTERFACE_ADDED, - description: `${newInterface.name} added to interfaces implemented by ${oldType.name}.`, + description: `${newInterface} added to interfaces implemented by ${oldType}.`, }); } for (const oldInterface of interfacesDiff.removed) { schemaChanges.push({ type: BreakingChangeType.IMPLEMENTED_INTERFACE_REMOVED, - description: `${oldType.name} no longer implements interface ${oldInterface.name}.`, + description: `${oldType} no longer implements interface ${oldInterface}.`, }); } @@ -352,12 +350,12 @@ function findFieldChanges( for (const oldField of fieldsDiff.removed) { schemaChanges.push({ type: BreakingChangeType.FIELD_REMOVED, - description: `${oldType.name}.${oldField.name} was removed.`, + description: `Field ${oldField} was removed.`, }); } for (const [oldField, newField] of fieldsDiff.persisted) { - schemaChanges.push(...findArgChanges(oldType, oldField, newField)); + schemaChanges.push(...findArgChanges(oldField, newField)); const isSafe = isChangeSafeForObjectOrInterfaceField( oldField.type, @@ -366,9 +364,7 @@ function findFieldChanges( if (!isSafe) { schemaChanges.push({ type: BreakingChangeType.FIELD_CHANGED_KIND, - description: - `${oldType.name}.${oldField.name} changed type from ` + - `${String(oldField.type)} to ${String(newField.type)}.`, + description: `Field ${newField} changed type from ${oldField.type} to ${newField.type}.`, }); } } @@ -377,7 +373,6 @@ function findFieldChanges( } function findArgChanges( - oldType: GraphQLObjectType | GraphQLInterfaceType, oldField: GraphQLField, newField: GraphQLField, ): Array { @@ -387,7 +382,7 @@ function findArgChanges( for (const oldArg of argsDiff.removed) { schemaChanges.push({ type: BreakingChangeType.ARG_REMOVED, - description: `${oldType.name}.${oldField.name} arg ${oldArg.name} was removed.`, + description: `Argument ${oldArg} was removed.`, }); } @@ -399,15 +394,13 @@ function findArgChanges( if (!isSafe) { schemaChanges.push({ type: BreakingChangeType.ARG_CHANGED_KIND, - description: - `${oldType.name}.${oldField.name} arg ${oldArg.name} has changed type from ` + - `${String(oldArg.type)} to ${String(newArg.type)}.`, + description: `Argument ${newArg} has changed type from ${oldArg.type} to ${newArg.type}.`, }); } else if (oldArg.defaultValue !== undefined) { if (newArg.defaultValue === undefined) { schemaChanges.push({ type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, - description: `${oldType.name}.${oldField.name} arg ${oldArg.name} defaultValue was removed.`, + description: `${oldArg} defaultValue was removed.`, }); } else { // Since we looking only for client's observable changes we should @@ -419,7 +412,7 @@ function findArgChanges( if (oldValueStr !== newValueStr) { schemaChanges.push({ type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, - description: `${oldType.name}.${oldField.name} arg ${oldArg.name} has changed defaultValue from ${oldValueStr} to ${newValueStr}.`, + description: `${oldArg} has changed defaultValue from ${oldValueStr} to ${newValueStr}.`, }); } } @@ -430,12 +423,12 @@ function findArgChanges( if (isRequiredArgument(newArg)) { schemaChanges.push({ type: BreakingChangeType.REQUIRED_ARG_ADDED, - description: `A required arg ${newArg.name} on ${oldType.name}.${oldField.name} was added.`, + description: `A required argument ${newArg} was added.`, }); } else { schemaChanges.push({ type: DangerousChangeType.OPTIONAL_ARG_ADDED, - description: `An optional arg ${newArg.name} on ${oldType.name}.${oldField.name} was added.`, + description: `An optional argument ${newArg} was added.`, }); } } diff --git a/src/validation/__tests__/NoDeprecatedCustomRule-test.ts b/src/validation/__tests__/NoDeprecatedCustomRule-test.ts index 12d66eafc2..a1483e67be 100644 --- a/src/validation/__tests__/NoDeprecatedCustomRule-test.ts +++ b/src/validation/__tests__/NoDeprecatedCustomRule-test.ts @@ -106,7 +106,7 @@ describe('Validate: no deprecated', () => { `).to.deep.equal([ { message: - 'Field "Query.someField" argument "deprecatedArg" is deprecated. Some arg reason.', + 'The argument Query.someField(deprecatedArg:) is deprecated. Some arg reason.', locations: [{ line: 3, column: 21 }], }, ]); @@ -150,7 +150,7 @@ describe('Validate: no deprecated', () => { `).to.deep.equal([ { message: - 'Directive "@someDirective" argument "deprecatedArg" is deprecated. Some arg reason.', + 'The argument @someDirective(deprecatedArg:) is deprecated. Some arg reason.', locations: [{ line: 3, column: 36 }], }, ]); @@ -255,7 +255,7 @@ describe('Validate: no deprecated', () => { it('reports error when a deprecated enum value is used', () => { const message = - 'The enum value "EnumType.DEPRECATED_VALUE" is deprecated. Some enum reason.'; + 'The enum value EnumType.DEPRECATED_VALUE is deprecated. Some enum reason.'; expectErrors(` query ( diff --git a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts index 7976f46bd2..3ca5d19837 100644 --- a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts +++ b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts @@ -165,7 +165,7 @@ describe('Validate: Provided required arguments', () => { `).to.deep.equal([ { message: - 'Field "multipleReqs" argument "req1" of type "Int!" is required, but it was not provided.', + 'Argument ComplicatedArgs.multipleReqs(req1:) of type "Int!" is required, but it was not provided.', locations: [{ line: 4, column: 13 }], }, ]); @@ -181,12 +181,12 @@ describe('Validate: Provided required arguments', () => { `).to.deep.equal([ { message: - 'Field "multipleReqs" argument "req1" of type "Int!" is required, but it was not provided.', + 'Argument ComplicatedArgs.multipleReqs(req1:) of type "Int!" is required, but it was not provided.', locations: [{ line: 4, column: 13 }], }, { message: - 'Field "multipleReqs" argument "req2" of type "Int!" is required, but it was not provided.', + 'Argument ComplicatedArgs.multipleReqs(req2:) of type "Int!" is required, but it was not provided.', locations: [{ line: 4, column: 13 }], }, ]); @@ -202,7 +202,7 @@ describe('Validate: Provided required arguments', () => { `).to.deep.equal([ { message: - 'Field "multipleReqs" argument "req2" of type "Int!" is required, but it was not provided.', + 'Argument ComplicatedArgs.multipleReqs(req2:) of type "Int!" is required, but it was not provided.', locations: [{ line: 4, column: 13 }], }, ]); @@ -241,12 +241,12 @@ describe('Validate: Provided required arguments', () => { `).to.deep.equal([ { message: - 'Directive "@include" argument "if" of type "Boolean!" is required, but it was not provided.', + 'Argument @include(if:) of type "Boolean!" is required, but it was not provided.', locations: [{ line: 3, column: 15 }], }, { message: - 'Directive "@skip" argument "if" of type "Boolean!" is required, but it was not provided.', + 'Argument @skip(if:) of type "Boolean!" is required, but it was not provided.', locations: [{ line: 4, column: 18 }], }, ]); @@ -274,7 +274,7 @@ describe('Validate: Provided required arguments', () => { `).to.deep.equal([ { message: - 'Directive "@test" argument "arg" of type "String!" is required, but it was not provided.', + 'Argument @test(arg:) of type "String!" is required, but it was not provided.', locations: [{ line: 3, column: 23 }], }, ]); @@ -288,7 +288,7 @@ describe('Validate: Provided required arguments', () => { `).to.deep.equal([ { message: - 'Directive "@include" argument "if" of type "Boolean!" is required, but it was not provided.', + 'Argument @include(if:) of type "Boolean!" is required, but it was not provided.', locations: [{ line: 3, column: 23 }], }, ]); @@ -303,7 +303,7 @@ describe('Validate: Provided required arguments', () => { `).to.deep.equal([ { message: - 'Directive "@deprecated" argument "reason" of type "String!" is required, but it was not provided.', + 'Argument @deprecated(reason:) of type "String!" is required, but it was not provided.', locations: [{ line: 3, column: 23 }], }, ]); @@ -325,7 +325,7 @@ describe('Validate: Provided required arguments', () => { ).to.deep.equal([ { message: - 'Directive "@test" argument "arg" of type "String!" is required, but it was not provided.', + 'Argument @test(arg:) of type "String!" is required, but it was not provided.', locations: [{ line: 4, column: 30 }], }, ]); @@ -347,7 +347,7 @@ describe('Validate: Provided required arguments', () => { ).to.deep.equal([ { message: - 'Directive "@test" argument "arg" of type "String!" is required, but it was not provided.', + 'Argument @test(arg:) of type "String!" is required, but it was not provided.', locations: [{ line: 2, column: 29 }], }, ]); diff --git a/src/validation/rules/FieldsOnCorrectTypeRule.ts b/src/validation/rules/FieldsOnCorrectTypeRule.ts index 1b2e066042..0507ee2961 100644 --- a/src/validation/rules/FieldsOnCorrectTypeRule.ts +++ b/src/validation/rules/FieldsOnCorrectTypeRule.ts @@ -54,7 +54,7 @@ export function FieldsOnCorrectTypeRule( // Report an error, including helpful suggestions. context.reportError( new GraphQLError( - `Cannot query field "${fieldName}" on type "${type.name}".` + + `Cannot query field "${fieldName}" on type "${type}".` + suggestion, node, ), diff --git a/src/validation/rules/KnownArgumentNamesRule.ts b/src/validation/rules/KnownArgumentNamesRule.ts index d769fe4966..a32bde09fd 100644 --- a/src/validation/rules/KnownArgumentNamesRule.ts +++ b/src/validation/rules/KnownArgumentNamesRule.ts @@ -26,15 +26,14 @@ export function KnownArgumentNamesRule(context: ValidationContext): ASTVisitor { Argument(argNode) { const argDef = context.getArgument(); const fieldDef = context.getFieldDef(); - const parentType = context.getParentType(); - if (!argDef && fieldDef && parentType) { + if (!argDef && fieldDef) { const argName = argNode.name.value; const knownArgsNames = fieldDef.args.map((arg) => arg.name); const suggestions = suggestionList(argName, knownArgsNames); context.reportError( new GraphQLError( - `Unknown argument "${argName}" on field "${parentType.name}.${fieldDef.name}".` + + `Unknown argument "${argName}" on field "${fieldDef}".` + didYouMean(suggestions), argNode, ), diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.ts b/src/validation/rules/ProvidedRequiredArgumentsRule.ts index e5c970d24f..d00a8ab21c 100644 --- a/src/validation/rules/ProvidedRequiredArgumentsRule.ts +++ b/src/validation/rules/ProvidedRequiredArgumentsRule.ts @@ -44,10 +44,9 @@ export function ProvidedRequiredArgumentsRule( ); for (const argDef of fieldDef.args) { if (!providedArgs.has(argDef.name) && isRequiredArgument(argDef)) { - const argTypeStr = inspect(argDef.type); context.reportError( new GraphQLError( - `Field "${fieldDef.name}" argument "${argDef.name}" of type "${argTypeStr}" is required, but it was not provided.`, + `Argument ${argDef} of type "${argDef.type}" is required, but it was not provided.`, fieldNode, ), ); @@ -107,7 +106,7 @@ export function ProvidedRequiredArgumentsOnDirectivesRule( : print(argDef.type); context.reportError( new GraphQLError( - `Directive "@${directiveName}" argument "${argName}" of type "${argType}" is required, but it was not provided.`, + `Argument @${directiveName}(${argName}:) of type "${argType}" is required, but it was not provided.`, directiveNode, ), ); diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index 6d5dc5c1ca..23df08f430 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -1,5 +1,4 @@ import { keyMap } from '../../jsutils/keyMap'; -import { inspect } from '../../jsutils/inspect'; import { didYouMean } from '../../jsutils/didYouMean'; import { suggestionList } from '../../jsutils/suggestionList'; @@ -51,10 +50,9 @@ export function ValuesOfCorrectTypeRule( for (const fieldDef of Object.values(type.getFields())) { const fieldNode = fieldNodeMap[fieldDef.name]; if (!fieldNode && isRequiredInputField(fieldDef)) { - const typeStr = inspect(fieldDef.type); context.reportError( new GraphQLError( - `Field "${type.name}.${fieldDef.name}" of required type "${typeStr}" was not provided.`, + `Field "${fieldDef}" of required type "${fieldDef.type}" was not provided.`, node, ), ); @@ -71,7 +69,7 @@ export function ValuesOfCorrectTypeRule( ); context.reportError( new GraphQLError( - `Field "${node.name.value}" is not defined by type "${parentType.name}".` + + `Field "${node.name.value}" is not defined by type "${parentType}".` + didYouMean(suggestions), node, ), @@ -83,7 +81,7 @@ export function ValuesOfCorrectTypeRule( if (isNonNullType(type)) { context.reportError( new GraphQLError( - `Expected value of type "${inspect(type)}", found ${print(node)}.`, + `Expected value of type "${type}", found ${print(node)}.`, node, ), ); @@ -111,10 +109,9 @@ function isValidValueNode(context: ValidationContext, node: ValueNode): void { const type = getNamedType(locationType); if (!isLeafType(type)) { - const typeStr = inspect(locationType); context.reportError( new GraphQLError( - `Expected value of type "${typeStr}", found ${print(node)}.`, + `Expected value of type "${locationType}", found ${print(node)}.`, node, ), ); @@ -126,22 +123,20 @@ function isValidValueNode(context: ValidationContext, node: ValueNode): void { try { const parseResult = type.parseLiteral(node, undefined /* variables */); if (parseResult === undefined) { - const typeStr = inspect(locationType); context.reportError( new GraphQLError( - `Expected value of type "${typeStr}", found ${print(node)}.`, + `Expected value of type "${locationType}", found ${print(node)}.`, node, ), ); } } catch (error) { - const typeStr = inspect(locationType); if (error instanceof GraphQLError) { context.reportError(error); } else { context.reportError( new GraphQLError( - `Expected value of type "${typeStr}", found ${print(node)}; ` + + `Expected value of type "${locationType}", found ${print(node)}; ` + error.message, node, undefined, diff --git a/src/validation/rules/VariablesInAllowedPositionRule.ts b/src/validation/rules/VariablesInAllowedPositionRule.ts index bf4c8f8f7c..9312f88548 100644 --- a/src/validation/rules/VariablesInAllowedPositionRule.ts +++ b/src/validation/rules/VariablesInAllowedPositionRule.ts @@ -1,4 +1,3 @@ -import { inspect } from '../../jsutils/inspect'; import type { Maybe } from '../../jsutils/Maybe'; import { GraphQLError } from '../../error/GraphQLError'; @@ -53,11 +52,9 @@ export function VariablesInAllowedPositionRule( defaultValue, ) ) { - const varTypeStr = inspect(varType); - const typeStr = inspect(type); context.reportError( new GraphQLError( - `Variable "$${varName}" of type "${varTypeStr}" used in position expecting type "${typeStr}".`, + `Variable "$${varName}" of type "${varType}" used in position expecting type "${type}".`, [varDef, node], ), ); diff --git a/src/validation/rules/custom/NoDeprecatedCustomRule.ts b/src/validation/rules/custom/NoDeprecatedCustomRule.ts index 38b688a203..1229db3d43 100644 --- a/src/validation/rules/custom/NoDeprecatedCustomRule.ts +++ b/src/validation/rules/custom/NoDeprecatedCustomRule.ts @@ -1,5 +1,3 @@ -import { invariant } from '../../../jsutils/invariant'; - import { GraphQLError } from '../../../error/GraphQLError'; import type { ASTVisitor } from '../../../language/visitor'; @@ -24,11 +22,9 @@ export function NoDeprecatedCustomRule(context: ValidationContext): ASTVisitor { const fieldDef = context.getFieldDef(); const deprecationReason = fieldDef?.deprecationReason; if (fieldDef && deprecationReason != null) { - const parentType = context.getParentType(); - invariant(parentType != null); context.reportError( new GraphQLError( - `The field ${parentType.name}.${fieldDef.name} is deprecated. ${deprecationReason}`, + `The field ${fieldDef} is deprecated. ${deprecationReason}`, node, ), ); @@ -38,25 +34,12 @@ export function NoDeprecatedCustomRule(context: ValidationContext): ASTVisitor { const argDef = context.getArgument(); const deprecationReason = argDef?.deprecationReason; if (argDef && deprecationReason != null) { - const directiveDef = context.getDirective(); - if (directiveDef != null) { - context.reportError( - new GraphQLError( - `Directive "@${directiveDef.name}" argument "${argDef.name}" is deprecated. ${deprecationReason}`, - node, - ), - ); - } else { - const parentType = context.getParentType(); - const fieldDef = context.getFieldDef(); - invariant(parentType != null && fieldDef != null); - context.reportError( - new GraphQLError( - `Field "${parentType.name}.${fieldDef.name}" argument "${argDef.name}" is deprecated. ${deprecationReason}`, - node, - ), - ); - } + context.reportError( + new GraphQLError( + `The argument ${argDef} is deprecated. ${deprecationReason}`, + node, + ), + ); } }, ObjectField(node) { @@ -67,7 +50,7 @@ export function NoDeprecatedCustomRule(context: ValidationContext): ASTVisitor { if (deprecationReason != null) { context.reportError( new GraphQLError( - `The input field ${inputObjectDef.name}.${inputFieldDef.name} is deprecated. ${deprecationReason}`, + `The input field ${inputFieldDef} is deprecated. ${deprecationReason}`, node, ), ); @@ -78,11 +61,9 @@ export function NoDeprecatedCustomRule(context: ValidationContext): ASTVisitor { const enumValueDef = context.getEnumValue(); const deprecationReason = enumValueDef?.deprecationReason; if (enumValueDef && deprecationReason != null) { - const enumTypeDef = getNamedType(context.getInputType()); - invariant(enumTypeDef != null); context.reportError( new GraphQLError( - `The enum value "${enumTypeDef.name}.${enumValueDef.name}" is deprecated. ${deprecationReason}`, + `The enum value ${enumValueDef} is deprecated. ${deprecationReason}`, node, ), );