Skip to content

Commit

Permalink
Switch to checking for assignment when inverted
Browse files Browse the repository at this point in the history
  • Loading branch information
tjenkinson committed Dec 29, 2024
1 parent 9630147 commit 8715473
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 105 deletions.
46 changes: 31 additions & 15 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand All @@ -44604,23 +44606,30 @@ 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);
}
}
}
}

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;
Expand All @@ -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)) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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?
Expand All @@ -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
~~~~~~~~
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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?
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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');
Expand Down
10 changes: 1 addition & 9 deletions tests/baselines/reference/truthinessPromiseCoercion.errors.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
truthinessPromiseCoercion.ts(7,9): error TS2801: This condition will always return true since this 'Promise<number>' is always defined.
truthinessPromiseCoercion.ts(8,11): error TS2801: This condition will always return true since this 'Promise<number>' is always defined.
truthinessPromiseCoercion.ts(11,5): error TS2801: This condition will always return true since this 'Promise<number>' is always defined.
truthinessPromiseCoercion.ts(12,7): error TS2801: This condition will always return true since this 'Promise<number>' is always defined.
truthinessPromiseCoercion.ts(32,9): error TS2801: This condition will always return true since this 'Promise<unknown>' is always defined.
truthinessPromiseCoercion.ts(40,9): error TS2801: This condition will always return true since this 'Promise<boolean>' is always defined.
truthinessPromiseCoercion.ts(43,9): error TS2801: This condition will always return true since this 'Promise<boolean>' is always defined.


==== truthinessPromiseCoercion.ts (7 errors) ====
==== truthinessPromiseCoercion.ts (5 errors) ====
declare const p: Promise<number>
declare const p2: null | Promise<number>
declare const obj: { p: Promise<unknown> }
Expand All @@ -19,19 +17,13 @@ truthinessPromiseCoercion.ts(43,9): error TS2801: This condition will always ret
!!! error TS2801: This condition will always return true since this 'Promise<number>' 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<number>' is always defined.
!!! related TS2773 truthinessPromiseCoercion.ts:8:11: Did you forget to use 'await'?
if (p2) {} // no err

p ? f.arguments : f.arguments;
~
!!! error TS2801: This condition will always return true since this 'Promise<number>' 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<number>' is always defined.
!!! related TS2773 truthinessPromiseCoercion.ts:12:7: Did you forget to use 'await'?
p2 ? f.arguments : f.arguments;
}

Expand Down

0 comments on commit 8715473

Please sign in to comment.