Skip to content

Commit eaf4f46

Browse files
a-tarasyuka-tarasyuk
authored
feat(40197): handle uncalled function checks in binary expressions (#40260)
1 parent 4283428 commit eaf4f46

15 files changed

+1069
-24
lines changed

‎src/compiler/binder.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,7 @@ namespace ts {
665665
}
666666
// We create a return control flow graph for IIFEs and constructors. For constructors
667667
// we use the return control flow graph in strict property initialization checks.
668-
currentReturnTarget = isIIFE || node.kind === SyntaxKind.Constructor || (isInJSFile && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression)) ? createBranchLabel() : undefined;
668+
currentReturnTarget = isIIFE || node.kind === SyntaxKind.Constructor || (isInJSFile(node) && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression)) ? createBranchLabel() : undefined;
669669
currentExceptionTarget = undefined;
670670
currentBreakTarget = undefined;
671671
currentContinueTarget = undefined;
@@ -686,7 +686,7 @@ namespace ts {
686686
if (currentReturnTarget) {
687687
addAntecedent(currentReturnTarget, currentFlow);
688688
currentFlow = finishFlowLabel(currentReturnTarget);
689-
if (node.kind === SyntaxKind.Constructor || (isInJSFile && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression))) {
689+
if (node.kind === SyntaxKind.Constructor || (isInJSFile(node) && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression))) {
690690
(<FunctionLikeDeclaration>node).returnFlowNode = currentFlow;
691691
}
692692
}

‎src/compiler/checker.ts

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30006,6 +30006,9 @@ namespace ts {
3000630006
workStacks.leftType[stackIndex] = leftType;
3000730007
const operator = node.operatorToken.kind;
3000830008
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken) {
30009+
if (operator === SyntaxKind.AmpersandAmpersandToken) {
30010+
checkTestingKnownTruthyCallableType(node.left, leftType);
30011+
}
3000930012
checkTruthinessOfType(leftType, node.left);
3001030013
}
3001130014
advanceState(CheckBinaryExpressionState.FinishCheck);
@@ -30539,7 +30542,7 @@ namespace ts {
3053930542

3054030543
function checkConditionalExpression(node: ConditionalExpression, checkMode?: CheckMode): Type {
3054130544
const type = checkTruthinessExpression(node.condition);
30542-
checkTestingKnownTruthyCallableType(node.condition, node.whenTrue, type);
30545+
checkTestingKnownTruthyCallableType(node.condition, type, node.whenTrue);
3054330546
const type1 = checkExpression(node.whenTrue, checkMode);
3054430547
const type2 = checkExpression(node.whenFalse, checkMode);
3054530548
return getUnionType([type1, type2], UnionReduction.Subtype);
@@ -33775,7 +33778,7 @@ namespace ts {
3377533778
// Grammar checking
3377633779
checkGrammarStatementInAmbientContext(node);
3377733780
const type = checkTruthinessExpression(node.expression);
33778-
checkTestingKnownTruthyCallableType(node.expression, node.thenStatement, type);
33781+
checkTestingKnownTruthyCallableType(node.expression, type, node.thenStatement);
3377933782
checkSourceElement(node.thenStatement);
3378033783

3378133784
if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
@@ -33785,16 +33788,16 @@ namespace ts {
3378533788
checkSourceElement(node.elseStatement);
3378633789
}
3378733790

33788-
function checkTestingKnownTruthyCallableType(condExpr: Expression, body: Statement | Expression, type: Type) {
33791+
function checkTestingKnownTruthyCallableType(condExpr: Expression, type: Type, body?: Statement | Expression) {
3378933792
if (!strictNullChecks) {
3379033793
return;
3379133794
}
3379233795

33793-
const testedNode = isIdentifier(condExpr)
33794-
? condExpr
33795-
: isPropertyAccessExpression(condExpr)
33796-
? condExpr.name
33797-
: undefined;
33796+
const location = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
33797+
const testedNode = isIdentifier(location) ? location
33798+
: isPropertyAccessExpression(location) ? location.name
33799+
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
33800+
: undefined;
3379833801

3379933802
if (!testedNode) {
3380033803
return;
@@ -33815,27 +33818,34 @@ namespace ts {
3381533818
return;
3381633819
}
3381733820

33818-
const testedFunctionSymbol = getSymbolAtLocation(testedNode);
33819-
if (!testedFunctionSymbol) {
33821+
const testedSymbol = getSymbolAtLocation(testedNode);
33822+
if (!testedSymbol) {
3382033823
return;
3382133824
}
3382233825

33823-
const functionIsUsedInBody = forEachChild(body, function check(childNode): boolean | undefined {
33826+
const isUsed = isBinaryExpression(condExpr.parent) ? isFunctionUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
33827+
: body ? isFunctionUsedInConditionBody(condExpr, body, testedNode, testedSymbol)
33828+
: false;
33829+
if (!isUsed) {
33830+
error(location, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
33831+
}
33832+
}
33833+
33834+
function isFunctionUsedInConditionBody(expr: Expression, body: Statement | Expression, testedNode: Node, testedSymbol: Symbol): boolean {
33835+
return !!forEachChild(body, function check(childNode): boolean | undefined {
3382433836
if (isIdentifier(childNode)) {
3382533837
const childSymbol = getSymbolAtLocation(childNode);
33826-
if (childSymbol && childSymbol === testedFunctionSymbol) {
33838+
if (childSymbol && childSymbol === testedSymbol) {
3382733839
// If the test was a simple identifier, the above check is sufficient
33828-
if (isIdentifier(condExpr)) {
33840+
if (isIdentifier(expr)) {
3382933841
return true;
3383033842
}
3383133843
// Otherwise we need to ensure the symbol is called on the same target
3383233844
let testedExpression = testedNode.parent;
3383333845
let childExpression = childNode.parent;
3383433846
while (testedExpression && childExpression) {
33835-
3383633847
if (isIdentifier(testedExpression) && isIdentifier(childExpression) ||
33837-
testedExpression.kind === SyntaxKind.ThisKeyword && childExpression.kind === SyntaxKind.ThisKeyword
33838-
) {
33848+
testedExpression.kind === SyntaxKind.ThisKeyword && childExpression.kind === SyntaxKind.ThisKeyword) {
3383933849
return getSymbolAtLocation(testedExpression) === getSymbolAtLocation(childExpression);
3384033850
}
3384133851

@@ -33852,13 +33862,18 @@ namespace ts {
3385233862
}
3385333863
}
3385433864
}
33855-
3385633865
return forEachChild(childNode, check);
3385733866
});
33867+
}
3385833868

33859-
if (!functionIsUsedInBody) {
33860-
error(condExpr, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
33869+
function isFunctionUsedInBinaryExpressionChain(node: Node, testedSymbol: Symbol): boolean {
33870+
while (isBinaryExpression(node) && node.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken) {
33871+
if (isCallExpression(node.right) && testedSymbol === getSymbolAtLocation(node.right.expression)) {
33872+
return true;
33873+
}
33874+
node = node.parent;
3386133875
}
33876+
return false;
3386233877
}
3386333878

3386433879
function checkDoStatement(node: DoStatement) {

‎src/compiler/emitter.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2811,7 +2811,7 @@ namespace ts {
28112811
if (isSimilarNode && currentSourceFile) {
28122812
pos = skipTrivia(currentSourceFile.text, pos);
28132813
}
2814-
if (emitLeadingCommentsOfPosition && isSimilarNode && contextNode.pos !== startPos) {
2814+
if (isSimilarNode && contextNode.pos !== startPos) {
28152815
const needsIndent = indentLeading && currentSourceFile && !positionsAreOnSameLine(startPos, pos, currentSourceFile);
28162816
if (needsIndent) {
28172817
increaseIndent();
@@ -2822,7 +2822,7 @@ namespace ts {
28222822
}
28232823
}
28242824
pos = writeTokenText(token, writer, pos);
2825-
if (emitTrailingCommentsOfPosition && isSimilarNode && contextNode.end !== pos) {
2825+
if (isSimilarNode && contextNode.end !== pos) {
28262826
emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ true);
28272827
}
28282828
return pos;
@@ -3468,7 +3468,7 @@ namespace ts {
34683468
// "comment1" is not considered to be leading comment for node.initializer
34693469
// but rather a trailing comment on the previous node.
34703470
const initializer = node.initializer;
3471-
if (emitTrailingCommentsOfPosition && (getEmitFlags(initializer) & EmitFlags.NoLeadingComments) === 0) {
3471+
if ((getEmitFlags(initializer) & EmitFlags.NoLeadingComments) === 0) {
34723472
const commentRange = getCommentRange(initializer);
34733473
emitTrailingCommentsOfPosition(commentRange.pos);
34743474
}

‎tests/baselines/reference/truthinessCallExpressionCoercion2.errors.txt

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(3,5): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
2+
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(6,10): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
3+
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(30,18): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
4+
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(36,46): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
5+
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(47,5): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
6+
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(50,10): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
7+
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(66,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
8+
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(69,14): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
9+
10+
11+
==== tests/cases/compiler/truthinessCallExpressionCoercion2.ts (8 errors) ====
12+
function test(required1: () => boolean, required2: () => boolean, optional?: () => boolean) {
13+
// error
14+
required1 && console.log('required');
15+
~~~~~~~~~
16+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
17+
18+
// error
19+
1 && required1 && console.log('required');
20+
~~~~~~~~~
21+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
22+
23+
// ok
24+
required1 && required1();
25+
26+
// ok
27+
required1 && 1 && required1();
28+
29+
// ok
30+
optional && console.log('optional');
31+
32+
// ok
33+
1 && optional && console.log('optional');
34+
35+
// ok
36+
!!required1 && console.log('not required');
37+
38+
// ok
39+
required1() && console.log('required call');
40+
41+
// ok
42+
required1 && required2 && required1() && required2();
43+
44+
// error
45+
required1 && required2 && required1() && console.log('foo');
46+
~~~~~~~~~
47+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
48+
}
49+
50+
function checksConsole() {
51+
// error
52+
typeof window !== 'undefined' && window.console &&
53+
((window.console as any).firebug || (window.console.exception && window.console.table));
54+
~~~~~~~~~~~~~~~~~~~~~~~~
55+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
56+
}
57+
58+
function checksPropertyAccess() {
59+
const x = {
60+
foo: {
61+
bar() { return true; }
62+
}
63+
}
64+
65+
// error
66+
x.foo.bar && console.log('x.foo.bar');
67+
~~~~~~~~~
68+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
69+
70+
// error
71+
1 && x.foo.bar && console.log('x.foo.bar');
72+
~~~~~~~~~
73+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
74+
75+
// ok
76+
x.foo.bar && x.foo.bar();
77+
78+
// ok
79+
x.foo.bar && 1 && x.foo.bar();
80+
}
81+
82+
class Foo {
83+
optional?: () => boolean;
84+
required() {
85+
return true;
86+
}
87+
test() {
88+
// error
89+
this.required && console.log('required');
90+
~~~~~~~~~~~~~
91+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
92+
93+
// error
94+
1 && this.required && console.log('required');
95+
~~~~~~~~~~~~~
96+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
97+
98+
// ok
99+
this.required && this.required();
100+
101+
// ok
102+
this.required && 1 && this.required();
103+
104+
// ok
105+
1 && this.optional && console.log('optional');
106+
}
107+
}
108+

0 commit comments

Comments
 (0)