Skip to content

Commit 130615a

Browse files
jwbayjwbay
committed
Flag non-nullable values with call expressions in if statements as errors
1 parent 2a2866c commit 130615a

7 files changed

+440
-2
lines changed

Diff for: ‎src/compiler/checker.ts

+19-1
Original file line numberDiff line numberDiff line change
@@ -27874,7 +27874,25 @@ namespace ts {
2787427874
// Grammar checking
2787527875
checkGrammarStatementInAmbientContext(node);
2787627876

27877-
checkTruthinessExpression(node.expression);
27877+
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+
2787827896
checkSourceElement(node.thenStatement);
2787927897

2788027898
if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {

Diff for: ‎src/compiler/diagnosticMessages.json

+4-1
Original file line numberDiff line numberDiff line change
@@ -2689,7 +2689,10 @@
26892689
"category": "Error",
26902690
"code": 2773
26912691
},
2692-
2692+
"This condition will always return true since the function is always defined. Did you mean to call it instead?": {
2693+
"category": "Error",
2694+
"code": 2774
2695+
},
26932696
"Import declaration '{0}' is using private name '{1}'.": {
26942697
"category": "Error",
26952698
"code": 4000

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

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
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?
2+
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?
4+
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?
6+
7+
8+
==== tests/cases/compiler/truthinessCallExpressionCoercion.ts (5 errors) ====
9+
function func() { return Math.random() > 0.5; }
10+
11+
if (func) { // error
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+
16+
function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) {
17+
if (required) { // error
18+
~~~~~~~~
19+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
20+
}
21+
22+
if (required()) { // ok
23+
}
24+
25+
if (optional) { // ok
26+
}
27+
}
28+
29+
function checksPropertyAndElementAccess() {
30+
const x = {
31+
foo: {
32+
bar() { }
33+
}
34+
}
35+
36+
if (x.foo.bar) { // error
37+
~~~~~~~~~
38+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
39+
}
40+
41+
if (x.foo['bar']) { // 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+
47+
function maybeBoolean(param: boolean | (() => boolean)) {
48+
if (param) { // ok
49+
}
50+
}
51+
52+
class Foo {
53+
maybeIsUser?: () => boolean;
54+
55+
isUser() {
56+
return true;
57+
}
58+
59+
test() {
60+
if (this.isUser) { // error
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+
65+
if (this.maybeIsUser) { // ok
66+
}
67+
}
68+
}
69+

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

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
//// [truthinessCallExpressionCoercion.ts]
2+
function func() { return Math.random() > 0.5; }
3+
4+
if (func) { // error
5+
}
6+
7+
function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) {
8+
if (required) { // error
9+
}
10+
11+
if (required()) { // ok
12+
}
13+
14+
if (optional) { // ok
15+
}
16+
}
17+
18+
function checksPropertyAndElementAccess() {
19+
const x = {
20+
foo: {
21+
bar() { }
22+
}
23+
}
24+
25+
if (x.foo.bar) { // error
26+
}
27+
28+
if (x.foo['bar']) { // error
29+
}
30+
}
31+
32+
function maybeBoolean(param: boolean | (() => boolean)) {
33+
if (param) { // ok
34+
}
35+
}
36+
37+
class Foo {
38+
maybeIsUser?: () => boolean;
39+
40+
isUser() {
41+
return true;
42+
}
43+
44+
test() {
45+
if (this.isUser) { // error
46+
}
47+
48+
if (this.maybeIsUser) { // ok
49+
}
50+
}
51+
}
52+
53+
54+
//// [truthinessCallExpressionCoercion.js]
55+
function func() { return Math.random() > 0.5; }
56+
if (func) { // error
57+
}
58+
function onlyErrorsWhenNonNullable(required, optional) {
59+
if (required) { // error
60+
}
61+
if (required()) { // ok
62+
}
63+
if (optional) { // ok
64+
}
65+
}
66+
function checksPropertyAndElementAccess() {
67+
var x = {
68+
foo: {
69+
bar: function () { }
70+
}
71+
};
72+
if (x.foo.bar) { // error
73+
}
74+
if (x.foo['bar']) { // error
75+
}
76+
}
77+
function maybeBoolean(param) {
78+
if (param) { // ok
79+
}
80+
}
81+
var Foo = /** @class */ (function () {
82+
function Foo() {
83+
}
84+
Foo.prototype.isUser = function () {
85+
return true;
86+
};
87+
Foo.prototype.test = function () {
88+
if (this.isUser) { // error
89+
}
90+
if (this.maybeIsUser) { // ok
91+
}
92+
};
93+
return Foo;
94+
}());

Diff for: ‎tests/baselines/reference/truthinessCallExpressionCoercion.symbols

+97
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
=== tests/cases/compiler/truthinessCallExpressionCoercion.ts ===
2+
function func() { return Math.random() > 0.5; }
3+
>func : Symbol(func, Decl(truthinessCallExpressionCoercion.ts, 0, 0))
4+
>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
5+
>Math : Symbol(Math, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
6+
>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
7+
8+
if (func) { // error
9+
>func : Symbol(func, Decl(truthinessCallExpressionCoercion.ts, 0, 0))
10+
}
11+
12+
function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) {
13+
>onlyErrorsWhenNonNullable : Symbol(onlyErrorsWhenNonNullable, Decl(truthinessCallExpressionCoercion.ts, 3, 1))
14+
>required : Symbol(required, Decl(truthinessCallExpressionCoercion.ts, 5, 35))
15+
>optional : Symbol(optional, Decl(truthinessCallExpressionCoercion.ts, 5, 59))
16+
17+
if (required) { // error
18+
>required : Symbol(required, Decl(truthinessCallExpressionCoercion.ts, 5, 35))
19+
}
20+
21+
if (required()) { // ok
22+
>required : Symbol(required, Decl(truthinessCallExpressionCoercion.ts, 5, 35))
23+
}
24+
25+
if (optional) { // ok
26+
>optional : Symbol(optional, Decl(truthinessCallExpressionCoercion.ts, 5, 59))
27+
}
28+
}
29+
30+
function checksPropertyAndElementAccess() {
31+
>checksPropertyAndElementAccess : Symbol(checksPropertyAndElementAccess, Decl(truthinessCallExpressionCoercion.ts, 14, 1))
32+
33+
const x = {
34+
>x : Symbol(x, Decl(truthinessCallExpressionCoercion.ts, 17, 9))
35+
36+
foo: {
37+
>foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15))
38+
39+
bar() { }
40+
>bar : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14))
41+
}
42+
}
43+
44+
if (x.foo.bar) { // error
45+
>x.foo.bar : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14))
46+
>x.foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15))
47+
>x : Symbol(x, Decl(truthinessCallExpressionCoercion.ts, 17, 9))
48+
>foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15))
49+
>bar : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14))
50+
}
51+
52+
if (x.foo['bar']) { // error
53+
>x.foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15))
54+
>x : Symbol(x, Decl(truthinessCallExpressionCoercion.ts, 17, 9))
55+
>foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15))
56+
>'bar' : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14))
57+
}
58+
}
59+
60+
function maybeBoolean(param: boolean | (() => boolean)) {
61+
>maybeBoolean : Symbol(maybeBoolean, Decl(truthinessCallExpressionCoercion.ts, 28, 1))
62+
>param : Symbol(param, Decl(truthinessCallExpressionCoercion.ts, 30, 22))
63+
64+
if (param) { // ok
65+
>param : Symbol(param, Decl(truthinessCallExpressionCoercion.ts, 30, 22))
66+
}
67+
}
68+
69+
class Foo {
70+
>Foo : Symbol(Foo, Decl(truthinessCallExpressionCoercion.ts, 33, 1))
71+
72+
maybeIsUser?: () => boolean;
73+
>maybeIsUser : Symbol(Foo.maybeIsUser, Decl(truthinessCallExpressionCoercion.ts, 35, 11))
74+
75+
isUser() {
76+
>isUser : Symbol(Foo.isUser, Decl(truthinessCallExpressionCoercion.ts, 36, 32))
77+
78+
return true;
79+
}
80+
81+
test() {
82+
>test : Symbol(Foo.test, Decl(truthinessCallExpressionCoercion.ts, 40, 5))
83+
84+
if (this.isUser) { // error
85+
>this.isUser : Symbol(Foo.isUser, Decl(truthinessCallExpressionCoercion.ts, 36, 32))
86+
>this : Symbol(Foo, Decl(truthinessCallExpressionCoercion.ts, 33, 1))
87+
>isUser : Symbol(Foo.isUser, Decl(truthinessCallExpressionCoercion.ts, 36, 32))
88+
}
89+
90+
if (this.maybeIsUser) { // ok
91+
>this.maybeIsUser : Symbol(Foo.maybeIsUser, Decl(truthinessCallExpressionCoercion.ts, 35, 11))
92+
>this : Symbol(Foo, Decl(truthinessCallExpressionCoercion.ts, 33, 1))
93+
>maybeIsUser : Symbol(Foo.maybeIsUser, Decl(truthinessCallExpressionCoercion.ts, 35, 11))
94+
}
95+
}
96+
}
97+

0 commit comments

Comments
 (0)