Flag non-nullable functions in if statements as errors

src/compiler/checker.ts

@@ -27874,7 +27874,8 @@ namespace ts {
             // Grammar checking
             checkGrammarStatementInAmbientContext(node);
 
-            checkTruthinessExpression(node.expression);
+            const type = checkTruthinessExpression(node.expression);
+            checkTestingKnownTruthyCallableType(node.expression, type);
             checkSourceElement(node.thenStatement);
 
             if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
@@ -27884,6 +27885,59 @@ namespace ts {
             checkSourceElement(node.elseStatement);
         }
 
+        function checkTestingKnownTruthyCallableType(testedNode: Expression, type: Type) {
+            if (!strictNullChecks) {
+                return;
+            }
+
+            if (testedNode.kind === SyntaxKind.Identifier ||
+                testedNode.kind === SyntaxKind.PropertyAccessExpression ||
+                testedNode.kind === SyntaxKind.ElementAccessExpression) {
+                const possiblyFalsy = getFalsyFlags(type);
+                if (possiblyFalsy) {
+                    return;
+                }
+
+                // While it technically should be invalid for any known-truthy value
+                // to be tested, we de-scope to functions that return a boolean 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) {
+                    return;
+                }
+
+                const alwaysReturnsBoolean = every(callSignatures, signature => {
+                    const returnType = getReturnTypeOfSignature(signature);
+                    const exactlyBoolean = TypeFlags.Boolean | TypeFlags.Union;
+                    if (returnType.flags === exactlyBoolean) {
+                        return true;
+                    }
+
+                    if (returnType.flags & TypeFlags.Union) {
+                        const allowedInUnion = TypeFlags.BooleanLike | TypeFlags.Nullable;
+                        let unionContainsBoolean = false;
+                        const unionContainsOnlyAllowedTypes = every((returnType as UnionType).types, innerType => {
+                            if (innerType.flags & TypeFlags.BooleanLike) {
+                                unionContainsBoolean = true;
+                            }
+
+                            return (innerType.flags | allowedInUnion) === allowedInUnion;
+                        });
+
+                        return unionContainsBoolean && unionContainsOnlyAllowedTypes;
+                    }
+
+                    return false;
+                });
+
+                if (alwaysReturnsBoolean) {
+                    error(testedNode, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
+                }
+            }
+        }
+
         function checkDoStatement(node: DoStatement) {
             // Grammar checking
             checkGrammarStatementInAmbientContext(node);

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,112 @@
+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(29,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(32,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(35,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(58,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(61,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(78,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 (8 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 onlyErrorsWhenTestingNonNullableFunctionType(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 (optional) { // ok
+        }
+
+        if (!!required) { // ok
+        }
+
+        if (required()) { // ok
+        }
+    }
+
+    function onlyErrorsWhenReturnsBoolean(
+        bool: () => boolean,
+        nullableBool: () => boolean | undefined,
+        nullableTrue: () => true | undefined,
+        nullable: () => undefined | null,
+        notABool: () => string,
+        unionWithBool: () => boolean | string,
+        nullableString: () => string | undefined
+    ) {
+        if (bool) { // error
+            ~~~~
+!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
+        }
+
+        if (nullableBool) { // error
+            ~~~~~~~~~~~~
+!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
+        }
+
+        if (nullableTrue) { // error
+            ~~~~~~~~~~~~
+!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
+        }
+
+        if (nullable) { // ok
+        }
+
+        if (notABool) { // ok
+        }
+
+        if (unionWithBool) { // ok
+        }
+
+        if (nullableString) { // ok
+        }
+    }
+
+    function checksPropertyAndElementAccess() {
+        const x = {
+            foo: {
+                bar() { return true; }
+            }
+        }
+
+        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,146 @@
+//// [truthinessCallExpressionCoercion.ts]
+function func() { return Math.random() > 0.5; }
+
+if (func) { // error
+}
+
+function onlyErrorsWhenTestingNonNullableFunctionType(required: () => boolean, optional?: () => boolean) {
+    if (required) { // error
+    }
+
+    if (optional) { // ok
+    }
+
+    if (!!required) { // ok
+    }
+
+    if (required()) { // ok
+    }
+}
+
+function onlyErrorsWhenReturnsBoolean(
+    bool: () => boolean,
+    nullableBool: () => boolean | undefined,
+    nullableTrue: () => true | undefined,
+    nullable: () => undefined | null,
+    notABool: () => string,
+    unionWithBool: () => boolean | string,
+    nullableString: () => string | undefined
+) {
+    if (bool) { // error
+    }
+
+    if (nullableBool) { // error
+    }
+
+    if (nullableTrue) { // error
+    }
+
+    if (nullable) { // ok
+    }
+
+    if (notABool) { // ok
+    }
+
+    if (unionWithBool) { // ok
+    }
+
+    if (nullableString) { // ok
+    }
+}
+
+function checksPropertyAndElementAccess() {
+    const x = {
+        foo: {
+            bar() { return true; }
+        }
+    }
+
+    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 onlyErrorsWhenTestingNonNullableFunctionType(required, optional) {
+    if (required) { // error
+    }
+    if (optional) { // ok
+    }
+    if (!!required) { // ok
+    }
+    if (required()) { // ok
+    }
+}
+function onlyErrorsWhenReturnsBoolean(bool, nullableBool, nullableTrue, nullable, notABool, unionWithBool, nullableString) {
+    if (bool) { // error
+    }
+    if (nullableBool) { // error
+    }
+    if (nullableTrue) { // error
+    }
+    if (nullable) { // ok
+    }
+    if (notABool) { // ok
+    }
+    if (unionWithBool) { // ok
+    }
+    if (nullableString) { // ok
+    }
+}
+function checksPropertyAndElementAccess() {
+    var x = {
+        foo: {
+            bar: function () { return true; }
+        }
+    };
+    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;
+}());