Skip to content

Commit cc1768c

Browse files
a-tarasyuka-tarasyuk
committed
feat(40197): handle uncalled function checks in binary expressions
1 parent 2b0278a commit cc1768c

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
@@ -660,7 +660,7 @@ namespace ts {
660660
}
661661
// We create a return control flow graph for IIFEs and constructors. For constructors
662662
// we use the return control flow graph in strict property initialization checks.
663-
currentReturnTarget = isIIFE || node.kind === SyntaxKind.Constructor || (isInJSFile && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression)) ? createBranchLabel() : undefined;
663+
currentReturnTarget = isIIFE || node.kind === SyntaxKind.Constructor || (isInJSFile(node) && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression)) ? createBranchLabel() : undefined;
664664
currentExceptionTarget = undefined;
665665
currentBreakTarget = undefined;
666666
currentContinueTarget = undefined;
@@ -681,7 +681,7 @@ namespace ts {
681681
if (currentReturnTarget) {
682682
addAntecedent(currentReturnTarget, currentFlow);
683683
currentFlow = finishFlowLabel(currentReturnTarget);
684-
if (node.kind === SyntaxKind.Constructor || (isInJSFile && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression))) {
684+
if (node.kind === SyntaxKind.Constructor || (isInJSFile(node) && (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression))) {
685685
(<FunctionLikeDeclaration>node).returnFlowNode = currentFlow;
686686
}
687687
}

‎src/compiler/checker.ts

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29377,6 +29377,9 @@ namespace ts {
2937729377
workStacks.leftType[stackIndex] = leftType;
2937829378
const operator = node.operatorToken.kind;
2937929379
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken) {
29380+
if (operator === SyntaxKind.AmpersandAmpersandToken) {
29381+
checkTestingKnownTruthyCallableType(node.left, leftType);
29382+
}
2938029383
checkTruthinessOfType(leftType, node.left);
2938129384
}
2938229385
advanceState(CheckBinaryExpressionState.FinishCheck);
@@ -29910,7 +29913,7 @@ namespace ts {
2991029913

2991129914
function checkConditionalExpression(node: ConditionalExpression, checkMode?: CheckMode): Type {
2991229915
const type = checkTruthinessExpression(node.condition);
29913-
checkTestingKnownTruthyCallableType(node.condition, node.whenTrue, type);
29916+
checkTestingKnownTruthyCallableType(node.condition, type, node.whenTrue);
2991429917
const type1 = checkExpression(node.whenTrue, checkMode);
2991529918
const type2 = checkExpression(node.whenFalse, checkMode);
2991629919
return getUnionType([type1, type2], UnionReduction.Subtype);
@@ -33125,7 +33128,7 @@ namespace ts {
3312533128
// Grammar checking
3312633129
checkGrammarStatementInAmbientContext(node);
3312733130
const type = checkTruthinessExpression(node.expression);
33128-
checkTestingKnownTruthyCallableType(node.expression, node.thenStatement, type);
33131+
checkTestingKnownTruthyCallableType(node.expression, type, node.thenStatement);
3312933132
checkSourceElement(node.thenStatement);
3313033133

3313133134
if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
@@ -33135,16 +33138,16 @@ namespace ts {
3313533138
checkSourceElement(node.elseStatement);
3313633139
}
3313733140

33138-
function checkTestingKnownTruthyCallableType(condExpr: Expression, body: Statement | Expression, type: Type) {
33141+
function checkTestingKnownTruthyCallableType(condExpr: Expression, type: Type, body?: Statement | Expression) {
3313933142
if (!strictNullChecks) {
3314033143
return;
3314133144
}
3314233145

33143-
const testedNode = isIdentifier(condExpr)
33144-
? condExpr
33145-
: isPropertyAccessExpression(condExpr)
33146-
? condExpr.name
33147-
: undefined;
33146+
const location = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
33147+
const testedNode = isIdentifier(location) ? location
33148+
: isPropertyAccessExpression(location) ? location.name
33149+
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
33150+
: undefined;
3314833151

3314933152
if (!testedNode) {
3315033153
return;
@@ -33165,27 +33168,34 @@ namespace ts {
3316533168
return;
3316633169
}
3316733170

33168-
const testedFunctionSymbol = getSymbolAtLocation(testedNode);
33169-
if (!testedFunctionSymbol) {
33171+
const testedSymbol = getSymbolAtLocation(testedNode);
33172+
if (!testedSymbol) {
3317033173
return;
3317133174
}
3317233175

33173-
const functionIsUsedInBody = forEachChild(body, function check(childNode): boolean | undefined {
33176+
const isUsed = isBinaryExpression(condExpr.parent) ? isFunctionUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
33177+
: body ? isFunctionUsedInConditionBody(condExpr, body, testedNode, testedSymbol)
33178+
: false;
33179+
if (!isUsed) {
33180+
error(location, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
33181+
}
33182+
}
33183+
33184+
function isFunctionUsedInConditionBody(expr: Expression, body: Statement | Expression, testedNode: Node, testedSymbol: Symbol): boolean {
33185+
return !!forEachChild(body, function check(childNode): boolean | undefined {
3317433186
if (isIdentifier(childNode)) {
3317533187
const childSymbol = getSymbolAtLocation(childNode);
33176-
if (childSymbol && childSymbol === testedFunctionSymbol) {
33188+
if (childSymbol && childSymbol === testedSymbol) {
3317733189
// If the test was a simple identifier, the above check is sufficient
33178-
if (isIdentifier(condExpr)) {
33190+
if (isIdentifier(expr)) {
3317933191
return true;
3318033192
}
3318133193
// Otherwise we need to ensure the symbol is called on the same target
3318233194
let testedExpression = testedNode.parent;
3318333195
let childExpression = childNode.parent;
3318433196
while (testedExpression && childExpression) {
33185-
3318633197
if (isIdentifier(testedExpression) && isIdentifier(childExpression) ||
33187-
testedExpression.kind === SyntaxKind.ThisKeyword && childExpression.kind === SyntaxKind.ThisKeyword
33188-
) {
33198+
testedExpression.kind === SyntaxKind.ThisKeyword && childExpression.kind === SyntaxKind.ThisKeyword) {
3318933199
return getSymbolAtLocation(testedExpression) === getSymbolAtLocation(childExpression);
3319033200
}
3319133201

@@ -33202,13 +33212,18 @@ namespace ts {
3320233212
}
3320333213
}
3320433214
}
33205-
3320633215
return forEachChild(childNode, check);
3320733216
});
33217+
}
3320833218

33209-
if (!functionIsUsedInBody) {
33210-
error(condExpr, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
33219+
function isFunctionUsedInBinaryExpressionChain(node: Node, testedSymbol: Symbol): boolean {
33220+
while (isBinaryExpression(node) && node.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken) {
33221+
if (isCallExpression(node.right) && testedSymbol === getSymbolAtLocation(node.right.expression)) {
33222+
return true;
33223+
}
33224+
node = node.parent;
3321133225
}
33226+
return false;
3321233227
}
3321333228

3321433229
function checkDoStatement(node: DoStatement) {

‎src/compiler/emitter.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2779,7 +2779,7 @@ namespace ts {
27792779
if (isSimilarNode && currentSourceFile) {
27802780
pos = skipTrivia(currentSourceFile.text, pos);
27812781
}
2782-
if (emitLeadingCommentsOfPosition && isSimilarNode && contextNode.pos !== startPos) {
2782+
if (isSimilarNode && contextNode.pos !== startPos) {
27832783
const needsIndent = indentLeading && currentSourceFile && !positionsAreOnSameLine(startPos, pos, currentSourceFile);
27842784
if (needsIndent) {
27852785
increaseIndent();
@@ -2790,7 +2790,7 @@ namespace ts {
27902790
}
27912791
}
27922792
pos = writeTokenText(token, writer, pos);
2793-
if (emitTrailingCommentsOfPosition && isSimilarNode && contextNode.end !== pos) {
2793+
if (isSimilarNode && contextNode.end !== pos) {
27942794
emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ true);
27952795
}
27962796
return pos;
@@ -3436,7 +3436,7 @@ namespace ts {
34363436
// "comment1" is not considered to be leading comment for node.initializer
34373437
// but rather a trailing comment on the previous node.
34383438
const initializer = node.initializer;
3439-
if (emitTrailingCommentsOfPosition && (getEmitFlags(initializer) & EmitFlags.NoLeadingComments) === 0) {
3439+
if ((getEmitFlags(initializer) & EmitFlags.NoLeadingComments) === 0) {
34403440
const commentRange = getCommentRange(initializer);
34413441
emitTrailingCommentsOfPosition(commentRange.pos);
34423442
}

‎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)