Skip to content

Commit 611f5b3

Browse files
jonhuejonhue
committed
Uncalled function checks only works with single conditional
1 parent 3fe05c8 commit 611f5b3

13 files changed

+352
-13
lines changed

‎src/compiler/checker.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34538,7 +34538,6 @@ namespace ts {
3453834538

3453934539
function checkTestingKnownTruthyCallableOrAwaitableType(condExpr: Expression, type: Type, body?: Statement | Expression) {
3454034540
if (!strictNullChecks) return;
34541-
if (getFalsyFlags(type)) return;
3454234541

3454334542
if (getAwaitedTypeOfPromise(type)) {
3454434543
errorAndMaybeSuggestAwait(
@@ -34549,7 +34548,12 @@ namespace ts {
3454934548
return;
3455034549
}
3455134550

34552-
const location = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
34551+
const location = isBinaryExpression(condExpr) ? condExpr.right
34552+
: isPrefixUnaryExpression(condExpr) ? condExpr.operand
34553+
: condExpr;
34554+
if (isBinaryExpression(condExpr) && condExpr.operatorToken.kind === SyntaxKind.BarBarToken) {
34555+
checkTestingKnownTruthyCallableOrAwaitableType(condExpr.left, type, body);
34556+
}
3455334557
const testedNode = isIdentifier(location) ? location
3455434558
: isPropertyAccessExpression(location) ? location.name
3455534559
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
@@ -34560,12 +34564,18 @@ namespace ts {
3456034564
return;
3456134565
}
3456234566

34567+
const testedType = isPrefixUnaryExpression(condExpr) ? checkExpression(location) : type;
34568+
console.log(0, typeToString(type), typeToString(testedType))
34569+
if (typeToString(testedType).includes("undefined")) { // maybeTypeOfKind(testedType, TypeFlags.Undefined)
34570+
return;
34571+
}
34572+
3456334573
// While it technically should be invalid for any known-truthy value
3456434574
// to be tested, we de-scope to functions unrefenced in the block as a
3456534575
// heuristic to identify the most common bugs. There are too many
3456634576
// false positives for values sourced from type definitions without
3456734577
// strictNullChecks otherwise.
34568-
const callSignatures = getSignaturesOfType(type, SignatureKind.Call);
34578+
const callSignatures = getSignaturesOfType(testedType, SignatureKind.Call);
3456934579
if (callSignatures.length === 0) {
3457034580
return;
3457134581
}
@@ -34578,7 +34588,12 @@ namespace ts {
3457834588
const isUsed = isBinaryExpression(condExpr.parent) && isFunctionUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
3457934589
|| body && isFunctionUsedInConditionBody(condExpr, body, testedNode, testedSymbol);
3458034590
if (!isUsed) {
34581-
error(location, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
34591+
if (getFalsyFlags(type)) {
34592+
error(location, Diagnostics.This_condition_will_always_return_false_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
34593+
}
34594+
else {
34595+
error(location, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
34596+
}
3458234597
}
3458334598
}
3458434599

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3260,6 +3260,10 @@
32603260
"category": "Error",
32613261
"code": 2801
32623262
},
3263+
"This condition will always return false since the function is always defined. Did you mean to call it instead?": {
3264+
"category": "Error",
3265+
"code": 2802
3266+
},
32633267

32643268
"Import declaration '{0}' is using private name '{1}'.": {
32653269
"category": "Error",

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@ tests/cases/compiler/truthinessCallExpressionCoercion2.ts(14,10): error TS2774:
33
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(41,18): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
44
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(44,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
55
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(48,11): 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(65,46): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
76
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(76,5): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
87
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(79,10): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
98
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(99,5): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
109
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(109,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
1110
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(112,14): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
1211

1312

14-
==== tests/cases/compiler/truthinessCallExpressionCoercion2.ts (11 errors) ====
13+
==== tests/cases/compiler/truthinessCallExpressionCoercion2.ts (10 errors) ====
1514
declare class A {
1615
static from(): string;
1716
}
@@ -87,8 +86,6 @@ tests/cases/compiler/truthinessCallExpressionCoercion2.ts(112,14): error TS2774:
8786
// error
8887
typeof window !== 'undefined' && window.console &&
8988
((window.console as any).firebug || (window.console.exception && window.console.table));
90-
~~~~~~~~~~~~~~~~~~~~~~~~
91-
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
9289
}
9390

9491
function checksPropertyAccess() {

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(5,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/uncalledFunctionChecksInConditional.ts(13,6): error TS2802: This condition will always return false since the function is always defined. Did you mean to call it instead?
3+
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(21,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/uncalledFunctionChecksInConditional.ts(21,14): 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/uncalledFunctionChecksInConditional.ts(25,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/uncalledFunctionChecksInConditional.ts(33,6): error TS2802: This condition will always return false since the function is always defined. Did you mean to call it instead?
7+
8+
9+
==== tests/cases/compiler/uncalledFunctionChecksInConditional.ts (6 errors) ====
10+
declare function isFoo(): boolean;
11+
declare function isBar(): boolean;
12+
declare const isUndefinedFoo: () => boolean | undefined;
13+
14+
if (isFoo) {
15+
~~~~~
16+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
17+
// error
18+
}
19+
20+
if (isUndefinedFoo) {
21+
// no error
22+
}
23+
24+
if (!isFoo) {
25+
~~~~~
26+
!!! error TS2802: This condition will always return false since the function is always defined. Did you mean to call it instead?
27+
// error
28+
}
29+
30+
if (!isUndefinedFoo) {
31+
// no error
32+
}
33+
34+
if (isFoo || isBar) {
35+
~~~~~
36+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
37+
~~~~~
38+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
39+
// error
40+
}
41+
42+
if (isFoo || isFoo()) {
43+
~~~~~
44+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
45+
// error
46+
}
47+
48+
if (isFoo && isFoo()) {
49+
// no error
50+
}
51+
52+
if (!isFoo || isFoo()) {
53+
~~~~~
54+
!!! error TS2802: This condition will always return false since the function is always defined. Did you mean to call it instead?
55+
// error
56+
}
57+
58+
if (!isUndefinedFoo || isFoo()) {
59+
// no error
60+
}
61+

‎tests/baselines/reference/uncalledFunctionChecksInConditional.js

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
//// [uncalledFunctionChecksInConditional.ts]
2+
declare function isFoo(): boolean;
3+
declare function isBar(): boolean;
4+
declare const isUndefinedFoo: () => boolean | undefined;
5+
6+
if (isFoo) {
7+
// error
8+
}
9+
10+
if (isUndefinedFoo) {
11+
// no error
12+
}
13+
14+
if (!isFoo) {
15+
// error
16+
}
17+
18+
if (!isUndefinedFoo) {
19+
// no error
20+
}
21+
22+
if (isFoo || isBar) {
23+
// error
24+
}
25+
26+
if (isFoo || isFoo()) {
27+
// error
28+
}
29+
30+
if (isFoo && isFoo()) {
31+
// no error
32+
}
33+
34+
if (!isFoo || isFoo()) {
35+
// error
36+
}
37+
38+
if (!isUndefinedFoo || isFoo()) {
39+
// no error
40+
}
41+
42+
43+
//// [uncalledFunctionChecksInConditional.js]
44+
if (isFoo) {
45+
// error
46+
}
47+
if (isUndefinedFoo) {
48+
// no error
49+
}
50+
if (!isFoo) {
51+
// error
52+
}
53+
if (!isUndefinedFoo) {
54+
// no error
55+
}
56+
if (isFoo || isBar) {
57+
// error
58+
}
59+
if (isFoo || isFoo()) {
60+
// error
61+
}
62+
if (isFoo && isFoo()) {
63+
// no error
64+
}
65+
if (!isFoo || isFoo()) {
66+
// error
67+
}
68+
if (!isUndefinedFoo || isFoo()) {
69+
// no error
70+
}

‎tests/baselines/reference/uncalledFunctionChecksInConditional.symbols

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
=== tests/cases/compiler/uncalledFunctionChecksInConditional.ts ===
2+
declare function isFoo(): boolean;
3+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
4+
5+
declare function isBar(): boolean;
6+
>isBar : Symbol(isBar, Decl(uncalledFunctionChecksInConditional.ts, 0, 34))
7+
8+
declare const isUndefinedFoo: () => boolean | undefined;
9+
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksInConditional.ts, 2, 13))
10+
11+
if (isFoo) {
12+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
13+
14+
// error
15+
}
16+
17+
if (isUndefinedFoo) {
18+
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksInConditional.ts, 2, 13))
19+
20+
// no error
21+
}
22+
23+
if (!isFoo) {
24+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
25+
26+
// error
27+
}
28+
29+
if (!isUndefinedFoo) {
30+
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksInConditional.ts, 2, 13))
31+
32+
// no error
33+
}
34+
35+
if (isFoo || isBar) {
36+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
37+
>isBar : Symbol(isBar, Decl(uncalledFunctionChecksInConditional.ts, 0, 34))
38+
39+
// error
40+
}
41+
42+
if (isFoo || isFoo()) {
43+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
44+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
45+
46+
// error
47+
}
48+
49+
if (isFoo && isFoo()) {
50+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
51+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
52+
53+
// no error
54+
}
55+
56+
if (!isFoo || isFoo()) {
57+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
58+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
59+
60+
// error
61+
}
62+
63+
if (!isUndefinedFoo || isFoo()) {
64+
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksInConditional.ts, 2, 13))
65+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
66+
67+
// no error
68+
}
69+

‎tests/baselines/reference/uncalledFunctionChecksInConditional.types

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
=== tests/cases/compiler/uncalledFunctionChecksInConditional.ts ===
2+
declare function isFoo(): boolean;
3+
>isFoo : () => boolean
4+
5+
declare function isBar(): boolean;
6+
>isBar : () => boolean
7+
8+
declare const isUndefinedFoo: () => boolean | undefined;
9+
>isUndefinedFoo : () => boolean | undefined
10+
11+
if (isFoo) {
12+
>isFoo : () => boolean
13+
14+
// error
15+
}
16+
17+
if (isUndefinedFoo) {
18+
>isUndefinedFoo : () => boolean | undefined
19+
20+
// no error
21+
}
22+
23+
if (!isFoo) {
24+
>!isFoo : false
25+
>isFoo : () => boolean
26+
27+
// error
28+
}
29+
30+
if (!isUndefinedFoo) {
31+
>!isUndefinedFoo : false
32+
>isUndefinedFoo : () => boolean | undefined
33+
34+
// no error
35+
}
36+
37+
if (isFoo || isBar) {
38+
>isFoo || isBar : () => boolean
39+
>isFoo : () => boolean
40+
>isBar : () => boolean
41+
42+
// error
43+
}
44+
45+
if (isFoo || isFoo()) {
46+
>isFoo || isFoo() : () => boolean
47+
>isFoo : () => boolean
48+
>isFoo() : boolean
49+
>isFoo : () => boolean
50+
51+
// error
52+
}
53+
54+
if (isFoo && isFoo()) {
55+
>isFoo && isFoo() : boolean
56+
>isFoo : () => boolean
57+
>isFoo() : boolean
58+
>isFoo : () => boolean
59+
60+
// no error
61+
}
62+
63+
if (!isFoo || isFoo()) {
64+
>!isFoo || isFoo() : boolean
65+
>!isFoo : false
66+
>isFoo : () => boolean
67+
>isFoo() : boolean
68+
>isFoo : () => boolean
69+
70+
// error
71+
}
72+
73+
if (!isUndefinedFoo || isFoo()) {
74+
>!isUndefinedFoo || isFoo() : boolean
75+
>!isUndefinedFoo : false
76+
>isUndefinedFoo : () => boolean | undefined
77+
>isFoo() : boolean
78+
>isFoo : () => boolean
79+
80+
// no error
81+
}
82+

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ tests/cases/conformance/types/unknown/unknownType2.ts(216,13): error TS2322: Typ
248248
else {
249249
const a: NumberEnum.A = u;
250250
}
251-
251+
252252

253253
if (u !== NumberEnum.A && u !== NumberEnum.B && u !== StringEnum.A) { }
254254
else {

‎tests/baselines/reference/unknownType2.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ function notNotEquals(u: unknown) {
241241
else {
242242
const a: NumberEnum.A = u;
243243
}
244-
244+
245245

246246
if (u !== NumberEnum.A && u !== NumberEnum.B && u !== StringEnum.A) { }
247247
else {

0 commit comments

Comments
 (0)