Flag non-nullable values with call expressions in if statements as errors

src/compiler/checker.ts

@@ -27930,7 +27930,25 @@ namespace ts {
             // Grammar checking
             checkGrammarStatementInAmbientContext(node);
 
-            checkTruthinessExpression(node.expression);
+            const type = checkTruthinessExpression(node.expression);
+
+            if (strictNullChecks &&
+                (node.expression.kind === SyntaxKind.Identifier ||
+                node.expression.kind === SyntaxKind.PropertyAccessExpression ||
+                node.expression.kind === SyntaxKind.ElementAccessExpression)) {
+                const possiblyFalsy = !!getFalsyFlags(type);
+                if (!possiblyFalsy) {
+                    // While it technically should be invalid for any known-truthy value
+                    // to be tested, we de-scope to functions as a heuristic to identify
+                    // the most common bugs. There are too many false positives for values
+                    // sourced from type definitions without strictNullChecks otherwise.
+                    const callSignatures = getSignaturesOfType(type, SignatureKind.Call);
+                    if (callSignatures.length > 0) {
+                        error(node.expression, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
+                    }
+                }
+            }
+
             checkSourceElement(node.thenStatement);
 
             if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {

src/compiler/diagnosticMessages.json

@@ -2689,7 +2689,10 @@
         "category": "Error",
         "code": 2773
     },
-
+    "This condition will always return true since the function is always defined. Did you mean to call it instead?": {
+        "category": "Error",
+        "code": 2774
+    },
     "Import declaration '{0}' is using private name '{1}'.": {
         "category": "Error",
         "code": 4000

tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt

@@ -0,0 +1,69 @@
+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?
+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?
+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?
+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?
+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?
+
+
+==== tests/cases/compiler/truthinessCallExpressionCoercion.ts (5 errors) ====
+    function func() { return Math.random() > 0.5; }
+    
+    if (func) { // error
+        ~~~~
+!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
+    }
+    
+    function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) {
+        if (required) { // error
+            ~~~~~~~~
+!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
+        }
+    
+        if (required()) { // ok
+        }
+    
+        if (optional) { // ok
+        }
+    }
+    
+    function checksPropertyAndElementAccess() {
+        const x = {
+            foo: {
+                bar() { }
+            }
+        }
+    
+        if (x.foo.bar) { // error
+            ~~~~~~~~~
+!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
+        }
+        
+        if (x.foo['bar']) { // error
+            ~~~~~~~~~~~~
+!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
+        }
+    }
+    
+    function maybeBoolean(param: boolean | (() => boolean)) {
+        if (param) { // ok
+        }
+    }
+    
+    class Foo {
+        maybeIsUser?: () => boolean;
+    
+        isUser() {
+            return true;
+        }
+    
+        test() {
+            if (this.isUser) { // error
+                ~~~~~~~~~~~
+!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
+            }
+    
+            if (this.maybeIsUser) { // ok
+            }
+        }
+    }
+    

tests/baselines/reference/truthinessCallExpressionCoercion.js

@@ -0,0 +1,94 @@
+//// [truthinessCallExpressionCoercion.ts]
+function func() { return Math.random() > 0.5; }
+
+if (func) { // error
+}
+
+function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) {
+    if (required) { // error
+    }
+
+    if (required()) { // ok
+    }
+
+    if (optional) { // ok
+    }
+}
+
+function checksPropertyAndElementAccess() {
+    const x = {
+        foo: {
+            bar() { }
+        }
+    }
+
+    if (x.foo.bar) { // error
+    }
+    
+    if (x.foo['bar']) { // error
+    }
+}
+
+function maybeBoolean(param: boolean | (() => boolean)) {
+    if (param) { // ok
+    }
+}
+
+class Foo {
+    maybeIsUser?: () => boolean;
+
+    isUser() {
+        return true;
+    }
+
+    test() {
+        if (this.isUser) { // error
+        }
+
+        if (this.maybeIsUser) { // ok
+        }
+    }
+}
+
+
+//// [truthinessCallExpressionCoercion.js]
+function func() { return Math.random() > 0.5; }
+if (func) { // error
+}
+function onlyErrorsWhenNonNullable(required, optional) {
+    if (required) { // error
+    }
+    if (required()) { // ok
+    }
+    if (optional) { // ok
+    }
+}
+function checksPropertyAndElementAccess() {
+    var x = {
+        foo: {
+            bar: function () { }
+        }
+    };
+    if (x.foo.bar) { // error
+    }
+    if (x.foo['bar']) { // error
+    }
+}
+function maybeBoolean(param) {
+    if (param) { // ok
+    }
+}
+var Foo = /** @class */ (function () {
+    function Foo() {
+    }
+    Foo.prototype.isUser = function () {
+        return true;
+    };
+    Foo.prototype.test = function () {
+        if (this.isUser) { // error
+        }
+        if (this.maybeIsUser) { // ok
+        }
+    };
+    return Foo;
+}());

tests/baselines/reference/truthinessCallExpressionCoercion.symbols

@@ -0,0 +1,97 @@
+=== tests/cases/compiler/truthinessCallExpressionCoercion.ts ===
+function func() { return Math.random() > 0.5; }
+>func : Symbol(func, Decl(truthinessCallExpressionCoercion.ts, 0, 0))
+>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
+>Math : Symbol(Math, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
+>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
+
+if (func) { // error
+>func : Symbol(func, Decl(truthinessCallExpressionCoercion.ts, 0, 0))
+}
+
+function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) {
+>onlyErrorsWhenNonNullable : Symbol(onlyErrorsWhenNonNullable, Decl(truthinessCallExpressionCoercion.ts, 3, 1))
+>required : Symbol(required, Decl(truthinessCallExpressionCoercion.ts, 5, 35))
+>optional : Symbol(optional, Decl(truthinessCallExpressionCoercion.ts, 5, 59))
+
+    if (required) { // error
+>required : Symbol(required, Decl(truthinessCallExpressionCoercion.ts, 5, 35))
+    }
+
+    if (required()) { // ok
+>required : Symbol(required, Decl(truthinessCallExpressionCoercion.ts, 5, 35))
+    }
+
+    if (optional) { // ok
+>optional : Symbol(optional, Decl(truthinessCallExpressionCoercion.ts, 5, 59))
+    }
+}
+
+function checksPropertyAndElementAccess() {
+>checksPropertyAndElementAccess : Symbol(checksPropertyAndElementAccess, Decl(truthinessCallExpressionCoercion.ts, 14, 1))
+
+    const x = {
+>x : Symbol(x, Decl(truthinessCallExpressionCoercion.ts, 17, 9))
+
+        foo: {
+>foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15))
+
+            bar() { }
+>bar : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14))
+        }
+    }
+
+    if (x.foo.bar) { // error
+>x.foo.bar : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14))
+>x.foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15))
+>x : Symbol(x, Decl(truthinessCallExpressionCoercion.ts, 17, 9))
+>foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15))
+>bar : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14))
+    }
+    
+    if (x.foo['bar']) { // error
+>x.foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15))
+>x : Symbol(x, Decl(truthinessCallExpressionCoercion.ts, 17, 9))
+>foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15))
+>'bar' : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14))
+    }
+}
+
+function maybeBoolean(param: boolean | (() => boolean)) {
+>maybeBoolean : Symbol(maybeBoolean, Decl(truthinessCallExpressionCoercion.ts, 28, 1))
+>param : Symbol(param, Decl(truthinessCallExpressionCoercion.ts, 30, 22))
+
+    if (param) { // ok
+>param : Symbol(param, Decl(truthinessCallExpressionCoercion.ts, 30, 22))
+    }
+}
+
+class Foo {
+>Foo : Symbol(Foo, Decl(truthinessCallExpressionCoercion.ts, 33, 1))
+
+    maybeIsUser?: () => boolean;
+>maybeIsUser : Symbol(Foo.maybeIsUser, Decl(truthinessCallExpressionCoercion.ts, 35, 11))
+
+    isUser() {
+>isUser : Symbol(Foo.isUser, Decl(truthinessCallExpressionCoercion.ts, 36, 32))
+
+        return true;
+    }
+
+    test() {
+>test : Symbol(Foo.test, Decl(truthinessCallExpressionCoercion.ts, 40, 5))
+
+        if (this.isUser) { // error
+>this.isUser : Symbol(Foo.isUser, Decl(truthinessCallExpressionCoercion.ts, 36, 32))
+>this : Symbol(Foo, Decl(truthinessCallExpressionCoercion.ts, 33, 1))
+>isUser : Symbol(Foo.isUser, Decl(truthinessCallExpressionCoercion.ts, 36, 32))
+        }
+
+        if (this.maybeIsUser) { // ok
+>this.maybeIsUser : Symbol(Foo.maybeIsUser, Decl(truthinessCallExpressionCoercion.ts, 35, 11))
+>this : Symbol(Foo, Decl(truthinessCallExpressionCoercion.ts, 33, 1))
+>maybeIsUser : Symbol(Foo.maybeIsUser, Decl(truthinessCallExpressionCoercion.ts, 35, 11))
+        }
+    }
+}
+

tests/baselines/reference/truthinessCallExpressionCoercion.types

@@ -0,0 +1,105 @@
+=== tests/cases/compiler/truthinessCallExpressionCoercion.ts ===
+function func() { return Math.random() > 0.5; }
+>func : () => boolean
+>Math.random() > 0.5 : boolean
+>Math.random() : number
+>Math.random : () => number
+>Math : Math
+>random : () => number
+>0.5 : 0.5
+
+if (func) { // error
+>func : () => boolean
+}
+
+function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) {
+>onlyErrorsWhenNonNullable : (required: () => boolean, optional?: (() => boolean) | undefined) => void
+>required : () => boolean
+>optional : (() => boolean) | undefined
+
+    if (required) { // error
+>required : () => boolean
+    }
+
+    if (required()) { // ok
+>required() : boolean
+>required : () => boolean
+    }
+
+    if (optional) { // ok
+>optional : (() => boolean) | undefined
+    }
+}
+
+function checksPropertyAndElementAccess() {
+>checksPropertyAndElementAccess : () => void
+
+    const x = {
+>x : { foo: { bar(): void; }; }
+>{        foo: {            bar() { }        }    } : { foo: { bar(): void; }; }
+
+        foo: {
+>foo : { bar(): void; }
+>{            bar() { }        } : { bar(): void; }
+
+            bar() { }
+>bar : () => void
+        }
+    }
+
+    if (x.foo.bar) { // error
+>x.foo.bar : () => void
+>x.foo : { bar(): void; }
+>x : { foo: { bar(): void; }; }
+>foo : { bar(): void; }
+>bar : () => void
+    }
+    
+    if (x.foo['bar']) { // error
+>x.foo['bar'] : () => void
+>x.foo : { bar(): void; }
+>x : { foo: { bar(): void; }; }
+>foo : { bar(): void; }
+>'bar' : "bar"
+    }
+}
+
+function maybeBoolean(param: boolean | (() => boolean)) {
+>maybeBoolean : (param: boolean | (() => boolean)) => void
+>param : boolean | (() => boolean)
+
+    if (param) { // ok
+>param : boolean | (() => boolean)
+    }
+}
+
+class Foo {
+>Foo : Foo
+
+    maybeIsUser?: () => boolean;
+>maybeIsUser : (() => boolean) | undefined
+
+    isUser() {
+>isUser : () => boolean
+
+        return true;
+>true : true
+    }
+
+    test() {
+>test : () => void
+
+        if (this.isUser) { // error
+>this.isUser : () => boolean
+>this : this
+>isUser : () => boolean
+        }
+
+        if (this.maybeIsUser) { // ok
+>this.maybeIsUser : (() => boolean) | undefined
+>this : this
+>maybeIsUser : (() => boolean) | undefined
+        }
+    }
+}
+

tests/cases/compiler/truthinessCallExpressionCoercion.ts

@@ -0,0 +1,52 @@
+// @strictNullChecks:true
+
+function func() { return Math.random() > 0.5; }
+
+if (func) { // error
+}
+
+function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) {
+    if (required) { // error
+    }
+
+    if (required()) { // ok
+    }
+
+    if (optional) { // ok
+    }
+}
+
+function checksPropertyAndElementAccess() {
+    const x = {
+        foo: {
+            bar() { }
+        }
+    }
+
+    if (x.foo.bar) { // error
+    }
+    
+    if (x.foo['bar']) { // error
+    }
+}
+
+function maybeBoolean(param: boolean | (() => boolean)) {
+    if (param) { // ok
+    }
+}
+
+class Foo {
+    maybeIsUser?: () => boolean;
+
+    isUser() {
+        return true;
+    }
+
+    test() {
+        if (this.isUser) { // error
+        }
+
+        if (this.maybeIsUser) { // ok
+        }
+    }
+}