Skip to content

Commit 2524fb1

Browse files
ahejlsbergahejlsberg
authored
Consistent narrowing by discriminant (#38311)
* Consistent requirements for narrowing by discriminant * Add tests
1 parent c219fda commit 2524fb1

File tree

5 files changed

+471
-8
lines changed

5 files changed

+471
-8
lines changed

‎src/compiler/checker.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20593,14 +20593,15 @@ namespace ts {
2059320593
}
2059420594

2059520595
function isMatchingReferenceDiscriminant(expr: Expression, computedType: Type) {
20596-
if (!(computedType.flags & TypeFlags.Union) || !isAccessExpression(expr)) {
20596+
const type = declaredType.flags & TypeFlags.Union ? declaredType : computedType;
20597+
if (!(type.flags & TypeFlags.Union) || !isAccessExpression(expr)) {
2059720598
return false;
2059820599
}
2059920600
const name = getAccessedPropertyName(expr);
2060020601
if (name === undefined) {
2060120602
return false;
2060220603
}
20603-
return isMatchingReference(reference, expr.expression) && isDiscriminantProperty(computedType, name);
20604+
return isMatchingReference(reference, expr.expression) && isDiscriminantProperty(type, name);
2060420605
}
2060520606

2060620607
function narrowTypeByDiscriminant(type: Type, access: AccessExpression, narrowType: (t: Type) => Type): Type {
@@ -20626,7 +20627,7 @@ namespace ts {
2062620627
if (strictNullChecks && assumeTrue && optionalChainContainsReference(expr, reference)) {
2062720628
type = getTypeWithFacts(type, TypeFacts.NEUndefinedOrNull);
2062820629
}
20629-
if (isMatchingReferenceDiscriminant(expr, declaredType)) {
20630+
if (isMatchingReferenceDiscriminant(expr, type)) {
2063020631
return narrowTypeByDiscriminant(type, <AccessExpression>expr, t => getTypeWithFacts(t, assumeTrue ? TypeFacts.Truthy : TypeFacts.Falsy));
2063120632
}
2063220633
return type;
@@ -20682,10 +20683,10 @@ namespace ts {
2068220683
type = narrowTypeByOptionalChainContainment(type, operator, left, assumeTrue);
2068320684
}
2068420685
}
20685-
if (isMatchingReferenceDiscriminant(left, declaredType)) {
20686+
if (isMatchingReferenceDiscriminant(left, type)) {
2068620687
return narrowTypeByDiscriminant(type, <AccessExpression>left, t => narrowTypeByEquality(t, operator, right, assumeTrue));
2068720688
}
20688-
if (isMatchingReferenceDiscriminant(right, declaredType)) {
20689+
if (isMatchingReferenceDiscriminant(right, type)) {
2068920690
return narrowTypeByDiscriminant(type, <AccessExpression>right, t => narrowTypeByEquality(t, operator, left, assumeTrue));
2069020691
}
2069120692
if (isMatchingConstructorReference(left)) {
@@ -21107,7 +21108,7 @@ namespace ts {
2110721108
!(getTypeFacts(predicate.type) & TypeFacts.EQUndefined)) {
2110821109
type = getTypeWithFacts(type, TypeFacts.NEUndefinedOrNull);
2110921110
}
21110-
if (isMatchingReferenceDiscriminant(predicateArgument, declaredType)) {
21111+
if (isMatchingReferenceDiscriminant(predicateArgument, type)) {
2111121112
return narrowTypeByDiscriminant(type, predicateArgument as AccessExpression, t => getNarrowedType(t, predicate.type!, assumeTrue, isTypeSubtypeOf));
2111221113
}
2111321114
}
@@ -21149,7 +21150,7 @@ namespace ts {
2114921150
if (isMatchingReference(reference, expr)) {
2115021151
return getTypeWithFacts(type, assumePresent ? TypeFacts.NEUndefinedOrNull : TypeFacts.EQUndefinedOrNull);
2115121152
}
21152-
if (isMatchingReferenceDiscriminant(expr, declaredType)) {
21153+
if (isMatchingReferenceDiscriminant(expr, type)) {
2115321154
return narrowTypeByDiscriminant(type, <AccessExpression>expr, t => getTypeWithFacts(t, assumePresent ? TypeFacts.NEUndefinedOrNull : TypeFacts.EQUndefinedOrNull));
2115421155
}
2115521156
return type;

‎tests/baselines/reference/discriminantPropertyCheck.js

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,67 @@ function func3(value: Partial<UnionOfBar>) {
174174
}
175175
}
176176
}
177+
178+
// Repro from #30557
179+
180+
interface TypeA {
181+
Name: "TypeA";
182+
Value1: "Cool stuff!";
183+
}
184+
185+
interface TypeB {
186+
Name: "TypeB";
187+
Value2: 0;
188+
}
189+
190+
type Type = TypeA | TypeB;
191+
192+
declare function isType(x: unknown): x is Type;
193+
194+
function WorksProperly(data: Type) {
195+
if (data.Name === "TypeA") {
196+
const value1 = data.Value1;
197+
}
198+
}
199+
200+
function DoesNotWork(data: unknown) {
201+
if (isType(data)) {
202+
if (data.Name === "TypeA") {
203+
const value1 = data.Value1;
204+
}
205+
}
206+
}
207+
208+
// Repro from #36777
209+
210+
type TestA = {
211+
type: 'testA';
212+
bananas: 3;
213+
}
214+
215+
type TestB = {
216+
type: 'testB';
217+
apples: 5;
218+
}
219+
220+
type AllTests = TestA | TestB;
221+
222+
type MapOfAllTests = Record<string, AllTests>;
223+
224+
const doTestingStuff = (mapOfTests: MapOfAllTests, ids: string[]) => {
225+
ids.forEach(id => {
226+
let test;
227+
test = mapOfTests[id];
228+
if (test.type === 'testA') {
229+
console.log(test.bananas);
230+
}
231+
switch (test.type) {
232+
case 'testA': {
233+
console.log(test.bananas);
234+
}
235+
}
236+
});
237+
};
177238

178239

179240
//// [discriminantPropertyCheck.js]
@@ -269,3 +330,29 @@ function func3(value) {
269330
}
270331
}
271332
}
333+
function WorksProperly(data) {
334+
if (data.Name === "TypeA") {
335+
var value1 = data.Value1;
336+
}
337+
}
338+
function DoesNotWork(data) {
339+
if (isType(data)) {
340+
if (data.Name === "TypeA") {
341+
var value1 = data.Value1;
342+
}
343+
}
344+
}
345+
var doTestingStuff = function (mapOfTests, ids) {
346+
ids.forEach(function (id) {
347+
var test;
348+
test = mapOfTests[id];
349+
if (test.type === 'testA') {
350+
console.log(test.bananas);
351+
}
352+
switch (test.type) {
353+
case 'testA': {
354+
console.log(test.bananas);
355+
}
356+
}
357+
});
358+
};

‎tests/baselines/reference/discriminantPropertyCheck.symbols

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,3 +508,159 @@ function func3(value: Partial<UnionOfBar>) {
508508
}
509509
}
510510

511+
// Repro from #30557
512+
513+
interface TypeA {
514+
>TypeA : Symbol(TypeA, Decl(discriminantPropertyCheck.ts, 174, 1))
515+
516+
Name: "TypeA";
517+
>Name : Symbol(TypeA.Name, Decl(discriminantPropertyCheck.ts, 178, 17))
518+
519+
Value1: "Cool stuff!";
520+
>Value1 : Symbol(TypeA.Value1, Decl(discriminantPropertyCheck.ts, 179, 18))
521+
}
522+
523+
interface TypeB {
524+
>TypeB : Symbol(TypeB, Decl(discriminantPropertyCheck.ts, 181, 1))
525+
526+
Name: "TypeB";
527+
>Name : Symbol(TypeB.Name, Decl(discriminantPropertyCheck.ts, 183, 17))
528+
529+
Value2: 0;
530+
>Value2 : Symbol(TypeB.Value2, Decl(discriminantPropertyCheck.ts, 184, 18))
531+
}
532+
533+
type Type = TypeA | TypeB;
534+
>Type : Symbol(Type, Decl(discriminantPropertyCheck.ts, 186, 1))
535+
>TypeA : Symbol(TypeA, Decl(discriminantPropertyCheck.ts, 174, 1))
536+
>TypeB : Symbol(TypeB, Decl(discriminantPropertyCheck.ts, 181, 1))
537+
538+
declare function isType(x: unknown): x is Type;
539+
>isType : Symbol(isType, Decl(discriminantPropertyCheck.ts, 188, 26))
540+
>x : Symbol(x, Decl(discriminantPropertyCheck.ts, 190, 24))
541+
>x : Symbol(x, Decl(discriminantPropertyCheck.ts, 190, 24))
542+
>Type : Symbol(Type, Decl(discriminantPropertyCheck.ts, 186, 1))
543+
544+
function WorksProperly(data: Type) {
545+
>WorksProperly : Symbol(WorksProperly, Decl(discriminantPropertyCheck.ts, 190, 47))
546+
>data : Symbol(data, Decl(discriminantPropertyCheck.ts, 192, 23))
547+
>Type : Symbol(Type, Decl(discriminantPropertyCheck.ts, 186, 1))
548+
549+
if (data.Name === "TypeA") {
550+
>data.Name : Symbol(Name, Decl(discriminantPropertyCheck.ts, 178, 17), Decl(discriminantPropertyCheck.ts, 183, 17))
551+
>data : Symbol(data, Decl(discriminantPropertyCheck.ts, 192, 23))
552+
>Name : Symbol(Name, Decl(discriminantPropertyCheck.ts, 178, 17), Decl(discriminantPropertyCheck.ts, 183, 17))
553+
554+
const value1 = data.Value1;
555+
>value1 : Symbol(value1, Decl(discriminantPropertyCheck.ts, 194, 13))
556+
>data.Value1 : Symbol(TypeA.Value1, Decl(discriminantPropertyCheck.ts, 179, 18))
557+
>data : Symbol(data, Decl(discriminantPropertyCheck.ts, 192, 23))
558+
>Value1 : Symbol(TypeA.Value1, Decl(discriminantPropertyCheck.ts, 179, 18))
559+
}
560+
}
561+
562+
function DoesNotWork(data: unknown) {
563+
>DoesNotWork : Symbol(DoesNotWork, Decl(discriminantPropertyCheck.ts, 196, 1))
564+
>data : Symbol(data, Decl(discriminantPropertyCheck.ts, 198, 21))
565+
566+
if (isType(data)) {
567+
>isType : Symbol(isType, Decl(discriminantPropertyCheck.ts, 188, 26))
568+
>data : Symbol(data, Decl(discriminantPropertyCheck.ts, 198, 21))
569+
570+
if (data.Name === "TypeA") {
571+
>data.Name : Symbol(Name, Decl(discriminantPropertyCheck.ts, 178, 17), Decl(discriminantPropertyCheck.ts, 183, 17))
572+
>data : Symbol(data, Decl(discriminantPropertyCheck.ts, 198, 21))
573+
>Name : Symbol(Name, Decl(discriminantPropertyCheck.ts, 178, 17), Decl(discriminantPropertyCheck.ts, 183, 17))
574+
575+
const value1 = data.Value1;
576+
>value1 : Symbol(value1, Decl(discriminantPropertyCheck.ts, 201, 17))
577+
>data.Value1 : Symbol(TypeA.Value1, Decl(discriminantPropertyCheck.ts, 179, 18))
578+
>data : Symbol(data, Decl(discriminantPropertyCheck.ts, 198, 21))
579+
>Value1 : Symbol(TypeA.Value1, Decl(discriminantPropertyCheck.ts, 179, 18))
580+
}
581+
}
582+
}
583+
584+
// Repro from #36777
585+
586+
type TestA = {
587+
>TestA : Symbol(TestA, Decl(discriminantPropertyCheck.ts, 204, 1))
588+
589+
type: 'testA';
590+
>type : Symbol(type, Decl(discriminantPropertyCheck.ts, 208, 14))
591+
592+
bananas: 3;
593+
>bananas : Symbol(bananas, Decl(discriminantPropertyCheck.ts, 209, 18))
594+
}
595+
596+
type TestB = {
597+
>TestB : Symbol(TestB, Decl(discriminantPropertyCheck.ts, 211, 1))
598+
599+
type: 'testB';
600+
>type : Symbol(type, Decl(discriminantPropertyCheck.ts, 213, 14))
601+
602+
apples: 5;
603+
>apples : Symbol(apples, Decl(discriminantPropertyCheck.ts, 214, 18))
604+
}
605+
606+
type AllTests = TestA | TestB;
607+
>AllTests : Symbol(AllTests, Decl(discriminantPropertyCheck.ts, 216, 1))
608+
>TestA : Symbol(TestA, Decl(discriminantPropertyCheck.ts, 204, 1))
609+
>TestB : Symbol(TestB, Decl(discriminantPropertyCheck.ts, 211, 1))
610+
611+
type MapOfAllTests = Record<string, AllTests>;
612+
>MapOfAllTests : Symbol(MapOfAllTests, Decl(discriminantPropertyCheck.ts, 218, 30))
613+
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --))
614+
>AllTests : Symbol(AllTests, Decl(discriminantPropertyCheck.ts, 216, 1))
615+
616+
const doTestingStuff = (mapOfTests: MapOfAllTests, ids: string[]) => {
617+
>doTestingStuff : Symbol(doTestingStuff, Decl(discriminantPropertyCheck.ts, 222, 5))
618+
>mapOfTests : Symbol(mapOfTests, Decl(discriminantPropertyCheck.ts, 222, 24))
619+
>MapOfAllTests : Symbol(MapOfAllTests, Decl(discriminantPropertyCheck.ts, 218, 30))
620+
>ids : Symbol(ids, Decl(discriminantPropertyCheck.ts, 222, 50))
621+
622+
ids.forEach(id => {
623+
>ids.forEach : Symbol(Array.forEach, Decl(lib.es5.d.ts, --, --))
624+
>ids : Symbol(ids, Decl(discriminantPropertyCheck.ts, 222, 50))
625+
>forEach : Symbol(Array.forEach, Decl(lib.es5.d.ts, --, --))
626+
>id : Symbol(id, Decl(discriminantPropertyCheck.ts, 223, 16))
627+
628+
let test;
629+
>test : Symbol(test, Decl(discriminantPropertyCheck.ts, 224, 11))
630+
631+
test = mapOfTests[id];
632+
>test : Symbol(test, Decl(discriminantPropertyCheck.ts, 224, 11))
633+
>mapOfTests : Symbol(mapOfTests, Decl(discriminantPropertyCheck.ts, 222, 24))
634+
>id : Symbol(id, Decl(discriminantPropertyCheck.ts, 223, 16))
635+
636+
if (test.type === 'testA') {
637+
>test.type : Symbol(type, Decl(discriminantPropertyCheck.ts, 208, 14), Decl(discriminantPropertyCheck.ts, 213, 14))
638+
>test : Symbol(test, Decl(discriminantPropertyCheck.ts, 224, 11))
639+
>type : Symbol(type, Decl(discriminantPropertyCheck.ts, 208, 14), Decl(discriminantPropertyCheck.ts, 213, 14))
640+
641+
console.log(test.bananas);
642+
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
643+
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
644+
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
645+
>test.bananas : Symbol(bananas, Decl(discriminantPropertyCheck.ts, 209, 18))
646+
>test : Symbol(test, Decl(discriminantPropertyCheck.ts, 224, 11))
647+
>bananas : Symbol(bananas, Decl(discriminantPropertyCheck.ts, 209, 18))
648+
}
649+
switch (test.type) {
650+
>test.type : Symbol(type, Decl(discriminantPropertyCheck.ts, 208, 14), Decl(discriminantPropertyCheck.ts, 213, 14))
651+
>test : Symbol(test, Decl(discriminantPropertyCheck.ts, 224, 11))
652+
>type : Symbol(type, Decl(discriminantPropertyCheck.ts, 208, 14), Decl(discriminantPropertyCheck.ts, 213, 14))
653+
654+
case 'testA': {
655+
console.log(test.bananas);
656+
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
657+
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
658+
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
659+
>test.bananas : Symbol(bananas, Decl(discriminantPropertyCheck.ts, 209, 18))
660+
>test : Symbol(test, Decl(discriminantPropertyCheck.ts, 224, 11))
661+
>bananas : Symbol(bananas, Decl(discriminantPropertyCheck.ts, 209, 18))
662+
}
663+
}
664+
});
665+
};
666+

0 commit comments

Comments
 (0)