From 14cee1f50350551a79870dd66507aea71115dd88 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Fri, 20 Mar 2020 13:14:02 -0700 Subject: [PATCH 1/4] For x && typeof x === 'object', narrow x to just type object --- src/compiler/checker.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4be7ea8841fdf..acacb4eaa07ca 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -18947,6 +18947,12 @@ namespace ts { return false; } + function containsTruthyCheck(source: Node, target: Node) { + return isMatchingReference(source, target) || + (target.kind === SyntaxKind.BinaryExpression && (target).operatorToken.kind === SyntaxKind.AmpersandAmpersandToken && + (isMatchingReference(source, (target).left) || isMatchingReference(source, (target).right))); + } + function getAccessedPropertyName(access: AccessExpression): __String | undefined { return access.kind === SyntaxKind.PropertyAccessExpression ? access.name.escapedText : isStringOrNumericLiteralLike(access.argumentExpression) ? escapeLeadingUnderscores(access.argumentExpression.text) : @@ -20343,15 +20349,22 @@ namespace ts { if (type.flags & TypeFlags.Any && literal.text === "function") { return type; } + if (assumeTrue && type.flags & TypeFlags.Unknown && literal.text === "object") { + // The pattern x && typeof x === 'object', where x is of type unknown, narrows x to type object. + if (typeOfExpr.parent.parent.kind === SyntaxKind.BinaryExpression) { + const expr = typeOfExpr.parent.parent; + if (expr.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken && expr.right === typeOfExpr.parent && containsTruthyCheck(reference, expr.left)) { + return nonPrimitiveType; + } + } + return getUnionType([nonPrimitiveType, nullType]); + } const facts = assumeTrue ? typeofEQFacts.get(literal.text) || TypeFacts.TypeofEQHostObject : typeofNEFacts.get(literal.text) || TypeFacts.TypeofNEHostObject; return getTypeWithFacts(assumeTrue ? mapType(type, narrowTypeForTypeof) : type, facts); function narrowTypeForTypeof(type: Type) { - 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. From 1380811a34570579ca3ffd3ee10691d72d05492e Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Fri, 20 Mar 2020 16:08:16 -0700 Subject: [PATCH 2/4] Add tests --- .../narrowingTruthyObject.errors.txt | 30 ++++++ .../reference/narrowingTruthyObject.js | 49 +++++++++ .../reference/narrowingTruthyObject.symbols | 68 ++++++++++++ .../reference/narrowingTruthyObject.types | 101 ++++++++++++++++++ tests/cases/compiler/narrowingTruthyObject.ts | 25 +++++ 5 files changed, 273 insertions(+) create mode 100644 tests/baselines/reference/narrowingTruthyObject.errors.txt create mode 100644 tests/baselines/reference/narrowingTruthyObject.js create mode 100644 tests/baselines/reference/narrowingTruthyObject.symbols create mode 100644 tests/baselines/reference/narrowingTruthyObject.types create mode 100644 tests/cases/compiler/narrowingTruthyObject.ts diff --git a/tests/baselines/reference/narrowingTruthyObject.errors.txt b/tests/baselines/reference/narrowingTruthyObject.errors.txt new file mode 100644 index 0000000000000..a35597516cacc --- /dev/null +++ b/tests/baselines/reference/narrowingTruthyObject.errors.txt @@ -0,0 +1,30 @@ +tests/cases/compiler/narrowingTruthyObject.ts(3,9): error TS2531: Object is possibly 'null'. + + +==== tests/cases/compiler/narrowingTruthyObject.ts (1 errors) ==== + function foo(x: unknown, b: boolean) { + if (typeof x === 'object') { + x.toString(); + ~ +!!! error TS2531: Object is possibly 'null'. + } + if (typeof x === 'object' && x) { + x.toString(); + } + if (x && typeof x === 'object') { + x.toString(); + } + if (b && x && typeof x === 'object') { + x.toString(); + } + if (x && b && typeof x === 'object') { + x.toString(); + } + } + + // Repro from #36870 + + function f1(x: unknown): any { + return x && typeof x === 'object' && x.hasOwnProperty('x'); + } + \ No newline at end of file diff --git a/tests/baselines/reference/narrowingTruthyObject.js b/tests/baselines/reference/narrowingTruthyObject.js new file mode 100644 index 0000000000000..8d46bece056d7 --- /dev/null +++ b/tests/baselines/reference/narrowingTruthyObject.js @@ -0,0 +1,49 @@ +//// [narrowingTruthyObject.ts] +function foo(x: unknown, b: boolean) { + if (typeof x === 'object') { + x.toString(); + } + if (typeof x === 'object' && x) { + x.toString(); + } + if (x && typeof x === 'object') { + x.toString(); + } + if (b && x && typeof x === 'object') { + x.toString(); + } + if (x && b && typeof x === 'object') { + x.toString(); + } +} + +// Repro from #36870 + +function f1(x: unknown): any { + return x && typeof x === 'object' && x.hasOwnProperty('x'); +} + + +//// [narrowingTruthyObject.js] +"use strict"; +function foo(x, b) { + if (typeof x === 'object') { + x.toString(); + } + if (typeof x === 'object' && x) { + x.toString(); + } + if (x && typeof x === 'object') { + x.toString(); + } + if (b && x && typeof x === 'object') { + x.toString(); + } + if (x && b && typeof x === 'object') { + x.toString(); + } +} +// Repro from #36870 +function f1(x) { + return x && typeof x === 'object' && x.hasOwnProperty('x'); +} diff --git a/tests/baselines/reference/narrowingTruthyObject.symbols b/tests/baselines/reference/narrowingTruthyObject.symbols new file mode 100644 index 0000000000000..44a82b93e1080 --- /dev/null +++ b/tests/baselines/reference/narrowingTruthyObject.symbols @@ -0,0 +1,68 @@ +=== tests/cases/compiler/narrowingTruthyObject.ts === +function foo(x: unknown, b: boolean) { +>foo : Symbol(foo, Decl(narrowingTruthyObject.ts, 0, 0)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>b : Symbol(b, Decl(narrowingTruthyObject.ts, 0, 24)) + + if (typeof x === 'object') { +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) + + x.toString(); +>x.toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) + } + if (typeof x === 'object' && x) { +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) + + x.toString(); +>x.toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) + } + if (x && typeof x === 'object') { +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) + + x.toString(); +>x.toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) + } + if (b && x && typeof x === 'object') { +>b : Symbol(b, Decl(narrowingTruthyObject.ts, 0, 24)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) + + x.toString(); +>x.toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) + } + if (x && b && typeof x === 'object') { +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>b : Symbol(b, Decl(narrowingTruthyObject.ts, 0, 24)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) + + x.toString(); +>x.toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) + } +} + +// Repro from #36870 + +function f1(x: unknown): any { +>f1 : Symbol(f1, Decl(narrowingTruthyObject.ts, 16, 1)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 20, 12)) + + return x && typeof x === 'object' && x.hasOwnProperty('x'); +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 20, 12)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 20, 12)) +>x.hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 20, 12)) +>hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.es5.d.ts, --, --)) +} + diff --git a/tests/baselines/reference/narrowingTruthyObject.types b/tests/baselines/reference/narrowingTruthyObject.types new file mode 100644 index 0000000000000..a2f96fbc70df7 --- /dev/null +++ b/tests/baselines/reference/narrowingTruthyObject.types @@ -0,0 +1,101 @@ +=== tests/cases/compiler/narrowingTruthyObject.ts === +function foo(x: unknown, b: boolean) { +>foo : (x: unknown, b: boolean) => void +>x : unknown +>b : boolean + + if (typeof x === 'object') { +>typeof x === 'object' : boolean +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>x : unknown +>'object' : "object" + + x.toString(); +>x.toString() : string +>x.toString : () => string +>x : object | null +>toString : () => string + } + if (typeof x === 'object' && x) { +>typeof x === 'object' && x : false | object | null +>typeof x === 'object' : boolean +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>x : unknown +>'object' : "object" +>x : object | null + + x.toString(); +>x.toString() : string +>x.toString : () => string +>x : object +>toString : () => string + } + if (x && typeof x === 'object') { +>x && typeof x === 'object' : boolean +>x : unknown +>typeof x === 'object' : boolean +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>x : unknown +>'object' : "object" + + x.toString(); +>x.toString() : string +>x.toString : () => string +>x : object +>toString : () => string + } + if (b && x && typeof x === 'object') { +>b && x && typeof x === 'object' : boolean +>b && x : unknown +>b : boolean +>x : unknown +>typeof x === 'object' : boolean +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>x : unknown +>'object' : "object" + + x.toString(); +>x.toString() : string +>x.toString : () => string +>x : object +>toString : () => string + } + if (x && b && typeof x === 'object') { +>x && b && typeof x === 'object' : boolean +>x && b : boolean +>x : unknown +>b : boolean +>typeof x === 'object' : boolean +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>x : unknown +>'object' : "object" + + x.toString(); +>x.toString() : string +>x.toString : () => string +>x : object +>toString : () => string + } +} + +// Repro from #36870 + +function f1(x: unknown): any { +>f1 : (x: unknown) => any +>x : unknown + + return x && typeof x === 'object' && x.hasOwnProperty('x'); +>x && typeof x === 'object' && x.hasOwnProperty('x') : boolean +>x && typeof x === 'object' : boolean +>x : unknown +>typeof x === 'object' : boolean +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>x : unknown +>'object' : "object" +>x.hasOwnProperty('x') : boolean +>x.hasOwnProperty : (v: string | number | symbol) => boolean +>x : object +>hasOwnProperty : (v: string | number | symbol) => boolean +>'x' : "x" +} + diff --git a/tests/cases/compiler/narrowingTruthyObject.ts b/tests/cases/compiler/narrowingTruthyObject.ts new file mode 100644 index 0000000000000..34e589d92463c --- /dev/null +++ b/tests/cases/compiler/narrowingTruthyObject.ts @@ -0,0 +1,25 @@ +// @strict: true + +function foo(x: unknown, b: boolean) { + if (typeof x === 'object') { + x.toString(); + } + if (typeof x === 'object' && x) { + x.toString(); + } + if (x && typeof x === 'object') { + x.toString(); + } + if (b && x && typeof x === 'object') { + x.toString(); + } + if (x && b && typeof x === 'object') { + x.toString(); + } +} + +// Repro from #36870 + +function f1(x: unknown): any { + return x && typeof x === 'object' && x.hasOwnProperty('x'); +} From 092a80563c18b6c87ed5c2995dbef38843155288 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Fri, 20 Mar 2020 16:46:39 -0700 Subject: [PATCH 3/4] Allow arbitrary nesting / add comments --- src/compiler/checker.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index acacb4eaa07ca..364e3f8913d85 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -18947,10 +18947,11 @@ namespace ts { return false; } - function containsTruthyCheck(source: Node, target: Node) { + // Given a source x, check if target matches x or is an && operation with an operand that matches x. + function containsTruthyCheck(source: Node, target: Node): boolean { return isMatchingReference(source, target) || (target.kind === SyntaxKind.BinaryExpression && (target).operatorToken.kind === SyntaxKind.AmpersandAmpersandToken && - (isMatchingReference(source, (target).left) || isMatchingReference(source, (target).right))); + (containsTruthyCheck(source, (target).left) || containsTruthyCheck(source, (target).right))); } function getAccessedPropertyName(access: AccessExpression): __String | undefined { @@ -20350,7 +20351,8 @@ namespace ts { return type; } if (assumeTrue && type.flags & TypeFlags.Unknown && literal.text === "object") { - // The pattern x && typeof x === 'object', where x is of type unknown, narrows x to type object. + // The pattern x && typeof x === 'object', where x is of type unknown, narrows x to type object. We don't + // need to check for the reverse typeof x === 'object' && x since that already narrows correctly. if (typeOfExpr.parent.parent.kind === SyntaxKind.BinaryExpression) { const expr = typeOfExpr.parent.parent; if (expr.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken && expr.right === typeOfExpr.parent && containsTruthyCheck(reference, expr.left)) { From b5ef182e434efd8c768bdb4a1d04c99b24b940d1 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Fri, 20 Mar 2020 16:48:17 -0700 Subject: [PATCH 4/4] Add additional tests --- .../narrowingTruthyObject.errors.txt | 6 +++ .../reference/narrowingTruthyObject.js | 12 ++++++ .../reference/narrowingTruthyObject.symbols | 34 +++++++++++++--- .../reference/narrowingTruthyObject.types | 40 +++++++++++++++++++ tests/cases/compiler/narrowingTruthyObject.ts | 6 +++ 5 files changed, 93 insertions(+), 5 deletions(-) diff --git a/tests/baselines/reference/narrowingTruthyObject.errors.txt b/tests/baselines/reference/narrowingTruthyObject.errors.txt index a35597516cacc..793e2fa5200cb 100644 --- a/tests/baselines/reference/narrowingTruthyObject.errors.txt +++ b/tests/baselines/reference/narrowingTruthyObject.errors.txt @@ -20,6 +20,12 @@ tests/cases/compiler/narrowingTruthyObject.ts(3,9): error TS2531: Object is poss if (x && b && typeof x === 'object') { x.toString(); } + if (x && b && b && typeof x === 'object') { + x.toString(); + } + if (b && b && x && b && b && typeof x === 'object') { + x.toString(); + } } // Repro from #36870 diff --git a/tests/baselines/reference/narrowingTruthyObject.js b/tests/baselines/reference/narrowingTruthyObject.js index 8d46bece056d7..719a7e2ade86a 100644 --- a/tests/baselines/reference/narrowingTruthyObject.js +++ b/tests/baselines/reference/narrowingTruthyObject.js @@ -15,6 +15,12 @@ function foo(x: unknown, b: boolean) { if (x && b && typeof x === 'object') { x.toString(); } + if (x && b && b && typeof x === 'object') { + x.toString(); + } + if (b && b && x && b && b && typeof x === 'object') { + x.toString(); + } } // Repro from #36870 @@ -42,6 +48,12 @@ function foo(x, b) { if (x && b && typeof x === 'object') { x.toString(); } + if (x && b && b && typeof x === 'object') { + x.toString(); + } + if (b && b && x && b && b && typeof x === 'object') { + x.toString(); + } } // Repro from #36870 function f1(x) { diff --git a/tests/baselines/reference/narrowingTruthyObject.symbols b/tests/baselines/reference/narrowingTruthyObject.symbols index 44a82b93e1080..9457ed95a8167 100644 --- a/tests/baselines/reference/narrowingTruthyObject.symbols +++ b/tests/baselines/reference/narrowingTruthyObject.symbols @@ -48,6 +48,30 @@ function foo(x: unknown, b: boolean) { x.toString(); >x.toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) >x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) + } + if (x && b && b && typeof x === 'object') { +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>b : Symbol(b, Decl(narrowingTruthyObject.ts, 0, 24)) +>b : Symbol(b, Decl(narrowingTruthyObject.ts, 0, 24)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) + + x.toString(); +>x.toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) + } + if (b && b && x && b && b && typeof x === 'object') { +>b : Symbol(b, Decl(narrowingTruthyObject.ts, 0, 24)) +>b : Symbol(b, Decl(narrowingTruthyObject.ts, 0, 24)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) +>b : Symbol(b, Decl(narrowingTruthyObject.ts, 0, 24)) +>b : Symbol(b, Decl(narrowingTruthyObject.ts, 0, 24)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) + + x.toString(); +>x.toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 0, 13)) >toString : Symbol(Object.toString, Decl(lib.es5.d.ts, --, --)) } } @@ -55,14 +79,14 @@ function foo(x: unknown, b: boolean) { // Repro from #36870 function f1(x: unknown): any { ->f1 : Symbol(f1, Decl(narrowingTruthyObject.ts, 16, 1)) ->x : Symbol(x, Decl(narrowingTruthyObject.ts, 20, 12)) +>f1 : Symbol(f1, Decl(narrowingTruthyObject.ts, 22, 1)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 26, 12)) return x && typeof x === 'object' && x.hasOwnProperty('x'); ->x : Symbol(x, Decl(narrowingTruthyObject.ts, 20, 12)) ->x : Symbol(x, Decl(narrowingTruthyObject.ts, 20, 12)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 26, 12)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 26, 12)) >x.hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.es5.d.ts, --, --)) ->x : Symbol(x, Decl(narrowingTruthyObject.ts, 20, 12)) +>x : Symbol(x, Decl(narrowingTruthyObject.ts, 26, 12)) >hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.es5.d.ts, --, --)) } diff --git a/tests/baselines/reference/narrowingTruthyObject.types b/tests/baselines/reference/narrowingTruthyObject.types index a2f96fbc70df7..bf8f9ead875d3 100644 --- a/tests/baselines/reference/narrowingTruthyObject.types +++ b/tests/baselines/reference/narrowingTruthyObject.types @@ -74,6 +74,46 @@ function foo(x: unknown, b: boolean) { >x.toString() : string >x.toString : () => string >x : object +>toString : () => string + } + if (x && b && b && typeof x === 'object') { +>x && b && b && typeof x === 'object' : boolean +>x && b && b : boolean +>x && b : boolean +>x : unknown +>b : boolean +>b : true +>typeof x === 'object' : boolean +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>x : unknown +>'object' : "object" + + x.toString(); +>x.toString() : string +>x.toString : () => string +>x : object +>toString : () => string + } + if (b && b && x && b && b && typeof x === 'object') { +>b && b && x && b && b && typeof x === 'object' : boolean +>b && b && x && b && b : true +>b && b && x && b : true +>b && b && x : unknown +>b && b : boolean +>b : boolean +>b : true +>x : unknown +>b : true +>b : true +>typeof x === 'object' : boolean +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>x : unknown +>'object' : "object" + + x.toString(); +>x.toString() : string +>x.toString : () => string +>x : object >toString : () => string } } diff --git a/tests/cases/compiler/narrowingTruthyObject.ts b/tests/cases/compiler/narrowingTruthyObject.ts index 34e589d92463c..edc72686c40a1 100644 --- a/tests/cases/compiler/narrowingTruthyObject.ts +++ b/tests/cases/compiler/narrowingTruthyObject.ts @@ -16,6 +16,12 @@ function foo(x: unknown, b: boolean) { if (x && b && typeof x === 'object') { x.toString(); } + if (x && b && b && typeof x === 'object') { + x.toString(); + } + if (b && b && x && b && b && typeof x === 'object') { + x.toString(); + } } // Repro from #36870