Disallow truthiness/nullishness checks on syntax that never varies on it (#59217)

Author: RyanCavanaugh

src/compiler/checker.ts

@@ -470,6 +470,7 @@ import {
     isAutoAccessorPropertyDeclaration,
     isAwaitExpression,
     isBinaryExpression,
+    isBinaryLogicalOperator,
     isBindableObjectDefinePropertyCall,
     isBindableStaticElementAccessExpression,
     isBindableStaticNameExpression,
@@ -899,6 +900,7 @@ import {
     NodeWithTypeArguments,
     NonNullChain,
     NonNullExpression,
+    NoSubstitutionTemplateLiteral,
     not,
     noTruncationMaximumTruncationLength,
     NumberLiteralType,
@@ -929,6 +931,7 @@ import {
     PatternAmbientModule,
     PlusToken,
     PostfixUnaryExpression,
+    PredicateSemantics,
     PrefixUnaryExpression,
     PrivateIdentifier,
     Program,
@@ -39470,7 +39473,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 return state;
             }
 
-            checkGrammarNullishCoalesceWithLogicalExpression(node);
+            checkNullishCoalesceOperands(node);
 
             const operator = node.operatorToken.kind;
             if (operator === SyntaxKind.EqualsToken && (node.left.kind === SyntaxKind.ObjectLiteralExpression || node.left.kind === SyntaxKind.ArrayLiteralExpression)) {
@@ -39503,7 +39506,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                     if (operator === SyntaxKind.AmpersandAmpersandToken || isIfStatement(parent)) {
                         checkTestingKnownTruthyCallableOrAwaitableOrEnumMemberType(node.left, leftType, isIfStatement(parent) ? parent.thenStatement : undefined);
                     }
-                    checkTruthinessOfType(leftType, node.left);
+                    if (isBinaryLogicalOperator(operator)) {
+                        checkTruthinessOfType(leftType, node.left);
+                    }
                 }
             }
         }
@@ -39569,7 +39574,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         }
     }
 
-    function checkGrammarNullishCoalesceWithLogicalExpression(node: BinaryExpression) {
+    function checkNullishCoalesceOperands(node: BinaryExpression) {
         const { left, operatorToken, right } = node;
         if (operatorToken.kind === SyntaxKind.QuestionQuestionToken) {
             if (isBinaryExpression(left) && (left.operatorToken.kind === SyntaxKind.BarBarToken || left.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken)) {
@@ -39578,7 +39583,60 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             if (isBinaryExpression(right) && (right.operatorToken.kind === SyntaxKind.BarBarToken || right.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken)) {
                 grammarErrorOnNode(right, Diagnostics._0_and_1_operations_cannot_be_mixed_without_parentheses, tokenToString(right.operatorToken.kind), tokenToString(operatorToken.kind));
             }
+
+            const leftTarget = skipOuterExpressions(left, OuterExpressionKinds.All);
+            const nullishSemantics = getSyntacticNullishnessSemantics(leftTarget);
+            if (nullishSemantics !== PredicateSemantics.Sometimes) {
+                if (node.parent.kind === SyntaxKind.BinaryExpression) {
+                    error(leftTarget, Diagnostics.This_binary_expression_is_never_nullish_Are_you_missing_parentheses);
+                }
+                else {
+                    if (nullishSemantics === PredicateSemantics.Always) {
+                        error(leftTarget, Diagnostics.This_expression_is_always_nullish);
+                    }
+                    else {
+                        error(leftTarget, Diagnostics.Right_operand_of_is_unreachable_because_the_left_operand_is_never_nullish);
+                    }
+                }
+            }
+        }
+    }
+
+    function getSyntacticNullishnessSemantics(node: Node): PredicateSemantics {
+        node = skipOuterExpressions(node);
+        switch (node.kind) {
+            case SyntaxKind.AwaitExpression:
+            case SyntaxKind.CallExpression:
+            case SyntaxKind.ElementAccessExpression:
+            case SyntaxKind.NewExpression:
+            case SyntaxKind.PropertyAccessExpression:
+            case SyntaxKind.YieldExpression:
+                return PredicateSemantics.Sometimes;
+            case SyntaxKind.BinaryExpression:
+                // List of operators that can produce null/undefined:
+                // = ??= ?? || ||= && &&=
+                switch ((node as BinaryExpression).operatorToken.kind) {
+                    case SyntaxKind.EqualsToken:
+                    case SyntaxKind.QuestionQuestionToken:
+                    case SyntaxKind.QuestionQuestionEqualsToken:
+                    case SyntaxKind.BarBarToken:
+                    case SyntaxKind.BarBarEqualsToken:
+                    case SyntaxKind.AmpersandAmpersandToken:
+                    case SyntaxKind.AmpersandAmpersandEqualsToken:
+                        return PredicateSemantics.Sometimes;
+                }
+                return PredicateSemantics.Never;
+            case SyntaxKind.ConditionalExpression:
+                return getSyntacticNullishnessSemantics((node as ConditionalExpression).whenTrue) | getSyntacticNullishnessSemantics((node as ConditionalExpression).whenFalse);
+            case SyntaxKind.NullKeyword:
+                return PredicateSemantics.Always;
+            case SyntaxKind.Identifier:
+                if (getResolvedSymbol(node as Identifier) === undefinedSymbol) {
+                    return PredicateSemantics.Always;
+                }
+                return PredicateSemantics.Sometimes;
         }
+        return PredicateSemantics.Never;
     }
 
     // Note that this and `checkBinaryExpression` above should behave mostly the same, except this elides some
@@ -39589,7 +39647,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             return checkDestructuringAssignment(left, checkExpression(right, checkMode), checkMode, right.kind === SyntaxKind.ThisKeyword);
         }
         let leftType: Type;
-        if (isLogicalOrCoalescingBinaryOperator(operator)) {
+        if (isBinaryLogicalOperator(operator)) {
             leftType = checkTruthinessExpression(left, checkMode);
         }
         else {
@@ -44217,9 +44275,57 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         if (type.flags & TypeFlags.Void) {
             error(node, Diagnostics.An_expression_of_type_void_cannot_be_tested_for_truthiness);
         }
+        else {
+            const semantics = getSyntacticTruthySemantics(node);
+            if (semantics !== PredicateSemantics.Sometimes) {
+                error(
+                    node,
+                    semantics === PredicateSemantics.Always ?
+                        Diagnostics.This_kind_of_expression_is_always_truthy :
+                        Diagnostics.This_kind_of_expression_is_always_falsy,
+                );
+            }
+        }
+
         return type;
     }
 
+    function getSyntacticTruthySemantics(node: Node): PredicateSemantics {
+        node = skipOuterExpressions(node);
+        switch (node.kind) {
+            case SyntaxKind.NumericLiteral:
+                // Allow `while(0)` or `while(1)`
+                if ((node as NumericLiteral).text === "0" || (node as NumericLiteral).text === "1") {
+                    return PredicateSemantics.Sometimes;
+                }
+                return PredicateSemantics.Always;
+            case SyntaxKind.ArrayLiteralExpression:
+            case SyntaxKind.ArrowFunction:
+            case SyntaxKind.BigIntLiteral:
+            case SyntaxKind.ClassExpression:
+            case SyntaxKind.FunctionExpression:
+            case SyntaxKind.JsxElement:
+            case SyntaxKind.JsxSelfClosingElement:
+            case SyntaxKind.ObjectLiteralExpression:
+            case SyntaxKind.RegularExpressionLiteral:
+                return PredicateSemantics.Always;
+            case SyntaxKind.VoidExpression:
+            case SyntaxKind.NullKeyword:
+                return PredicateSemantics.Never;
+            case SyntaxKind.NoSubstitutionTemplateLiteral:
+            case SyntaxKind.StringLiteral:
+                return !!(node as StringLiteral | NoSubstitutionTemplateLiteral).text ? PredicateSemantics.Always : PredicateSemantics.Never;
+            case SyntaxKind.ConditionalExpression:
+                return getSyntacticTruthySemantics((node as ConditionalExpression).whenTrue) | getSyntacticTruthySemantics((node as ConditionalExpression).whenFalse);
+            case SyntaxKind.Identifier:
+                if (getResolvedSymbol(node as Identifier) === undefinedSymbol) {
+                    return PredicateSemantics.Never;
+                }
+                return PredicateSemantics.Sometimes;
+        }
+        return PredicateSemantics.Sometimes;
+    }
+
     function checkTruthinessExpression(node: Expression, checkMode?: CheckMode) {
         return checkTruthinessOfType(checkExpression(node, checkMode), node);
     }

src/compiler/diagnosticMessages.json

@@ -3915,7 +3915,26 @@
         "category": "Error",
         "code": 2868
     },
-
+    "Right operand of ?? is unreachable because the left operand is never nullish.": {
+        "category": "Error",
+        "code": 2869
+    },
+    "This binary expression is never nullish. Are you missing parentheses?": {
+        "category": "Error",
+        "code": 2870
+    },
+    "This expression is always nullish.": {
+        "category": "Error",
+        "code": 2871
+    },
+    "This kind of expression is always truthy.": {
+        "category": "Error",
+        "code": 2872
+    },
+    "This kind of expression is always falsy.": {
+        "category": "Error",
+        "code": 2873
+    },
     "Import declaration '{0}' is using private name '{1}'.": {
         "category": "Error",
         "code": 4000

src/compiler/types.ts

@@ -923,6 +923,14 @@ export const enum RelationComparisonResult {
     Overflow            = ComplexityOverflow | StackDepthOverflow,
 }
 
+/** @internal */
+export const enum PredicateSemantics {
+    None = 0,
+    Always = 1 << 0,
+    Never = 1 << 1,
+    Sometimes = Always | Never,
+}
+
 /** @internal */
 export type NodeId = number;
 

src/compiler/utilities.ts

@@ -7190,7 +7190,8 @@ export function modifierToFlag(token: SyntaxKind): ModifierFlags {
     return ModifierFlags.None;
 }
 
-function isBinaryLogicalOperator(token: SyntaxKind): boolean {
+/** @internal */
+export function isBinaryLogicalOperator(token: SyntaxKind): boolean {
     return token === SyntaxKind.BarBarToken || token === SyntaxKind.AmpersandAmpersandToken;
 }
 

tests/baselines/reference/aliasUsageInOrExpression.errors.txt

@@ -0,0 +1,31 @@
+aliasUsageInOrExpression_main.ts(10,40): error TS2873: This kind of expression is always falsy.
+aliasUsageInOrExpression_main.ts(11,40): error TS2873: This kind of expression is always falsy.
+
+
+==== aliasUsageInOrExpression_main.ts (2 errors) ====
+    import Backbone = require("./aliasUsageInOrExpression_backbone");
+    import moduleA = require("./aliasUsageInOrExpression_moduleA");
+    interface IHasVisualizationModel {
+        VisualizationModel: typeof Backbone.Model;
+    }
+    var i: IHasVisualizationModel;
+    var d1 = i || moduleA;
+    var d2: IHasVisualizationModel = i || moduleA;
+    var d2: IHasVisualizationModel = moduleA || i;
+    var e: { x: IHasVisualizationModel } = <{ x: IHasVisualizationModel }>null || { x: moduleA };
+                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+!!! error TS2873: This kind of expression is always falsy.
+    var f: { x: IHasVisualizationModel } = <{ x: IHasVisualizationModel }>null ? { x: moduleA } : null;
+                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+!!! error TS2873: This kind of expression is always falsy.
+==== aliasUsageInOrExpression_backbone.ts (0 errors) ====
+    export class Model {
+        public someData: string;
+    }
+    
+==== aliasUsageInOrExpression_moduleA.ts (0 errors) ====
+    import Backbone = require("./aliasUsageInOrExpression_backbone");
+    export class VisualizationModel extends Backbone.Model {
+        // interesting stuff here
+    }
+    

tests/baselines/reference/bestCommonTypeWithContextualTyping.errors.txt

@@ -0,0 +1,26 @@
+bestCommonTypeWithContextualTyping.ts(19,31): error TS2873: This kind of expression is always falsy.
+
+
+==== bestCommonTypeWithContextualTyping.ts (1 errors) ====
+    interface Contextual {
+        dummy;
+        p?: number;
+    }
+    
+    interface Ellement {
+        dummy;
+        p: any;
+    }
+    
+    var e: Ellement;
+    
+    // All of these should pass. Neither type is a supertype of the other, but the RHS should
+    // always use Ellement in these examples (not Contextual). Because Ellement is assignable
+    // to Contextual, no errors.
+    var arr: Contextual[] = [e]; // Ellement[]
+    var obj: { [s: string]: Contextual } = { s: e }; // { s: Ellement; [s: string]: Ellement }
+    
+    var conditional: Contextual = null ? e : e; // Ellement
+                                  ~~~~
+!!! error TS2873: This kind of expression is always falsy.
+    var contextualOr: Contextual = e || e; // Ellement

tests/baselines/reference/bestCommonTypeWithContextualTyping.types

@@ -4,6 +4,7 @@
 interface Contextual {
     dummy;
 >dummy : any
+>      : ^^^
 
     p?: number;
 >p : number
@@ -13,9 +14,11 @@ interface Contextual {
 interface Ellement {
     dummy;
 >dummy : any
+>      : ^^^
 
     p: any;
 >p : any
+>  : ^^^
 }
 
 var e: Ellement;

tests/baselines/reference/checkJsdocReturnTag1.errors.txt

@@ -0,0 +1,28 @@
+returns.js(20,12): error TS2872: This kind of expression is always truthy.
+
+
+==== returns.js (1 errors) ====
+    // @ts-check
+    /**
+     * @returns {string} This comment is not currently exposed
+     */
+    function f() {
+        return "hello";
+    }
+    
+    /**
+     * @returns {string=} This comment is not currently exposed
+     */
+    function f1() {
+        return "hello world";
+    }
+    
+    /**
+     * @returns {string|number} This comment is not currently exposed
+     */
+    function f2() {
+        return 5 || "hello";
+               ~
+!!! error TS2872: This kind of expression is always truthy.
+    }
+    

tests/baselines/reference/checkJsdocReturnTag2.errors.txt

@@ -1,9 +1,10 @@
 returns.js(6,5): error TS2322: Type 'number' is not assignable to type 'string'.
 returns.js(13,5): error TS2322: Type 'number | boolean' is not assignable to type 'string | number'.
   Type 'boolean' is not assignable to type 'string | number'.
+returns.js(13,12): error TS2872: This kind of expression is always truthy.
 
 
-==== returns.js (2 errors) ====
+==== returns.js (3 errors) ====
     // @ts-check
     /**
      * @returns {string} This comment is not currently exposed
@@ -22,5 +23,7 @@ returns.js(13,5): error TS2322: Type 'number | boolean' is not assignable to typ
         ~~~~~~
 !!! error TS2322: Type 'number | boolean' is not assignable to type 'string | number'.
 !!! error TS2322:   Type 'boolean' is not assignable to type 'string | number'.
+               ~
+!!! error TS2872: This kind of expression is always truthy.
     }
 

tests/baselines/reference/computedPropertyNames46_ES5.errors.txt

@@ -0,0 +1,9 @@
+computedPropertyNames46_ES5.ts(2,6): error TS2873: This kind of expression is always falsy.
+
+
+==== computedPropertyNames46_ES5.ts (1 errors) ====
+    var o = {
+        ["" || 0]: 0
+         ~~
+!!! error TS2873: This kind of expression is always falsy.
+    };