From 8715473448508d502922cec996384f8da1190c81 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sun, 29 Dec 2024 21:07:31 +0000 Subject: [PATCH] Switch to checking for assignment when inverted --- src/compiler/checker.ts | 46 +++++++++---- ...xtualOverloadListFromArrayUnion.errors.txt | 69 ------------------- ...ruthinessCallExpressionCoercion.errors.txt | 5 +- ...uthinessCallExpressionCoercion1.errors.txt | 5 +- ...uthinessCallExpressionCoercion2.errors.txt | 5 +- .../truthinessPromiseCoercion.errors.txt | 10 +-- 6 files changed, 35 insertions(+), 105 deletions(-) delete mode 100644 tests/baselines/reference/contextualOverloadListFromArrayUnion.errors.txt diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 1cb1eb828efee..520631a8ff841 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -44539,30 +44539,30 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { function checkTestingKnownTruthyCallableOrAwaitableOrEnumMemberType(condExpr: Expression, condType: Type, body?: Statement | Expression) { if (!strictNullChecks) return; - bothHelper(condExpr, condType, body); + bothHelper(condExpr, body); - function bothHelper(condExpr: Expression, condType: Type, body: Expression | Statement | undefined) { + function bothHelper(condExpr: Expression, body: Expression | Statement | undefined) { condExpr = skipParentheses(condExpr); - helper(condExpr, condType, body); + helper(condExpr, body); while (isBinaryExpression(condExpr) && (condExpr.operatorToken.kind === SyntaxKind.BarBarToken || condExpr.operatorToken.kind === SyntaxKind.QuestionQuestionToken)) { condExpr = skipParentheses(condExpr.left); - helper(condExpr, condType, body); + helper(condExpr, body); } } - function helper(condExpr: Expression, condType: Type, body: Expression | Statement | undefined) { + function helper(condExpr: Expression, body: Expression | Statement | undefined) { let location = condExpr; - // let checkAwaitOnly = false; + let inverted = false; while (isPrefixUnaryExpression(location)) { location = skipParentheses(location.operand); - // checkAwaitOnly = true; + inverted = !inverted; } location = isLogicalOrCoalescingBinaryExpression(location) ? skipParentheses(location.right) : location; if (isModuleExportsAccessExpression(location)) { return; } if (isLogicalOrCoalescingBinaryExpression(location)) { - bothHelper(location, condType, body); + bothHelper(location, body); return; } const type = location === condExpr ? condType : checkExpression(location); @@ -44593,8 +44593,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { return; } + const isUsedInBody = inverted && isIfStatement(condExpr.parent) ? testedSymbol && isSymbolUsedInConditionBody(condExpr, condExpr.parent.parent, testedNode, testedSymbol, /*checkForAssignment*/ true) : testedSymbol && body && isSymbolUsedInConditionBody(condExpr, body, testedNode, testedSymbol, /*checkForAssignment*/ false); const isUsed = testedSymbol && isBinaryExpression(condExpr.parent) && isSymbolUsedInBinaryExpressionChain(condExpr.parent, testedSymbol) - || testedSymbol && body && isSymbolUsedInConditionBody(condExpr, body, testedNode, testedSymbol); + || isUsedInBody; + if (!isUsed) { if (isPromise) { errorAndMaybeSuggestAwait( @@ -44604,7 +44606,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { getTypeNameForErrorDisplay(type), ); } - // else if (!checkAwaitOnly) { else { error(location, Diagnostics.This_condition_will_always_return_true_since_this_function_is_always_defined_Did_you_mean_to_call_it_instead); } @@ -44612,15 +44613,23 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { } } - function isSymbolUsedInConditionBody(expr: Expression, body: Statement | Expression, testedNode: Node, testedSymbol: Symbol): boolean { + function isSymbolUsedInConditionBody(expr: Expression, body: Statement | Expression | Node, testedNode: Node, testedSymbol: Symbol, checkForAssignment: boolean): boolean { return !!forEachChild(body, function check(childNode): boolean | undefined { if (isIdentifier(childNode)) { const childSymbol = getSymbolAtLocation(childNode); if (childSymbol && childSymbol === testedSymbol) { - // If the test was a simple identifier, the above check is sufficient - if (isIdentifier(expr) || isIdentifier(testedNode) && isBinaryExpression(testedNode.parent)) { - return true; + if (checkForAssignment) { + if (isIdentifier(expr) && isBinaryExpression(childNode.parent) && childNode.parent.operatorToken.kind === SyntaxKind.EqualsToken) { + return true; + } } + else { + // If the test was a simple identifier, the above check is sufficient + if (isIdentifier(expr) || isIdentifier(testedNode) && isBinaryExpression(testedNode.parent)) { + return true; + } + } + // Otherwise we need to ensure the symbol is called on the same target let testedExpression = testedNode.parent; let childExpression = childNode.parent; @@ -44629,7 +44638,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { isIdentifier(testedExpression) && isIdentifier(childExpression) || testedExpression.kind === SyntaxKind.ThisKeyword && childExpression.kind === SyntaxKind.ThisKeyword ) { - return getSymbolAtLocation(testedExpression) === getSymbolAtLocation(childExpression); + const sameSymbol = getSymbolAtLocation(testedExpression) === getSymbolAtLocation(childExpression); + if (sameSymbol) { + if (checkForAssignment) { + return isPropertyAccessExpression(childNode.parent) && isBinaryExpression(childExpression.parent.parent) && childExpression.parent.parent.operatorToken.kind === SyntaxKind.EqualsToken; + } + return true; + } + return false; } else if (isPropertyAccessExpression(testedExpression) && isPropertyAccessExpression(childExpression)) { if (getSymbolAtLocation(testedExpression.name) !== getSymbolAtLocation(childExpression.name)) { diff --git a/tests/baselines/reference/contextualOverloadListFromArrayUnion.errors.txt b/tests/baselines/reference/contextualOverloadListFromArrayUnion.errors.txt deleted file mode 100644 index 7def0f968e165..0000000000000 --- a/tests/baselines/reference/contextualOverloadListFromArrayUnion.errors.txt +++ /dev/null @@ -1,69 +0,0 @@ -three.ts(28,14): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? - - -==== one.ts (0 errors) ==== - declare const y: never[] | string[]; - export const yThen = y.map(item => item.length); -==== two.ts (0 errors) ==== - declare const y: number[][] | string[]; - export const yThen = y.map(item => item.length); -==== three.ts (1 errors) ==== - // #42504 - interface ResizeObserverCallback { - (entries: ResizeObserverEntry[], observer: ResizeObserver): void; - } - interface ResizeObserverCallback { // duplicate for effect - (entries: ResizeObserverEntry[], observer: ResizeObserver): void; - } - - const resizeObserver = new ResizeObserver(([entry]) => { - entry - }); - // comment in #35501 - interface Callback { - (error: null, result: T): unknown - (error: Error, result: null): unknown - } - - interface Task { - (callback: Callback): unknown - } - - export function series(tasks: Task[], callback: Callback): void { - let index = 0 - let results: T[] = [] - - function next() { - let task = tasks[index] - if (!task) { - ~~~~ -!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? - callback(null, results) - } else { - task((error, result) => { - if (error) { - callback(error, null) - } else { - // must use postfix-!, since `error` and `result` don't have a - // causal relationship when the overloads are combined - results.push(result!) - next() - } - }) - } - } - next() - } - - series([ - cb => setTimeout(() => cb(null, 1), 300), - cb => setTimeout(() => cb(null, 2), 200), - cb => setTimeout(() => cb(null, 3), 100), - ], (error, results) => { - if (error) { - console.error(error) - } else { - console.log(results) - } - }) - \ No newline at end of file diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt b/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt index 239cea1d41f40..492cbdfcbffc1 100644 --- a/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt +++ b/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt @@ -1,5 +1,4 @@ truthinessCallExpressionCoercion.ts(2,9): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? -truthinessCallExpressionCoercion.ts(8,11): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion.ts(18,9): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion.ts(36,9): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion.ts(50,9): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? @@ -8,7 +7,7 @@ truthinessCallExpressionCoercion.ts(76,9): error TS2774: This condition will alw truthinessCallExpressionCoercion.ts(82,9): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? -==== truthinessCallExpressionCoercion.ts (8 errors) ==== +==== truthinessCallExpressionCoercion.ts (7 errors) ==== function onlyErrorsWhenTestingNonNullableFunctionType(required: () => boolean, optional?: () => boolean) { if (required) { // error ~~~~~~~~ @@ -19,8 +18,6 @@ truthinessCallExpressionCoercion.ts(82,9): error TS2774: This condition will alw } if (!!required) { // ok - ~~~~~~~~ -!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? } if (required()) { // ok diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion1.errors.txt b/tests/baselines/reference/truthinessCallExpressionCoercion1.errors.txt index 9c972898bb280..978a1565af7f9 100644 --- a/tests/baselines/reference/truthinessCallExpressionCoercion1.errors.txt +++ b/tests/baselines/reference/truthinessCallExpressionCoercion1.errors.txt @@ -1,12 +1,11 @@ truthinessCallExpressionCoercion1.ts(3,5): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? -truthinessCallExpressionCoercion1.ts(9,7): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion1.ts(19,5): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion1.ts(33,5): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion1.ts(46,5): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion1.ts(76,9): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? -==== truthinessCallExpressionCoercion1.ts (6 errors) ==== +==== truthinessCallExpressionCoercion1.ts (5 errors) ==== function onlyErrorsWhenTestingNonNullableFunctionType(required: () => boolean, optional?: () => boolean) { // error required ? console.log('required') : undefined; @@ -18,8 +17,6 @@ truthinessCallExpressionCoercion1.ts(76,9): error TS2774: This condition will al // ok !!required ? console.log('not required') : undefined; - ~~~~~~~~ -!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? // ok required() ? console.log('required call') : undefined; diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion2.errors.txt b/tests/baselines/reference/truthinessCallExpressionCoercion2.errors.txt index 977ff398c8daf..d0cb3fd84095e 100644 --- a/tests/baselines/reference/truthinessCallExpressionCoercion2.errors.txt +++ b/tests/baselines/reference/truthinessCallExpressionCoercion2.errors.txt @@ -1,6 +1,5 @@ truthinessCallExpressionCoercion2.ts(11,5): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion2.ts(14,10): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? -truthinessCallExpressionCoercion2.ts(29,7): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion2.ts(41,18): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion2.ts(44,9): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? truthinessCallExpressionCoercion2.ts(48,9): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? @@ -36,7 +35,7 @@ truthinessCallExpressionCoercion2.ts(180,9): error TS2774: This condition will a truthinessCallExpressionCoercion2.ts(183,14): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? -==== truthinessCallExpressionCoercion2.ts (36 errors) ==== +==== truthinessCallExpressionCoercion2.ts (35 errors) ==== declare class A { static from(): string; } @@ -70,8 +69,6 @@ truthinessCallExpressionCoercion2.ts(183,14): error TS2774: This condition will // ok !!required1 && console.log('not required'); - ~~~~~~~~~ -!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead? // ok required1() && console.log('required call'); diff --git a/tests/baselines/reference/truthinessPromiseCoercion.errors.txt b/tests/baselines/reference/truthinessPromiseCoercion.errors.txt index 8907fcb574c2b..9a7a1e17a158c 100644 --- a/tests/baselines/reference/truthinessPromiseCoercion.errors.txt +++ b/tests/baselines/reference/truthinessPromiseCoercion.errors.txt @@ -1,13 +1,11 @@ truthinessPromiseCoercion.ts(7,9): error TS2801: This condition will always return true since this 'Promise' is always defined. -truthinessPromiseCoercion.ts(8,11): error TS2801: This condition will always return true since this 'Promise' is always defined. truthinessPromiseCoercion.ts(11,5): error TS2801: This condition will always return true since this 'Promise' is always defined. -truthinessPromiseCoercion.ts(12,7): error TS2801: This condition will always return true since this 'Promise' is always defined. truthinessPromiseCoercion.ts(32,9): error TS2801: This condition will always return true since this 'Promise' is always defined. truthinessPromiseCoercion.ts(40,9): error TS2801: This condition will always return true since this 'Promise' is always defined. truthinessPromiseCoercion.ts(43,9): error TS2801: This condition will always return true since this 'Promise' is always defined. -==== truthinessPromiseCoercion.ts (7 errors) ==== +==== truthinessPromiseCoercion.ts (5 errors) ==== declare const p: Promise declare const p2: null | Promise declare const obj: { p: Promise } @@ -19,9 +17,6 @@ truthinessPromiseCoercion.ts(43,9): error TS2801: This condition will always ret !!! error TS2801: This condition will always return true since this 'Promise' is always defined. !!! related TS2773 truthinessPromiseCoercion.ts:7:9: Did you forget to use 'await'? if (!!p) {} // no err - ~ -!!! error TS2801: This condition will always return true since this 'Promise' is always defined. -!!! related TS2773 truthinessPromiseCoercion.ts:8:11: Did you forget to use 'await'? if (p2) {} // no err p ? f.arguments : f.arguments; @@ -29,9 +24,6 @@ truthinessPromiseCoercion.ts(43,9): error TS2801: This condition will always ret !!! error TS2801: This condition will always return true since this 'Promise' is always defined. !!! related TS2773 truthinessPromiseCoercion.ts:11:5: Did you forget to use 'await'? !!p ? f.arguments : f.arguments; - ~ -!!! error TS2801: This condition will always return true since this 'Promise' is always defined. -!!! related TS2773 truthinessPromiseCoercion.ts:12:7: Did you forget to use 'await'? p2 ? f.arguments : f.arguments; }