Skip to content

Commit bf6be75

Browse files
a-tarasyuka-tarasyuk
authored
feat(36048): handle uncalled function checks in ternaries (#36402)
1 parent c8147cb commit bf6be75

6 files changed

+688
-10
lines changed

Diff for: ‎src/compiler/checker.ts

+10-10
Original file line numberDiff line numberDiff line change
@@ -27929,7 +27929,8 @@ namespace ts {
2792927929
}
2793027930

2793127931
function checkConditionalExpression(node: ConditionalExpression, checkMode?: CheckMode): Type {
27932-
checkTruthinessExpression(node.condition);
27932+
const type = checkTruthinessExpression(node.condition);
27933+
checkTestingKnownTruthyCallableType(node.condition, node.whenTrue, type);
2793327934
const type1 = checkExpression(node.whenTrue, checkMode);
2793427935
const type2 = checkExpression(node.whenFalse, checkMode);
2793527936
return getUnionType([type1, type2], UnionReduction.Subtype);
@@ -31051,9 +31052,8 @@ namespace ts {
3105131052
function checkIfStatement(node: IfStatement) {
3105231053
// Grammar checking
3105331054
checkGrammarStatementInAmbientContext(node);
31054-
3105531055
const type = checkTruthinessExpression(node.expression);
31056-
checkTestingKnownTruthyCallableType(node, type);
31056+
checkTestingKnownTruthyCallableType(node.expression, node.thenStatement, type);
3105731057
checkSourceElement(node.thenStatement);
3105831058

3105931059
if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
@@ -31063,15 +31063,15 @@ namespace ts {
3106331063
checkSourceElement(node.elseStatement);
3106431064
}
3106531065

31066-
function checkTestingKnownTruthyCallableType(ifStatement: IfStatement, type: Type) {
31066+
function checkTestingKnownTruthyCallableType(condExpr: Expression, body: Statement | Expression, type: Type) {
3106731067
if (!strictNullChecks) {
3106831068
return;
3106931069
}
3107031070

31071-
const testedNode = isIdentifier(ifStatement.expression)
31072-
? ifStatement.expression
31073-
: isPropertyAccessExpression(ifStatement.expression)
31074-
? ifStatement.expression.name
31071+
const testedNode = isIdentifier(condExpr)
31072+
? condExpr
31073+
: isPropertyAccessExpression(condExpr)
31074+
? condExpr.name
3107531075
: undefined;
3107631076

3107731077
if (!testedNode) {
@@ -31098,7 +31098,7 @@ namespace ts {
3109831098
return;
3109931099
}
3110031100

31101-
const functionIsUsedInBody = forEachChild(ifStatement.thenStatement, function check(childNode): boolean | undefined {
31101+
const functionIsUsedInBody = forEachChild(body, function check(childNode): boolean | undefined {
3110231102
if (isIdentifier(childNode)) {
3110331103
const childSymbol = getSymbolAtLocation(childNode);
3110431104
if (childSymbol && childSymbol.id === testedFunctionSymbol.id) {
@@ -31110,7 +31110,7 @@ namespace ts {
3111031110
});
3111131111

3111231112
if (!functionIsUsedInBody) {
31113-
error(ifStatement.expression, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
31113+
error(condExpr, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
3111431114
}
3111531115
}
3111631116

Diff for: ‎tests/baselines/reference/truthinessCallExpressionCoercion1.errors.txt

+85
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+

Diff for: ‎tests/baselines/reference/truthinessCallExpressionCoercion1.js

+122
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)