From 75dd873319f26c58f37c4cc23886bb8be3808d78 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 14 Oct 2024 14:05:16 +0300 Subject: [PATCH] polish(ast): prefer undefined over empty arrays (#4206) see: https://github.com/graphql/graphql-js/pull/2405#issuecomment-579077133 Manually created ASTs always allowed undefined in place of empty arrays; this change simply updates the parser to more closely follow the manual approach. This is therefore not technically a breaking change, but considering that there may be tools not aware of this, we have labelled it a BREAKING_CHANGE to highlight it for consumers. This closes #2203 as our tests now cover the undefined case by default whenever there is an empty array. --- src/execution/execute.ts | 2 - src/execution/values.ts | 2 - src/language/__tests__/parser-test.ts | 26 ++--- src/language/__tests__/schema-parser-test.ts | 100 +++++++++--------- src/language/parser.ts | 69 ++++++------ src/language/printer.ts | 2 - src/type/__tests__/validation-test.ts | 65 +++++++++++- src/type/validate.ts | 9 +- .../__tests__/buildASTSchema-test.ts | 2 +- .../__tests__/coerceInputValue-test.ts | 9 +- .../__tests__/replaceVariables-test.ts | 2 +- src/utilities/extendSchema.ts | 26 ++--- .../rules/KnownArgumentNamesRule.ts | 2 - src/validation/rules/NoUnusedVariablesRule.ts | 4 - .../rules/OverlappingFieldsCanBeMergedRule.ts | 5 +- .../rules/ProvidedRequiredArgumentsRule.ts | 8 -- .../UniqueArgumentDefinitionNamesRule.ts | 6 -- .../rules/UniqueArgumentNamesRule.ts | 2 - .../rules/UniqueEnumValueNamesRule.ts | 2 - .../rules/UniqueFieldDefinitionNamesRule.ts | 2 - .../rules/UniqueOperationTypesRule.ts | 2 - .../rules/UniqueVariableNamesRule.ts | 2 - 22 files changed, 184 insertions(+), 165 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index a35156cb..be2be7d7 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -561,8 +561,6 @@ export function validateExecutionArgs( return [new GraphQLError('Must provide an operation.')]; } - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const variableDefinitions = operation.variableDefinitions ?? []; const hideSuggestions = args.hideSuggestions ?? false; diff --git a/src/execution/values.ts b/src/execution/values.ts index 5040193c..77050858 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -224,8 +224,6 @@ export function experimentalGetArgumentValues( ): { [argument: string]: unknown } { const coercedValues: { [argument: string]: unknown } = {}; - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const argumentNodes = node.arguments ?? []; const argNodeMap = new Map(argumentNodes.map((arg) => [arg.name.value, arg])); diff --git a/src/language/__tests__/parser-test.ts b/src/language/__tests__/parser-test.ts index 02b5f99c..71efd7b8 100644 --- a/src/language/__tests__/parser-test.ts +++ b/src/language/__tests__/parser-test.ts @@ -259,8 +259,8 @@ describe('Parser', () => { loc: { start: 0, end: 40 }, operation: 'query', name: undefined, - variableDefinitions: [], - directives: [], + variableDefinitions: undefined, + directives: undefined, selectionSet: { kind: Kind.SELECTION_SET, loc: { start: 0, end: 40 }, @@ -290,7 +290,7 @@ describe('Parser', () => { loc: { start: 9, end: 14 }, }, ], - directives: [], + directives: undefined, selectionSet: { kind: Kind.SELECTION_SET, loc: { start: 16, end: 38 }, @@ -304,8 +304,8 @@ describe('Parser', () => { loc: { start: 22, end: 24 }, value: 'id', }, - arguments: [], - directives: [], + arguments: undefined, + directives: undefined, selectionSet: undefined, }, { @@ -317,8 +317,8 @@ describe('Parser', () => { loc: { start: 30, end: 34 }, value: 'name', }, - arguments: [], - directives: [], + arguments: undefined, + directives: undefined, selectionSet: undefined, }, ], @@ -349,8 +349,8 @@ describe('Parser', () => { loc: { start: 0, end: 29 }, operation: 'query', name: undefined, - variableDefinitions: [], - directives: [], + variableDefinitions: undefined, + directives: undefined, selectionSet: { kind: Kind.SELECTION_SET, loc: { start: 6, end: 29 }, @@ -364,8 +364,8 @@ describe('Parser', () => { loc: { start: 10, end: 14 }, value: 'node', }, - arguments: [], - directives: [], + arguments: undefined, + directives: undefined, selectionSet: { kind: Kind.SELECTION_SET, loc: { start: 15, end: 27 }, @@ -379,8 +379,8 @@ describe('Parser', () => { loc: { start: 21, end: 23 }, value: 'id', }, - arguments: [], - directives: [], + arguments: undefined, + directives: undefined, selectionSet: undefined, }, ], diff --git a/src/language/__tests__/schema-parser-test.ts b/src/language/__tests__/schema-parser-test.ts index 27ac5c1b..3780c833 100644 --- a/src/language/__tests__/schema-parser-test.ts +++ b/src/language/__tests__/schema-parser-test.ts @@ -31,7 +31,7 @@ function nameNode(name: unknown, loc: unknown) { } function fieldNode(name: unknown, type: unknown, loc: unknown) { - return fieldNodeWithArgs(name, type, [], loc); + return fieldNodeWithArgs(name, type, undefined, loc); } function fieldNodeWithArgs( @@ -46,7 +46,7 @@ function fieldNodeWithArgs( name, arguments: args, type, - directives: [], + directives: undefined, loc, }; } @@ -56,7 +56,7 @@ function enumValueNode(name: unknown, loc: unknown) { kind: 'EnumValueDefinition', name: nameNode(name, loc), description: undefined, - directives: [], + directives: undefined, loc, }; } @@ -73,7 +73,7 @@ function inputValueNode( description: undefined, type, defaultValue, - directives: [], + directives: undefined, loc, }; } @@ -93,8 +93,8 @@ describe('Schema Parser', () => { kind: 'ObjectTypeDefinition', name: nameNode('Hello', { start: 5, end: 10 }), description: undefined, - interfaces: [], - directives: [], + interfaces: undefined, + directives: undefined, fields: [ fieldNode( nameNode('world', { start: 15, end: 20 }), @@ -180,8 +180,8 @@ describe('Schema Parser', () => { { kind: 'ObjectTypeExtension', name: nameNode('Hello', { start: 12, end: 17 }), - interfaces: [], - directives: [], + interfaces: undefined, + directives: undefined, fields: [ fieldNode( nameNode('world', { start: 22, end: 27 }), @@ -206,8 +206,8 @@ describe('Schema Parser', () => { kind: 'ObjectTypeExtension', name: nameNode('Hello', { start: 12, end: 17 }), interfaces: [typeNode('Greeting', { start: 29, end: 37 })], - directives: [], - fields: [], + directives: undefined, + fields: undefined, loc: { start: 0, end: 37 }, }, ], @@ -224,8 +224,8 @@ describe('Schema Parser', () => { kind: 'InterfaceTypeExtension', name: nameNode('Hello', { start: 17, end: 22 }), interfaces: [typeNode('Greeting', { start: 34, end: 42 })], - directives: [], - fields: [], + directives: undefined, + fields: undefined, loc: { start: 0, end: 42 }, }, ], @@ -247,16 +247,16 @@ describe('Schema Parser', () => { kind: 'ObjectTypeExtension', name: nameNode('Hello', { start: 19, end: 24 }), interfaces: [typeNode('Greeting', { start: 36, end: 44 })], - directives: [], - fields: [], + directives: undefined, + fields: undefined, loc: { start: 7, end: 44 }, }, { kind: 'ObjectTypeExtension', name: nameNode('Hello', { start: 64, end: 69 }), interfaces: [typeNode('SecondGreeting', { start: 81, end: 95 })], - directives: [], - fields: [], + directives: undefined, + fields: undefined, loc: { start: 52, end: 95 }, }, ], @@ -309,16 +309,16 @@ describe('Schema Parser', () => { kind: 'InterfaceTypeExtension', name: nameNode('Hello', { start: 24, end: 29 }), interfaces: [typeNode('Greeting', { start: 41, end: 49 })], - directives: [], - fields: [], + directives: undefined, + fields: undefined, loc: { start: 7, end: 49 }, }, { kind: 'InterfaceTypeExtension', name: nameNode('Hello', { start: 74, end: 79 }), interfaces: [typeNode('SecondGreeting', { start: 91, end: 105 })], - directives: [], - fields: [], + directives: undefined, + fields: undefined, loc: { start: 57, end: 105 }, }, ], @@ -381,7 +381,7 @@ describe('Schema Parser', () => { definitions: [ { kind: 'SchemaExtension', - directives: [], + directives: undefined, operationTypes: [ { kind: 'OperationTypeDefinition', @@ -409,11 +409,11 @@ describe('Schema Parser', () => { { kind: 'Directive', name: nameNode('directive', { start: 15, end: 24 }), - arguments: [], + arguments: undefined, loc: { start: 14, end: 24 }, }, ], - operationTypes: [], + operationTypes: undefined, loc: { start: 0, end: 24 }, }, ], @@ -449,8 +449,8 @@ describe('Schema Parser', () => { kind: 'ObjectTypeDefinition', name: nameNode('Hello', { start: 5, end: 10 }), description: undefined, - interfaces: [], - directives: [], + interfaces: undefined, + directives: undefined, fields: [ fieldNode( nameNode('world', { start: 15, end: 20 }), @@ -479,7 +479,7 @@ describe('Schema Parser', () => { name: nameNode('Hello', { start: 10, end: 15 }), description: undefined, interfaces: [typeNode('World', { start: 27, end: 32 })], - directives: [], + directives: undefined, fields: [ fieldNode( nameNode('field', { start: 35, end: 40 }), @@ -505,7 +505,7 @@ describe('Schema Parser', () => { name: nameNode('Hello', { start: 5, end: 10 }), description: undefined, interfaces: [typeNode('World', { start: 22, end: 27 })], - directives: [], + directives: undefined, fields: [ fieldNode( nameNode('field', { start: 30, end: 35 }), @@ -534,7 +534,7 @@ describe('Schema Parser', () => { typeNode('Wo', { start: 22, end: 24 }), typeNode('rld', { start: 27, end: 30 }), ], - directives: [], + directives: undefined, fields: [ fieldNode( nameNode('field', { start: 33, end: 38 }), @@ -562,7 +562,7 @@ describe('Schema Parser', () => { typeNode('Wo', { start: 27, end: 29 }), typeNode('rld', { start: 32, end: 35 }), ], - directives: [], + directives: undefined, fields: [ fieldNode( nameNode('field', { start: 38, end: 43 }), @@ -591,7 +591,7 @@ describe('Schema Parser', () => { typeNode('Wo', { start: 24, end: 26 }), typeNode('rld', { start: 29, end: 32 }), ], - directives: [], + directives: undefined, fields: [ fieldNode( nameNode('field', { start: 35, end: 40 }), @@ -621,7 +621,7 @@ describe('Schema Parser', () => { typeNode('Wo', { start: 29, end: 31 }), typeNode('rld', { start: 34, end: 37 }), ], - directives: [], + directives: undefined, fields: [ fieldNode( nameNode('field', { start: 40, end: 45 }), @@ -646,7 +646,7 @@ describe('Schema Parser', () => { kind: 'EnumTypeDefinition', name: nameNode('Hello', { start: 5, end: 10 }), description: undefined, - directives: [], + directives: undefined, values: [enumValueNode('WORLD', { start: 13, end: 18 })], loc: { start: 0, end: 20 }, }, @@ -665,7 +665,7 @@ describe('Schema Parser', () => { kind: 'EnumTypeDefinition', name: nameNode('Hello', { start: 5, end: 10 }), description: undefined, - directives: [], + directives: undefined, values: [ enumValueNode('WO', { start: 13, end: 15 }), enumValueNode('RLD', { start: 17, end: 20 }), @@ -691,8 +691,8 @@ describe('Schema Parser', () => { kind: 'InterfaceTypeDefinition', name: nameNode('Hello', { start: 10, end: 15 }), description: undefined, - interfaces: [], - directives: [], + interfaces: undefined, + directives: undefined, fields: [ fieldNode( nameNode('world', { start: 20, end: 25 }), @@ -721,8 +721,8 @@ describe('Schema Parser', () => { kind: 'ObjectTypeDefinition', name: nameNode('Hello', { start: 5, end: 10 }), description: undefined, - interfaces: [], - directives: [], + interfaces: undefined, + directives: undefined, fields: [ fieldNodeWithArgs( nameNode('world', { start: 15, end: 20 }), @@ -759,8 +759,8 @@ describe('Schema Parser', () => { kind: 'ObjectTypeDefinition', name: nameNode('Hello', { start: 5, end: 10 }), description: undefined, - interfaces: [], - directives: [], + interfaces: undefined, + directives: undefined, fields: [ fieldNodeWithArgs( nameNode('world', { start: 15, end: 20 }), @@ -801,8 +801,8 @@ describe('Schema Parser', () => { kind: 'ObjectTypeDefinition', name: nameNode('Hello', { start: 5, end: 10 }), description: undefined, - interfaces: [], - directives: [], + interfaces: undefined, + directives: undefined, fields: [ fieldNodeWithArgs( nameNode('world', { start: 15, end: 20 }), @@ -843,8 +843,8 @@ describe('Schema Parser', () => { kind: 'ObjectTypeDefinition', name: nameNode('Hello', { start: 5, end: 10 }), description: undefined, - interfaces: [], - directives: [], + interfaces: undefined, + directives: undefined, fields: [ fieldNodeWithArgs( nameNode('world', { start: 15, end: 20 }), @@ -883,7 +883,7 @@ describe('Schema Parser', () => { kind: 'UnionTypeDefinition', name: nameNode('Hello', { start: 6, end: 11 }), description: undefined, - directives: [], + directives: undefined, types: [typeNode('World', { start: 14, end: 19 })], loc: { start: 0, end: 19 }, }, @@ -902,7 +902,7 @@ describe('Schema Parser', () => { kind: 'UnionTypeDefinition', name: nameNode('Hello', { start: 6, end: 11 }), description: undefined, - directives: [], + directives: undefined, types: [ typeNode('Wo', { start: 14, end: 16 }), typeNode('Rld', { start: 19, end: 22 }), @@ -924,7 +924,7 @@ describe('Schema Parser', () => { kind: 'UnionTypeDefinition', name: nameNode('Hello', { start: 6, end: 11 }), description: undefined, - directives: [], + directives: undefined, types: [ typeNode('Wo', { start: 16, end: 18 }), typeNode('Rld', { start: 21, end: 24 }), @@ -974,7 +974,7 @@ describe('Schema Parser', () => { kind: 'ScalarTypeDefinition', name: nameNode('Hello', { start: 7, end: 12 }), description: undefined, - directives: [], + directives: undefined, loc: { start: 0, end: 12 }, }, ], @@ -995,7 +995,7 @@ input Hello { kind: 'InputObjectTypeDefinition', name: nameNode('Hello', { start: 7, end: 12 }), description: undefined, - directives: [], + directives: undefined, fields: [ inputValueNode( nameNode('world', { start: 17, end: 22 }), @@ -1037,7 +1037,7 @@ input Hello { value: 'foo', loc: { start: 11, end: 14 }, }, - arguments: [], + arguments: undefined, repeatable: false, locations: [ { @@ -1073,7 +1073,7 @@ input Hello { value: 'foo', loc: { start: 11, end: 14 }, }, - arguments: [], + arguments: undefined, repeatable: true, locations: [ { diff --git a/src/language/parser.ts b/src/language/parser.ts index 521d55a1..5e70bcb9 100644 --- a/src/language/parser.ts +++ b/src/language/parser.ts @@ -319,8 +319,8 @@ export class Parser { kind: Kind.OPERATION_DEFINITION, operation: OperationTypeNode.QUERY, name: undefined, - variableDefinitions: [], - directives: [], + variableDefinitions: undefined, + directives: undefined, selectionSet: this.parseSelectionSet(), }); } @@ -359,7 +359,7 @@ export class Parser { /** * VariableDefinitions : ( VariableDefinition+ ) */ - parseVariableDefinitions(): Array { + parseVariableDefinitions(): Array | undefined { return this.optionalMany( TokenKind.PAREN_L, this.parseVariableDefinition, @@ -455,15 +455,15 @@ export class Parser { /** * Arguments[Const] : ( Argument[?Const]+ ) */ - parseArguments(isConst: true): Array; - parseArguments(isConst: boolean): Array; - parseArguments(isConst: boolean): Array { + parseArguments(isConst: true): Array | undefined; + parseArguments(isConst: boolean): Array | undefined; + parseArguments(isConst: boolean): Array | undefined { const item = isConst ? this.parseConstArgument : this.parseArgument; return this.optionalMany(TokenKind.PAREN_L, item, TokenKind.PAREN_R); } /* experimental */ - parseFragmentArguments(): Array { + parseFragmentArguments(): Array | undefined { const item = this.parseFragmentArgument; return this.optionalMany(TokenKind.PAREN_L, item, TokenKind.PAREN_R); } @@ -733,17 +733,20 @@ export class Parser { /** * Directives[Const] : Directive[?Const]+ */ - parseDirectives(isConst: true): Array; - parseDirectives(isConst: boolean): Array; - parseDirectives(isConst: boolean): Array { + parseDirectives(isConst: true): Array | undefined; + parseDirectives(isConst: boolean): Array | undefined; + parseDirectives(isConst: boolean): Array | undefined { const directives = []; while (this.peek(TokenKind.AT)) { directives.push(this.parseDirective(isConst)); } - return directives; + if (directives.length) { + return directives; + } + return undefined; } - parseConstDirectives(): Array { + parseConstDirectives(): Array | undefined { return this.parseDirectives(true); } @@ -904,10 +907,10 @@ export class Parser { * - implements `&`? NamedType * - ImplementsInterfaces & NamedType */ - parseImplementsInterfaces(): Array { + parseImplementsInterfaces(): Array | undefined { return this.expectOptionalKeyword('implements') ? this.delimitedMany(TokenKind.AMP, this.parseNamedType) - : []; + : undefined; } /** @@ -915,7 +918,7 @@ export class Parser { * FieldsDefinition : { FieldDefinition+ } * ``` */ - parseFieldsDefinition(): Array { + parseFieldsDefinition(): Array | undefined { return this.optionalMany( TokenKind.BRACE_L, this.parseFieldDefinition, @@ -948,7 +951,7 @@ export class Parser { /** * ArgumentsDefinition : ( InputValueDefinition+ ) */ - parseArgumentDefs(): Array { + parseArgumentDefs(): Array | undefined { return this.optionalMany( TokenKind.PAREN_L, this.parseInputValueDef, @@ -1028,10 +1031,10 @@ export class Parser { * - = `|`? NamedType * - UnionMemberTypes | NamedType */ - parseUnionMemberTypes(): Array { + parseUnionMemberTypes(): Array | undefined { return this.expectOptionalToken(TokenKind.EQUALS) ? this.delimitedMany(TokenKind.PIPE, this.parseNamedType) - : []; + : undefined; } /** @@ -1059,7 +1062,7 @@ export class Parser { * EnumValuesDefinition : { EnumValueDefinition+ } * ``` */ - parseEnumValuesDefinition(): Array { + parseEnumValuesDefinition(): Array | undefined { return this.optionalMany( TokenKind.BRACE_L, this.parseEnumValueDefinition, @@ -1128,7 +1131,7 @@ export class Parser { * InputFieldsDefinition : { InputValueDefinition+ } * ``` */ - parseInputFieldsDefinition(): Array { + parseInputFieldsDefinition(): Array | undefined { return this.optionalMany( TokenKind.BRACE_L, this.parseInputValueDef, @@ -1191,7 +1194,7 @@ export class Parser { this.parseOperationTypeDefinition, TokenKind.BRACE_R, ); - if (directives.length === 0 && operationTypes.length === 0) { + if (directives === undefined && operationTypes === undefined) { throw this.unexpected(); } return this.node(start, { @@ -1211,7 +1214,7 @@ export class Parser { this.expectKeyword('scalar'); const name = this.parseName(); const directives = this.parseConstDirectives(); - if (directives.length === 0) { + if (directives === undefined) { throw this.unexpected(); } return this.node(start, { @@ -1236,9 +1239,9 @@ export class Parser { const directives = this.parseConstDirectives(); const fields = this.parseFieldsDefinition(); if ( - interfaces.length === 0 && - directives.length === 0 && - fields.length === 0 + interfaces === undefined && + directives === undefined && + fields === undefined ) { throw this.unexpected(); } @@ -1266,9 +1269,9 @@ export class Parser { const directives = this.parseConstDirectives(); const fields = this.parseFieldsDefinition(); if ( - interfaces.length === 0 && - directives.length === 0 && - fields.length === 0 + interfaces === undefined && + directives === undefined && + fields === undefined ) { throw this.unexpected(); } @@ -1293,7 +1296,7 @@ export class Parser { const name = this.parseName(); const directives = this.parseConstDirectives(); const types = this.parseUnionMemberTypes(); - if (directives.length === 0 && types.length === 0) { + if (directives === undefined && types === undefined) { throw this.unexpected(); } return this.node(start, { @@ -1316,7 +1319,7 @@ export class Parser { const name = this.parseName(); const directives = this.parseConstDirectives(); const values = this.parseEnumValuesDefinition(); - if (directives.length === 0 && values.length === 0) { + if (directives === undefined && values === undefined) { throw this.unexpected(); } return this.node(start, { @@ -1339,7 +1342,7 @@ export class Parser { const name = this.parseName(); const directives = this.parseConstDirectives(); const fields = this.parseInputFieldsDefinition(); - if (directives.length === 0 && fields.length === 0) { + if (directives === undefined && fields === undefined) { throw this.unexpected(); } return this.node(start, { @@ -1550,7 +1553,7 @@ export class Parser { openKind: TokenKind, parseFn: () => T, closeKind: TokenKind, - ): Array { + ): Array | undefined { if (this.expectOptionalToken(openKind)) { const nodes = []; do { @@ -1558,7 +1561,7 @@ export class Parser { } while (!this.expectOptionalToken(closeKind)); return nodes; } - return []; + return undefined; } /** diff --git a/src/language/printer.ts b/src/language/printer.ts index c2858188..bf2959e5 100644 --- a/src/language/printer.ts +++ b/src/language/printer.ts @@ -358,8 +358,6 @@ function indent(str: string): string { } function hasMultilineItems(maybeArray: Maybe>): boolean { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ return maybeArray?.some((str) => str.includes('\n')) ?? false; } diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 3c2fe215..ca476359 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -1,4 +1,4 @@ -import { expect } from 'chai'; +import { assert, expect } from 'chai'; import { describe, it } from 'mocha'; import { dedent } from '../../__testUtils__/dedent.js'; @@ -807,6 +807,40 @@ describe('Type System: Union types must be valid', () => { ]); } }); + + it('rejects a Union type with non-Object members types with malformed AST', () => { + const schema = buildSchema(` + type Query { + test: BadUnion + } + + type TypeA { + field: String + } + + type TypeB { + field: String + } + + union BadUnion = + | TypeA + | String + | TypeB + `); + + const badUnionNode = (schema.getType('BadUnion') as GraphQLUnionType) + ?.astNode; + assert(badUnionNode); + /* @ts-expect-error */ + badUnionNode.types = undefined; + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Union type BadUnion can only include Object types, it cannot include String.', + }, + ]); + }); }); describe('Type System: Input Objects must have fields', () => { @@ -1209,6 +1243,35 @@ describe('Type System: Objects can only implement unique interfaces', () => { ]); }); + it('rejects an Object implementing a non-Interface type with malformed AST', () => { + const schema = buildSchema(` + type Query { + test: BadObject + } + + input SomeInputObject { + field: String + } + + type BadObject implements SomeInputObject { + field: String + } + `); + + const badObjectNode = (schema.getType('BadObject') as GraphQLObjectType) + ?.astNode; + assert(badObjectNode); + /* @ts-expect-error */ + badObjectNode.interfaces = undefined; + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Type BadObject must only implement Interface types, it cannot implement SomeInputObject.', + }, + ]); + }); + it('rejects an Object implementing the same interface twice', () => { const schema = buildSchema(` type Query { diff --git a/src/type/validate.ts b/src/type/validate.ts index 3a69a8c6..9fe05263 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -162,10 +162,7 @@ function getOperationTypeNode( operation: OperationTypeNode, ): Maybe { return [schema.astNode, ...schema.extensionASTNodes] - .flatMap( - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - (schemaNode) => /* c8 ignore next */ schemaNode?.operationTypes ?? [], - ) + .flatMap((schemaNode) => schemaNode?.operationTypes ?? []) .find((operationNode) => operationNode.operation === operation)?.type; } @@ -642,9 +639,8 @@ function getAllImplementsInterfaceNodes( | InterfaceTypeExtensionNode > = astNode != null ? [astNode, ...extensionASTNodes] : extensionASTNodes; - // FIXME: https://github.com/graphql/graphql-js/issues/2203 return nodes - .flatMap((typeNode) => /* c8 ignore next */ typeNode.interfaces ?? []) + .flatMap((typeNode) => typeNode.interfaces ?? []) .filter((ifaceNode) => ifaceNode.name.value === iface.name); } @@ -656,7 +652,6 @@ function getUnionMemberTypeNodes( const nodes: ReadonlyArray = astNode != null ? [astNode, ...extensionASTNodes] : extensionASTNodes; - // FIXME: https://github.com/graphql/graphql-js/issues/2203 return nodes .flatMap((unionNode) => /* c8 ignore next */ unionNode.types ?? []) .filter((typeNode) => typeNode.name.value === typeName); diff --git a/src/utilities/__tests__/buildASTSchema-test.ts b/src/utilities/__tests__/buildASTSchema-test.ts index e199abc2..5a794a12 100644 --- a/src/utilities/__tests__/buildASTSchema-test.ts +++ b/src/utilities/__tests__/buildASTSchema-test.ts @@ -339,7 +339,7 @@ describe('Schema Builder', () => { const definition = parse(sdl).definitions[0]; expect( definition.kind === 'InterfaceTypeDefinition' && definition.interfaces, - ).to.deep.equal([], 'The interfaces property must be an empty array.'); + ).to.deep.equal(undefined, 'The interfaces property must not be defined.'); expect(cycleSDL(sdl)).to.equal(sdl); }); diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index a886d31a..2a44ae62 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -639,7 +639,7 @@ describe('coerceInputLiteral', () => { parser.expectToken(TokenKind.SOF); const variableValuesOrErrors = getVariableValues( new GraphQLSchema({}), - parser.parseVariableDefinitions(), + parser.parseVariableDefinitions() ?? [], inputs, ); invariant(variableValuesOrErrors.variableValues !== undefined); @@ -926,6 +926,13 @@ describe('coerceInputLiteral', () => { requiredBool: true, }); test('{ requiredBool: $foo }', testInputObj, undefined); + testWithVariables( + '', + {}, + '{ requiredBool: $foo }', + testInputObj, + undefined, + ); testWithVariables( '($foo: Boolean)', { foo: true }, diff --git a/src/utilities/__tests__/replaceVariables-test.ts b/src/utilities/__tests__/replaceVariables-test.ts index 2ab97164..61b0780d 100644 --- a/src/utilities/__tests__/replaceVariables-test.ts +++ b/src/utilities/__tests__/replaceVariables-test.ts @@ -24,7 +24,7 @@ function testVariables(variableDefs: string, inputs: ReadOnlyObjMap) { parser.expectToken(TokenKind.SOF); const variableValuesOrErrors = getVariableValues( new GraphQLSchema({ types: [GraphQLInt] }), - parser.parseVariableDefinitions(), + parser.parseVariableDefinitions() ?? [], inputs, ); invariant(variableValuesOrErrors.variableValues !== undefined); diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index f43c1533..36e357b2 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -440,9 +440,7 @@ export function extendSchemaImpl( } { const opTypes = {}; for (const node of nodes) { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - const operationTypesNodes = - /* c8 ignore next */ node.operationTypes ?? []; + const operationTypesNodes = node.operationTypes ?? []; for (const operationType of operationTypesNodes) { // Note: While this could make early assertions to get the correctly @@ -498,8 +496,7 @@ export function extendSchemaImpl( ): GraphQLFieldConfigMap { const fieldConfigMap = Object.create(null); for (const node of nodes) { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - const nodeFields = /* c8 ignore next */ node.fields ?? []; + const nodeFields = node.fields ?? []; for (const field of nodeFields) { fieldConfigMap[field.name.value] = { @@ -520,8 +517,7 @@ export function extendSchemaImpl( function buildArgumentMap( args: Maybe>, ): GraphQLFieldConfigArgumentMap { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - const argsNodes = /* c8 ignore next */ args ?? []; + const argsNodes = args ?? []; const argConfigMap = Object.create(null); for (const arg of argsNodes) { @@ -548,8 +544,7 @@ export function extendSchemaImpl( ): GraphQLInputFieldConfigMap { const inputFieldMap = Object.create(null); for (const node of nodes) { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - const fieldsNodes = /* c8 ignore next */ node.fields ?? []; + const fieldsNodes = node.fields ?? []; for (const field of fieldsNodes) { // Note: While this could make assertions to get the correctly typed @@ -574,8 +569,7 @@ export function extendSchemaImpl( ): GraphQLEnumValueConfigMap { const enumValueMap = Object.create(null); for (const node of nodes) { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - const valuesNodes = /* c8 ignore next */ node.values ?? []; + const valuesNodes = node.values ?? []; for (const value of valuesNodes) { enumValueMap[value.name.value] = { @@ -600,10 +594,7 @@ export function extendSchemaImpl( // values below, that would throw immediately while type system // validation with validateSchema() will produce more actionable results. // @ts-expect-error - return nodes.flatMap( - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - (node) => /* c8 ignore next */ node.interfaces?.map(getNamedType) ?? [], - ); + return nodes.flatMap((node) => node.interfaces?.map(getNamedType) ?? []); } function buildUnionTypes( @@ -613,10 +604,7 @@ export function extendSchemaImpl( // values below, that would throw immediately while type system // validation with validateSchema() will produce more actionable results. // @ts-expect-error - return nodes.flatMap( - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - (node) => /* c8 ignore next */ node.types?.map(getNamedType) ?? [], - ); + return nodes.flatMap((node) => node.types?.map(getNamedType) ?? []); } function buildType(astNode: TypeDefinitionNode): GraphQLNamedType { diff --git a/src/validation/rules/KnownArgumentNamesRule.ts b/src/validation/rules/KnownArgumentNamesRule.ts index cbf3e94b..8db00b14 100644 --- a/src/validation/rules/KnownArgumentNamesRule.ts +++ b/src/validation/rules/KnownArgumentNamesRule.ts @@ -99,8 +99,6 @@ export function KnownArgumentNamesOnDirectivesRule( const astDefinitions = context.getDocument().definitions; for (const def of astDefinitions) { if (def.kind === Kind.DIRECTIVE_DEFINITION) { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const argsNodes = def.arguments ?? []; directiveArgs.set( diff --git a/src/validation/rules/NoUnusedVariablesRule.ts b/src/validation/rules/NoUnusedVariablesRule.ts index e55e1c16..6d0bd087 100644 --- a/src/validation/rules/NoUnusedVariablesRule.ts +++ b/src/validation/rules/NoUnusedVariablesRule.ts @@ -19,8 +19,6 @@ export function NoUnusedVariablesRule(context: ValidationContext): ASTVisitor { const argumentNameUsed = new Set( usages.map(({ node }) => node.name.value), ); - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const variableDefinitions = fragment.variableDefinitions ?? []; for (const varDef of variableDefinitions) { const argName = varDef.variable.name.value; @@ -44,8 +42,6 @@ export function NoUnusedVariablesRule(context: ValidationContext): ASTVisitor { } } - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const variableDefinitions = operation.variableDefinitions ?? []; for (const variableDef of variableDefinitions) { const variableName = variableDef.variable.name.value; diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index dae909c6..c74ecc7f 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -705,9 +705,8 @@ function findConflict( } } - // FIXME https://github.com/graphql/graphql-js/issues/2203 - const directives1 = /* c8 ignore next */ node1.directives ?? []; - const directives2 = /* c8 ignore next */ node2.directives ?? []; + const directives1 = node1.directives ?? []; + const directives2 = node2.directives ?? []; if (!sameStreams(directives1, varMap1, directives2, varMap2)) { return [ [responseName, 'they have differing stream directives'], diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.ts b/src/validation/rules/ProvidedRequiredArgumentsRule.ts index 72737746..5576b4f2 100644 --- a/src/validation/rules/ProvidedRequiredArgumentsRule.ts +++ b/src/validation/rules/ProvidedRequiredArgumentsRule.ts @@ -47,8 +47,6 @@ export function ProvidedRequiredArgumentsRule( } const providedArgs = new Set( - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ fieldNode.arguments?.map((arg) => arg.name.value), ); for (const argDef of fieldDef.args) { @@ -83,8 +81,6 @@ export function ProvidedRequiredArgumentsRule( } const providedArgs = new Set( - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ spreadNode.arguments?.map((arg) => arg.name.value), ); for (const [ @@ -138,8 +134,6 @@ export function ProvidedRequiredArgumentsOnDirectivesRule( const astDefinitions = context.getDocument().definitions; for (const def of astDefinitions) { if (def.kind === Kind.DIRECTIVE_DEFINITION) { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const argNodes = def.arguments ?? []; requiredArgsMap.set( @@ -160,8 +154,6 @@ export function ProvidedRequiredArgumentsOnDirectivesRule( const directiveName = directiveNode.name.value; const requiredArgs = requiredArgsMap.get(directiveName); if (requiredArgs != null) { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const argNodes = directiveNode.arguments ?? []; const argNodeMap = new Set(argNodes.map((arg) => arg.name.value)); for (const [argName, argDef] of requiredArgs.entries()) { diff --git a/src/validation/rules/UniqueArgumentDefinitionNamesRule.ts b/src/validation/rules/UniqueArgumentDefinitionNamesRule.ts index 9904477b..ada4fe94 100644 --- a/src/validation/rules/UniqueArgumentDefinitionNamesRule.ts +++ b/src/validation/rules/UniqueArgumentDefinitionNamesRule.ts @@ -22,8 +22,6 @@ export function UniqueArgumentDefinitionNamesRule( ): ASTVisitor { return { DirectiveDefinition(directiveNode) { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const argumentNodes = directiveNode.arguments ?? []; return checkArgUniqueness(`@${directiveNode.name.value}`, argumentNodes); @@ -40,15 +38,11 @@ export function UniqueArgumentDefinitionNamesRule( }) { const typeName = typeNode.name.value; - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const fieldNodes = typeNode.fields ?? []; for (const fieldDef of fieldNodes) { const fieldName = fieldDef.name.value; - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const argumentNodes = fieldDef.arguments ?? []; checkArgUniqueness(`${typeName}.${fieldName}`, argumentNodes); diff --git a/src/validation/rules/UniqueArgumentNamesRule.ts b/src/validation/rules/UniqueArgumentNamesRule.ts index 2baba71c..c4305e6e 100644 --- a/src/validation/rules/UniqueArgumentNamesRule.ts +++ b/src/validation/rules/UniqueArgumentNamesRule.ts @@ -26,8 +26,6 @@ export function UniqueArgumentNamesRule( function checkArgUniqueness(parentNode: { arguments?: ReadonlyArray | undefined; }) { - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const argumentNodes = parentNode.arguments ?? []; const seenArgs = groupBy(argumentNodes, (arg) => arg.name.value); diff --git a/src/validation/rules/UniqueEnumValueNamesRule.ts b/src/validation/rules/UniqueEnumValueNamesRule.ts index 8342880f..74d8b651 100644 --- a/src/validation/rules/UniqueEnumValueNamesRule.ts +++ b/src/validation/rules/UniqueEnumValueNamesRule.ts @@ -39,8 +39,6 @@ export function UniqueEnumValueNamesRule( knownValueNames.set(typeName, valueNames); } - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const valueNodes = node.values ?? []; for (const valueDef of valueNodes) { diff --git a/src/validation/rules/UniqueFieldDefinitionNamesRule.ts b/src/validation/rules/UniqueFieldDefinitionNamesRule.ts index e776bb4d..cf8e6186 100644 --- a/src/validation/rules/UniqueFieldDefinitionNamesRule.ts +++ b/src/validation/rules/UniqueFieldDefinitionNamesRule.ts @@ -51,8 +51,6 @@ export function UniqueFieldDefinitionNamesRule( knownFieldNames.set(typeName, fieldNames); } - // FIXME: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const fieldNodes = node.fields ?? []; for (const fieldDef of fieldNodes) { diff --git a/src/validation/rules/UniqueOperationTypesRule.ts b/src/validation/rules/UniqueOperationTypesRule.ts index 51e4cfe6..cf4c191e 100644 --- a/src/validation/rules/UniqueOperationTypesRule.ts +++ b/src/validation/rules/UniqueOperationTypesRule.ts @@ -35,8 +35,6 @@ export function UniqueOperationTypesRule( function checkOperationTypes( node: SchemaDefinitionNode | SchemaExtensionNode, ) { - // See: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const operationTypesNodes = node.operationTypes ?? []; for (const operationType of operationTypesNodes) { diff --git a/src/validation/rules/UniqueVariableNamesRule.ts b/src/validation/rules/UniqueVariableNamesRule.ts index 8c067263..509fefbe 100644 --- a/src/validation/rules/UniqueVariableNamesRule.ts +++ b/src/validation/rules/UniqueVariableNamesRule.ts @@ -16,8 +16,6 @@ export function UniqueVariableNamesRule( ): ASTVisitor { return { OperationDefinition(operationNode) { - // See: https://github.com/graphql/graphql-js/issues/2203 - /* c8 ignore next */ const variableDefinitions = operationNode.variableDefinitions ?? []; const seenVariableDefinitions = groupBy(