Skip to content

Commit e2e6330

Browse files
jonhuejonhue
committed
review
1 parent b17401e commit e2e6330

6 files changed

+344
-53
lines changed

‎src/compiler/checker.ts

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -34958,7 +34958,6 @@ namespace ts {
3495834958

3495934959
function checkTestingKnownTruthyCallableOrAwaitableType(condExpr: Expression, type: Type, body?: Statement | Expression) {
3496034960
if (!strictNullChecks) return;
34961-
if (getFalsyFlags(type)) return;
3496234961

3496334962
if (getAwaitedTypeOfPromise(type)) {
3496434963
errorAndMaybeSuggestAwait(
@@ -34969,39 +34968,50 @@ namespace ts {
3496934968
return;
3497034969
}
3497134970

34972-
const location = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
34973-
if (isBinaryExpression(condExpr) && condExpr.operatorToken.kind === SyntaxKind.BarBarToken) {
34974-
checkTestingKnownTruthyCallableOrAwaitableType(condExpr.left, type, body);
34975-
}
34976-
const testedNode = isIdentifier(location) ? location
34977-
: isPropertyAccessExpression(location) ? location.name
34978-
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
34979-
: undefined;
34980-
const isPropertyExpressionCast = isPropertyAccessExpression(location)
34981-
&& isAssertionExpression(skipParentheses(location.expression));
34982-
if (!testedNode || isPropertyExpressionCast) {
34983-
return;
34971+
helper(condExpr, body);
34972+
while (isBinaryExpression(condExpr) && condExpr.operatorToken.kind === SyntaxKind.BarBarToken) {
34973+
condExpr = condExpr.left;
34974+
helper(condExpr, body);
3498434975
}
3498534976

34986-
// While it technically should be invalid for any known-truthy value
34987-
// to be tested, we de-scope to functions unrefenced in the block as a
34988-
// heuristic to identify the most common bugs. There are too many
34989-
// false positives for values sourced from type definitions without
34990-
// strictNullChecks otherwise.
34991-
const callSignatures = getSignaturesOfType(type, SignatureKind.Call);
34992-
if (callSignatures.length === 0) {
34993-
return;
34994-
}
34977+
function helper(condExpr: Expression, body: Expression | Statement | undefined) {
34978+
const location = isBinaryExpression(condExpr) &&
34979+
(condExpr.operatorToken.kind === SyntaxKind.BarBarToken || condExpr.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken)
34980+
? condExpr.right
34981+
: condExpr;
34982+
const type = checkTruthinessExpression(location);
34983+
if (getFalsyFlags(type)) return;
3499534984

34996-
const testedSymbol = getSymbolAtLocation(testedNode);
34997-
if (!testedSymbol) {
34998-
return;
34999-
}
34985+
// While it technically should be invalid for any known-truthy value
34986+
// to be tested, we de-scope to functions unrefenced in the block as a
34987+
// heuristic to identify the most common bugs. There are too many
34988+
// false positives for values sourced from type definitions without
34989+
// strictNullChecks otherwise.
34990+
const callSignatures = getSignaturesOfType(type, SignatureKind.Call);
34991+
if (callSignatures.length === 0) {
34992+
return;
34993+
}
3500034994

35001-
const isUsed = isBinaryExpression(condExpr.parent) && isFunctionUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
35002-
|| body && isFunctionUsedInConditionBody(condExpr, body, testedNode, testedSymbol);
35003-
if (!isUsed) {
35004-
error(location, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
34995+
const testedNode = isIdentifier(location) ? location
34996+
: isPropertyAccessExpression(location) ? location.name
34997+
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
34998+
: undefined;
34999+
const isPropertyExpressionCast = isPropertyAccessExpression(location)
35000+
&& isAssertionExpression(skipParentheses(location.expression));
35001+
if (!testedNode || isPropertyExpressionCast) {
35002+
return;
35003+
}
35004+
35005+
const testedSymbol = getSymbolAtLocation(testedNode);
35006+
if (!testedSymbol) {
35007+
return;
35008+
}
35009+
35010+
const isUsed = isBinaryExpression(condExpr.parent) && isFunctionUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
35011+
|| body && isFunctionUsedInConditionBody(condExpr, body, testedNode, testedSymbol);
35012+
if (!isUsed) {
35013+
error(location, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
35014+
}
3500535015
}
3500635016
}
3500735017

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

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,81 @@
1-
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(4,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(8,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/uncalledFunctionChecksInConditional.ts(8,14): 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(12,5): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
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(9,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/uncalledFunctionChecksInConditional.ts(9,14): 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(13,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/uncalledFunctionChecksInConditional.ts(32,10): 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(36,5): 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/uncalledFunctionChecksInConditional.ts(40,22): 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/uncalledFunctionChecksInConditional.ts(44,16): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
9+
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(48,22): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
510

611

7-
==== tests/cases/compiler/uncalledFunctionChecksInConditional.ts (4 errors) ====
12+
==== tests/cases/compiler/uncalledFunctionChecksInConditional.ts (9 errors) ====
813
declare function isFoo(): boolean;
914
declare function isBar(): boolean;
15+
declare const isUndefinedFoo: (() => boolean) | undefined;
1016

1117
if (isFoo) {
1218
~~~~~
1319
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
14-
// error
20+
// error on isFoo
1521
}
1622

1723
if (isFoo || isBar) {
1824
~~~~~
1925
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
2026
~~~~~
2127
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
22-
// error
28+
// error on isFoo, isBar
2329
}
2430

2531
if (isFoo || isFoo()) {
2632
~~~~~
2733
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
28-
// error
34+
// error on isFoo
35+
}
36+
37+
if (isUndefinedFoo || isFoo()) {
38+
// no error
2939
}
3040

3141
if (isFoo && isFoo()) {
3242
// no error
3343
}
44+
45+
declare const x: boolean;
46+
declare const ux: boolean | undefined;
47+
declare const y: boolean;
48+
declare const uy: boolean | undefined;
49+
declare function z(): boolean;
50+
declare const uz: (() => boolean) | undefined;
51+
52+
if (x || isFoo) {
53+
~~~~~
54+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
55+
// error on isFoo
56+
}
57+
58+
if (isFoo || x) {
59+
~~~~~
60+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
61+
// error on isFoo
62+
}
63+
64+
if (x || y || z() || isFoo) {
65+
~~~~~
66+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
67+
// error on isFoo
68+
}
69+
70+
if (x || uy || z || isUndefinedFoo) {
71+
~
72+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
73+
// error on z
74+
}
75+
76+
if (ux || y || uz || isFoo) {
77+
~~~~~
78+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
79+
// error on isFoo
80+
}
3481

‎tests/baselines/reference/uncalledFunctionChecksInConditional.js

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,84 @@
11
//// [uncalledFunctionChecksInConditional.ts]
22
declare function isFoo(): boolean;
33
declare function isBar(): boolean;
4+
declare const isUndefinedFoo: (() => boolean) | undefined;
45

56
if (isFoo) {
6-
// error
7+
// error on isFoo
78
}
89

910
if (isFoo || isBar) {
10-
// error
11+
// error on isFoo, isBar
1112
}
1213

1314
if (isFoo || isFoo()) {
14-
// error
15+
// error on isFoo
16+
}
17+
18+
if (isUndefinedFoo || isFoo()) {
19+
// no error
1520
}
1621

1722
if (isFoo && isFoo()) {
1823
// no error
1924
}
25+
26+
declare const x: boolean;
27+
declare const ux: boolean | undefined;
28+
declare const y: boolean;
29+
declare const uy: boolean | undefined;
30+
declare function z(): boolean;
31+
declare const uz: (() => boolean) | undefined;
32+
33+
if (x || isFoo) {
34+
// error on isFoo
35+
}
36+
37+
if (isFoo || x) {
38+
// error on isFoo
39+
}
40+
41+
if (x || y || z() || isFoo) {
42+
// error on isFoo
43+
}
44+
45+
if (x || uy || z || isUndefinedFoo) {
46+
// error on z
47+
}
48+
49+
if (ux || y || uz || isFoo) {
50+
// error on isFoo
51+
}
2052

2153

2254
//// [uncalledFunctionChecksInConditional.js]
2355
if (isFoo) {
24-
// error
56+
// error on isFoo
2557
}
2658
if (isFoo || isBar) {
27-
// error
59+
// error on isFoo, isBar
2860
}
2961
if (isFoo || isFoo()) {
30-
// error
62+
// error on isFoo
63+
}
64+
if (isUndefinedFoo || isFoo()) {
65+
// no error
3166
}
3267
if (isFoo && isFoo()) {
3368
// no error
3469
}
70+
if (x || isFoo) {
71+
// error on isFoo
72+
}
73+
if (isFoo || x) {
74+
// error on isFoo
75+
}
76+
if (x || y || z() || isFoo) {
77+
// error on isFoo
78+
}
79+
if (x || uy || z || isUndefinedFoo) {
80+
// error on z
81+
}
82+
if (ux || y || uz || isFoo) {
83+
// error on isFoo
84+
}

‎tests/baselines/reference/uncalledFunctionChecksInConditional.symbols

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,34 @@ declare function isFoo(): boolean;
55
declare function isBar(): boolean;
66
>isBar : Symbol(isBar, Decl(uncalledFunctionChecksInConditional.ts, 0, 34))
77

8+
declare const isUndefinedFoo: (() => boolean) | undefined;
9+
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksInConditional.ts, 2, 13))
10+
811
if (isFoo) {
912
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
1013

11-
// error
14+
// error on isFoo
1215
}
1316

1417
if (isFoo || isBar) {
1518
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
1619
>isBar : Symbol(isBar, Decl(uncalledFunctionChecksInConditional.ts, 0, 34))
1720

18-
// error
21+
// error on isFoo, isBar
1922
}
2023

2124
if (isFoo || isFoo()) {
2225
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
2326
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
2427

25-
// error
28+
// error on isFoo
29+
}
30+
31+
if (isUndefinedFoo || isFoo()) {
32+
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksInConditional.ts, 2, 13))
33+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
34+
35+
// no error
2636
}
2737

2838
if (isFoo && isFoo()) {
@@ -32,3 +42,62 @@ if (isFoo && isFoo()) {
3242
// no error
3343
}
3444

45+
declare const x: boolean;
46+
>x : Symbol(x, Decl(uncalledFunctionChecksInConditional.ts, 24, 13))
47+
48+
declare const ux: boolean | undefined;
49+
>ux : Symbol(ux, Decl(uncalledFunctionChecksInConditional.ts, 25, 13))
50+
51+
declare const y: boolean;
52+
>y : Symbol(y, Decl(uncalledFunctionChecksInConditional.ts, 26, 13))
53+
54+
declare const uy: boolean | undefined;
55+
>uy : Symbol(uy, Decl(uncalledFunctionChecksInConditional.ts, 27, 13))
56+
57+
declare function z(): boolean;
58+
>z : Symbol(z, Decl(uncalledFunctionChecksInConditional.ts, 27, 38))
59+
60+
declare const uz: (() => boolean) | undefined;
61+
>uz : Symbol(uz, Decl(uncalledFunctionChecksInConditional.ts, 29, 13))
62+
63+
if (x || isFoo) {
64+
>x : Symbol(x, Decl(uncalledFunctionChecksInConditional.ts, 24, 13))
65+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
66+
67+
// error on isFoo
68+
}
69+
70+
if (isFoo || x) {
71+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
72+
>x : Symbol(x, Decl(uncalledFunctionChecksInConditional.ts, 24, 13))
73+
74+
// error on isFoo
75+
}
76+
77+
if (x || y || z() || isFoo) {
78+
>x : Symbol(x, Decl(uncalledFunctionChecksInConditional.ts, 24, 13))
79+
>y : Symbol(y, Decl(uncalledFunctionChecksInConditional.ts, 26, 13))
80+
>z : Symbol(z, Decl(uncalledFunctionChecksInConditional.ts, 27, 38))
81+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
82+
83+
// error on isFoo
84+
}
85+
86+
if (x || uy || z || isUndefinedFoo) {
87+
>x : Symbol(x, Decl(uncalledFunctionChecksInConditional.ts, 24, 13))
88+
>uy : Symbol(uy, Decl(uncalledFunctionChecksInConditional.ts, 27, 13))
89+
>z : Symbol(z, Decl(uncalledFunctionChecksInConditional.ts, 27, 38))
90+
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksInConditional.ts, 2, 13))
91+
92+
// error on z
93+
}
94+
95+
if (ux || y || uz || isFoo) {
96+
>ux : Symbol(ux, Decl(uncalledFunctionChecksInConditional.ts, 25, 13))
97+
>y : Symbol(y, Decl(uncalledFunctionChecksInConditional.ts, 26, 13))
98+
>uz : Symbol(uz, Decl(uncalledFunctionChecksInConditional.ts, 29, 13))
99+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
100+
101+
// error on isFoo
102+
}
103+

0 commit comments

Comments
 (0)