Skip to content

Commit dea5a3a

Browse files
yaacovCRyaacovCR
committed
use the change in this PR to close 2203
1 parent 11b076c commit dea5a3a

16 files changed

+75
-62
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/extendSchema.ts‎

Lines changed: 7 additions & 15 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] = {
@@ -601,8 +595,7 @@ export function extendSchemaImpl(
601595
// validation with validateSchema() will produce more actionable results.
602596
// @ts-expect-error
603597
return nodes.flatMap(
604-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
605-
(node) => /* c8 ignore next */ node.interfaces?.map(getNamedType) ?? [],
598+
(node) => node.interfaces?.map(getNamedType) ?? [],
606599
);
607600
}
608601

@@ -614,8 +607,7 @@ export function extendSchemaImpl(
614607
// validation with validateSchema() will produce more actionable results.
615608
// @ts-expect-error
616609
return nodes.flatMap(
617-
// FIXME: https://github.com/graphql/graphql-js/issues/2203
618-
(node) => /* c8 ignore next */ node.types?.map(getNamedType) ?? [],
610+
(node) => node.types?.map(getNamedType) ?? [],
619611
);
620612
}
621613

‎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()) {

0 commit comments

Comments
 (0)