From e1842594708272b8e550c651cbf4721486f36f61 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 23 Aug 2024 17:22:28 +0300 Subject: [PATCH 1/5] standardize error messages prior to introducing schema coordinates extracted from #3808 PR #3808 uses schema coordinates to improve GraphQL-JS error messages. To reduce the size of the PR, this commit standardizes error messages according to the general pattern that will be introduced with schema coordinates without introducing the coordinates themselves, in the hopes of aiding review of the later PR. Parenthetically, extracting these changes out of #3808 demonstrates how the use of schema coordinates improves our error messages. For example, it is difficult when printing an error message regarding an argument to easily look up the field of that argument, and, if it a nested input field, the full field path. It is similarly sometimes difficult to know the type of the field or argument triggering the error. Embedding schema coordinates within GraphQL-JS makes this trivial. This PR simply omits the changes from #3808 that are directly enabled by the use of schema coordinates. #3808 can then be rebased on top of this PR to demonstrate the code changes and the direct improvement in the various error messages. --- src/execution/__tests__/variables-test.ts | 13 +- src/execution/values.ts | 4 +- src/type/__tests__/introspection-test.ts | 2 +- src/type/__tests__/validation-test.ts | 4 +- src/type/validate.ts | 8 +- .../__tests__/coerceInputValue-test.ts | 3 +- .../__tests__/findBreakingChanges-test.ts | 149 +++++++++--------- src/utilities/coerceInputValue.ts | 2 +- src/utilities/findBreakingChanges.ts | 38 ++--- .../DeferStreamDirectiveLabelRule-test.ts | 11 +- .../__tests__/NoDeprecatedCustomRule-test.ts | 4 +- .../ProvidedRequiredArgumentsRule-test.ts | 22 +-- .../StreamDirectiveOnListFieldRule-test.ts | 2 +- .../__tests__/ValuesOfCorrectTypeRule-test.ts | 2 +- .../rules/DeferStreamDirectiveLabelRule.ts | 4 +- .../rules/ProvidedRequiredArgumentsRule.ts | 4 +- .../rules/StreamDirectiveOnListFieldRule.ts | 2 +- .../rules/ValuesOfCorrectTypeRule.ts | 2 +- .../rules/custom/NoDeprecatedCustomRule.ts | 4 +- 19 files changed, 149 insertions(+), 131 deletions(-) diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index 6e4b39e810..778ee6d594 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -226,7 +226,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Argument "input" has invalid value ["foo", "bar", "baz"].', + 'Argument "input" of type "TestInputObject" has invalid value ["foo", "bar", "baz"].', path: ['fieldWithObjectInput'], locations: [{ line: 3, column: 41 }], }, @@ -262,7 +262,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Argument "input" has invalid value { c: "foo", e: "bar" }.', + 'Argument "input" of type "TestInputObject" has invalid value { c: "foo", e: "bar" }.', path: ['fieldWithObjectInput'], locations: [{ line: 3, column: 41 }], }, @@ -462,7 +462,7 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" got invalid value { a: "foo", b: "bar" }; Field "c" of required type "String!" was not provided.', + 'Variable "$input" got invalid value { a: "foo", b: "bar" }; Field "TestInputObject.c" of required type "String!" was not provided.', locations: [{ line: 2, column: 16 }], }, ], @@ -481,12 +481,12 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" got invalid value { a: "foo" } at "input.na"; Field "c" of required type "String!" was not provided.', + 'Variable "$input" got invalid value { a: "foo" } at "input.na"; Field "TestInputObject.c" of required type "String!" was not provided.', locations: [{ line: 2, column: 18 }], }, { message: - 'Variable "$input" got invalid value { na: { a: "foo" } }; Field "nb" of required type "String!" was not provided.', + 'Variable "$input" got invalid value { na: { a: "foo" } }; Field "TestNestedInputObject.nb" of required type "String!" was not provided.', locations: [{ line: 2, column: 18 }], }, ], @@ -1042,7 +1042,8 @@ describe('Execute: Handles inputs', () => { }, errors: [ { - message: 'Argument "input" has invalid value WRONG_TYPE.', + message: + 'Argument "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 5511911c78..8f7a3d62bd 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -215,7 +215,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 "${name}" of type "${inspect( + argType, + )}" has invalid value ${print(valueNode)}.`, { nodes: valueNode }, ); } diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index e6e5d0943e..ddef977fbb 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -1644,7 +1644,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__/validation-test.ts b/src/type/__tests__/validation-test.ts index 34bd352e2c..3c2fe21500 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2089,7 +2089,7 @@ describe('Objects must adhere to Interface they implement', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { 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 }, @@ -2546,7 +2546,7 @@ describe('Interfaces must adhere to Interface they implement', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { 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/validate.ts b/src/type/validate.ts index d00cec790f..6399c43d94 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -433,7 +433,13 @@ function validateTypeImplementsInterface( 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}.`, + `Argument "${ + type.name + }.${fieldName}(${argName}:)" must not be required type "${inspect( + typeArg.type, + )}" if not provided by the Interface field "${ + iface.name + }.${fieldName}".`, [typeArg.astNode, ifaceField.astNode], ); } diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index 7712c91a39..b0b8306fa0 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -238,7 +238,8 @@ describe('coerceInputValue', () => { const result = coerceValue({ bar: 123 }, TestInputObject); expectErrors(result).to.deep.equal([ { - error: 'Field "foo" of required type "Int!" was not provided.', + error: + 'Field "TestInputObject.foo" of required type "Int!" was not provided.', path: [], value: { bar: 123 }, }, diff --git a/src/utilities/__tests__/findBreakingChanges-test.ts b/src/utilities/__tests__/findBreakingChanges-test.ts index 5d92522fe3..8d7a3f8a56 100644 --- a/src/utilities/__tests__/findBreakingChanges-test.ts +++ b/src/utilities/__tests__/findBreakingChanges-test.ts @@ -57,7 +57,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([]); @@ -149,55 +149,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!]].', }, ]); }); @@ -245,48 +246,55 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.FIELD_REMOVED, - description: 'InputType1.field2 was removed.', + description: 'Field InputType1.field2 was removed.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'InputType1.field1 changed type from String to Int.', + description: 'Field InputType1.field1 changed type from String to Int.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'InputType1.field3 changed type from [String] to String.', + description: + 'Field InputType1.field3 changed type from [String] to String.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'InputType1.field5 changed type from String to String!.', + description: + 'Field InputType1.field5 changed type from String to String!.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'InputType1.field6 changed type from [Int] to [Int]!.', + description: + 'Field InputType1.field6 changed type from [Int] to [Int]!.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'InputType1.field8 changed type from Int to [Int]!.', + description: 'Field InputType1.field8 changed type from Int to [Int]!.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'InputType1.field9 changed type from [Int] to [Int!].', + description: + 'Field InputType1.field9 changed type from [Int] to [Int!].', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'InputType1.field11 changed type from [Int] to [[Int]].', + description: + 'Field InputType1.field11 changed type from [Int] to [[Int]].', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'InputType1.field12 changed type from [[Int]] to [Int].', + description: + 'Field InputType1.field12 changed type from [[Int]] to [Int].', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, - description: 'InputType1.field13 changed type from Int! to [Int]!.', + description: + 'Field InputType1.field13 changed type from Int! to [Int]!.', }, { type: BreakingChangeType.FIELD_CHANGED_KIND, description: - 'InputType1.field15 changed type from [[Int]!] to [[Int!]!].', + 'Field InputType1.field15 changed type from [[Int]!] to [[Int!]!].', }, ]); }); @@ -310,8 +318,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.', }, ]); }); @@ -360,7 +367,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.', }, ]); }); @@ -389,15 +396,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.', }, ]); }); @@ -451,62 +458,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!]!].', }, ]); }); @@ -532,7 +539,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.', }, ]); }); @@ -721,12 +729,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, @@ -745,34 +752,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.', }, ]); }); @@ -790,7 +798,7 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.DIRECTIVE_REMOVED, - description: 'DirectiveThatIsRemoved was removed.', + description: 'Directive @DirectiveThatIsRemoved was removed.', }, ]); }); @@ -810,7 +818,7 @@ describe('findBreakingChanges', () => { expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { type: BreakingChangeType.DIRECTIVE_REMOVED, - description: `${GraphQLDeprecatedDirective.name} was removed.`, + description: `Directive ${GraphQLDeprecatedDirective} was removed.`, }, ]); }); @@ -827,7 +835,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.', }, ]); }); @@ -849,7 +857,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.', }, ]); }); @@ -866,7 +874,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.', }, ]); }); @@ -883,7 +891,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.', }, ]); }); @@ -943,27 +951,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] }] }.', }, ]); }); @@ -1051,7 +1059,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.', }, ]); }); @@ -1143,8 +1151,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.', }, ]); }); @@ -1189,12 +1196,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, @@ -1225,7 +1232,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/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index 9aa49abed5..fe4b825b93 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -110,7 +110,7 @@ function coerceInputValueImpl( pathToArray(path), inputValue, new GraphQLError( - `Field "${field.name}" of required type "${typeStr}" was not provided.`, + `Field "${type.name}.${field.name}" of required type "${typeStr}" was not provided.`, ), ); } diff --git a/src/utilities/findBreakingChanges.ts b/src/utilities/findBreakingChanges.ts index 793d29bc83..4c5091ef97 100644 --- a/src/utilities/findBreakingChanges.ts +++ b/src/utilities/findBreakingChanges.ts @@ -124,7 +124,7 @@ function findDirectiveChanges( for (const oldDirective of directivesDiff.removed) { schemaChanges.push({ type: BreakingChangeType.DIRECTIVE_REMOVED, - description: `${oldDirective.name} was removed.`, + description: `Directive @${oldDirective.name} was removed.`, }); } @@ -135,7 +135,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 @${oldDirective.name}(${newArg.name}:) was added.`, }); } } @@ -143,14 +143,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 @${oldDirective.name}(${oldArg.name}:) 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.name}.`, }); } @@ -158,7 +158,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.name}.`, }); } } @@ -231,12 +231,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 ${oldType.name}.${newField.name} 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 ${oldType.name}.${newField.name} was added.`, }); } } @@ -244,7 +244,7 @@ function findInputObjectTypeChanges( for (const oldField of fieldsDiff.removed) { schemaChanges.push({ type: BreakingChangeType.FIELD_REMOVED, - description: `${oldType.name}.${oldField.name} was removed.`, + description: `Field ${oldType.name}.${oldField.name} was removed.`, }); } @@ -257,7 +257,7 @@ function findInputObjectTypeChanges( schemaChanges.push({ type: BreakingChangeType.FIELD_CHANGED_KIND, description: - `${oldType.name}.${oldField.name} changed type from ` + + `Field ${oldType.name}.${oldField.name} changed type from ` + `${String(oldField.type)} to ${String(newField.type)}.`, }); } @@ -300,14 +300,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 ${oldType.name}.${newValue.name} 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 ${oldType.name}.${oldValue.name} was removed.`, }); } @@ -351,7 +351,7 @@ function findFieldChanges( for (const oldField of fieldsDiff.removed) { schemaChanges.push({ type: BreakingChangeType.FIELD_REMOVED, - description: `${oldType.name}.${oldField.name} was removed.`, + description: `Field ${oldType.name}.${oldField.name} was removed.`, }); } @@ -366,7 +366,7 @@ function findFieldChanges( schemaChanges.push({ type: BreakingChangeType.FIELD_CHANGED_KIND, description: - `${oldType.name}.${oldField.name} changed type from ` + + `Field ${oldType.name}.${oldField.name} changed type from ` + `${String(oldField.type)} to ${String(newField.type)}.`, }); } @@ -386,7 +386,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 ${oldType.name}.${oldField.name}(${oldArg.name}:) was removed.`, }); } @@ -399,14 +399,14 @@ function findArgChanges( schemaChanges.push({ type: BreakingChangeType.ARG_CHANGED_KIND, description: - `${oldType.name}.${oldField.name} arg ${oldArg.name} has changed type from ` + + `Argument ${oldType.name}.${oldField.name}(${oldArg.name}:) has changed type from ` + `${String(oldArg.type)} to ${String(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: `${oldType.name}.${oldField.name}(${oldArg.name}:) defaultValue was removed.`, }); } else { // Since we looking only for client's observable changes we should @@ -418,7 +418,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: `${oldType.name}.${oldField.name}(${oldArg.name}:) has changed defaultValue from ${oldValueStr} to ${newValueStr}.`, }); } } @@ -429,12 +429,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 ${oldType.name}.${oldField.name}(${newArg.name}:) 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 ${oldType.name}.${oldField.name}(${newArg.name}:) was added.`, }); } } diff --git a/src/validation/__tests__/DeferStreamDirectiveLabelRule-test.ts b/src/validation/__tests__/DeferStreamDirectiveLabelRule-test.ts index 44709e00c5..4126c6e578 100644 --- a/src/validation/__tests__/DeferStreamDirectiveLabelRule-test.ts +++ b/src/validation/__tests__/DeferStreamDirectiveLabelRule-test.ts @@ -63,7 +63,7 @@ describe('Validate: Defer/Stream directive labels', () => { } `).toDeepEqual([ { - message: 'Directive "defer"\'s label argument must be a static string.', + message: 'Argument "@defer(label:)" must be a static string.', locations: [{ line: 4, column: 25 }], }, ]); @@ -101,7 +101,8 @@ describe('Validate: Defer/Stream directive labels', () => { } `).toDeepEqual([ { - message: 'Defer/Stream directive label argument must be unique.', + message: + 'Value for arguments "defer(label:)" and "stream(label:)" must be unique across all Defer/Stream directive usages.', locations: [ { line: 4, column: 25 }, { line: 5, column: 25 }, @@ -139,8 +140,7 @@ describe('Validate: Defer/Stream directive labels', () => { } `).toDeepEqual([ { - message: - 'Directive "stream"\'s label argument must be a static string.', + message: 'Argument "@stream(label:)" must be a static string.', locations: [{ line: 6, column: 39 }], }, ]); @@ -160,7 +160,8 @@ describe('Validate: Defer/Stream directive labels', () => { } `).toDeepEqual([ { - message: 'Defer/Stream directive label argument must be unique.', + message: + 'Value for arguments "defer(label:)" and "stream(label:)" must be unique across all Defer/Stream directive usages.', locations: [ { line: 4, column: 26 }, { line: 6, column: 39 }, diff --git a/src/validation/__tests__/NoDeprecatedCustomRule-test.ts b/src/validation/__tests__/NoDeprecatedCustomRule-test.ts index 96823684fc..90dfb0dbdc 100644 --- a/src/validation/__tests__/NoDeprecatedCustomRule-test.ts +++ b/src/validation/__tests__/NoDeprecatedCustomRule-test.ts @@ -106,7 +106,7 @@ describe('Validate: no deprecated', () => { `).toDeepEqual([ { 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', () => { `).toDeepEqual([ { message: - 'Directive "@someDirective" argument "deprecatedArg" is deprecated. Some arg reason.', + 'The argument "@someDirective(deprecatedArg:)" is deprecated. Some arg reason.', locations: [{ line: 3, column: 36 }], }, ]); diff --git a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts index 6f0d223c15..8605023f0a 100644 --- a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts +++ b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts @@ -168,7 +168,7 @@ describe('Validate: Provided required arguments', () => { `).toDeepEqual([ { message: - 'Field "multipleReqs" argument "req1" of type "Int!" is required, but it was not provided.', + 'Argument "multipleReqs(req1:)" of type "Int!" is required, but it was not provided.', locations: [{ line: 4, column: 13 }], }, ]); @@ -184,12 +184,12 @@ describe('Validate: Provided required arguments', () => { `).toDeepEqual([ { message: - 'Field "multipleReqs" argument "req1" of type "Int!" is required, but it was not provided.', + 'Argument "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 "multipleReqs(req2:)" of type "Int!" is required, but it was not provided.', locations: [{ line: 4, column: 13 }], }, ]); @@ -205,7 +205,7 @@ describe('Validate: Provided required arguments', () => { `).toDeepEqual([ { message: - 'Field "multipleReqs" argument "req2" of type "Int!" is required, but it was not provided.', + 'Argument "multipleReqs(req2:)" of type "Int!" is required, but it was not provided.', locations: [{ line: 4, column: 13 }], }, ]); @@ -244,12 +244,12 @@ describe('Validate: Provided required arguments', () => { `).toDeepEqual([ { 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 }], }, ]); @@ -277,7 +277,7 @@ describe('Validate: Provided required arguments', () => { `).toDeepEqual([ { 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 }], }, ]); @@ -291,7 +291,7 @@ describe('Validate: Provided required arguments', () => { `).toDeepEqual([ { 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 }], }, ]); @@ -306,7 +306,7 @@ describe('Validate: Provided required arguments', () => { `).toDeepEqual([ { 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 }], }, ]); @@ -328,7 +328,7 @@ describe('Validate: Provided required arguments', () => { ).toDeepEqual([ { 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 }], }, ]); @@ -350,7 +350,7 @@ describe('Validate: Provided required arguments', () => { ).toDeepEqual([ { 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/__tests__/StreamDirectiveOnListFieldRule-test.ts b/src/validation/__tests__/StreamDirectiveOnListFieldRule-test.ts index a35c31232d..ec49d42c29 100644 --- a/src/validation/__tests__/StreamDirectiveOnListFieldRule-test.ts +++ b/src/validation/__tests__/StreamDirectiveOnListFieldRule-test.ts @@ -71,7 +71,7 @@ describe('Validate: Stream directive on list field', () => { `).toDeepEqual([ { message: - 'Stream directive cannot be used on non-list field "name" on type "Human".', + 'Directive "@stream" cannot be used on non-list field "Human.name".', locations: [{ line: 3, column: 14 }], }, ]); diff --git a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts index c36ebb6992..cd71952f53 100644 --- a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts +++ b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts @@ -1104,7 +1104,7 @@ describe('Validate: Values of correct type', () => { `).toDeepEqual([ { message: - 'Variable "string" must be non-nullable to be used for OneOf Input Object "OneOfInput".', + 'Variable "$string" must be non-nullable to be used for OneOf Input Object "OneOfInput".', locations: [{ line: 4, column: 37 }], }, ]); diff --git a/src/validation/rules/DeferStreamDirectiveLabelRule.ts b/src/validation/rules/DeferStreamDirectiveLabelRule.ts index 45537af436..2b6d35a816 100644 --- a/src/validation/rules/DeferStreamDirectiveLabelRule.ts +++ b/src/validation/rules/DeferStreamDirectiveLabelRule.ts @@ -36,7 +36,7 @@ export function DeferStreamDirectiveLabelRule( if (labelValue.kind !== Kind.STRING) { context.reportError( new GraphQLError( - `Directive "${node.name.value}"'s label argument must be a static string.`, + `Argument "@${node.name.value}(label:)" must be a static string.`, { nodes: node }, ), ); @@ -47,7 +47,7 @@ export function DeferStreamDirectiveLabelRule( if (knownLabel != null) { context.reportError( new GraphQLError( - 'Defer/Stream directive label argument must be unique.', + 'Value for arguments "defer(label:)" and "stream(label:)" must be unique across all Defer/Stream directive usages.', { nodes: [knownLabel, node] }, ), ); diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.ts b/src/validation/rules/ProvidedRequiredArgumentsRule.ts index 72d104b852..d71fc4fcd9 100644 --- a/src/validation/rules/ProvidedRequiredArgumentsRule.ts +++ b/src/validation/rules/ProvidedRequiredArgumentsRule.ts @@ -46,7 +46,7 @@ export function ProvidedRequiredArgumentsRule( 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 "${fieldDef.name}(${argDef.name}:)" of type "${argTypeStr}" is required, but it was not provided.`, { nodes: fieldNode }, ), ); @@ -115,7 +115,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.`, { nodes: directiveNode }, ), ); diff --git a/src/validation/rules/StreamDirectiveOnListFieldRule.ts b/src/validation/rules/StreamDirectiveOnListFieldRule.ts index 34346e249a..16504d84d2 100644 --- a/src/validation/rules/StreamDirectiveOnListFieldRule.ts +++ b/src/validation/rules/StreamDirectiveOnListFieldRule.ts @@ -31,7 +31,7 @@ export function StreamDirectiveOnListFieldRule( ) { context.reportError( new GraphQLError( - `Stream directive cannot be used on non-list field "${fieldDef.name}" on type "${parentType.name}".`, + `Directive "@stream" cannot be used on non-list field "${parentType.name}.${fieldDef.name}".`, { nodes: node }, ), ); diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index 3b9a1ee645..5915bdd7f4 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -221,7 +221,7 @@ function validateOneOfInputObject( if (isNullableVariable) { context.reportError( new GraphQLError( - `Variable "${variableName}" must be non-nullable to be used for OneOf Input Object "${type.name}".`, + `Variable "$${variableName}" must be non-nullable to be used for OneOf Input Object "${type.name}".`, { nodes: [node] }, ), ); diff --git a/src/validation/rules/custom/NoDeprecatedCustomRule.ts b/src/validation/rules/custom/NoDeprecatedCustomRule.ts index 375373eb1d..2dc865b253 100644 --- a/src/validation/rules/custom/NoDeprecatedCustomRule.ts +++ b/src/validation/rules/custom/NoDeprecatedCustomRule.ts @@ -42,7 +42,7 @@ export function NoDeprecatedCustomRule(context: ValidationContext): ASTVisitor { if (directiveDef != null) { context.reportError( new GraphQLError( - `Directive "@${directiveDef.name}" argument "${argDef.name}" is deprecated. ${deprecationReason}`, + `The argument "@${directiveDef.name}(${argDef.name}:)" is deprecated. ${deprecationReason}`, { nodes: node }, ), ); @@ -52,7 +52,7 @@ export function NoDeprecatedCustomRule(context: ValidationContext): ASTVisitor { invariant(parentType != null && fieldDef != null); context.reportError( new GraphQLError( - `Field "${parentType.name}.${fieldDef.name}" argument "${argDef.name}" is deprecated. ${deprecationReason}`, + `The argument "${parentType.name}.${fieldDef.name}(${argDef.name}:)" is deprecated. ${deprecationReason}`, { nodes: node }, ), ); From ccd814a93aaf20de36a6a2e834ad798e24cf8183 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 25 Aug 2024 22:31:48 +0300 Subject: [PATCH 2/5] add type name to ProvidedRequiredArguments rule errors --- src/type/__tests__/introspection-test.ts | 2 +- .../ProvidedRequiredArgumentsRule-test.ts | 8 ++++---- .../rules/ProvidedRequiredArgumentsRule.ts | 18 ++++++++++++++++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index ddef977fbb..6c9b32ae2f 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -1644,7 +1644,7 @@ describe('Introspection', () => { errors: [ { message: - 'Argument "__type(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/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts index 8605023f0a..3b2c71acf1 100644 --- a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts +++ b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts @@ -168,7 +168,7 @@ describe('Validate: Provided required arguments', () => { `).toDeepEqual([ { message: - 'Argument "multipleReqs(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 }], }, ]); @@ -184,12 +184,12 @@ describe('Validate: Provided required arguments', () => { `).toDeepEqual([ { message: - 'Argument "multipleReqs(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: - 'Argument "multipleReqs(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 }], }, ]); @@ -205,7 +205,7 @@ describe('Validate: Provided required arguments', () => { `).toDeepEqual([ { message: - 'Argument "multipleReqs(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 }], }, ]); diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.ts b/src/validation/rules/ProvidedRequiredArgumentsRule.ts index d71fc4fcd9..f870020e55 100644 --- a/src/validation/rules/ProvidedRequiredArgumentsRule.ts +++ b/src/validation/rules/ProvidedRequiredArgumentsRule.ts @@ -8,8 +8,14 @@ import { print } from '../../language/printer.js'; import type { ASTVisitor } from '../../language/visitor.js'; import type { GraphQLArgument } from '../../type/definition.js'; -import { isRequiredArgument, isType } from '../../type/definition.js'; +import { + getNamedType, + isCompositeType, + isRequiredArgument, + isType, +} from '../../type/definition.js'; import { specifiedDirectives } from '../../type/directives.js'; +import { isIntrospectionType } from '../../type/introspection.js'; import type { SDLValidationContext, @@ -43,10 +49,18 @@ export function ProvidedRequiredArgumentsRule( ); for (const argDef of fieldDef.args) { if (!providedArgs.has(argDef.name) && isRequiredArgument(argDef)) { + const fieldType = getNamedType(context.getType()); + const parentType = context.getParentType(); + const parentTypeStr = + fieldType && isIntrospectionType(fieldType) + ? '.' + : parentType && isCompositeType(parentType) + ? `${parentType.name}.` + : ''; const argTypeStr = inspect(argDef.type); context.reportError( new GraphQLError( - `Argument "${fieldDef.name}(${argDef.name}:)" of type "${argTypeStr}" is required, but it was not provided.`, + `Argument "${parentTypeStr}${fieldDef.name}(${argDef.name}:)" of type "${argTypeStr}" is required, but it was not provided.`, { nodes: fieldNode }, ), ); From f2feb87bc331b9f9ec0df74dfa7d7e105f3bded3 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 25 Aug 2024 23:01:56 +0300 Subject: [PATCH 3/5] use directive and type toString methods instead of `.name` --- src/execution/execute.ts | 16 +++---- src/type/__tests__/introspection-test.ts | 4 +- src/type/validate.ts | 46 +++++++++---------- src/utilities/coerceInputValue.ts | 12 ++--- src/utilities/findBreakingChanges.ts | 42 ++++++++--------- src/utilities/printSchema.ts | 23 ++++------ .../DeferStreamDirectiveOnRootFieldRule.ts | 8 ++-- .../rules/FieldsOnCorrectTypeRule.ts | 2 +- .../rules/KnownArgumentNamesRule.ts | 2 +- .../rules/ProvidedRequiredArgumentsRule.ts | 2 +- .../rules/StreamDirectiveOnListFieldRule.ts | 2 +- .../rules/ValuesOfCorrectTypeRule.ts | 10 ++-- .../rules/custom/NoDeprecatedCustomRule.ts | 4 +- 13 files changed, 84 insertions(+), 89 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 99582b828d..93767667f3 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -891,7 +891,7 @@ function completeValue( ); if ((completed as GraphQLWrappedResult)[0] === null) { throw new Error( - `Cannot return null for non-nullable field ${info.parentType.name}.${info.fieldName}.`, + `Cannot return null for non-nullable field ${info.parentType}.${info.fieldName}.`, ); } return completed; @@ -1241,7 +1241,7 @@ function completeListValue( if (!isIterableObject(result)) { throw new GraphQLError( - `Expected Iterable, but did not find one for field "${info.parentType.name}.${info.fieldName}".`, + `Expected Iterable, but did not find one for field "${info.parentType}.${info.fieldName}".`, ); } @@ -1548,7 +1548,7 @@ function ensureValidRuntimeType( ): GraphQLObjectType { if (runtimeTypeName == null) { throw new GraphQLError( - `Abstract type "${returnType.name}" must resolve to an Object type at runtime for field "${info.parentType.name}.${info.fieldName}". Either the "${returnType.name}" type should provide a "resolveType" function or each possible type should provide an "isTypeOf" function.`, + `Abstract type "${returnType}" must resolve to an Object type at runtime for field "${info.parentType}.${info.fieldName}". Either the "${returnType}" type should provide a "resolveType" function or each possible type should provide an "isTypeOf" function.`, { nodes: toNodes(fieldGroup) }, ); } @@ -1563,7 +1563,7 @@ function ensureValidRuntimeType( if (typeof runtimeTypeName !== 'string') { throw new GraphQLError( - `Abstract type "${returnType.name}" must resolve to an Object type at runtime for field "${info.parentType.name}.${info.fieldName}" with ` + + `Abstract type "${returnType}" must resolve to an Object type at runtime for field "${info.parentType}.${info.fieldName}" with ` + `value ${inspect(result)}, received "${inspect(runtimeTypeName)}".`, ); } @@ -1571,21 +1571,21 @@ function ensureValidRuntimeType( const runtimeType = exeContext.schema.getType(runtimeTypeName); if (runtimeType == null) { throw new GraphQLError( - `Abstract type "${returnType.name}" was resolved to a type "${runtimeTypeName}" that does not exist inside the schema.`, + `Abstract type "${returnType}" was resolved to a type "${runtimeTypeName}" that does not exist inside the schema.`, { nodes: toNodes(fieldGroup) }, ); } if (!isObjectType(runtimeType)) { throw new GraphQLError( - `Abstract type "${returnType.name}" was resolved to a non-object type "${runtimeTypeName}".`, + `Abstract type "${returnType}" was resolved to a non-object type "${runtimeTypeName}".`, { nodes: toNodes(fieldGroup) }, ); } if (!exeContext.schema.isSubType(returnType, runtimeType)) { throw new GraphQLError( - `Runtime Object type "${runtimeType.name}" is not a possible type for "${returnType.name}".`, + `Runtime Object type "${runtimeType}" is not a possible type for "${returnType}".`, { nodes: toNodes(fieldGroup) }, ); } @@ -1651,7 +1651,7 @@ function invalidReturnTypeError( fieldGroup: FieldGroup, ): GraphQLError { return new GraphQLError( - `Expected value of type "${returnType.name}" but got: ${inspect(result)}.`, + `Expected value of type "${returnType}" but got: ${inspect(result)}.`, { nodes: toNodes(fieldGroup) }, ); } diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index 6c9b32ae2f..3ff904a4a8 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -1738,11 +1738,11 @@ describe('Introspection', () => { _3: any, info: GraphQLResolveInfo, ): never { - expect.fail(`Called on ${info.parentType.name}::${info.fieldName}`); + expect.fail(`Called on ${info.parentType}::${info.fieldName}`); } function typeResolver(_1: any, _2: any, info: GraphQLResolveInfo): never { - expect.fail(`Called on ${info.parentType.name}::${info.fieldName}`); + expect.fail(`Called on ${info.parentType}::${info.fieldName}`); } /* c8 ignore stop */ diff --git a/src/type/validate.ts b/src/type/validate.ts index 6399c43d94..3a69a8c6eb 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -148,7 +148,7 @@ function validateRootTypes(context: SchemaValidationContext): void { if (operationTypes.length > 1) { const operationList = andList(operationTypes); context.reportError( - `All root types must be different, "${rootType.name}" type is used as ${operationList} root types.`, + `All root types must be different, "${rootType}" type is used as ${operationList} root types.`, operationTypes.map((operationType) => getOperationTypeNode(schema, operationType), ), @@ -185,7 +185,7 @@ function validateDirectives(context: SchemaValidationContext): void { if (directive.locations.length === 0) { context.reportError( - `Directive @${directive.name} must include 1 or more locations.`, + `Directive ${directive} must include 1 or more locations.`, directive.astNode, ); } @@ -198,7 +198,7 @@ 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 ${directive}(${arg.name}:) must be Input Type ` + `but got: ${inspect(arg.type)}.`, arg.astNode, ); @@ -206,7 +206,7 @@ function validateDirectives(context: SchemaValidationContext): void { if (isRequiredArgument(arg) && arg.deprecationReason != null) { context.reportError( - `Required argument @${directive.name}(${arg.name}:) cannot be deprecated.`, + `Required argument ${directive}(${arg.name}:) cannot be deprecated.`, [getDeprecatedDirectiveNode(arg.astNode), arg.astNode?.type], ); } @@ -282,7 +282,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, ]); @@ -295,7 +295,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 ${type}.${field.name} must be Output Type ` + `but got: ${inspect(field.type)}.`, field.astNode?.type, ); @@ -311,7 +311,7 @@ function validateFields( // 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 ${type}.${field.name}(${argName}:) must be Input ` + `Type but got: ${inspect(arg.type)}.`, arg.astNode?.type, ); @@ -319,7 +319,7 @@ function validateFields( if (isRequiredArgument(arg) && arg.deprecationReason != null) { context.reportError( - `Required argument ${type.name}.${field.name}(${argName}:) cannot be deprecated.`, + `Required argument ${type}.${field.name}(${argName}:) cannot be deprecated.`, [getDeprecatedDirectiveNode(arg.astNode), arg.astNode?.type], ); } @@ -344,7 +344,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; @@ -352,7 +352,7 @@ function validateInterfaces( if (ifaceTypeNames.has(iface.name)) { context.reportError( - `Type ${type.name} can only implement ${iface.name} once.`, + `Type ${type} can only implement ${iface.name} once.`, getAllImplementsInterfaceNodes(type, iface), ); continue; @@ -380,7 +380,7 @@ function validateTypeImplementsInterface( // Assert interface field exists on type. if (typeField == null) { context.reportError( - `Interface field ${iface.name}.${fieldName} expected but ${type.name} does not provide it.`, + `Interface field ${iface.name}.${fieldName} expected but ${type} does not provide it.`, [ifaceField.astNode, type.astNode, ...type.extensionASTNodes], ); continue; @@ -391,7 +391,7 @@ function validateTypeImplementsInterface( if (!isTypeSubTypeOf(context.schema, typeField.type, ifaceField.type)) { context.reportError( `Interface field ${iface.name}.${fieldName} expects type ` + - `${inspect(ifaceField.type)} but ${type.name}.${fieldName} ` + + `${inspect(ifaceField.type)} but ${type}.${fieldName} ` + `is type ${inspect(typeField.type)}.`, [ifaceField.astNode?.type, typeField.astNode?.type], ); @@ -405,7 +405,7 @@ function validateTypeImplementsInterface( // 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 ${iface.name}.${fieldName}(${argName}:) expected but ${type}.${fieldName} does not provide it.`, [ifaceArg.astNode, typeField.astNode], ); continue; @@ -418,7 +418,7 @@ function validateTypeImplementsInterface( context.reportError( `Interface field argument ${iface.name}.${fieldName}(${argName}:) ` + `expects type ${inspect(ifaceArg.type)} but ` + - `${type.name}.${fieldName}(${argName}:) is type ` + + `${type}.${fieldName}(${argName}:) is type ` + `${inspect(typeArg.type)}.`, [ifaceArg.astNode?.type, typeArg.astNode?.type], ); @@ -433,9 +433,7 @@ function validateTypeImplementsInterface( const ifaceArg = ifaceField.args.find((arg) => arg.name === argName); if (!ifaceArg && isRequiredArgument(typeArg)) { context.reportError( - `Argument "${ - type.name - }.${fieldName}(${argName}:)" must not be required type "${inspect( + `Argument "${type}.${fieldName}(${argName}:)" must not be required type "${inspect( typeArg.type, )}" if not provided by the Interface field "${ iface.name @@ -457,8 +455,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.name} because it would create a circular reference.` + : `Type ${type} must implement ${transitive.name} because it is implemented by ${iface.name}.`, [ ...getAllImplementsInterfaceNodes(iface, transitive), ...getAllImplementsInterfaceNodes(type, iface), @@ -485,7 +483,7 @@ function validateUnionMembers( for (const memberType of memberTypes) { if (includedTypeNames.has(memberType.name)) { context.reportError( - `Union type ${union.name} can only include type ${memberType.name} once.`, + `Union type ${union.name} can only include type ${memberType} once.`, getUnionMemberTypeNodes(union, memberType.name), ); continue; @@ -509,7 +507,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], ); } @@ -567,14 +565,14 @@ function validateOneOfInputObjectField( ): void { if (isNonNullType(field.type)) { context.reportError( - `OneOf input field ${type.name}.${field.name} must be nullable.`, + `OneOf input field ${type}.${field.name} must be nullable.`, field.astNode?.type, ); } if (field.defaultValue !== undefined) { context.reportError( - `OneOf input field ${type.name}.${field.name} cannot have a default value.`, + `OneOf input field ${type}.${field.name} cannot have a default value.`, field.astNode, ); } @@ -620,7 +618,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/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index fe4b825b93..7b3a61926c 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -90,7 +90,7 @@ function coerceInputValueImpl( onError( pathToArray(path), inputValue, - new GraphQLError(`Expected type "${type.name}" to be an object.`), + new GraphQLError(`Expected type "${type}" to be an object.`), ); return; } @@ -110,7 +110,7 @@ function coerceInputValueImpl( pathToArray(path), inputValue, new GraphQLError( - `Field "${type.name}.${field.name}" of required type "${typeStr}" was not provided.`, + `Field "${type}.${field.name}" of required type "${typeStr}" was not provided.`, ), ); } @@ -136,7 +136,7 @@ function coerceInputValueImpl( pathToArray(path), inputValue, new GraphQLError( - `Field "${fieldName}" is not defined by type "${type.name}".` + + `Field "${fieldName}" is not defined by type "${type}".` + didYouMean(suggestions), ), ); @@ -150,7 +150,7 @@ function coerceInputValueImpl( pathToArray(path), inputValue, new GraphQLError( - `Exactly one key must be specified for OneOf type "${type.name}".`, + `Exactly one key must be specified for OneOf type "${type}".`, ), ); } @@ -184,7 +184,7 @@ function coerceInputValueImpl( onError( pathToArray(path), inputValue, - new GraphQLError(`Expected type "${type.name}". ` + error.message, { + new GraphQLError(`Expected type "${type}". ` + error.message, { originalError: error, }), ); @@ -195,7 +195,7 @@ function coerceInputValueImpl( onError( pathToArray(path), inputValue, - new GraphQLError(`Expected type "${type.name}".`), + new GraphQLError(`Expected type "${type}".`), ); } return parseResult; diff --git a/src/utilities/findBreakingChanges.ts b/src/utilities/findBreakingChanges.ts index 4c5091ef97..7017c43ff9 100644 --- a/src/utilities/findBreakingChanges.ts +++ b/src/utilities/findBreakingChanges.ts @@ -182,8 +182,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,7 +208,7 @@ function findTypeChanges( schemaChanges.push({ type: BreakingChangeType.TYPE_CHANGED_KIND, description: - `${oldType.name} changed from ` + + `${oldType} changed from ` + `${typeKindName(oldType)} to ${typeKindName(newType)}.`, }); } @@ -231,12 +231,12 @@ function findInputObjectTypeChanges( if (isRequiredInputField(newField)) { schemaChanges.push({ type: BreakingChangeType.REQUIRED_INPUT_FIELD_ADDED, - description: `A required field ${oldType.name}.${newField.name} was added.`, + description: `A required field ${oldType}.${newField.name} was added.`, }); } else { schemaChanges.push({ type: DangerousChangeType.OPTIONAL_INPUT_FIELD_ADDED, - description: `An optional field ${oldType.name}.${newField.name} was added.`, + description: `An optional field ${oldType}.${newField.name} was added.`, }); } } @@ -244,7 +244,7 @@ function findInputObjectTypeChanges( for (const oldField of fieldsDiff.removed) { schemaChanges.push({ type: BreakingChangeType.FIELD_REMOVED, - description: `Field ${oldType.name}.${oldField.name} was removed.`, + description: `Field ${oldType}.${oldField.name} was removed.`, }); } @@ -257,7 +257,7 @@ function findInputObjectTypeChanges( schemaChanges.push({ type: BreakingChangeType.FIELD_CHANGED_KIND, description: - `Field ${oldType.name}.${oldField.name} changed type from ` + + `Field ${oldType}.${oldField.name} changed type from ` + `${String(oldField.type)} to ${String(newField.type)}.`, }); } @@ -276,14 +276,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}.`, }); } @@ -300,14 +300,14 @@ function findEnumTypeChanges( for (const newValue of valuesDiff.added) { schemaChanges.push({ type: DangerousChangeType.VALUE_ADDED_TO_ENUM, - description: `Enum value ${oldType.name}.${newValue.name} was added.`, + description: `Enum value ${oldType}.${newValue.name} was added.`, }); } for (const oldValue of valuesDiff.removed) { schemaChanges.push({ type: BreakingChangeType.VALUE_REMOVED_FROM_ENUM, - description: `Enum value ${oldType.name}.${oldValue.name} was removed.`, + description: `Enum value ${oldType}.${oldValue.name} was removed.`, }); } @@ -324,14 +324,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.name} 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.name}.`, }); } @@ -351,7 +351,7 @@ function findFieldChanges( for (const oldField of fieldsDiff.removed) { schemaChanges.push({ type: BreakingChangeType.FIELD_REMOVED, - description: `Field ${oldType.name}.${oldField.name} was removed.`, + description: `Field ${oldType}.${oldField.name} was removed.`, }); } @@ -366,7 +366,7 @@ function findFieldChanges( schemaChanges.push({ type: BreakingChangeType.FIELD_CHANGED_KIND, description: - `Field ${oldType.name}.${oldField.name} changed type from ` + + `Field ${oldType}.${oldField.name} changed type from ` + `${String(oldField.type)} to ${String(newField.type)}.`, }); } @@ -386,7 +386,7 @@ function findArgChanges( for (const oldArg of argsDiff.removed) { schemaChanges.push({ type: BreakingChangeType.ARG_REMOVED, - description: `Argument ${oldType.name}.${oldField.name}(${oldArg.name}:) was removed.`, + description: `Argument ${oldType}.${oldField.name}(${oldArg.name}:) was removed.`, }); } @@ -399,14 +399,14 @@ function findArgChanges( schemaChanges.push({ type: BreakingChangeType.ARG_CHANGED_KIND, description: - `Argument ${oldType.name}.${oldField.name}(${oldArg.name}:) has changed type from ` + + `Argument ${oldType}.${oldField.name}(${oldArg.name}:) has changed type from ` + `${String(oldArg.type)} to ${String(newArg.type)}.`, }); } else if (oldArg.defaultValue !== undefined) { if (newArg.defaultValue === undefined) { schemaChanges.push({ type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, - description: `${oldType.name}.${oldField.name}(${oldArg.name}:) defaultValue was removed.`, + description: `${oldType}.${oldField.name}(${oldArg.name}:) defaultValue was removed.`, }); } else { // Since we looking only for client's observable changes we should @@ -418,7 +418,7 @@ function findArgChanges( if (oldValueStr !== newValueStr) { schemaChanges.push({ type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, - description: `${oldType.name}.${oldField.name}(${oldArg.name}:) has changed defaultValue from ${oldValueStr} to ${newValueStr}.`, + description: `${oldType}.${oldField.name}(${oldArg.name}:) has changed defaultValue from ${oldValueStr} to ${newValueStr}.`, }); } } @@ -429,12 +429,12 @@ function findArgChanges( if (isRequiredArgument(newArg)) { schemaChanges.push({ type: BreakingChangeType.REQUIRED_ARG_ADDED, - description: `A required argument ${oldType.name}.${oldField.name}(${newArg.name}:) was added.`, + description: `A required argument ${oldType}.${oldField.name}(${newArg.name}:) was added.`, }); } else { schemaChanges.push({ type: DangerousChangeType.OPTIONAL_ARG_ADDED, - description: `An optional argument ${oldType.name}.${oldField.name}(${newArg.name}:) was added.`, + description: `An optional argument ${oldType}.${oldField.name}(${newArg.name}:) was added.`, }); } } diff --git a/src/utilities/printSchema.ts b/src/utilities/printSchema.ts index c4caffc616..7d4a5e4f5a 100644 --- a/src/utilities/printSchema.ts +++ b/src/utilities/printSchema.ts @@ -86,9 +86,9 @@ function printSchemaDefinition(schema: GraphQLSchema): Maybe { return ( printDescription(schema) + 'schema {\n' + - (queryType ? ` query: ${queryType.name}\n` : '') + - (mutationType ? ` mutation: ${mutationType.name}\n` : '') + - (subscriptionType ? ` subscription: ${subscriptionType.name}\n` : '') + + (queryType ? ` query: ${queryType}\n` : '') + + (mutationType ? ` mutation: ${mutationType}\n` : '') + + (subscriptionType ? ` subscription: ${subscriptionType}\n` : '') + '}' ); } @@ -148,9 +148,7 @@ export function printType(type: GraphQLNamedType): string { } function printScalar(type: GraphQLScalarType): string { - return ( - printDescription(type) + `scalar ${type.name}` + printSpecifiedByURL(type) - ); + return printDescription(type) + `scalar ${type}` + printSpecifiedByURL(type); } function printImplementedInterfaces( @@ -165,7 +163,7 @@ function printImplementedInterfaces( function printObject(type: GraphQLObjectType): string { return ( printDescription(type) + - `type ${type.name}` + + `type ${type}` + printImplementedInterfaces(type) + printFields(type) ); @@ -174,7 +172,7 @@ function printObject(type: GraphQLObjectType): string { function printInterface(type: GraphQLInterfaceType): string { return ( printDescription(type) + - `interface ${type.name}` + + `interface ${type}` + printImplementedInterfaces(type) + printFields(type) ); @@ -183,7 +181,7 @@ function printInterface(type: GraphQLInterfaceType): string { function printUnion(type: GraphQLUnionType): string { const types = type.getTypes(); const possibleTypes = types.length ? ' = ' + types.join(' | ') : ''; - return printDescription(type) + 'union ' + type.name + possibleTypes; + return printDescription(type) + `union ${type.name}` + possibleTypes; } function printEnum(type: GraphQLEnumType): string { @@ -197,7 +195,7 @@ function printEnum(type: GraphQLEnumType): string { printDeprecated(value.deprecationReason), ); - return printDescription(type) + `enum ${type.name}` + printBlock(values); + return printDescription(type) + `enum ${type}` + printBlock(values); } function printInputObject(type: GraphQLInputObjectType): string { @@ -206,7 +204,7 @@ function printInputObject(type: GraphQLInputObjectType): string { ); return ( printDescription(type) + - `input ${type.name}` + + `input ${type}` + (type.isOneOf ? ' @oneOf' : '') + printBlock(fields) ); @@ -272,8 +270,7 @@ function printInputValue(arg: GraphQLInputField): string { export function printDirective(directive: GraphQLDirective): string { return ( printDescription(directive) + - 'directive @' + - directive.name + + `directive ${directive}` + printArgs(directive.args) + (directive.isRepeatable ? ' repeatable' : '') + ' on ' + diff --git a/src/validation/rules/DeferStreamDirectiveOnRootFieldRule.ts b/src/validation/rules/DeferStreamDirectiveOnRootFieldRule.ts index b0d2dda18f..ce7c20664c 100644 --- a/src/validation/rules/DeferStreamDirectiveOnRootFieldRule.ts +++ b/src/validation/rules/DeferStreamDirectiveOnRootFieldRule.ts @@ -26,7 +26,7 @@ export function DeferStreamDirectiveOnRootFieldRule( if (mutationType && parentType === mutationType) { context.reportError( new GraphQLError( - `Defer directive cannot be used on root mutation type "${parentType.name}".`, + `Defer directive cannot be used on root mutation type "${parentType}".`, { nodes: node }, ), ); @@ -34,7 +34,7 @@ export function DeferStreamDirectiveOnRootFieldRule( if (subscriptionType && parentType === subscriptionType) { context.reportError( new GraphQLError( - `Defer directive cannot be used on root subscription type "${parentType.name}".`, + `Defer directive cannot be used on root subscription type "${parentType}".`, { nodes: node }, ), ); @@ -44,7 +44,7 @@ export function DeferStreamDirectiveOnRootFieldRule( if (mutationType && parentType === mutationType) { context.reportError( new GraphQLError( - `Stream directive cannot be used on root mutation type "${parentType.name}".`, + `Stream directive cannot be used on root mutation type "${parentType}".`, { nodes: node }, ), ); @@ -52,7 +52,7 @@ export function DeferStreamDirectiveOnRootFieldRule( if (subscriptionType && parentType === subscriptionType) { context.reportError( new GraphQLError( - `Stream directive cannot be used on root subscription type "${parentType.name}".`, + `Stream directive cannot be used on root subscription type "${parentType}".`, { nodes: node }, ), ); diff --git a/src/validation/rules/FieldsOnCorrectTypeRule.ts b/src/validation/rules/FieldsOnCorrectTypeRule.ts index 5d61e08b03..c6fce9e89b 100644 --- a/src/validation/rules/FieldsOnCorrectTypeRule.ts +++ b/src/validation/rules/FieldsOnCorrectTypeRule.ts @@ -56,7 +56,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, { nodes: node }, ), diff --git a/src/validation/rules/KnownArgumentNamesRule.ts b/src/validation/rules/KnownArgumentNamesRule.ts index 2f09348778..01046436b4 100644 --- a/src/validation/rules/KnownArgumentNamesRule.ts +++ b/src/validation/rules/KnownArgumentNamesRule.ts @@ -37,7 +37,7 @@ export function KnownArgumentNamesRule(context: ValidationContext): ASTVisitor { const suggestions = suggestionList(argName, knownArgsNames); context.reportError( new GraphQLError( - `Unknown argument "${argName}" on field "${parentType.name}.${fieldDef.name}".` + + `Unknown argument "${argName}" on field "${parentType}.${fieldDef.name}".` + didYouMean(suggestions), { nodes: argNode }, ), diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.ts b/src/validation/rules/ProvidedRequiredArgumentsRule.ts index f870020e55..af3fce210b 100644 --- a/src/validation/rules/ProvidedRequiredArgumentsRule.ts +++ b/src/validation/rules/ProvidedRequiredArgumentsRule.ts @@ -55,7 +55,7 @@ export function ProvidedRequiredArgumentsRule( fieldType && isIntrospectionType(fieldType) ? '.' : parentType && isCompositeType(parentType) - ? `${parentType.name}.` + ? `${parentType}.` : ''; const argTypeStr = inspect(argDef.type); context.reportError( diff --git a/src/validation/rules/StreamDirectiveOnListFieldRule.ts b/src/validation/rules/StreamDirectiveOnListFieldRule.ts index 16504d84d2..223a5e66d3 100644 --- a/src/validation/rules/StreamDirectiveOnListFieldRule.ts +++ b/src/validation/rules/StreamDirectiveOnListFieldRule.ts @@ -31,7 +31,7 @@ export function StreamDirectiveOnListFieldRule( ) { context.reportError( new GraphQLError( - `Directive "@stream" cannot be used on non-list field "${parentType.name}.${fieldDef.name}".`, + `Directive "@stream" cannot be used on non-list field "${parentType}.${fieldDef.name}".`, { nodes: node }, ), ); diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index 5915bdd7f4..a86a66bd47 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -74,7 +74,7 @@ export function ValuesOfCorrectTypeRule( const typeStr = inspect(fieldDef.type); context.reportError( new GraphQLError( - `Field "${type.name}.${fieldDef.name}" of required type "${typeStr}" was not provided.`, + `Field "${type}.${fieldDef.name}" of required type "${typeStr}" was not provided.`, { nodes: node }, ), ); @@ -101,7 +101,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), { nodes: node }, ), @@ -193,7 +193,7 @@ function validateOneOfInputObject( if (isNotExactlyOneField) { context.reportError( new GraphQLError( - `OneOf Input Object "${type.name}" must specify exactly one key.`, + `OneOf Input Object "${type}" must specify exactly one key.`, { nodes: [node] }, ), ); @@ -206,7 +206,7 @@ function validateOneOfInputObject( if (isNullLiteral) { context.reportError( - new GraphQLError(`Field "${type.name}.${keys[0]}" must be non-null.`, { + new GraphQLError(`Field "${type}.${keys[0]}" must be non-null.`, { nodes: [node], }), ); @@ -221,7 +221,7 @@ function validateOneOfInputObject( if (isNullableVariable) { context.reportError( new GraphQLError( - `Variable "$${variableName}" must be non-nullable to be used for OneOf Input Object "${type.name}".`, + `Variable "$${variableName}" must be non-nullable to be used for OneOf Input Object "${type}".`, { nodes: [node] }, ), ); diff --git a/src/validation/rules/custom/NoDeprecatedCustomRule.ts b/src/validation/rules/custom/NoDeprecatedCustomRule.ts index 2dc865b253..1e3328039e 100644 --- a/src/validation/rules/custom/NoDeprecatedCustomRule.ts +++ b/src/validation/rules/custom/NoDeprecatedCustomRule.ts @@ -28,7 +28,7 @@ export function NoDeprecatedCustomRule(context: ValidationContext): ASTVisitor { invariant(parentType != null); context.reportError( new GraphQLError( - `The field ${parentType.name}.${fieldDef.name} is deprecated. ${deprecationReason}`, + `The field ${parentType}.${fieldDef.name} is deprecated. ${deprecationReason}`, { nodes: node }, ), ); @@ -52,7 +52,7 @@ export function NoDeprecatedCustomRule(context: ValidationContext): ASTVisitor { invariant(parentType != null && fieldDef != null); context.reportError( new GraphQLError( - `The argument "${parentType.name}.${fieldDef.name}(${argDef.name}:)" is deprecated. ${deprecationReason}`, + `The argument "${parentType}.${fieldDef.name}(${argDef.name}:)" is deprecated. ${deprecationReason}`, { nodes: node }, ), ); From ed95121143d3b6d111864e3f719e04cce4f655ba Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 25 Aug 2024 23:08:53 +0300 Subject: [PATCH 4/5] fix coverage --- .../rules/ProvidedRequiredArgumentsRule.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.ts b/src/validation/rules/ProvidedRequiredArgumentsRule.ts index af3fce210b..b65e8f2d48 100644 --- a/src/validation/rules/ProvidedRequiredArgumentsRule.ts +++ b/src/validation/rules/ProvidedRequiredArgumentsRule.ts @@ -10,7 +10,6 @@ import type { ASTVisitor } from '../../language/visitor.js'; import type { GraphQLArgument } from '../../type/definition.js'; import { getNamedType, - isCompositeType, isRequiredArgument, isType, } from '../../type/definition.js'; @@ -50,17 +49,14 @@ export function ProvidedRequiredArgumentsRule( for (const argDef of fieldDef.args) { if (!providedArgs.has(argDef.name) && isRequiredArgument(argDef)) { const fieldType = getNamedType(context.getType()); - const parentType = context.getParentType(); - const parentTypeStr = + const parentType = fieldType && isIntrospectionType(fieldType) - ? '.' - : parentType && isCompositeType(parentType) - ? `${parentType}.` - : ''; + ? '' + : context.getParentType(); const argTypeStr = inspect(argDef.type); context.reportError( new GraphQLError( - `Argument "${parentTypeStr}${fieldDef.name}(${argDef.name}:)" of type "${argTypeStr}" is required, but it was not provided.`, + `Argument "${parentType}.${fieldDef.name}(${argDef.name}:)" of type "${argTypeStr}" is required, but it was not provided.`, { nodes: fieldNode }, ), ); From 8635c9cf295ee5d2ce0e78b009ce07635a4dcd56 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 26 Aug 2024 15:04:24 +0300 Subject: [PATCH 5/5] better coverage fix --- .../rules/ProvidedRequiredArgumentsRule.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.ts b/src/validation/rules/ProvidedRequiredArgumentsRule.ts index b65e8f2d48..c42823b2bd 100644 --- a/src/validation/rules/ProvidedRequiredArgumentsRule.ts +++ b/src/validation/rules/ProvidedRequiredArgumentsRule.ts @@ -49,14 +49,19 @@ export function ProvidedRequiredArgumentsRule( for (const argDef of fieldDef.args) { if (!providedArgs.has(argDef.name) && isRequiredArgument(argDef)) { const fieldType = getNamedType(context.getType()); - const parentType = - fieldType && isIntrospectionType(fieldType) - ? '' - : context.getParentType(); + let parentTypeStr: string | undefined; + if (fieldType && isIntrospectionType(fieldType)) { + parentTypeStr = '.'; + } else { + const parentType = context.getParentType(); + if (parentType) { + parentTypeStr = `${context.getParentType()}.`; + } + } const argTypeStr = inspect(argDef.type); context.reportError( new GraphQLError( - `Argument "${parentType}.${fieldDef.name}(${argDef.name}:)" of type "${argTypeStr}" is required, but it was not provided.`, + `Argument "${parentTypeStr}${fieldDef.name}(${argDef.name}:)" of type "${argTypeStr}" is required, but it was not provided.`, { nodes: fieldNode }, ), );