Skip to content

Commit 2ca0fda

Browse files
committed
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 688f93c commit 2ca0fda

File tree

2 files changed

+55
-8
lines changed

2 files changed

+55
-8
lines changed

src/type/__tests__/validation-test.js

+42-1
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ describe('Type System: Union types must be valid', () => {
635635
]);
636636
});
637637

638-
it('rejects a Union type with non-Object members types', () => {
638+
it('rejects a Union type with non-Object member types', () => {
639639
let schema = buildSchema(`
640640
type Query {
641641
test: BadUnion
@@ -695,6 +695,47 @@ describe('Type System: Union types must be valid', () => {
695695
]);
696696
}
697697
});
698+
699+
it('rejects a Union type with duplicate non-Object member types', () => {
700+
let schema = buildSchema(`
701+
type Query {
702+
test: BadUnion
703+
}
704+
705+
type TypeA {
706+
field: String
707+
}
708+
709+
union BadUnion =
710+
| Int
711+
| String
712+
| TypeA
713+
| Int
714+
| String
715+
`);
716+
717+
schema = extendSchema(schema, parse('extend union BadUnion = Int'));
718+
719+
expect(validateSchema(schema)).to.deep.equal([
720+
{
721+
message:
722+
'Union type BadUnion can only include Object types, it cannot include Int.',
723+
locations: [
724+
{ line: 11, column: 11 },
725+
{ line: 14, column: 11 },
726+
{ line: 1, column: 25 },
727+
],
728+
},
729+
{
730+
message:
731+
'Union type BadUnion can only include Object types, it cannot include String.',
732+
locations: [
733+
{ line: 12, column: 11 },
734+
{ line: 15, column: 11 },
735+
],
736+
},
737+
]);
738+
});
698739
});
699740

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

src/type/validate.js

+13-7
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,20 @@ function validateUnionMembers(
438438
}
439439

440440
const includedTypeNames = Object.create(null);
441+
const includedInvalidTypes = Object.create(null);
441442
for (const memberType of memberTypes) {
443+
if (!isObjectType(memberType)) {
444+
const typeString = inspect(memberType);
445+
if (!includedInvalidTypes[typeString]) {
446+
context.reportError(
447+
`Union type ${union.name} can only include Object types, ` +
448+
`it cannot include ${typeString}.`,
449+
getUnionMemberTypeNodes(union, String(memberType)),
450+
);
451+
includedInvalidTypes[typeString] = true;
452+
}
453+
continue;
454+
}
442455
if (includedTypeNames[memberType.name]) {
443456
context.reportError(
444457
`Union type ${union.name} can only include type ${memberType.name} once.`,
@@ -447,13 +460,6 @@ function validateUnionMembers(
447460
continue;
448461
}
449462
includedTypeNames[memberType.name] = true;
450-
if (!isObjectType(memberType)) {
451-
context.reportError(
452-
`Union type ${union.name} can only include Object types, ` +
453-
`it cannot include ${inspect(memberType)}.`,
454-
getUnionMemberTypeNodes(union, String(memberType)),
455-
);
456-
}
457463
}
458464
}
459465

0 commit comments

Comments
 (0)