Skip to content

Improve uncalled function check related to parenthesized and binary expressions #50756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,10 @@ import {
isJSDocTypeAlias,
isJsonSourceFile,
isLeftHandSideExpression,
isLogicalOrCoalescingAssignmentExpression,
isLogicalOrCoalescingAssignmentOperator,
isLogicalOrCoalescingBinaryExpression,
isLogicalOrCoalescingBinaryOperator,
isModuleAugmentationExternal,
isModuleBlock,
isModuleDeclaration,
Expand Down Expand Up @@ -1356,17 +1359,13 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
node = (node as PrefixUnaryExpression).operand;
}
else {
return node.kind === SyntaxKind.BinaryExpression && (
(node as BinaryExpression).operatorToken.kind === SyntaxKind.AmpersandAmpersandToken ||
(node as BinaryExpression).operatorToken.kind === SyntaxKind.BarBarToken ||
(node as BinaryExpression).operatorToken.kind === SyntaxKind.QuestionQuestionToken);
return isLogicalOrCoalescingBinaryExpression(node);
}
}
}

function isLogicalAssignmentExpression(node: Node) {
node = skipParentheses(node);
return isBinaryExpression(node) && isLogicalOrCoalescingAssignmentOperator(node.operatorToken.kind);
return isLogicalOrCoalescingAssignmentExpression(skipParentheses(node));
}

function isTopLevelLogicalExpression(node: Node): boolean {
Expand Down Expand Up @@ -1838,10 +1837,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
// we'll need to handle the `bindLogicalExpression` scenarios in this state machine, too
// For now, though, since the common cases are chained `+`, leaving it recursive is fine
const operator = node.operatorToken.kind;
if (operator === SyntaxKind.AmpersandAmpersandToken ||
operator === SyntaxKind.BarBarToken ||
operator === SyntaxKind.QuestionQuestionToken ||
isLogicalOrCoalescingAssignmentOperator(operator)) {
if (isLogicalOrCoalescingBinaryOperator(operator) || isLogicalOrCoalescingAssignmentOperator(operator)) {
if (isTopLevelLogicalExpression(node)) {
const postExpressionLabel = createBranchLabel();
bindLogicalLikeExpression(node, postExpressionLabel, postExpressionLabel);
Expand Down
43 changes: 26 additions & 17 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,8 @@ import {
isLiteralExpressionOfObject,
isLiteralImportTypeNode,
isLiteralTypeNode,
isLogicalOrCoalescingBinaryExpression,
isLogicalOrCoalescingBinaryOperator,
isMetaProperty,
isMethodDeclaration,
isMethodSignature,
Expand Down Expand Up @@ -35324,13 +35326,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
setLeftType(state, leftType);
setLastResult(state, /*type*/ undefined);
const operator = operatorToken.kind;
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken) {
if (operator === SyntaxKind.AmpersandAmpersandToken) {
let parent = node.parent;
while (parent.kind === SyntaxKind.ParenthesizedExpression
|| isBinaryExpression(parent) && (parent.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken || parent.operatorToken.kind === SyntaxKind.BarBarToken)) {
parent = parent.parent;
}
if (isLogicalOrCoalescingBinaryOperator(operator)) {
let parent = node.parent;
while (parent.kind === SyntaxKind.ParenthesizedExpression || isLogicalOrCoalescingBinaryExpression(parent)) {
parent = parent.parent;
}
if (operator === SyntaxKind.AmpersandAmpersandToken || isIfStatement(parent)) {
checkTestingKnownTruthyCallableOrAwaitableType(node.left, leftType, isIfStatement(parent) ? parent.thenStatement : undefined);
}
checkTruthinessOfType(leftType, node.left);
Expand Down Expand Up @@ -35419,7 +35420,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return checkDestructuringAssignment(left, checkExpression(right, checkMode), checkMode, right.kind === SyntaxKind.ThisKeyword);
}
let leftType: Type;
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken) {
if (isLogicalOrCoalescingBinaryOperator(operator)) {
leftType = checkTruthinessExpression(left, checkMode);
}
else {
Expand Down Expand Up @@ -39652,19 +39653,28 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {

function checkTestingKnownTruthyCallableOrAwaitableType(condExpr: Expression, condType: Type, body?: Statement | Expression) {
if (!strictNullChecks) return;
bothHelper(condExpr, body);

function bothHelper(condExpr: Expression, body: Expression | Statement | undefined) {
condExpr = skipParentheses(condExpr);

helper(condExpr, body);
while (isBinaryExpression(condExpr) && condExpr.operatorToken.kind === SyntaxKind.BarBarToken) {
condExpr = condExpr.left;
helper(condExpr, body);

while (isBinaryExpression(condExpr) && (condExpr.operatorToken.kind === SyntaxKind.BarBarToken || condExpr.operatorToken.kind === SyntaxKind.QuestionQuestionToken)) {
condExpr = skipParentheses(condExpr.left);
helper(condExpr, body);
}
}

function helper(condExpr: Expression, body: Expression | Statement | undefined) {
const location = isBinaryExpression(condExpr) &&
(condExpr.operatorToken.kind === SyntaxKind.BarBarToken || condExpr.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken)
? condExpr.right
: condExpr;
if (isModuleExportsAccessExpression(location)) return;
const location = isLogicalOrCoalescingBinaryExpression(condExpr) ? skipParentheses(condExpr.right) : condExpr;
if (isModuleExportsAccessExpression(location)) {
return;
}
if (isLogicalOrCoalescingBinaryExpression(location)) {
bothHelper(location, body);
return;
}
const type = location === condExpr ? condType : checkTruthinessExpression(location);
const isPropertyExpressionCast = isPropertyAccessExpression(location) && isTypeAssertion(location.expression);
if (!(getTypeFacts(type) & TypeFacts.Truthy) || isPropertyExpressionCast) return;
Expand All @@ -39682,7 +39692,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {

const testedNode = isIdentifier(location) ? location
: isPropertyAccessExpression(location) ? location.name
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
: undefined;
const testedSymbol = testedNode && getSymbolAtLocation(testedNode);
if (!testedSymbol && !isPromise) {
Expand Down
14 changes: 3 additions & 11 deletions src/compiler/transformers/es2021.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {
AssignmentExpression,
BinaryExpression,
Bundle,
chainBundle,
getNonAssignmentOperatorForCompoundAssignment,
Expand All @@ -14,7 +13,6 @@ import {
Node,
skipParentheses,
SourceFile,
SyntaxKind,
Token,
TransformationContext,
TransformFlags,
Expand Down Expand Up @@ -43,16 +41,10 @@ export function transformES2021(context: TransformationContext): (x: SourceFile
if ((node.transformFlags & TransformFlags.ContainsES2021) === 0) {
return node;
}
switch (node.kind) {
case SyntaxKind.BinaryExpression:
const binaryExpression = node as BinaryExpression;
if (isLogicalOrCoalescingAssignmentExpression(binaryExpression)) {
return transformLogicalAssignment(binaryExpression);
}
// falls through
default:
return visitEachChild(node, visitor, context);
if (isLogicalOrCoalescingAssignmentExpression(node)) {
return transformLogicalAssignment(node);
}
return visitEachChild(node, visitor, context);
}

function transformLogicalAssignment(binaryExpression: AssignmentExpression<Token<LogicalOrCoalescingAssignmentOperator>>): VisitResult<Node> {
Expand Down
23 changes: 18 additions & 5 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ import {
LiteralImportTypeNode,
LiteralLikeElementAccessExpression,
LiteralLikeNode,
LogicalOperator,
LogicalOrCoalescingAssignmentOperator,
map,
mapDefined,
Expand Down Expand Up @@ -6075,11 +6076,13 @@ export function modifierToFlag(token: SyntaxKind): ModifierFlags {
return ModifierFlags.None;
}

function isBinaryLogicalOperator(token: SyntaxKind): boolean {
return token === SyntaxKind.BarBarToken || token === SyntaxKind.AmpersandAmpersandToken;
}

/** @internal */
export function isLogicalOperator(token: SyntaxKind): boolean {
return token === SyntaxKind.BarBarToken
|| token === SyntaxKind.AmpersandAmpersandToken
|| token === SyntaxKind.ExclamationToken;
return isBinaryLogicalOperator(token) || token === SyntaxKind.ExclamationToken;
}

/** @internal */
Expand All @@ -6090,8 +6093,18 @@ export function isLogicalOrCoalescingAssignmentOperator(token: SyntaxKind): toke
}

/** @internal */
export function isLogicalOrCoalescingAssignmentExpression(expr: BinaryExpression): expr is AssignmentExpression<Token<LogicalOrCoalescingAssignmentOperator>> {
return isLogicalOrCoalescingAssignmentOperator(expr.operatorToken.kind);
export function isLogicalOrCoalescingAssignmentExpression(expr: Node): expr is AssignmentExpression<Token<LogicalOrCoalescingAssignmentOperator>> {
return isBinaryExpression(expr) && isLogicalOrCoalescingAssignmentOperator(expr.operatorToken.kind);
}

/** @internal */
export function isLogicalOrCoalescingBinaryOperator(token: SyntaxKind): token is LogicalOperator | SyntaxKind.QuestionQuestionToken {
return isBinaryLogicalOperator(token) || token === SyntaxKind.QuestionQuestionToken;
}

/** @internal */
export function isLogicalOrCoalescingBinaryExpression(expr: Node): expr is BinaryExpression {
return isBinaryExpression(expr) && isLogicalOrCoalescingBinaryOperator(expr.operatorToken.kind);
}

/** @internal */
Expand Down
Loading