Skip to content

Commit

Permalink
Consider all union types matching discriminator for excess property c…
Browse files Browse the repository at this point in the history 
…hecks

Fixes microsoft#51873
  • Loading branch information
@nebkat
nebkat committed Dec 14, 2022
1 parent e4816ed commit ca945a2
Show file tree
Hide file tree
Showing 12 changed files with 463 additions and 198 deletions.
10 changes: 1 addition & 9 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22061,15 +22061,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (match === -1) {
return defaultValue;
}
// make sure exactly 1 matches before returning it
let nextMatch = discriminable.indexOf(/*searchElement*/ true, match + 1);
while (nextMatch !== -1) {
if (!isTypeIdenticalTo(target.types[match], target.types[nextMatch])) {
return defaultValue;
}
nextMatch = discriminable.indexOf(/*searchElement*/ true, nextMatch + 1);
}
return target.types[match];
return getUnionType(target.types.filter((_, index) => discriminable[index]));
}

/**
Expand Down
54 changes: 52 additions & 2 deletions tests/baselines/reference/excessPropertyCheckWithMultipleDiscriminants.errors.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,17 @@ tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(30,5): erro
Object literal may only specify known properties, and 'multipleOf' does not exist in type 'Float'.
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(41,5): error TS2322: Type '{ p1: "left"; p2: false; p3: number; p4: string; }' is not assignable to type 'DisjointDiscriminants'.
Object literal may only specify known properties, and 'p3' does not exist in type '{ p1: "left"; p2: boolean; }'.
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(50,5): error TS2322: Type '{ p1: "left"; p2: true; p3: number; p4: string; }' is not assignable to type 'DisjointDiscriminants'.
Object literal may only specify known properties, and 'p4' does not exist in type '{ p1: "left"; p2: true; p3: number; } | { p1: "left"; p2: boolean; }'.
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(57,5): error TS2322: Type '{ p1: "right"; p2: false; p3: number; p4: string; }' is not assignable to type 'DisjointDiscriminants'.
Object literal may only specify known properties, and 'p3' does not exist in type '{ p1: "right"; p2: false; p4: string; }'.
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(83,5): error TS2322: Type '{ type: "A"; n: number; a: number; b: number; }' is not assignable to type 'CommonWithOverlappingOptionals'.
Object literal may only specify known properties, and 'b' does not exist in type 'Common | (Common & A)'.
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(93,5): error TS2322: Type '{ type: "A"; n: number; a: number; b: number; }' is not assignable to type 'CommonWithDisjointOverlappingOptionals'.
Object literal may only specify known properties, and 'b' does not exist in type 'Common | A'.


==== tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts (3 errors) ====
==== tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts (6 errors) ====
// Repro from #32657

interface Base<T> {
Expand Down Expand Up @@ -57,12 +63,15 @@ tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(57,5): erro
p4: "hello"
};

// This has no excess error because variant one and three are both applicable.
// This has excess error because variant two is not applicable.
const b: DisjointDiscriminants = {
p1: 'left',
p2: true,
p3: 42,
p4: "hello"
~~~~~~~~~~~
!!! error TS2322: Type '{ p1: "left"; p2: true; p3: number; p4: string; }' is not assignable to type 'DisjointDiscriminants'.
!!! error TS2322: Object literal may only specify known properties, and 'p4' does not exist in type '{ p1: "left"; p2: true; p3: number; } | { p1: "left"; p2: boolean; }'.
};

// This has excess error because variant two is the only applicable case
Expand All @@ -75,4 +84,45 @@ tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(57,5): erro
!!! error TS2322: Object literal may only specify known properties, and 'p3' does not exist in type '{ p1: "right"; p2: false; p4: string; }'.
p4: "hello"
};

// Repro from #51873

interface Common {
type: "A" | "B" | "C" | "D";
n: number;
}
interface A {
type: "A";
a?: number;
}
interface B {
type: "B";
b?: number;
}

type CommonWithOverlappingOptionals = Common | (Common & A) | (Common & B);

// Should reject { b } because reduced to Common | (Common & A)
const c1: CommonWithOverlappingOptionals = {
type: "A",
n: 1,
a: 1,
b: 1 // excess property
~~~~
!!! error TS2322: Type '{ type: "A"; n: number; a: number; b: number; }' is not assignable to type 'CommonWithOverlappingOptionals'.
!!! error TS2322: Object literal may only specify known properties, and 'b' does not exist in type 'Common | (Common & A)'.
}

type CommonWithDisjointOverlappingOptionals = Common | A | B;

// Should still reject { b } because reduced to Common | A, even though these are now disjoint
const c2: CommonWithDisjointOverlappingOptionals = {
type: "A",
n: 1,
a: 1,
b: 1 // excess property
~~~~
!!! error TS2322: Type '{ type: "A"; n: number; a: number; b: number; }' is not assignable to type 'CommonWithDisjointOverlappingOptionals'.
!!! error TS2322: Object literal may only specify known properties, and 'b' does not exist in type 'Common | A'.
}

53 changes: 51 additions & 2 deletions tests/baselines/reference/excessPropertyCheckWithMultipleDiscriminants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const a: DisjointDiscriminants = {
p4: "hello"
};

// This has no excess error because variant one and three are both applicable.
// This has excess error because variant two is not applicable.
const b: DisjointDiscriminants = {
p1: 'left',
p2: true,
Expand All @@ -58,6 +58,41 @@ const c: DisjointDiscriminants = {
p3: 42,
p4: "hello"
};

// Repro from #51873

interface Common {
type: "A" | "B" | "C" | "D";
n: number;
}
interface A {
type: "A";
a?: number;
}
interface B {
type: "B";
b?: number;
}

type CommonWithOverlappingOptionals = Common | (Common & A) | (Common & B);

// Should reject { b } because reduced to Common | (Common & A)
const c1: CommonWithOverlappingOptionals = {
type: "A",
n: 1,
a: 1,
b: 1 // excess property
}

type CommonWithDisjointOverlappingOptionals = Common | A | B;

// Should still reject { b } because reduced to Common | A, even though these are now disjoint
const c2: CommonWithDisjointOverlappingOptionals = {
type: "A",
n: 1,
a: 1,
b: 1 // excess property
}


//// [excessPropertyCheckWithMultipleDiscriminants.js]
Expand All @@ -75,7 +110,7 @@ var a = {
p3: 42,
p4: "hello"
};
// This has no excess error because variant one and three are both applicable.
// This has excess error because variant two is not applicable.
var b = {
p1: 'left',
p2: true,
Expand All @@ -89,3 +124,17 @@ var c = {
p3: 42,
p4: "hello"
};
// Should reject { b } because reduced to Common | (Common & A)
var c1 = {
type: "A",
n: 1,
a: 1,
b: 1 // excess property
};
// Should still reject { b } because reduced to Common | A, even though these are now disjoint
var c2 = {
type: "A",
n: 1,
a: 1,
b: 1 // excess property
};
82 changes: 81 additions & 1 deletion tests/baselines/reference/excessPropertyCheckWithMultipleDiscriminants.symbols
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const a: DisjointDiscriminants = {

};

// This has no excess error because variant one and three are both applicable.
// This has excess error because variant two is not applicable.
const b: DisjointDiscriminants = {
>b : Symbol(b, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 45, 5))
>DisjointDiscriminants : Symbol(DisjointDiscriminants, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 31, 1))
Expand Down Expand Up @@ -141,3 +141,83 @@ const c: DisjointDiscriminants = {

};

// Repro from #51873

interface Common {
>Common : Symbol(Common, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 58, 2))

type: "A" | "B" | "C" | "D";
>type : Symbol(Common.type, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 62, 18))

n: number;
>n : Symbol(Common.n, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 63, 32))
}
interface A {
>A : Symbol(A, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 65, 1))

type: "A";
>type : Symbol(A.type, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 66, 13))

a?: number;
>a : Symbol(A.a, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 67, 14))
}
interface B {
>B : Symbol(B, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 69, 1))

type: "B";
>type : Symbol(B.type, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 70, 13))

b?: number;
>b : Symbol(B.b, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 71, 14))
}

type CommonWithOverlappingOptionals = Common | (Common & A) | (Common & B);
>CommonWithOverlappingOptionals : Symbol(CommonWithOverlappingOptionals, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 73, 1))
>Common : Symbol(Common, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 58, 2))
>Common : Symbol(Common, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 58, 2))
>A : Symbol(A, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 65, 1))
>Common : Symbol(Common, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 58, 2))
>B : Symbol(B, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 69, 1))

// Should reject { b } because reduced to Common | (Common & A)
const c1: CommonWithOverlappingOptionals = {
>c1 : Symbol(c1, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 78, 5))
>CommonWithOverlappingOptionals : Symbol(CommonWithOverlappingOptionals, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 73, 1))

type: "A",
>type : Symbol(type, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 78, 44))

n: 1,
>n : Symbol(n, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 79, 14))

a: 1,
>a : Symbol(a, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 80, 9))

b: 1 // excess property
>b : Symbol(b, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 81, 9))
}

type CommonWithDisjointOverlappingOptionals = Common | A | B;
>CommonWithDisjointOverlappingOptionals : Symbol(CommonWithDisjointOverlappingOptionals, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 83, 1))
>Common : Symbol(Common, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 58, 2))
>A : Symbol(A, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 65, 1))
>B : Symbol(B, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 69, 1))

// Should still reject { b } because reduced to Common | A, even though these are now disjoint
const c2: CommonWithDisjointOverlappingOptionals = {
>c2 : Symbol(c2, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 88, 5))
>CommonWithDisjointOverlappingOptionals : Symbol(CommonWithDisjointOverlappingOptionals, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 83, 1))

type: "A",
>type : Symbol(type, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 88, 52))

n: 1,
>n : Symbol(n, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 89, 14))

a: 1,
>a : Symbol(a, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 90, 9))

b: 1 // excess property
>b : Symbol(b, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 91, 9))
}

76 changes: 75 additions & 1 deletion tests/baselines/reference/excessPropertyCheckWithMultipleDiscriminants.types
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ const a: DisjointDiscriminants = {

};

// This has no excess error because variant one and three are both applicable.
// This has excess error because variant two is not applicable.
const b: DisjointDiscriminants = {
>b : DisjointDiscriminants
>{ p1: 'left', p2: true, p3: 42, p4: "hello"} : { p1: "left"; p2: true; p3: number; p4: string; }
Expand Down Expand Up @@ -139,3 +139,77 @@ const c: DisjointDiscriminants = {

};

// Repro from #51873

interface Common {
type: "A" | "B" | "C" | "D";
>type : "A" | "B" | "C" | "D"

n: number;
>n : number
}
interface A {
type: "A";
>type : "A"

a?: number;
>a : number
}
interface B {
type: "B";
>type : "B"

b?: number;
>b : number
}

type CommonWithOverlappingOptionals = Common | (Common & A) | (Common & B);
>CommonWithOverlappingOptionals : Common | (Common & A) | (Common & B)

// Should reject { b } because reduced to Common | (Common & A)
const c1: CommonWithOverlappingOptionals = {
>c1 : CommonWithOverlappingOptionals
>{ type: "A", n: 1, a: 1, b: 1 // excess property} : { type: "A"; n: number; a: number; b: number; }

type: "A",
>type : "A"
>"A" : "A"

n: 1,
>n : number
>1 : 1

a: 1,
>a : number
>1 : 1

b: 1 // excess property
>b : number
>1 : 1
}

type CommonWithDisjointOverlappingOptionals = Common | A | B;
>CommonWithDisjointOverlappingOptionals : Common | A | B

// Should still reject { b } because reduced to Common | A, even though these are now disjoint
const c2: CommonWithDisjointOverlappingOptionals = {
>c2 : CommonWithDisjointOverlappingOptionals
>{ type: "A", n: 1, a: 1, b: 1 // excess property} : { type: "A"; n: number; a: number; b: number; }

type: "A",
>type : "A"
>"A" : "A"

n: 1,
>n : number
>1 : 1

a: 1,
>a : number
>1 : 1

b: 1 // excess property
>b : number
>1 : 1
}

Loading

0 comments on commit ca945a2

Please sign in to comment.