Skip to content

Commit 125c2b7

Browse files
yaacovCRyaacovCR
committed
use the change in this PR to close 2203
1 parent 513edb4 commit 125c2b7

18 files changed

+84
-68
lines changed

‎src/execution/execute.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,6 @@ export function validateExecutionArgs(
561561
return [new GraphQLError('Must provide an operation.')];
562562
}
563563

564-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
565-
/* c8 ignore next */
566564
const variableDefinitions = operation.variableDefinitions ?? [];
567565
const hideSuggestions = args.hideSuggestions ?? false;
568566

‎src/execution/values.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,6 @@ export function experimentalGetArgumentValues(
224224
): { [argument: string]: unknown } {
225225
const coercedValues: { [argument: string]: unknown } = {};
226226

227-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
228-
/* c8 ignore next */
229227
const argumentNodes = node.arguments ?? [];
230228
const argNodeMap = new Map(argumentNodes.map((arg) => [arg.name.value, arg]));
231229

‎src/language/printer.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,6 @@ function indent(str: string): string {
358358
}
359359

360360
function hasMultilineItems(maybeArray: Maybe<ReadonlyArray<string>>): boolean {
361-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
362-
/* c8 ignore next */
363361
return maybeArray?.some((str) => str.includes('\n')) ?? false;
364362
}
365363

‎src/type/__tests__/validation-test.ts

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { expect } from 'chai';
1+
import { assert, expect } from 'chai';
22
import { describe, it } from 'mocha';
33

44
import { dedent } from '../../__testUtils__/dedent.js';
@@ -807,6 +807,40 @@ describe('Type System: Union types must be valid', () => {
807807
]);
808808
}
809809
});
810+
811+
it('rejects a Union type with non-Object members types with malformed AST', () => {
812+
const schema = buildSchema(`
813+
type Query {
814+
test: BadUnion
815+
}
816+
817+
type TypeA {
818+
field: String
819+
}
820+
821+
type TypeB {
822+
field: String
823+
}
824+
825+
union BadUnion =
826+
| TypeA
827+
| String
828+
| TypeB
829+
`);
830+
831+
const badUnionNode = (schema.getType('BadUnion') as GraphQLUnionType)
832+
?.astNode;
833+
assert(badUnionNode);
834+
/* @ts-expect-error */
835+
badUnionNode.types = undefined;
836+
837+
expectJSON(validateSchema(schema)).toDeepEqual([
838+
{
839+
message:
840+
'Union type BadUnion can only include Object types, it cannot include String.',
841+
},
842+
]);
843+
});
810844
});
811845

812846
describe('Type System: Input Objects must have fields', () => {
@@ -1209,6 +1243,35 @@ describe('Type System: Objects can only implement unique interfaces', () => {
12091243
]);
12101244
});
12111245

1246+
it('rejects an Object implementing a non-Interface type with malformed AST', () => {
1247+
const schema = buildSchema(`
1248+
type Query {
1249+
test: BadObject
1250+
}
1251+
1252+
input SomeInputObject {
1253+
field: String
1254+
}
1255+
1256+
type BadObject implements SomeInputObject {
1257+
field: String
1258+
}
1259+
`);
1260+
1261+
const badObjectNode = (schema.getType('BadObject') as GraphQLObjectType)
1262+
?.astNode;
1263+
assert(badObjectNode);
1264+
/* @ts-expect-error */
1265+
badObjectNode.interfaces = undefined;
1266+
1267+
expectJSON(validateSchema(schema)).toDeepEqual([
1268+
{
1269+
message:
1270+
'Type BadObject must only implement Interface types, it cannot implement SomeInputObject.',
1271+
},
1272+
]);
1273+
});
1274+
12121275
it('rejects an Object implementing the same interface twice', () => {
12131276
const schema = buildSchema(`
12141277
type Query {

‎src/type/validate.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,7 @@ function getOperationTypeNode(
162162
operation: OperationTypeNode,
163163
): Maybe<ASTNode> {
164164
return [schema.astNode, ...schema.extensionASTNodes]
165-
.flatMap(
166-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
167-
(schemaNode) => /* c8 ignore next */ schemaNode?.operationTypes ?? [],
168-
)
165+
.flatMap((schemaNode) => schemaNode?.operationTypes ?? [])
169166
.find((operationNode) => operationNode.operation === operation)?.type;
170167
}
171168

@@ -642,9 +639,8 @@ function getAllImplementsInterfaceNodes(
642639
| InterfaceTypeExtensionNode
643640
> = astNode != null ? [astNode, ...extensionASTNodes] : extensionASTNodes;
644641

645-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
646642
return nodes
647-
.flatMap((typeNode) => /* c8 ignore next */ typeNode.interfaces ?? [])
643+
.flatMap((typeNode) => typeNode.interfaces ?? [])
648644
.filter((ifaceNode) => ifaceNode.name.value === iface.name);
649645
}
650646

@@ -656,7 +652,6 @@ function getUnionMemberTypeNodes(
656652
const nodes: ReadonlyArray<UnionTypeDefinitionNode | UnionTypeExtensionNode> =
657653
astNode != null ? [astNode, ...extensionASTNodes] : extensionASTNodes;
658654

659-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
660655
return nodes
661656
.flatMap((unionNode) => /* c8 ignore next */ unionNode.types ?? [])
662657
.filter((typeNode) => typeNode.name.value === typeName);

‎src/utilities/__tests__/coerceInputValue-test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ describe('coerceInputLiteral', () => {
639639
parser.expectToken(TokenKind.SOF);
640640
const variableValuesOrErrors = getVariableValues(
641641
new GraphQLSchema({}),
642-
parser.parseVariableDefinitions(),
642+
parser.parseVariableDefinitions() ?? [],
643643
inputs,
644644
);
645645
invariant(variableValuesOrErrors.variableValues !== undefined);
@@ -926,6 +926,13 @@ describe('coerceInputLiteral', () => {
926926
requiredBool: true,
927927
});
928928
test('{ requiredBool: $foo }', testInputObj, undefined);
929+
testWithVariables(
930+
'',
931+
{},
932+
'{ requiredBool: $foo }',
933+
testInputObj,
934+
undefined,
935+
);
929936
testWithVariables(
930937
'($foo: Boolean)',
931938
{ foo: true },

‎src/utilities/__tests__/replaceVariables-test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function testVariables(variableDefs: string, inputs: ReadOnlyObjMap<unknown>) {
2424
parser.expectToken(TokenKind.SOF);
2525
const variableValuesOrErrors = getVariableValues(
2626
new GraphQLSchema({ types: [GraphQLInt] }),
27-
parser.parseVariableDefinitions(),
27+
parser.parseVariableDefinitions() ?? [],
2828
inputs,
2929
);
3030
invariant(variableValuesOrErrors.variableValues !== undefined);

‎src/utilities/extendSchema.ts

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -440,9 +440,7 @@ export function extendSchemaImpl(
440440
} {
441441
const opTypes = {};
442442
for (const node of nodes) {
443-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
444-
const operationTypesNodes =
445-
/* c8 ignore next */ node.operationTypes ?? [];
443+
const operationTypesNodes = node.operationTypes ?? [];
446444

447445
for (const operationType of operationTypesNodes) {
448446
// Note: While this could make early assertions to get the correctly
@@ -498,8 +496,7 @@ export function extendSchemaImpl(
498496
): GraphQLFieldConfigMap<unknown, unknown> {
499497
const fieldConfigMap = Object.create(null);
500498
for (const node of nodes) {
501-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
502-
const nodeFields = /* c8 ignore next */ node.fields ?? [];
499+
const nodeFields = node.fields ?? [];
503500

504501
for (const field of nodeFields) {
505502
fieldConfigMap[field.name.value] = {
@@ -520,8 +517,7 @@ export function extendSchemaImpl(
520517
function buildArgumentMap(
521518
args: Maybe<ReadonlyArray<InputValueDefinitionNode>>,
522519
): GraphQLFieldConfigArgumentMap {
523-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
524-
const argsNodes = /* c8 ignore next */ args ?? [];
520+
const argsNodes = args ?? [];
525521

526522
const argConfigMap = Object.create(null);
527523
for (const arg of argsNodes) {
@@ -548,8 +544,7 @@ export function extendSchemaImpl(
548544
): GraphQLInputFieldConfigMap {
549545
const inputFieldMap = Object.create(null);
550546
for (const node of nodes) {
551-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
552-
const fieldsNodes = /* c8 ignore next */ node.fields ?? [];
547+
const fieldsNodes = node.fields ?? [];
553548

554549
for (const field of fieldsNodes) {
555550
// Note: While this could make assertions to get the correctly typed
@@ -574,8 +569,7 @@ export function extendSchemaImpl(
574569
): GraphQLEnumValueConfigMap {
575570
const enumValueMap = Object.create(null);
576571
for (const node of nodes) {
577-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
578-
const valuesNodes = /* c8 ignore next */ node.values ?? [];
572+
const valuesNodes = node.values ?? [];
579573

580574
for (const value of valuesNodes) {
581575
enumValueMap[value.name.value] = {
@@ -600,10 +594,7 @@ export function extendSchemaImpl(
600594
// values below, that would throw immediately while type system
601595
// validation with validateSchema() will produce more actionable results.
602596
// @ts-expect-error
603-
return nodes.flatMap(
604-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
605-
(node) => /* c8 ignore next */ node.interfaces?.map(getNamedType) ?? [],
606-
);
597+
return nodes.flatMap((node) => node.interfaces?.map(getNamedType) ?? []);
607598
}
608599

609600
function buildUnionTypes(
@@ -613,10 +604,7 @@ export function extendSchemaImpl(
613604
// values below, that would throw immediately while type system
614605
// validation with validateSchema() will produce more actionable results.
615606
// @ts-expect-error
616-
return nodes.flatMap(
617-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
618-
(node) => /* c8 ignore next */ node.types?.map(getNamedType) ?? [],
619-
);
607+
return nodes.flatMap((node) => node.types?.map(getNamedType) ?? []);
620608
}
621609

622610
function buildType(astNode: TypeDefinitionNode): GraphQLNamedType {

‎src/validation/rules/KnownArgumentNamesRule.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,6 @@ export function KnownArgumentNamesOnDirectivesRule(
9999
const astDefinitions = context.getDocument().definitions;
100100
for (const def of astDefinitions) {
101101
if (def.kind === Kind.DIRECTIVE_DEFINITION) {
102-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
103-
/* c8 ignore next */
104102
const argsNodes = def.arguments ?? [];
105103

106104
directiveArgs.set(

‎src/validation/rules/NoUnusedVariablesRule.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ export function NoUnusedVariablesRule(context: ValidationContext): ASTVisitor {
1919
const argumentNameUsed = new Set<string>(
2020
usages.map(({ node }) => node.name.value),
2121
);
22-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
23-
/* c8 ignore next */
2422
const variableDefinitions = fragment.variableDefinitions ?? [];
2523
for (const varDef of variableDefinitions) {
2624
const argName = varDef.variable.name.value;
@@ -44,8 +42,6 @@ export function NoUnusedVariablesRule(context: ValidationContext): ASTVisitor {
4442
}
4543
}
4644

47-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
48-
/* c8 ignore next */
4945
const variableDefinitions = operation.variableDefinitions ?? [];
5046
for (const variableDef of variableDefinitions) {
5147
const variableName = variableDef.variable.name.value;

‎src/validation/rules/OverlappingFieldsCanBeMergedRule.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -705,9 +705,8 @@ function findConflict(
705705
}
706706
}
707707

708-
// FIXME https://github.com/graphql/graphql-js/issues/2203
709-
const directives1 = /* c8 ignore next */ node1.directives ?? [];
710-
const directives2 = /* c8 ignore next */ node2.directives ?? [];
708+
const directives1 = node1.directives ?? [];
709+
const directives2 = node2.directives ?? [];
711710
if (!sameStreams(directives1, varMap1, directives2, varMap2)) {
712711
return [
713712
[responseName, 'they have differing stream directives'],

‎src/validation/rules/ProvidedRequiredArgumentsRule.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ export function ProvidedRequiredArgumentsRule(
4747
}
4848

4949
const providedArgs = new Set(
50-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
51-
/* c8 ignore next */
5250
fieldNode.arguments?.map((arg) => arg.name.value),
5351
);
5452
for (const argDef of fieldDef.args) {
@@ -83,8 +81,6 @@ export function ProvidedRequiredArgumentsRule(
8381
}
8482

8583
const providedArgs = new Set(
86-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
87-
/* c8 ignore next */
8884
spreadNode.arguments?.map((arg) => arg.name.value),
8985
);
9086
for (const [
@@ -138,8 +134,6 @@ export function ProvidedRequiredArgumentsOnDirectivesRule(
138134
const astDefinitions = context.getDocument().definitions;
139135
for (const def of astDefinitions) {
140136
if (def.kind === Kind.DIRECTIVE_DEFINITION) {
141-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
142-
/* c8 ignore next */
143137
const argNodes = def.arguments ?? [];
144138

145139
requiredArgsMap.set(
@@ -160,8 +154,6 @@ export function ProvidedRequiredArgumentsOnDirectivesRule(
160154
const directiveName = directiveNode.name.value;
161155
const requiredArgs = requiredArgsMap.get(directiveName);
162156
if (requiredArgs != null) {
163-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
164-
/* c8 ignore next */
165157
const argNodes = directiveNode.arguments ?? [];
166158
const argNodeMap = new Set(argNodes.map((arg) => arg.name.value));
167159
for (const [argName, argDef] of requiredArgs.entries()) {

‎src/validation/rules/UniqueArgumentDefinitionNamesRule.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ export function UniqueArgumentDefinitionNamesRule(
2222
): ASTVisitor {
2323
return {
2424
DirectiveDefinition(directiveNode) {
25-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
26-
/* c8 ignore next */
2725
const argumentNodes = directiveNode.arguments ?? [];
2826

2927
return checkArgUniqueness(`@${directiveNode.name.value}`, argumentNodes);
@@ -40,15 +38,11 @@ export function UniqueArgumentDefinitionNamesRule(
4038
}) {
4139
const typeName = typeNode.name.value;
4240

43-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
44-
/* c8 ignore next */
4541
const fieldNodes = typeNode.fields ?? [];
4642

4743
for (const fieldDef of fieldNodes) {
4844
const fieldName = fieldDef.name.value;
4945

50-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
51-
/* c8 ignore next */
5246
const argumentNodes = fieldDef.arguments ?? [];
5347

5448
checkArgUniqueness(`${typeName}.${fieldName}`, argumentNodes);

‎src/validation/rules/UniqueArgumentNamesRule.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ export function UniqueArgumentNamesRule(
2626
function checkArgUniqueness(parentNode: {
2727
arguments?: ReadonlyArray<ArgumentNode> | undefined;
2828
}) {
29-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
30-
/* c8 ignore next */
3129
const argumentNodes = parentNode.arguments ?? [];
3230

3331
const seenArgs = groupBy(argumentNodes, (arg) => arg.name.value);

‎src/validation/rules/UniqueEnumValueNamesRule.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ export function UniqueEnumValueNamesRule(
3939
knownValueNames.set(typeName, valueNames);
4040
}
4141

42-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
43-
/* c8 ignore next */
4442
const valueNodes = node.values ?? [];
4543

4644
for (const valueDef of valueNodes) {

‎src/validation/rules/UniqueFieldDefinitionNamesRule.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@ export function UniqueFieldDefinitionNamesRule(
5151
knownFieldNames.set(typeName, fieldNames);
5252
}
5353

54-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
55-
/* c8 ignore next */
5654
const fieldNodes = node.fields ?? [];
5755

5856
for (const fieldDef of fieldNodes) {

‎src/validation/rules/UniqueOperationTypesRule.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ export function UniqueOperationTypesRule(
3535
function checkOperationTypes(
3636
node: SchemaDefinitionNode | SchemaExtensionNode,
3737
) {
38-
// See: https://github.com/graphql/graphql-js/issues/2203
39-
/* c8 ignore next */
4038
const operationTypesNodes = node.operationTypes ?? [];
4139

4240
for (const operationType of operationTypesNodes) {

‎src/validation/rules/UniqueVariableNamesRule.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ export function UniqueVariableNamesRule(
1616
): ASTVisitor {
1717
return {
1818
OperationDefinition(operationNode) {
19-
// See: https://github.com/graphql/graphql-js/issues/2203
20-
/* c8 ignore next */
2119
const variableDefinitions = operationNode.variableDefinitions ?? [];
2220

2321
const seenVariableDefinitions = groupBy(

0 commit comments

Comments
 (0)