From 343343414216c290cd68534ff50e39ee9beecb10 Mon Sep 17 00:00:00 2001 From: Titian Cernicova-Dragomir Date: Tue, 31 Mar 2020 00:45:32 +0300 Subject: [PATCH] Fixed issue where missing method call went unreported if the call target symbol did no have an id assigned or if the called property was used inside the if block on a different target. (#35862) --- src/compiler/checker.ts | 27 ++++++- ...ruthinessCallExpressionCoercion.errors.txt | 34 ++++++++- .../truthinessCallExpressionCoercion.js | 40 +++++++++- .../truthinessCallExpressionCoercion.symbols | 76 +++++++++++++++++++ .../truthinessCallExpressionCoercion.types | 71 +++++++++++++++++ .../truthinessCallExpressionCoercion.ts | 25 ++++++ 6 files changed, 268 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3c4384e02bd..d12687bdb5b 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -31620,8 +31620,31 @@ namespace ts { const functionIsUsedInBody = forEachChild(body, function check(childNode): boolean | undefined { if (isIdentifier(childNode)) { const childSymbol = getSymbolAtLocation(childNode); - if (childSymbol && childSymbol.id === testedFunctionSymbol.id) { - return true; + if (childSymbol && childSymbol === testedFunctionSymbol) { + // If the test was a simple identifier, the above check is sufficient + if (isIdentifier(ifStatement.expression)) { + return true; + } + // Otherwise we need to ensure the symbol is called on the same target + let testedExpression = testedNode.parent; + let childExpression = childNode.parent; + while (testedExpression && childExpression) { + + if (isIdentifier(testedExpression) && isIdentifier(childExpression)) { + return getSymbolAtLocation(testedExpression) === getSymbolAtLocation(childExpression); + } + + if (isPropertyAccessExpression(testedExpression) && isPropertyAccessExpression(childExpression)) { + if (getSymbolAtLocation(testedExpression.name) !== getSymbolAtLocation(childExpression.name)) { + return false; + } + childExpression = childExpression.expression; + testedExpression = testedExpression.expression; + } + else { + return false; + } + } } } diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt b/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt index 1599a70db4e..0e0b2c98404 100644 --- a/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt +++ b/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt @@ -3,9 +3,11 @@ tests/cases/compiler/truthinessCallExpressionCoercion.ts(18,9): error TS2774: Th tests/cases/compiler/truthinessCallExpressionCoercion.ts(36,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? tests/cases/compiler/truthinessCallExpressionCoercion.ts(50,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? tests/cases/compiler/truthinessCallExpressionCoercion.ts(66,13): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? +tests/cases/compiler/truthinessCallExpressionCoercion.ts(76,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? +tests/cases/compiler/truthinessCallExpressionCoercion.ts(82,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? -==== tests/cases/compiler/truthinessCallExpressionCoercion.ts (5 errors) ==== +==== tests/cases/compiler/truthinessCallExpressionCoercion.ts (7 errors) ==== function onlyErrorsWhenTestingNonNullableFunctionType(required: () => boolean, optional?: () => boolean) { if (required) { // error ~~~~~~~~ @@ -88,4 +90,32 @@ tests/cases/compiler/truthinessCallExpressionCoercion.ts(66,13): error TS2774: T } } } - \ No newline at end of file + + // Test for GH-35557 where ids were not assigned for a symbol. + function A(stats: StatsBase) { + if (stats.isDirectory) { // err + ~~~~~~~~~~~~~~~~~ +!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? + console.log(`[Directory] ${stats.ctime}`) + } + } + + function B(a: Nested, b: Nested) { + if (a.stats.isDirectory) { // err + ~~~~~~~~~~~~~~~~~~~ +!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? + b.stats.isDirectory(); + } + if (a.stats.isDirectory) { // ok + a.stats.isDirectory(); + } + } + + interface StatsBase { + isDirectory(): boolean; + ctime: number; + } + + interface Nested { + stats: StatsBase; + } \ No newline at end of file diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion.js b/tests/baselines/reference/truthinessCallExpressionCoercion.js index 19ab1722c81..3f43e629671 100644 --- a/tests/baselines/reference/truthinessCallExpressionCoercion.js +++ b/tests/baselines/reference/truthinessCallExpressionCoercion.js @@ -71,7 +71,31 @@ class Foo { } } } - + +// Test for GH-35557 where ids were not assigned for a symbol. +function A(stats: StatsBase) { + if (stats.isDirectory) { // err + console.log(`[Directory] ${stats.ctime}`) + } +} + +function B(a: Nested, b: Nested) { + if (a.stats.isDirectory) { // err + b.stats.isDirectory(); + } + if (a.stats.isDirectory) { // ok + a.stats.isDirectory(); + } +} + +interface StatsBase { + isDirectory(): boolean; + ctime: number; +} + +interface Nested { + stats: StatsBase; +} //// [truthinessCallExpressionCoercion.js] function onlyErrorsWhenTestingNonNullableFunctionType(required, optional) { @@ -132,3 +156,17 @@ var Foo = /** @class */ (function () { }; return Foo; }()); +// Test for GH-35557 where ids were not assigned for a symbol. +function A(stats) { + if (stats.isDirectory) { // err + console.log("[Directory] " + stats.ctime); + } +} +function B(a, b) { + if (a.stats.isDirectory) { // err + b.stats.isDirectory(); + } + if (a.stats.isDirectory) { // ok + a.stats.isDirectory(); + } +} diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion.symbols b/tests/baselines/reference/truthinessCallExpressionCoercion.symbols index cbfe456a82c..3a2af7d0d05 100644 --- a/tests/baselines/reference/truthinessCallExpressionCoercion.symbols +++ b/tests/baselines/reference/truthinessCallExpressionCoercion.symbols @@ -151,3 +151,79 @@ class Foo { } } +// Test for GH-35557 where ids were not assigned for a symbol. +function A(stats: StatsBase) { +>A : Symbol(A, Decl(truthinessCallExpressionCoercion.ts, 71, 1)) +>stats : Symbol(stats, Decl(truthinessCallExpressionCoercion.ts, 74, 11)) +>StatsBase : Symbol(StatsBase, Decl(truthinessCallExpressionCoercion.ts, 87, 1)) + + if (stats.isDirectory) { // err +>stats.isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) +>stats : Symbol(stats, Decl(truthinessCallExpressionCoercion.ts, 74, 11)) +>isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) + + console.log(`[Directory] ${stats.ctime}`) +>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) +>console : Symbol(console, Decl(lib.dom.d.ts, --, --)) +>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) +>stats.ctime : Symbol(StatsBase.ctime, Decl(truthinessCallExpressionCoercion.ts, 90, 27)) +>stats : Symbol(stats, Decl(truthinessCallExpressionCoercion.ts, 74, 11)) +>ctime : Symbol(StatsBase.ctime, Decl(truthinessCallExpressionCoercion.ts, 90, 27)) + } +} + +function B(a: Nested, b: Nested) { +>B : Symbol(B, Decl(truthinessCallExpressionCoercion.ts, 78, 1)) +>a : Symbol(a, Decl(truthinessCallExpressionCoercion.ts, 80, 11)) +>Nested : Symbol(Nested, Decl(truthinessCallExpressionCoercion.ts, 92, 1)) +>b : Symbol(b, Decl(truthinessCallExpressionCoercion.ts, 80, 21)) +>Nested : Symbol(Nested, Decl(truthinessCallExpressionCoercion.ts, 92, 1)) + + if (a.stats.isDirectory) { // err +>a.stats.isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) +>a.stats : Symbol(Nested.stats, Decl(truthinessCallExpressionCoercion.ts, 94, 18)) +>a : Symbol(a, Decl(truthinessCallExpressionCoercion.ts, 80, 11)) +>stats : Symbol(Nested.stats, Decl(truthinessCallExpressionCoercion.ts, 94, 18)) +>isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) + + b.stats.isDirectory(); +>b.stats.isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) +>b.stats : Symbol(Nested.stats, Decl(truthinessCallExpressionCoercion.ts, 94, 18)) +>b : Symbol(b, Decl(truthinessCallExpressionCoercion.ts, 80, 21)) +>stats : Symbol(Nested.stats, Decl(truthinessCallExpressionCoercion.ts, 94, 18)) +>isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) + } + if (a.stats.isDirectory) { // ok +>a.stats.isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) +>a.stats : Symbol(Nested.stats, Decl(truthinessCallExpressionCoercion.ts, 94, 18)) +>a : Symbol(a, Decl(truthinessCallExpressionCoercion.ts, 80, 11)) +>stats : Symbol(Nested.stats, Decl(truthinessCallExpressionCoercion.ts, 94, 18)) +>isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) + + a.stats.isDirectory(); +>a.stats.isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) +>a.stats : Symbol(Nested.stats, Decl(truthinessCallExpressionCoercion.ts, 94, 18)) +>a : Symbol(a, Decl(truthinessCallExpressionCoercion.ts, 80, 11)) +>stats : Symbol(Nested.stats, Decl(truthinessCallExpressionCoercion.ts, 94, 18)) +>isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) + } +} + +interface StatsBase { +>StatsBase : Symbol(StatsBase, Decl(truthinessCallExpressionCoercion.ts, 87, 1)) +>T : Symbol(T, Decl(truthinessCallExpressionCoercion.ts, 89, 20)) + + isDirectory(): boolean; +>isDirectory : Symbol(StatsBase.isDirectory, Decl(truthinessCallExpressionCoercion.ts, 89, 24)) + + ctime: number; +>ctime : Symbol(StatsBase.ctime, Decl(truthinessCallExpressionCoercion.ts, 90, 27)) +} + +interface Nested { +>Nested : Symbol(Nested, Decl(truthinessCallExpressionCoercion.ts, 92, 1)) + + stats: StatsBase; +>stats : Symbol(Nested.stats, Decl(truthinessCallExpressionCoercion.ts, 94, 18)) +>StatsBase : Symbol(StatsBase, Decl(truthinessCallExpressionCoercion.ts, 87, 1)) +} diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion.types b/tests/baselines/reference/truthinessCallExpressionCoercion.types index 004228c40f6..6b934f44f22 100644 --- a/tests/baselines/reference/truthinessCallExpressionCoercion.types +++ b/tests/baselines/reference/truthinessCallExpressionCoercion.types @@ -177,3 +177,74 @@ class Foo { } } +// Test for GH-35557 where ids were not assigned for a symbol. +function A(stats: StatsBase) { +>A : (stats: StatsBase) => void +>stats : StatsBase + + if (stats.isDirectory) { // err +>stats.isDirectory : () => boolean +>stats : StatsBase +>isDirectory : () => boolean + + console.log(`[Directory] ${stats.ctime}`) +>console.log(`[Directory] ${stats.ctime}`) : void +>console.log : (message?: any, ...optionalParams: any[]) => void +>console : Console +>log : (message?: any, ...optionalParams: any[]) => void +>`[Directory] ${stats.ctime}` : string +>stats.ctime : number +>stats : StatsBase +>ctime : number + } +} + +function B(a: Nested, b: Nested) { +>B : (a: Nested, b: Nested) => void +>a : Nested +>b : Nested + + if (a.stats.isDirectory) { // err +>a.stats.isDirectory : () => boolean +>a.stats : StatsBase +>a : Nested +>stats : StatsBase +>isDirectory : () => boolean + + b.stats.isDirectory(); +>b.stats.isDirectory() : boolean +>b.stats.isDirectory : () => boolean +>b.stats : StatsBase +>b : Nested +>stats : StatsBase +>isDirectory : () => boolean + } + if (a.stats.isDirectory) { // ok +>a.stats.isDirectory : () => boolean +>a.stats : StatsBase +>a : Nested +>stats : StatsBase +>isDirectory : () => boolean + + a.stats.isDirectory(); +>a.stats.isDirectory() : boolean +>a.stats.isDirectory : () => boolean +>a.stats : StatsBase +>a : Nested +>stats : StatsBase +>isDirectory : () => boolean + } +} + +interface StatsBase { + isDirectory(): boolean; +>isDirectory : () => boolean + + ctime: number; +>ctime : number +} + +interface Nested { + stats: StatsBase; +>stats : StatsBase +} diff --git a/tests/cases/compiler/truthinessCallExpressionCoercion.ts b/tests/cases/compiler/truthinessCallExpressionCoercion.ts index 431a0b0463c..a094e8c716e 100644 --- a/tests/cases/compiler/truthinessCallExpressionCoercion.ts +++ b/tests/cases/compiler/truthinessCallExpressionCoercion.ts @@ -72,3 +72,28 @@ class Foo { } } } + +// Test for GH-35557 where ids were not assigned for a symbol. +function A(stats: StatsBase) { + if (stats.isDirectory) { // err + console.log(`[Directory] ${stats.ctime}`) + } +} + +function B(a: Nested, b: Nested) { + if (a.stats.isDirectory) { // err + b.stats.isDirectory(); + } + if (a.stats.isDirectory) { // ok + a.stats.isDirectory(); + } +} + +interface StatsBase { + isDirectory(): boolean; + ctime: number; +} + +interface Nested { + stats: StatsBase; +} \ No newline at end of file