Skip to content

Commit ce4dcf3

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

15 files changed

+694
-19
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: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29327,6 +29327,9 @@ namespace ts {
2932729327
workStacks.leftType[stackIndex] = leftType;
2932829328
const operator = node.operatorToken.kind;
2932929329
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken) {
29330+
if (operator === SyntaxKind.AmpersandAmpersandToken) {
29331+
checkTestingKnownTruthyCallableType(node.left, leftType);
29332+
}
2933029333
checkTruthinessOfType(leftType, node.left);
2933129334
}
2933229335
advanceState(CheckBinaryExpressionState.FinishCheck);
@@ -29860,7 +29863,7 @@ namespace ts {
2986029863

2986129864
function checkConditionalExpression(node: ConditionalExpression, checkMode?: CheckMode): Type {
2986229865
const type = checkTruthinessExpression(node.condition);
29863-
checkTestingKnownTruthyCallableType(node.condition, node.whenTrue, type);
29866+
checkTestingKnownTruthyCallableType(node.condition, type, node.whenTrue);
2986429867
const type1 = checkExpression(node.whenTrue, checkMode);
2986529868
const type2 = checkExpression(node.whenFalse, checkMode);
2986629869
return getUnionType([type1, type2], UnionReduction.Subtype);
@@ -33069,7 +33072,7 @@ namespace ts {
3306933072
// Grammar checking
3307033073
checkGrammarStatementInAmbientContext(node);
3307133074
const type = checkTruthinessExpression(node.expression);
33072-
checkTestingKnownTruthyCallableType(node.expression, node.thenStatement, type);
33075+
checkTestingKnownTruthyCallableType(node.expression, type, node.thenStatement);
3307333076
checkSourceElement(node.thenStatement);
3307433077

3307533078
if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
@@ -33079,16 +33082,15 @@ namespace ts {
3307933082
checkSourceElement(node.elseStatement);
3308033083
}
3308133084

33082-
function checkTestingKnownTruthyCallableType(condExpr: Expression, body: Statement | Expression, type: Type) {
33085+
function checkTestingKnownTruthyCallableType(condExpr: Expression, type: Type, body?: Statement | Expression) {
3308333086
if (!strictNullChecks) {
3308433087
return;
3308533088
}
3308633089

33087-
const testedNode = isIdentifier(condExpr)
33088-
? condExpr
33089-
: isPropertyAccessExpression(condExpr)
33090-
? condExpr.name
33091-
: undefined;
33090+
const expression = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
33091+
const testedNode = isIdentifier(expression) ? expression :
33092+
isPropertyAccessExpression(expression) ? expression.name :
33093+
isBinaryExpression(expression) && isIdentifier(expression.right) ? expression.right : undefined;
3309233094

3309333095
if (!testedNode) {
3309433096
return;
@@ -33114,7 +33116,7 @@ namespace ts {
3311433116
return;
3311533117
}
3311633118

33117-
const functionIsUsedInBody = forEachChild(body, function check(childNode): boolean | undefined {
33119+
const functionIsUsedInBody = body && forEachChild(body, function check(childNode): boolean | undefined {
3311833120
if (isIdentifier(childNode)) {
3311933121
const childSymbol = getSymbolAtLocation(childNode);
3312033122
if (childSymbol && childSymbol === testedFunctionSymbol) {
@@ -33126,10 +33128,8 @@ namespace ts {
3312633128
let testedExpression = testedNode.parent;
3312733129
let childExpression = childNode.parent;
3312833130
while (testedExpression && childExpression) {
33129-
3313033131
if (isIdentifier(testedExpression) && isIdentifier(childExpression) ||
33131-
testedExpression.kind === SyntaxKind.ThisKeyword && childExpression.kind === SyntaxKind.ThisKeyword
33132-
) {
33132+
testedExpression.kind === SyntaxKind.ThisKeyword && childExpression.kind === SyntaxKind.ThisKeyword) {
3313333133
return getSymbolAtLocation(testedExpression) === getSymbolAtLocation(childExpression);
3313433134
}
3313533135

@@ -33149,9 +33149,8 @@ namespace ts {
3314933149

3315033150
return forEachChild(childNode, check);
3315133151
});
33152-
3315333152
if (!functionIsUsedInBody) {
33154-
error(condExpr, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
33153+
error(expression, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
3315533154
}
3315633155
}
3315733156

‎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: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
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(29,5): 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(32,10): 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(42,9): 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(45,14): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
7+
8+
9+
==== tests/cases/compiler/truthinessCallExpressionCoercion2.ts (6 errors) ====
10+
function test(required: () => boolean, optional?: () => boolean) {
11+
// error
12+
required && console.log('required');
13+
~~~~~~~~
14+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
15+
16+
// error
17+
1 && required && console.log('required');
18+
~~~~~~~~
19+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
20+
21+
// ok
22+
optional && console.log('optional');
23+
24+
// ok
25+
1 && optional && console.log('optional');
26+
27+
// ok
28+
!!required && console.log('not required');
29+
30+
// ok
31+
required() && console.log('required call');
32+
}
33+
34+
function checksPropertyAccess() {
35+
const x = {
36+
foo: {
37+
bar() { return true; }
38+
}
39+
}
40+
41+
// error
42+
x.foo.bar && console.log('x.foo.bar');
43+
~~~~~~~~~
44+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
45+
46+
// error
47+
1 && x.foo.bar && console.log('x.foo.bar');
48+
~~~~~~~~~
49+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
50+
}
51+
52+
class Foo {
53+
optional?: () => boolean;
54+
required() {
55+
return true;
56+
}
57+
test() {
58+
// error
59+
this.required && console.log('required');
60+
~~~~~~~~~~~~~
61+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
62+
63+
// error
64+
1 && this.required && console.log('required');
65+
~~~~~~~~~~~~~
66+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
67+
68+
// ok
69+
1 && this.optional && console.log('optional');
70+
}
71+
}
72+

‎tests/baselines/reference/truthinessCallExpressionCoercion2.js

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
//// [truthinessCallExpressionCoercion2.ts]
2+
function test(required: () => boolean, optional?: () => boolean) {
3+
// error
4+
required && console.log('required');
5+
6+
// error
7+
1 && required && console.log('required');
8+
9+
// ok
10+
optional && console.log('optional');
11+
12+
// ok
13+
1 && optional && console.log('optional');
14+
15+
// ok
16+
!!required && console.log('not required');
17+
18+
// ok
19+
required() && console.log('required call');
20+
}
21+
22+
function checksPropertyAccess() {
23+
const x = {
24+
foo: {
25+
bar() { return true; }
26+
}
27+
}
28+
29+
// error
30+
x.foo.bar && console.log('x.foo.bar');
31+
32+
// error
33+
1 && x.foo.bar && console.log('x.foo.bar');
34+
}
35+
36+
class Foo {
37+
optional?: () => boolean;
38+
required() {
39+
return true;
40+
}
41+
test() {
42+
// error
43+
this.required && console.log('required');
44+
45+
// error
46+
1 && this.required && console.log('required');
47+
48+
// ok
49+
1 && this.optional && console.log('optional');
50+
}
51+
}
52+
53+
54+
//// [truthinessCallExpressionCoercion2.js]
55+
function test(required, optional) {
56+
// error
57+
required && console.log('required');
58+
// error
59+
1 && required && console.log('required');
60+
// ok
61+
optional && console.log('optional');
62+
// ok
63+
1 && optional && console.log('optional');
64+
// ok
65+
!!required && console.log('not required');
66+
// ok
67+
required() && console.log('required call');
68+
}
69+
function checksPropertyAccess() {
70+
var x = {
71+
foo: {
72+
bar: function () { return true; }
73+
}
74+
};
75+
// error
76+
x.foo.bar && console.log('x.foo.bar');
77+
// error
78+
1 && x.foo.bar && console.log('x.foo.bar');
79+
}
80+
var Foo = /** @class */ (function () {
81+
function Foo() {
82+
}
83+
Foo.prototype.required = function () {
84+
return true;
85+
};
86+
Foo.prototype.test = function () {
87+
// error
88+
this.required && console.log('required');
89+
// error
90+
1 && this.required && console.log('required');
91+
// ok
92+
1 && this.optional && console.log('optional');
93+
};
94+
return Foo;
95+
}());

0 commit comments

Comments
 (0)