From acb8b1f65fbb59510c007586307ee3c9206c0b65 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 17 Sep 2018 09:06:26 -0700 Subject: [PATCH 1/5] Correct falsiness for {} empty object type --- src/compiler/checker.ts | 53 ++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 11f279d27d531..99bbd58e6b3c2 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -601,6 +601,8 @@ namespace ts { FunctionFacts = FunctionStrictFacts | EQUndefined | EQNull | EQUndefinedOrNull | Falsy, UndefinedFacts = TypeofNEString | TypeofNENumber | TypeofNEBoolean | TypeofNESymbol | TypeofNEObject | TypeofNEFunction | TypeofNEHostObject | EQUndefined | EQUndefinedOrNull | NENull | Falsy, NullFacts = TypeofEQObject | TypeofNEString | TypeofNENumber | TypeofNEBoolean | TypeofNESymbol | TypeofNEFunction | TypeofNEHostObject | EQNull | EQUndefinedOrNull | NEUndefined | Falsy, + EmptyObjectStrictFacts = All & ~(EQUndefined | EQNull | EQUndefinedOrNull), + EmptyObjectFacts = All, } const typeofEQFacts = createMapFromTemplate({ @@ -11774,8 +11776,12 @@ namespace ts { const simplified = getSimplifiedType((target).type); const constraint = simplified !== (target).type ? simplified : getConstraintOfType((target).type); if (constraint) { - if (result = isRelatedTo(source, getIndexType(constraint, (target as IndexType).stringsOnly), reportErrors)) { - return result; + // We require Ternary.True here such that circular constraints don't cause + // false positives. For example, given 'T extends { [K in keyof T]: string }', + // 'keyof T' has itself as its constraint and produces a Ternary.Maybe when + // related to other types. + if (isRelatedTo(source, getIndexType(constraint, (target as IndexType).stringsOnly), reportErrors) === Ternary.True) { + return Ternary.True; } } } @@ -14169,9 +14175,11 @@ namespace ts { (type === falseType || type === regularFalseType) ? TypeFacts.FalseFacts : TypeFacts.TrueFacts; } if (flags & TypeFlags.Object) { - return isFunctionObjectType(type) ? - strictNullChecks ? TypeFacts.FunctionStrictFacts : TypeFacts.FunctionFacts : - strictNullChecks ? TypeFacts.ObjectStrictFacts : TypeFacts.ObjectFacts; + return getObjectFlags(type) & ObjectFlags.Anonymous && isEmptyObjectType(type) ? + strictNullChecks ? TypeFacts.EmptyObjectStrictFacts : TypeFacts.EmptyObjectFacts : + isFunctionObjectType(type) ? + strictNullChecks ? TypeFacts.FunctionStrictFacts : TypeFacts.FunctionFacts : + strictNullChecks ? TypeFacts.ObjectStrictFacts : TypeFacts.ObjectFacts; } if (flags & (TypeFlags.Void | TypeFlags.Undefined)) { return TypeFacts.UndefinedFacts; @@ -15083,23 +15091,24 @@ namespace ts { return getTypeWithFacts(assumeTrue ? mapType(type, narrowTypeForTypeof) : type, facts); function narrowTypeForTypeof(type: Type) { - if (assumeTrue && !(type.flags & TypeFlags.Union)) { - if (type.flags & TypeFlags.Unknown && literal.text === "object") { - return getUnionType([nonPrimitiveType, nullType]); - } - // We narrow a non-union type to an exact primitive type if the non-union type - // is a supertype of that primitive type. For example, type 'any' can be narrowed - // to one of the primitive types. - const targetType = literal.text === "function" ? globalFunctionType : typeofTypesByName.get(literal.text); - if (targetType) { - if (isTypeSubtypeOf(targetType, type)) { - return isTypeAny(type) ? targetType : getIntersectionType([type, targetType]); // Intersection to handle `string` being a subtype of `keyof T` - } - if (type.flags & TypeFlags.Instantiable) { - const constraint = getBaseConstraintOfType(type) || anyType; - if (isTypeSubtypeOf(targetType, constraint)) { - return getIntersectionType([type, targetType]); - } + if (type.flags & TypeFlags.Unknown && literal.text === "object") { + return getUnionType([nonPrimitiveType, nullType]); + } + // We narrow a non-union type to an exact primitive type if the non-union type + // is a supertype of that primitive type. For example, type 'any' can be narrowed + // to one of the primitive types. + const targetType = literal.text === "function" ? globalFunctionType : typeofTypesByName.get(literal.text); + if (targetType) { + if (isTypeSubtypeOf(type, targetType)) { + return type; + } + if (isTypeSubtypeOf(targetType, type)) { + return targetType; + } + if (type.flags & TypeFlags.Instantiable) { + const constraint = getBaseConstraintOfType(type) || anyType; + if (isTypeSubtypeOf(targetType, constraint)) { + return getIntersectionType([type, targetType]); } } } From 46de5067b038536545dd9d7f956971e9160d15e8 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 17 Sep 2018 09:06:38 -0700 Subject: [PATCH 2/5] Fix resulting issue in compiler --- src/services/importTracker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/importTracker.ts b/src/services/importTracker.ts index 86bee92eaec48..9872b37840305 100644 --- a/src/services/importTracker.ts +++ b/src/services/importTracker.ts @@ -385,7 +385,7 @@ namespace ts.FindAllReferences { } /** Iterates over all statements at the top level or in module declarations. Returns the first truthy result. */ - function forEachPossibleImportOrExportStatement(sourceFileLike: SourceFileLike, action: (statement: Statement) => T): T | undefined { + function forEachPossibleImportOrExportStatement(sourceFileLike: SourceFileLike, action: (statement: Statement) => T) { return forEach(sourceFileLike.kind === SyntaxKind.SourceFile ? sourceFileLike.statements : sourceFileLike.body!.statements, statement => // TODO: GH#18217 action(statement) || (isAmbientModuleDeclaration(statement) && forEach(statement.body && statement.body.statements, action))); } From 92c17cebcb86a1b2ca0086af02aa7e93d2731c0c Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 17 Sep 2018 09:15:52 -0700 Subject: [PATCH 3/5] Accept new baselines --- tests/baselines/reference/mappedTypes4.types | 2 +- .../reference/recursiveTypeRelations.errors.txt | 9 ++++++++- .../reference/strictTypeofUnionNarrowing.types | 6 +++--- .../typeGuardOfFormTypeOfPrimitiveSubtype.types | 12 ++++++------ 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/tests/baselines/reference/mappedTypes4.types b/tests/baselines/reference/mappedTypes4.types index 039aabb7306d3..fac8d982e9210 100644 --- a/tests/baselines/reference/mappedTypes4.types +++ b/tests/baselines/reference/mappedTypes4.types @@ -45,7 +45,7 @@ function boxify(obj: T): Boxified { } return obj; >obj : any ->obj : never +>obj : T } type A = { a: string }; diff --git a/tests/baselines/reference/recursiveTypeRelations.errors.txt b/tests/baselines/reference/recursiveTypeRelations.errors.txt index 8c5596958c389..e39ca75dab97d 100644 --- a/tests/baselines/reference/recursiveTypeRelations.errors.txt +++ b/tests/baselines/reference/recursiveTypeRelations.errors.txt @@ -1,9 +1,12 @@ tests/cases/compiler/recursiveTypeRelations.ts(8,5): error TS2391: Function implementation is missing or not immediately following the declaration. tests/cases/compiler/recursiveTypeRelations.ts(27,38): error TS2304: Cannot find name 'ClassNameObject'. +tests/cases/compiler/recursiveTypeRelations.ts(27,55): error TS2345: Argument of type '(obj: any, key: keyof S) => any' is not assignable to parameter of type '(previousValue: any, currentValue: string, currentIndex: number, array: string[]) => any'. + Types of parameters 'key' and 'currentValue' are incompatible. + Type 'string' is not assignable to type 'keyof S'. tests/cases/compiler/recursiveTypeRelations.ts(27,61): error TS2304: Cannot find name 'ClassNameObject'. -==== tests/cases/compiler/recursiveTypeRelations.ts (3 errors) ==== +==== tests/cases/compiler/recursiveTypeRelations.ts (4 errors) ==== // Repro from #14896 type Attributes = { @@ -35,6 +38,10 @@ tests/cases/compiler/recursiveTypeRelations.ts(27,61): error TS2304: Cannot find return Object.keys(arg).reduce((obj: ClassNameObject, key: keyof S) => { ~~~~~~~~~~~~~~~ !!! error TS2304: Cannot find name 'ClassNameObject'. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +!!! error TS2345: Argument of type '(obj: any, key: keyof S) => any' is not assignable to parameter of type '(previousValue: any, currentValue: string, currentIndex: number, array: string[]) => any'. +!!! error TS2345: Types of parameters 'key' and 'currentValue' are incompatible. +!!! error TS2345: Type 'string' is not assignable to type 'keyof S'. ~~~~~~~~~~~~~~~ !!! error TS2304: Cannot find name 'ClassNameObject'. const exportedClassName = styles[key]; diff --git a/tests/baselines/reference/strictTypeofUnionNarrowing.types b/tests/baselines/reference/strictTypeofUnionNarrowing.types index 3039ece15d68b..1ec44e14ff03a 100644 --- a/tests/baselines/reference/strictTypeofUnionNarrowing.types +++ b/tests/baselines/reference/strictTypeofUnionNarrowing.types @@ -12,7 +12,7 @@ function stringify1(anything: { toString(): string } | undefined): string { >"string" : "string" >anything.toUpperCase() : string >anything.toUpperCase : () => string ->anything : { toString(): string; } & string +>anything : string >toUpperCase : () => string >"" : "" } @@ -29,7 +29,7 @@ function stringify2(anything: {} | undefined): string { >"string" : "string" >anything.toUpperCase() : string >anything.toUpperCase : () => string ->anything : string & {} +>anything : string >toUpperCase : () => string >"" : "" } @@ -64,7 +64,7 @@ function stringify4(anything: { toString?(): string } | undefined): string { >"string" : "string" >anything.toUpperCase() : string >anything.toUpperCase : () => string ->anything : {} & string +>anything : string >toUpperCase : () => string >"" : "" } diff --git a/tests/baselines/reference/typeGuardOfFormTypeOfPrimitiveSubtype.types b/tests/baselines/reference/typeGuardOfFormTypeOfPrimitiveSubtype.types index 6510b73d44d53..4787c07d75807 100644 --- a/tests/baselines/reference/typeGuardOfFormTypeOfPrimitiveSubtype.types +++ b/tests/baselines/reference/typeGuardOfFormTypeOfPrimitiveSubtype.types @@ -14,7 +14,7 @@ if (typeof a === "number") { let c: number = a; >c : number ->a : number & {} +>a : number } if (typeof a === "string") { >typeof a === "string" : boolean @@ -24,7 +24,7 @@ if (typeof a === "string") { let c: string = a; >c : string ->a : string & {} +>a : string } if (typeof a === "boolean") { >typeof a === "boolean" : boolean @@ -34,7 +34,7 @@ if (typeof a === "boolean") { let c: boolean = a; >c : boolean ->a : (false & {}) | (true & {}) +>a : boolean } if (typeof b === "number") { @@ -45,7 +45,7 @@ if (typeof b === "number") { let c: number = b; >c : number ->b : { toString(): string; } & number +>b : number } if (typeof b === "string") { >typeof b === "string" : boolean @@ -55,7 +55,7 @@ if (typeof b === "string") { let c: string = b; >c : string ->b : { toString(): string; } & string +>b : string } if (typeof b === "boolean") { >typeof b === "boolean" : boolean @@ -65,6 +65,6 @@ if (typeof b === "boolean") { let c: boolean = b; >c : boolean ->b : ({ toString(): string; } & false) | ({ toString(): string; } & true) +>b : boolean } From eb06af19013e0265895d446b0b9f177dcf3541fa Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 17 Sep 2018 13:01:53 -0700 Subject: [PATCH 4/5] Add tests --- .../controlFlow/controlFlowTruthiness.ts | 27 +++++++++++++++++++ .../keyof/keyofAndIndexedAccessErrors.ts | 6 +++++ 2 files changed, 33 insertions(+) diff --git a/tests/cases/conformance/controlFlow/controlFlowTruthiness.ts b/tests/cases/conformance/controlFlow/controlFlowTruthiness.ts index ba1947da13b9a..0245f9bfea012 100644 --- a/tests/cases/conformance/controlFlow/controlFlowTruthiness.ts +++ b/tests/cases/conformance/controlFlow/controlFlowTruthiness.ts @@ -68,3 +68,30 @@ function f6() { y; // string | undefined } } + +function f7(x: {}) { + if (x) { + x; // {} + } + else { + x; // {} + } +} + +function f8(x: T) { + if (x) { + x; // {} + } + else { + x; // {} + } +} + +function f9(x: T) { + if (x) { + x; // {} + } + else { + x; // never + } +} \ No newline at end of file diff --git a/tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts b/tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts index bb2b9d951179b..3660ff418e8a6 100644 --- a/tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts +++ b/tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts @@ -116,3 +116,9 @@ function f3, U extends T, J extends K>( tk = uj; uj = tk; // error } + +// The constraint of 'keyof T' is 'keyof T' +function f4(k: keyof T) { + k = 42; // error + k = "hello"; // error +} From 17080eb58f27a75cc2bb8e21308b7990d3751849 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 17 Sep 2018 13:02:01 -0700 Subject: [PATCH 5/5] Accept new baselines --- .../reference/controlFlowTruthiness.js | 52 ++++++++++++++++++- .../reference/controlFlowTruthiness.symbols | 51 ++++++++++++++++++ .../reference/controlFlowTruthiness.types | 47 +++++++++++++++++ .../keyofAndIndexedAccessErrors.errors.txt | 14 ++++- .../reference/keyofAndIndexedAccessErrors.js | 11 ++++ .../keyofAndIndexedAccessErrors.symbols | 16 ++++++ .../keyofAndIndexedAccessErrors.types | 16 ++++++ 7 files changed, 205 insertions(+), 2 deletions(-) diff --git a/tests/baselines/reference/controlFlowTruthiness.js b/tests/baselines/reference/controlFlowTruthiness.js index b57befbd6415a..a841619ace51f 100644 --- a/tests/baselines/reference/controlFlowTruthiness.js +++ b/tests/baselines/reference/controlFlowTruthiness.js @@ -67,7 +67,33 @@ function f6() { y; // string | undefined } } - + +function f7(x: {}) { + if (x) { + x; // {} + } + else { + x; // {} + } +} + +function f8(x: T) { + if (x) { + x; // {} + } + else { + x; // {} + } +} + +function f9(x: T) { + if (x) { + x; // {} + } + else { + x; // never + } +} //// [controlFlowTruthiness.js] function f1() { @@ -131,3 +157,27 @@ function f6() { y; // string | undefined } } +function f7(x) { + if (x) { + x; // {} + } + else { + x; // {} + } +} +function f8(x) { + if (x) { + x; // {} + } + else { + x; // {} + } +} +function f9(x) { + if (x) { + x; // {} + } + else { + x; // never + } +} diff --git a/tests/baselines/reference/controlFlowTruthiness.symbols b/tests/baselines/reference/controlFlowTruthiness.symbols index 72dd304f8106d..098a81e5164e2 100644 --- a/tests/baselines/reference/controlFlowTruthiness.symbols +++ b/tests/baselines/reference/controlFlowTruthiness.symbols @@ -140,3 +140,54 @@ function f6() { } } +function f7(x: {}) { +>f7 : Symbol(f7, Decl(controlFlowTruthiness.ts, 67, 1)) +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 69, 12)) + + if (x) { +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 69, 12)) + + x; // {} +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 69, 12)) + } + else { + x; // {} +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 69, 12)) + } +} + +function f8(x: T) { +>f8 : Symbol(f8, Decl(controlFlowTruthiness.ts, 76, 1)) +>T : Symbol(T, Decl(controlFlowTruthiness.ts, 78, 12)) +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 78, 15)) +>T : Symbol(T, Decl(controlFlowTruthiness.ts, 78, 12)) + + if (x) { +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 78, 15)) + + x; // {} +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 78, 15)) + } + else { + x; // {} +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 78, 15)) + } +} + +function f9(x: T) { +>f9 : Symbol(f9, Decl(controlFlowTruthiness.ts, 85, 1)) +>T : Symbol(T, Decl(controlFlowTruthiness.ts, 87, 12)) +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 87, 30)) +>T : Symbol(T, Decl(controlFlowTruthiness.ts, 87, 12)) + + if (x) { +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 87, 30)) + + x; // {} +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 87, 30)) + } + else { + x; // never +>x : Symbol(x, Decl(controlFlowTruthiness.ts, 87, 30)) + } +} diff --git a/tests/baselines/reference/controlFlowTruthiness.types b/tests/baselines/reference/controlFlowTruthiness.types index 225f6d0e7e25b..454d094f40651 100644 --- a/tests/baselines/reference/controlFlowTruthiness.types +++ b/tests/baselines/reference/controlFlowTruthiness.types @@ -157,3 +157,50 @@ function f6() { } } +function f7(x: {}) { +>f7 : (x: {}) => void +>x : {} + + if (x) { +>x : {} + + x; // {} +>x : {} + } + else { + x; // {} +>x : {} + } +} + +function f8(x: T) { +>f8 : (x: T) => void +>x : T + + if (x) { +>x : T + + x; // {} +>x : T + } + else { + x; // {} +>x : T + } +} + +function f9(x: T) { +>f9 : (x: T) => void +>x : T + + if (x) { +>x : T + + x; // {} +>x : T + } + else { + x; // never +>x : never + } +} diff --git a/tests/baselines/reference/keyofAndIndexedAccessErrors.errors.txt b/tests/baselines/reference/keyofAndIndexedAccessErrors.errors.txt index dd5cbe050a902..4053189aea3f8 100644 --- a/tests/baselines/reference/keyofAndIndexedAccessErrors.errors.txt +++ b/tests/baselines/reference/keyofAndIndexedAccessErrors.errors.txt @@ -61,9 +61,11 @@ tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts(114,5): error Type 'string' is not assignable to type 'J'. tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts(117,5): error TS2322: Type 'T[K]' is not assignable to type 'U[J]'. Type 'T' is not assignable to type 'U'. +tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts(122,5): error TS2322: Type '42' is not assignable to type 'keyof T'. +tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts(123,5): error TS2322: Type '"hello"' is not assignable to type 'keyof T'. -==== tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts (36 errors) ==== +==== tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts (38 errors) ==== class Shape { name: string; width: number; @@ -281,4 +283,14 @@ tests/cases/conformance/types/keyof/keyofAndIndexedAccessErrors.ts(117,5): error !!! error TS2322: Type 'T[K]' is not assignable to type 'U[J]'. !!! error TS2322: Type 'T' is not assignable to type 'U'. } + + // The constraint of 'keyof T' is 'keyof T' + function f4(k: keyof T) { + k = 42; // error + ~ +!!! error TS2322: Type '42' is not assignable to type 'keyof T'. + k = "hello"; // error + ~ +!!! error TS2322: Type '"hello"' is not assignable to type 'keyof T'. + } \ No newline at end of file diff --git a/tests/baselines/reference/keyofAndIndexedAccessErrors.js b/tests/baselines/reference/keyofAndIndexedAccessErrors.js index c64f4df533178..a3124ce4c711b 100644 --- a/tests/baselines/reference/keyofAndIndexedAccessErrors.js +++ b/tests/baselines/reference/keyofAndIndexedAccessErrors.js @@ -117,6 +117,12 @@ function f3, U extends T, J extends K>( tk = uj; uj = tk; // error } + +// The constraint of 'keyof T' is 'keyof T' +function f4(k: keyof T) { + k = 42; // error + k = "hello"; // error +} //// [keyofAndIndexedAccessErrors.js] @@ -178,3 +184,8 @@ function f3(t, k, tk, u, j, uk, tj, uj) { tk = uj; uj = tk; // error } +// The constraint of 'keyof T' is 'keyof T' +function f4(k) { + k = 42; // error + k = "hello"; // error +} diff --git a/tests/baselines/reference/keyofAndIndexedAccessErrors.symbols b/tests/baselines/reference/keyofAndIndexedAccessErrors.symbols index f5c5f41dddfc0..72739eb76d7e4 100644 --- a/tests/baselines/reference/keyofAndIndexedAccessErrors.symbols +++ b/tests/baselines/reference/keyofAndIndexedAccessErrors.symbols @@ -412,3 +412,19 @@ function f3, U extends T, J extends K>( >tk : Symbol(tk, Decl(keyofAndIndexedAccessErrors.ts, 99, 15)) } +// The constraint of 'keyof T' is 'keyof T' +function f4(k: keyof T) { +>f4 : Symbol(f4, Decl(keyofAndIndexedAccessErrors.ts, 117, 1)) +>T : Symbol(T, Decl(keyofAndIndexedAccessErrors.ts, 120, 12)) +>K : Symbol(K, Decl(keyofAndIndexedAccessErrors.ts, 120, 25)) +>T : Symbol(T, Decl(keyofAndIndexedAccessErrors.ts, 120, 12)) +>k : Symbol(k, Decl(keyofAndIndexedAccessErrors.ts, 120, 50)) +>T : Symbol(T, Decl(keyofAndIndexedAccessErrors.ts, 120, 12)) + + k = 42; // error +>k : Symbol(k, Decl(keyofAndIndexedAccessErrors.ts, 120, 50)) + + k = "hello"; // error +>k : Symbol(k, Decl(keyofAndIndexedAccessErrors.ts, 120, 50)) +} + diff --git a/tests/baselines/reference/keyofAndIndexedAccessErrors.types b/tests/baselines/reference/keyofAndIndexedAccessErrors.types index c15f5ebb6efd3..e5f8b6ee8e39c 100644 --- a/tests/baselines/reference/keyofAndIndexedAccessErrors.types +++ b/tests/baselines/reference/keyofAndIndexedAccessErrors.types @@ -396,3 +396,19 @@ function f3, U extends T, J extends K>( >tk : T[K] } +// The constraint of 'keyof T' is 'keyof T' +function f4(k: keyof T) { +>f4 : (k: keyof T) => void +>k : keyof T + + k = 42; // error +>k = 42 : 42 +>k : keyof T +>42 : 42 + + k = "hello"; // error +>k = "hello" : "hello" +>k : keyof T +>"hello" : "hello" +} +