Skip to content

Commit 94dbe2f

Browse files
a-tarasyuka-tarasyuk
committed
feat(36048): handle uncalled function checks in ternaries
1 parent 3b919e2 commit 94dbe2f

6 files changed

+688
-10
lines changed

‎src/compiler/checker.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27578,7 +27578,8 @@ namespace ts {
2757827578
}
2757927579

2758027580
function checkConditionalExpression(node: ConditionalExpression, checkMode?: CheckMode): Type {
27581-
checkTruthinessExpression(node.condition);
27581+
const type = checkTruthinessExpression(node.condition);
27582+
checkTestingKnownTruthyCallableType(node.condition, node.whenTrue, type);
2758227583
const type1 = checkExpression(node.whenTrue, checkMode);
2758327584
const type2 = checkExpression(node.whenFalse, checkMode);
2758427585
return getUnionType([type1, type2], UnionReduction.Subtype);
@@ -30685,9 +30686,8 @@ namespace ts {
3068530686
function checkIfStatement(node: IfStatement) {
3068630687
// Grammar checking
3068730688
checkGrammarStatementInAmbientContext(node);
30688-
3068930689
const type = checkTruthinessExpression(node.expression);
30690-
checkTestingKnownTruthyCallableType(node, type);
30690+
checkTestingKnownTruthyCallableType(node.expression, node.thenStatement, type);
3069130691
checkSourceElement(node.thenStatement);
3069230692

3069330693
if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
@@ -30697,15 +30697,15 @@ namespace ts {
3069730697
checkSourceElement(node.elseStatement);
3069830698
}
3069930699

30700-
function checkTestingKnownTruthyCallableType(ifStatement: IfStatement, type: Type) {
30700+
function checkTestingKnownTruthyCallableType(condExpr: Expression, body: Statement | Expression, type: Type) {
3070130701
if (!strictNullChecks) {
3070230702
return;
3070330703
}
3070430704

30705-
const testedNode = isIdentifier(ifStatement.expression)
30706-
? ifStatement.expression
30707-
: isPropertyAccessExpression(ifStatement.expression)
30708-
? ifStatement.expression.name
30705+
const testedNode = isIdentifier(condExpr)
30706+
? condExpr
30707+
: isPropertyAccessExpression(condExpr)
30708+
? condExpr.name
3070930709
: undefined;
3071030710

3071130711
if (!testedNode) {
@@ -30732,7 +30732,7 @@ namespace ts {
3073230732
return;
3073330733
}
3073430734

30735-
const functionIsUsedInBody = forEachChild(ifStatement.thenStatement, function check(childNode): boolean | undefined {
30735+
const functionIsUsedInBody = forEachChild(body, function check(childNode): boolean | undefined {
3073630736
if (isIdentifier(childNode)) {
3073730737
const childSymbol = getSymbolAtLocation(childNode);
3073830738
if (childSymbol && childSymbol.id === testedFunctionSymbol.id) {
@@ -30744,7 +30744,7 @@ namespace ts {
3074430744
});
3074530745

3074630746
if (!functionIsUsedInBody) {
30747-
error(ifStatement.expression, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
30747+
error(condExpr, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
3074830748
}
3074930749
}
3075030750

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

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

‎tests/baselines/reference/truthinessCallExpressionCoercion1.js

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
//// [truthinessCallExpressionCoercion1.ts]
2+
function onlyErrorsWhenTestingNonNullableFunctionType(required: () => boolean, optional?: () => boolean) {
3+
// error
4+
required ? console.log('required') : undefined;
5+
6+
// ok
7+
optional ? console.log('optional') : undefined;
8+
9+
// ok
10+
!!required ? console.log('not required') : undefined;
11+
12+
// ok
13+
required() ? console.log('required call') : undefined;
14+
}
15+
16+
function onlyErrorsWhenUnusedInBody() {
17+
function test() { return Math.random() > 0.5; }
18+
19+
// error
20+
test ? console.log('test') : undefined;
21+
22+
// ok
23+
test ? console.log(test) : undefined;
24+
25+
// ok
26+
test ? test() : undefined;
27+
28+
// ok
29+
test
30+
? [() => null].forEach(() => { test(); })
31+
: undefined;
32+
33+
// error
34+
test
35+
? [() => null].forEach(test => { test() })
36+
: undefined;
37+
}
38+
39+
function checksPropertyAccess() {
40+
const x = {
41+
foo: {
42+
bar() { return true; }
43+
}
44+
}
45+
46+
// error
47+
x.foo.bar ? console.log('x.foo.bar') : undefined;
48+
49+
// ok
50+
x.foo.bar ? x.foo.bar : undefined;
51+
}
52+
53+
class Foo {
54+
maybeIsUser?: () => boolean;
55+
56+
isUser() {
57+
return true;
58+
}
59+
60+
test() {
61+
// error
62+
this.isUser ? console.log('this.isUser') : undefined;
63+
64+
// ok
65+
this.maybeIsUser ? console.log('this.maybeIsUser') : undefined;
66+
}
67+
}
68+
69+
70+
//// [truthinessCallExpressionCoercion1.js]
71+
function onlyErrorsWhenTestingNonNullableFunctionType(required, optional) {
72+
// error
73+
required ? console.log('required') : undefined;
74+
// ok
75+
optional ? console.log('optional') : undefined;
76+
// ok
77+
!!required ? console.log('not required') : undefined;
78+
// ok
79+
required() ? console.log('required call') : undefined;
80+
}
81+
function onlyErrorsWhenUnusedInBody() {
82+
function test() { return Math.random() > 0.5; }
83+
// error
84+
test ? console.log('test') : undefined;
85+
// ok
86+
test ? console.log(test) : undefined;
87+
// ok
88+
test ? test() : undefined;
89+
// ok
90+
test
91+
? [function () { return null; }].forEach(function () { test(); })
92+
: undefined;
93+
// error
94+
test
95+
? [function () { return null; }].forEach(function (test) { test(); })
96+
: undefined;
97+
}
98+
function checksPropertyAccess() {
99+
var x = {
100+
foo: {
101+
bar: function () { return true; }
102+
}
103+
};
104+
// error
105+
x.foo.bar ? console.log('x.foo.bar') : undefined;
106+
// ok
107+
x.foo.bar ? x.foo.bar : undefined;
108+
}
109+
var Foo = /** @class */ (function () {
110+
function Foo() {
111+
}
112+
Foo.prototype.isUser = function () {
113+
return true;
114+
};
115+
Foo.prototype.test = function () {
116+
// error
117+
this.isUser ? console.log('this.isUser') : undefined;
118+
// ok
119+
this.maybeIsUser ? console.log('this.maybeIsUser') : undefined;
120+
};
121+
return Foo;
122+
}());

0 commit comments

Comments
 (0)