Skip to content

Excess property checks for discriminated unions #16363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Oct 6, 2017
38 changes: 31 additions & 7 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9062,6 +9062,13 @@ namespace ts {
(isTypeSubsetOf(globalObjectType, target) || (!isComparingJsxAttributes && isEmptyObjectType(target)))) {
return false;
}
if (target.flags & TypeFlags.Union) {
const discriminantType = findMatchingDiscriminantType(source, target as UnionType);
if (discriminantType) {
// check excess properties against discriminant type only, not the entire union
return hasExcessProperties(source, discriminantType, reportErrors);
}
}
for (const prop of getPropertiesOfObjectType(source)) {
if (!isKnownProperty(target, prop.escapedName, isComparingJsxAttributes)) {
if (reportErrors) {
Expand Down Expand Up @@ -9138,20 +9145,24 @@ namespace ts {
}

function findMatchingDiscriminantType(source: Type, target: UnionOrIntersectionType) {
let match: Type;
const sourceProperties = getPropertiesOfObjectType(source);
if (sourceProperties) {
for (const sourceProperty of sourceProperties) {
if (isDiscriminantProperty(target, sourceProperty.escapedName)) {
const sourceType = getTypeOfSymbol(sourceProperty);
for (const type of target.types) {
const targetType = getTypeOfPropertyOfType(type, sourceProperty.escapedName);
if (targetType && isRelatedTo(sourceType, targetType)) {
return type;
const sourceProperty = findSingleDiscriminantProperty(sourceProperties, target);
if (sourceProperty) {
const sourceType = getTypeOfSymbol(sourceProperty);
for (const type of target.types) {
const targetType = getTypeOfPropertyOfType(type, sourceProperty.escapedName);
if (targetType && isRelatedTo(sourceType, targetType)) {
if (match) {
return undefined;
}
match = type;
}
}
}
}
return match;
}

function typeRelatedToEachType(source: Type, target: IntersectionType, reportErrors: boolean): Ternary {
Expand Down Expand Up @@ -11154,6 +11165,19 @@ namespace ts {
return false;
}

function findSingleDiscriminantProperty(sourceProperties: Symbol[], target: Type): Symbol | undefined {
let result: Symbol;
for (const sourceProperty of sourceProperties) {
if (isDiscriminantProperty(target, sourceProperty.escapedName)) {
if (result) {
return undefined;
}
result = sourceProperty;
}
}
return result;
}

function isOrContainsMatchingReference(source: Node, target: Node) {
return isMatchingReference(source, target) || containsMatchingReference(source, target);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
tests/cases/compiler/discriminatedUnionErrorMessage.ts(8,5): error TS2322: Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Shape'.
Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Square'.
Property 'size' is missing in type '{ kind: "sq"; x: number; y: number; }'.
tests/cases/compiler/discriminatedUnionErrorMessage.ts(10,5): error TS2322: Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Shape'.
Object literal may only specify known properties, and 'x' does not exist in type 'Square'.


==== tests/cases/compiler/discriminatedUnionErrorMessage.ts (1 errors) ====
Expand All @@ -12,12 +11,11 @@ tests/cases/compiler/discriminatedUnionErrorMessage.ts(8,5): error TS2322: Type
| Rectangle
| Circle;
let shape: Shape = {
~~~~~
!!! error TS2322: Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Shape'.
!!! error TS2322: Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Square'.
!!! error TS2322: Property 'size' is missing in type '{ kind: "sq"; x: number; y: number; }'.
kind: "sq",
x: 12,
~~~~~
!!! error TS2322: Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Shape'.
!!! error TS2322: Object literal may only specify known properties, and 'x' does not exist in type 'Square'.
y: 13,
}

99 changes: 99 additions & 0 deletions tests/baselines/reference/excessPropertyCheckWithUnions.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
tests/cases/compiler/excessPropertyCheckWithUnions.ts(10,30): error TS2322: Type '{ tag: "T"; a1: string; }' is not assignable to type 'ADT'.
Object literal may only specify known properties, and 'a1' does not exist in type '{ tag: "T"; }'.
tests/cases/compiler/excessPropertyCheckWithUnions.ts(11,21): error TS2322: Type '{ tag: "A"; d20: 12; }' is not assignable to type 'ADT'.
Object literal may only specify known properties, and 'd20' does not exist in type '{ tag: "A"; a1: string; }'.
tests/cases/compiler/excessPropertyCheckWithUnions.ts(12,1): error TS2322: Type '{ tag: "D"; }' is not assignable to type 'ADT'.
Type '{ tag: "D"; }' is not assignable to type '{ tag: "D"; d20: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20; }'.
Property 'd20' is missing in type '{ tag: "D"; }'.
tests/cases/compiler/excessPropertyCheckWithUnions.ts(33,28): error TS2322: Type '{ tag: "A"; x: string; extra: number; }' is not assignable to type 'Ambiguous'.
Object literal may only specify known properties, and 'extra' does not exist in type 'Ambiguous'.
tests/cases/compiler/excessPropertyCheckWithUnions.ts(34,26): error TS2322: Type '{ tag: "A"; y: number; extra: number; }' is not assignable to type 'Ambiguous'.
Object literal may only specify known properties, and 'extra' does not exist in type 'Ambiguous'.
tests/cases/compiler/excessPropertyCheckWithUnions.ts(39,1): error TS2322: Type '{ tag: "A"; }' is not assignable to type 'Ambiguous'.
Type '{ tag: "A"; }' is not assignable to type '{ tag: "C"; }'.
Types of property 'tag' are incompatible.
Type '"A"' is not assignable to type '"C"'.
tests/cases/compiler/excessPropertyCheckWithUnions.ts(40,1): error TS2322: Type '{ tag: "A"; z: true; }' is not assignable to type 'Ambiguous'.
Type '{ tag: "A"; z: true; }' is not assignable to type '{ tag: "C"; }'.
Types of property 'tag' are incompatible.
Type '"A"' is not assignable to type '"C"'.


==== tests/cases/compiler/excessPropertyCheckWithUnions.ts (7 errors) ====
type ADT = {
tag: "A",
a1: string
} | {
tag: "D",
d20: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20
} | {
tag: "T",
}
let wrong: ADT = { tag: "T", a1: "extra" }
~~~~~~~~~~~
!!! error TS2322: Type '{ tag: "T"; a1: string; }' is not assignable to type 'ADT'.
!!! error TS2322: Object literal may only specify known properties, and 'a1' does not exist in type '{ tag: "T"; }'.
wrong = { tag: "A", d20: 12 }
~~~~~~~
!!! error TS2322: Type '{ tag: "A"; d20: 12; }' is not assignable to type 'ADT'.
!!! error TS2322: Object literal may only specify known properties, and 'd20' does not exist in type '{ tag: "A"; a1: string; }'.
wrong = { tag: "D" }
~~~~~
!!! error TS2322: Type '{ tag: "D"; }' is not assignable to type 'ADT'.
!!! error TS2322: Type '{ tag: "D"; }' is not assignable to type '{ tag: "D"; d20: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20; }'.
!!! error TS2322: Property 'd20' is missing in type '{ tag: "D"; }'.

type Ambiguous = {
tag: "A",
x: string
} | {
tag: "A",
y: number
} | {
tag: "B",
z: boolean
} | {
tag: "C"
}
let amb: Ambiguous
// no error for ambiguous tag, even when it could satisfy both constituents at once
amb = { tag: "A", x: "hi" }
amb = { tag: "A", y: 12 }
amb = { tag: "A", x: "hi", y: 12 }

// correctly error on excess property 'extra', even when ambiguous
amb = { tag: "A", x: "hi", extra: 12 }
~~~~~~~~~
!!! error TS2322: Type '{ tag: "A"; x: string; extra: number; }' is not assignable to type 'Ambiguous'.
!!! error TS2322: Object literal may only specify known properties, and 'extra' does not exist in type 'Ambiguous'.
amb = { tag: "A", y: 12, extra: 12 }
~~~~~~~~~
!!! error TS2322: Type '{ tag: "A"; y: number; extra: number; }' is not assignable to type 'Ambiguous'.
!!! error TS2322: Object literal may only specify known properties, and 'extra' does not exist in type 'Ambiguous'.

// assignability errors still work.
// But note that the error for `z: true` is the fallback one of reporting on
// the last constituent since assignability error reporting can't find a single best discriminant either.
amb = { tag: "A" }
~~~
!!! error TS2322: Type '{ tag: "A"; }' is not assignable to type 'Ambiguous'.
!!! error TS2322: Type '{ tag: "A"; }' is not assignable to type '{ tag: "C"; }'.
!!! error TS2322: Types of property 'tag' are incompatible.
!!! error TS2322: Type '"A"' is not assignable to type '"C"'.
amb = { tag: "A", z: true }
~~~
!!! error TS2322: Type '{ tag: "A"; z: true; }' is not assignable to type 'Ambiguous'.
!!! error TS2322: Type '{ tag: "A"; z: true; }' is not assignable to type '{ tag: "C"; }'.
!!! error TS2322: Types of property 'tag' are incompatible.
!!! error TS2322: Type '"A"' is not assignable to type '"C"'.

type Overlapping =
| { a: 1, b: 1, first: string }
| { a: 2, second: string }
| { b: 3, third: string }
let over: Overlapping

// these two are not reported because there are two discriminant properties
over = { a: 1, b: 1, first: "ok", second: "error" }
over = { a: 1, b: 1, first: "ok", third: "error" }

74 changes: 74 additions & 0 deletions tests/baselines/reference/excessPropertyCheckWithUnions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
//// [excessPropertyCheckWithUnions.ts]
type ADT = {
tag: "A",
a1: string
} | {
tag: "D",
d20: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20
} | {
tag: "T",
}
let wrong: ADT = { tag: "T", a1: "extra" }
wrong = { tag: "A", d20: 12 }
wrong = { tag: "D" }

type Ambiguous = {
tag: "A",
x: string
} | {
tag: "A",
y: number
} | {
tag: "B",
z: boolean
} | {
tag: "C"
}
let amb: Ambiguous
// no error for ambiguous tag, even when it could satisfy both constituents at once
amb = { tag: "A", x: "hi" }
amb = { tag: "A", y: 12 }
amb = { tag: "A", x: "hi", y: 12 }

// correctly error on excess property 'extra', even when ambiguous
amb = { tag: "A", x: "hi", extra: 12 }
amb = { tag: "A", y: 12, extra: 12 }

// assignability errors still work.
// But note that the error for `z: true` is the fallback one of reporting on
// the last constituent since assignability error reporting can't find a single best discriminant either.
amb = { tag: "A" }
amb = { tag: "A", z: true }

type Overlapping =
| { a: 1, b: 1, first: string }
| { a: 2, second: string }
| { b: 3, third: string }
let over: Overlapping

// these two are not reported because there are two discriminant properties
over = { a: 1, b: 1, first: "ok", second: "error" }
over = { a: 1, b: 1, first: "ok", third: "error" }


//// [excessPropertyCheckWithUnions.js]
var wrong = { tag: "T", a1: "extra" };
wrong = { tag: "A", d20: 12 };
wrong = { tag: "D" };
var amb;
// no error for ambiguous tag, even when it could satisfy both constituents at once
amb = { tag: "A", x: "hi" };
amb = { tag: "A", y: 12 };
amb = { tag: "A", x: "hi", y: 12 };
// correctly error on excess property 'extra', even when ambiguous
amb = { tag: "A", x: "hi", extra: 12 };
amb = { tag: "A", y: 12, extra: 12 };
// assignability errors still work.
// But note that the error for `z: true` is the fallback one of reporting on
// the last constituent since assignability error reporting can't find a single best discriminant either.
amb = { tag: "A" };
amb = { tag: "A", z: true };
var over;
// these two are not reported because there are two discriminant properties
over = { a: 1, b: 1, first: "ok", second: "error" };
over = { a: 1, b: 1, first: "ok", third: "error" };
Loading