Skip to content

Commit e81f5ec

Browse files
jwbayjwbay
committed
Only error when testing functions that return booleans
1 parent 130615a commit e81f5ec

6 files changed

+310
-92
lines changed

‎src/compiler/checker.ts

+47-18
Original file line numberDiff line numberDiff line change
@@ -27875,24 +27875,7 @@ namespace ts {
2787527875
checkGrammarStatementInAmbientContext(node);
2787627876

2787727877
const type = checkTruthinessExpression(node.expression);
27878-
27879-
if (strictNullChecks &&
27880-
(node.expression.kind === SyntaxKind.Identifier ||
27881-
node.expression.kind === SyntaxKind.PropertyAccessExpression ||
27882-
node.expression.kind === SyntaxKind.ElementAccessExpression)) {
27883-
const possiblyFalsy = !!getFalsyFlags(type);
27884-
if (!possiblyFalsy) {
27885-
// While it technically should be invalid for any known-truthy value
27886-
// to be tested, we de-scope to functions as a heuristic to identify
27887-
// the most common bugs. There are too many false positives for values
27888-
// sourced from type definitions without strictNullChecks otherwise.
27889-
const callSignatures = getSignaturesOfType(type, SignatureKind.Call);
27890-
if (callSignatures.length > 0) {
27891-
error(node.expression, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
27892-
}
27893-
}
27894-
}
27895-
27878+
checkTestingKnownTruthyCallableType(node.expression, type);
2789627879
checkSourceElement(node.thenStatement);
2789727880

2789827881
if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
@@ -27902,6 +27885,52 @@ namespace ts {
2790227885
checkSourceElement(node.elseStatement);
2790327886
}
2790427887

27888+
function checkTestingKnownTruthyCallableType(testedNode: Expression, type: Type) {
27889+
if (!strictNullChecks) {
27890+
return;
27891+
}
27892+
27893+
if (testedNode.kind === SyntaxKind.Identifier ||
27894+
testedNode.kind === SyntaxKind.PropertyAccessExpression ||
27895+
testedNode.kind === SyntaxKind.ElementAccessExpression) {
27896+
const possiblyFalsy = getFalsyFlags(type);
27897+
if (possiblyFalsy) {
27898+
return;
27899+
}
27900+
27901+
// While it technically should be invalid for any known-truthy value
27902+
// to be tested, we de-scope to functions that return a boolean as a
27903+
// heuristic to identify the most common bugs. There are too many
27904+
// false positives for values sourced from type definitions without
27905+
// strictNullChecks otherwise.
27906+
const callSignatures = getSignaturesOfType(type, SignatureKind.Call);
27907+
if (callSignatures.length === 0) {
27908+
return;
27909+
}
27910+
27911+
const alwaysReturnsBoolean = every(callSignatures, signature => {
27912+
const returnType = getReturnTypeOfSignature(signature);
27913+
const exactlyBoolean = TypeFlags.Boolean | TypeFlags.Union;
27914+
if ((returnType.flags & exactlyBoolean) === exactlyBoolean) {
27915+
return true;
27916+
}
27917+
27918+
if (returnType.flags & TypeFlags.Union) {
27919+
const allowedInUnion = TypeFlags.Boolean | TypeFlags.BooleanLiteral | TypeFlags.Nullable;
27920+
return every((returnType as UnionType).types, innerType => {
27921+
return (innerType.flags | allowedInUnion) === allowedInUnion;
27922+
});
27923+
}
27924+
27925+
return false;
27926+
});
27927+
27928+
if (alwaysReturnsBoolean) {
27929+
error(testedNode, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
27930+
}
27931+
}
27932+
}
27933+
2790527934
function checkDoStatement(node: DoStatement) {
2790627935
// Grammar checking
2790727936
checkGrammarStatementInAmbientContext(node);

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

+39-7
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,75 @@
11
tests/cases/compiler/truthinessCallExpressionCoercion.ts(3,5): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
22
tests/cases/compiler/truthinessCallExpressionCoercion.ts(7,9): 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/truthinessCallExpressionCoercion.ts(24,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
43
tests/cases/compiler/truthinessCallExpressionCoercion.ts(27,9): 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/truthinessCallExpressionCoercion.ts(44,13): 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/truthinessCallExpressionCoercion.ts(30,9): 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/truthinessCallExpressionCoercion.ts(50,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/truthinessCallExpressionCoercion.ts(53,9): 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/truthinessCallExpressionCoercion.ts(70,13): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
68

79

8-
==== tests/cases/compiler/truthinessCallExpressionCoercion.ts (5 errors) ====
10+
==== tests/cases/compiler/truthinessCallExpressionCoercion.ts (7 errors) ====
911
function func() { return Math.random() > 0.5; }
1012

1113
if (func) { // error
1214
~~~~
1315
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
1416
}
1517

16-
function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) {
18+
function onlyErrorsWhenTestingNonNullableFunctionType(required: () => boolean, optional?: () => boolean) {
1719
if (required) { // error
1820
~~~~~~~~
1921
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
2022
}
2123

24+
if (optional) { // ok
25+
}
26+
27+
if (!!required) { // ok
28+
}
29+
2230
if (required()) { // ok
2331
}
32+
}
2433

25-
if (optional) { // ok
34+
function onlyErrorsWhenReturnsBoolean(
35+
bool: () => boolean,
36+
nullableBool: () => boolean | undefined,
37+
notABool: () => string,
38+
unionWithBool: () => boolean | string,
39+
nullableString: () => string | undefined
40+
) {
41+
if (bool) { // error
42+
~~~~
43+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
44+
}
45+
46+
if (nullableBool) { // error
47+
~~~~~~~~~~~~
48+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
49+
}
50+
51+
if (notABool) { // ok
52+
}
53+
54+
if (unionWithBool) { // ok
55+
}
56+
57+
if (nullableString) { // ok
2658
}
2759
}
2860

2961
function checksPropertyAndElementAccess() {
3062
const x = {
3163
foo: {
32-
bar() { }
64+
bar() { return true; }
3365
}
3466
}
3567

3668
if (x.foo.bar) { // error
3769
~~~~~~~~~
3870
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
3971
}
40-
72+
4173
if (x.foo['bar']) { // error
4274
~~~~~~~~~~~~
4375
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?

‎tests/baselines/reference/truthinessCallExpressionCoercion.js

+47-7
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,53 @@ function func() { return Math.random() > 0.5; }
44
if (func) { // error
55
}
66

7-
function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) {
7+
function onlyErrorsWhenTestingNonNullableFunctionType(required: () => boolean, optional?: () => boolean) {
88
if (required) { // error
99
}
1010

11+
if (optional) { // ok
12+
}
13+
14+
if (!!required) { // ok
15+
}
16+
1117
if (required()) { // ok
1218
}
19+
}
1320

14-
if (optional) { // ok
21+
function onlyErrorsWhenReturnsBoolean(
22+
bool: () => boolean,
23+
nullableBool: () => boolean | undefined,
24+
notABool: () => string,
25+
unionWithBool: () => boolean | string,
26+
nullableString: () => string | undefined
27+
) {
28+
if (bool) { // error
29+
}
30+
31+
if (nullableBool) { // error
32+
}
33+
34+
if (notABool) { // ok
35+
}
36+
37+
if (unionWithBool) { // ok
38+
}
39+
40+
if (nullableString) { // ok
1541
}
1642
}
1743

1844
function checksPropertyAndElementAccess() {
1945
const x = {
2046
foo: {
21-
bar() { }
47+
bar() { return true; }
2248
}
2349
}
2450

2551
if (x.foo.bar) { // error
2652
}
27-
53+
2854
if (x.foo['bar']) { // error
2955
}
3056
}
@@ -55,18 +81,32 @@ class Foo {
5581
function func() { return Math.random() > 0.5; }
5682
if (func) { // error
5783
}
58-
function onlyErrorsWhenNonNullable(required, optional) {
84+
function onlyErrorsWhenTestingNonNullableFunctionType(required, optional) {
5985
if (required) { // error
6086
}
87+
if (optional) { // ok
88+
}
89+
if (!!required) { // ok
90+
}
6191
if (required()) { // ok
6292
}
63-
if (optional) { // ok
93+
}
94+
function onlyErrorsWhenReturnsBoolean(bool, nullableBool, notABool, unionWithBool, nullableString) {
95+
if (bool) { // error
96+
}
97+
if (nullableBool) { // error
98+
}
99+
if (notABool) { // ok
100+
}
101+
if (unionWithBool) { // ok
102+
}
103+
if (nullableString) { // ok
64104
}
65105
}
66106
function checksPropertyAndElementAccess() {
67107
var x = {
68108
foo: {
69-
bar: function () { }
109+
bar: function () { return true; }
70110
}
71111
};
72112
if (x.foo.bar) { // error

0 commit comments

Comments
 (0)