Skip to content

Commit 3eba0c8

Browse files
CitoyaacovCR
authored andcommitted
Improve error reporting for union member types
First make sure that the included type is an object type, because that would be the more severe problem. Only then we can be sure that the type has a name and check for duplicates. Also, create only one error per different non-object type.
1 parent b09b295 commit 3eba0c8

File tree

2 files changed

+55
-8
lines changed

2 files changed

+55
-8
lines changed

src/type/__tests__/validation-test.ts

+42-1
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,7 @@ describe('Type System: Union types must be valid', () => {
747747
]);
748748
});
749749

750-
it('rejects a Union type with non-Object members types', () => {
750+
it('rejects a Union type with non-Object member types', () => {
751751
let schema = buildSchema(`
752752
type Query {
753753
test: BadUnion
@@ -807,6 +807,47 @@ describe('Type System: Union types must be valid', () => {
807807
]);
808808
}
809809
});
810+
811+
it('rejects a Union type with duplicate non-Object member types', () => {
812+
let schema = buildSchema(`
813+
type Query {
814+
test: BadUnion
815+
}
816+
817+
type TypeA {
818+
field: String
819+
}
820+
821+
union BadUnion =
822+
| Int
823+
| String
824+
| TypeA
825+
| Int
826+
| String
827+
`);
828+
829+
schema = extendSchema(schema, parse('extend union BadUnion = Int'));
830+
831+
expect(validateSchema(schema)).to.deep.equal([
832+
{
833+
message:
834+
'Union type BadUnion can only include Object types, it cannot include Int.',
835+
locations: [
836+
{ line: 11, column: 11 },
837+
{ line: 14, column: 11 },
838+
{ line: 1, column: 25 },
839+
],
840+
},
841+
{
842+
message:
843+
'Union type BadUnion can only include Object types, it cannot include String.',
844+
locations: [
845+
{ line: 12, column: 11 },
846+
{ line: 15, column: 11 },
847+
],
848+
},
849+
]);
850+
});
810851
});
811852

812853
describe('Type System: Input Objects must have fields', () => {

src/type/validate.ts

+13-7
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,20 @@ function validateUnionMembers(
480480
}
481481

482482
const includedTypeNames = new Set<string>();
483+
const includedInvalidTypes = new Set<string>();
483484
for (const memberType of memberTypes) {
485+
if (!isObjectType(memberType)) {
486+
const typeString = inspect(memberType);
487+
if (!includedInvalidTypes.has(typeString)) {
488+
context.reportError(
489+
`Union type ${union.name} can only include Object types, ` +
490+
`it cannot include ${inspect(memberType)}.`,
491+
getUnionMemberTypeNodes(union, String(memberType)),
492+
);
493+
includedInvalidTypes.add(typeString);
494+
}
495+
continue;
496+
}
484497
if (includedTypeNames.has(memberType.name)) {
485498
context.reportError(
486499
`Union type ${union.name} can only include type ${memberType} once.`,
@@ -489,13 +502,6 @@ function validateUnionMembers(
489502
continue;
490503
}
491504
includedTypeNames.add(memberType.name);
492-
if (!isObjectType(memberType)) {
493-
context.reportError(
494-
`Union type ${union.name} can only include Object types, ` +
495-
`it cannot include ${inspect(memberType)}.`,
496-
getUnionMemberTypeNodes(union, String(memberType)),
497-
);
498-
}
499505
}
500506
}
501507

0 commit comments

Comments
 (0)