Add noUnnecessaryCasts compiler option, quickfix

Author: weswigham

src/compiler/checker.ts

@@ -19882,6 +19882,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                     }
                 }
                 if (sourceFlags & TypeFlags.Conditional) {
+                    if ((source as ConditionalType).root === (target as ConditionalType).root) {
+                        // Two instantiations of the same conditional type, just check instantiated outer type parameter equality
+                        const params = (source as ConditionalType).root.outerTypeParameters || [];
+                        const sourceTypeArguments = map(params, t => (source as ConditionalType).mapper ? getMappedType(t, (source as ConditionalType).mapper!) : t);
+                        const targetTypeArguments = map(params, t => (target as ConditionalType).mapper ? getMappedType(t, (target as ConditionalType).mapper!) : t);
+                        return typeArgumentsRelatedTo(sourceTypeArguments, targetTypeArguments, map(params, () => VarianceFlags.Unmeasurable), /*reportErrors*/ false, IntersectionState.None);
+                    }
                     if ((source as ConditionalType).root.isDistributive === (target as ConditionalType).root.isDistributive) {
                         if (result = isRelatedTo((source as ConditionalType).checkType, (target as ConditionalType).checkType, RecursionFlags.Both, /*reportErrors*/ false)) {
                             if (result &= isRelatedTo((source as ConditionalType).extendsType, (target as ConditionalType).extendsType, RecursionFlags.Both, /*reportErrors*/ false)) {
@@ -26071,6 +26078,24 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             node.kind === SyntaxKind.PropertyDeclaration)!;
     }
 
+    function getControlFlowContainerForIdentifier(node: Identifier, declaration: Declaration, symbol: Symbol, typeFromSymbol: Type) {
+        // The declaration container is the innermost function that encloses the declaration of the variable
+        // or parameter. The flow container is the innermost function starting with which we analyze the control
+        // flow graph to determine the control flow based type.
+        const isParameter = getRootDeclaration(declaration).kind === SyntaxKind.Parameter;
+        const declarationContainer = getControlFlowContainer(declaration);
+        let flowContainer = getControlFlowContainer(node);
+        // When the control flow originates in a function expression or arrow function and we are referencing
+        // a const variable or parameter from an outer function, we extend the origin of the control flow
+        // analysis to include the immediately enclosing function.
+        while (flowContainer !== declarationContainer && (flowContainer.kind === SyntaxKind.FunctionExpression ||
+            flowContainer.kind === SyntaxKind.ArrowFunction || isObjectLiteralOrClassExpressionMethodOrAccessor(flowContainer)) &&
+            (isConstVariable(symbol) && typeFromSymbol !== autoArrayType || isParameter && !isSymbolAssigned(symbol))) {
+            flowContainer = getControlFlowContainer(flowContainer);
+        }
+        return flowContainer;
+    }
+
     // Check if a parameter or catch variable is assigned anywhere
     function isSymbolAssigned(symbol: Symbol) {
         if (!symbol.valueDeclaration) {
@@ -26432,24 +26457,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         }
 
         type = getNarrowableTypeForReference(type, node, checkMode);
-
-        // The declaration container is the innermost function that encloses the declaration of the variable
-        // or parameter. The flow container is the innermost function starting with which we analyze the control
-        // flow graph to determine the control flow based type.
         const isParameter = getRootDeclaration(declaration).kind === SyntaxKind.Parameter;
         const declarationContainer = getControlFlowContainer(declaration);
-        let flowContainer = getControlFlowContainer(node);
-        const isOuterVariable = flowContainer !== declarationContainer;
-        const isSpreadDestructuringAssignmentTarget = node.parent && node.parent.parent && isSpreadAssignment(node.parent) && isDestructuringAssignmentTarget(node.parent.parent);
+        const flowContainer = getControlFlowContainerForIdentifier(node, declaration, localOrExportSymbol, type);
         const isModuleExports = symbol.flags & SymbolFlags.ModuleExports;
-        // When the control flow originates in a function expression or arrow function and we are referencing
-        // a const variable or parameter from an outer function, we extend the origin of the control flow
-        // analysis to include the immediately enclosing function.
-        while (flowContainer !== declarationContainer && (flowContainer.kind === SyntaxKind.FunctionExpression ||
-            flowContainer.kind === SyntaxKind.ArrowFunction || isObjectLiteralOrClassExpressionMethodOrAccessor(flowContainer)) &&
-            (isConstVariable(localOrExportSymbol) && type !== autoArrayType || isParameter && !isSymbolAssigned(localOrExportSymbol))) {
-            flowContainer = getControlFlowContainer(flowContainer);
-        }
+        const isOuterVariable = getControlFlowContainer(node) !== declarationContainer;
+        const isSpreadDestructuringAssignmentTarget = node.parent && node.parent.parent && isSpreadAssignment(node.parent) && isDestructuringAssignmentTarget(node.parent.parent);
         // We only look for uninitialized variables in strict null checking mode, and only when we can analyze
         // the entire control flow graph from the variable's declaration (i.e. when the flow container and
         // declaration container are the same).
@@ -32762,23 +32775,29 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
     }
 
     function checkAssertionWorker(errNode: Node, type: TypeNode, expression: UnaryExpression | Expression, checkMode?: CheckMode) {
-        let exprType = checkExpression(expression, checkMode);
+        const exprType = checkExpression(expression, checkMode);
         if (isConstTypeReference(type)) {
             if (!isValidConstAssertionArgument(expression)) {
                 error(expression, Diagnostics.A_const_assertions_can_only_be_applied_to_references_to_enum_members_or_string_number_boolean_array_or_object_literals);
             }
             return getRegularTypeOfLiteralType(exprType);
         }
         checkSourceElement(type);
-        exprType = getRegularTypeOfObjectLiteral(getBaseTypeOfLiteralType(exprType));
         const targetType = getTypeFromTypeNode(type);
         if (!isErrorType(targetType)) {
             addLazyDiagnostic(() => {
-                const widenedType = getWidenedType(exprType);
+                const regularType = getRegularTypeOfObjectLiteral(getBaseTypeOfLiteralType(exprType));
+                const widenedType = getWidenedType(regularType);
                 if (!isTypeComparableTo(targetType, widenedType)) {
-                    checkTypeComparableTo(exprType, targetType, errNode,
+                    checkTypeComparableTo(regularType, targetType, errNode,
                         Diagnostics.Conversion_of_type_0_to_type_1_may_be_a_mistake_because_neither_type_sufficiently_overlaps_with_the_other_If_this_was_intentional_convert_the_expression_to_unknown_first);
                 }
+                // Assertions, as a side effect, disable widening - some casts may rely on this, so
+                // if the expression type was widenable, we shouldn't assume the cast is extraneous.
+                // (Generally speaking, such casts are better served with `as const` casts nowadays though!)
+                else if (widenedType === exprType && isTypeIdenticalTo(targetType, widenedType)) {
+                    errorOrSuggestion(!!compilerOptions.noUnnecessaryCasts, errNode, Diagnostics.Type_cast_has_no_effect_on_the_type_of_this_expression);
+                }
             });
         }
         return targetType;
@@ -32791,8 +32810,37 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
     }
 
     function checkNonNullAssertion(node: NonNullExpression) {
-        return node.flags & NodeFlags.OptionalChain ? checkNonNullChain(node as NonNullChain) :
-            getNonNullableType(checkExpression(node.expression));
+        const exprType = checkExpression(node.expression);
+        const resultType = node.flags & NodeFlags.OptionalChain ? checkNonNullChain(node as NonNullChain) : getNonNullableType(exprType);
+        if (!isErrorType(resultType)) {
+            addLazyDiagnostic(() => {
+                if (isTypeIdenticalTo(exprType, resultType)) {
+                    // A non-null assertion on an identifier may also suppress a used-before definition, so may still be useful even if the type is unchanged by it
+                    // Identifiers on the left-hand-side of an assignment expression, ofc, can't do this, since they're for the assigned type itself.
+                    if (isIdentifier(node.expression) && !containsUndefinedType(exprType) && !(isAssignmentExpression(node.parent) && node.parent.left === node)) {
+                        const symbol = getSymbolAtLocation(node.expression);
+                        const declaration = symbol?.valueDeclaration;
+                        if (declaration) {
+                            const isParameter = getRootDeclaration(declaration).kind === SyntaxKind.Parameter;
+                            if (!isParameter) {
+                                const flowContainer = getControlFlowContainerForIdentifier(node.expression, declaration, symbol, exprType);
+                                // We need a fake reference that isn't parented to the nonnull expression, since the non-null will affect control flow's result
+                                // by suppressing the initial return type at the end of the flow
+                                const fakeReference = factory.cloneNode(node.expression);
+                                setParent(fakeReference, node.parent);
+                                fakeReference.flowNode = node.expression.flowNode;
+                                const flowType = getFlowTypeOfReference(fakeReference, exprType, undefinedType, flowContainer);
+                                if (containsUndefinedType(flowType)) {
+                                    return; // Cast is required to suppress a use before assignment control flow error
+                                }
+                            }
+                        }
+                    }
+                    errorOrSuggestion(!!compilerOptions.noUnnecessaryCasts, node, Diagnostics.Type_cast_has_no_effect_on_the_type_of_this_expression);
+                }
+            });
+        }
+        return resultType;
     }
 
     function checkExpressionWithTypeArguments(node: ExpressionWithTypeArguments | TypeQueryNode) {

src/compiler/commandLineParser.ts

@@ -800,6 +800,15 @@ const commandOptionsWithoutBuild: CommandLineOption[] = [
         description: Diagnostics.Raise_an_error_when_a_function_parameter_isn_t_read,
         defaultValueDescription: false,
     },
+    {
+        name: "noUnnecessaryCasts",
+        type: "boolean",
+        affectsSemanticDiagnostics: true,
+        affectsBuildInfo: true,
+        category: Diagnostics.Type_Checking,
+        description: Diagnostics.Raise_an_error_when_a_type_cast_does_not_affect_the_type_of_an_expression,
+        defaultValueDescription: false,
+    },
     {
         name: "exactOptionalPropertyTypes",
         type: "boolean",

src/compiler/diagnosticMessages.json

@@ -5919,6 +5919,10 @@
         "category": "Message",
         "code": 6803
     },
+    "Raise an error when a type cast does not affect the type of an expression.": {
+        "category": "Message",
+        "code": 6804
+    },
 
     "one of:": {
         "category": "Message",
@@ -6534,6 +6538,10 @@
         "category": "Suggestion",
         "code": 80008
     },
+    "Type cast has no effect on the type of this expression.": {
+        "category": "Error",
+        "code": 80009
+    },
 
     "Add missing 'super()' call": {
         "category": "Message",
@@ -7392,6 +7400,14 @@
         "category": "Message",
         "code": 95175
     },
+    "Remove unnecessary type cast.": {
+        "category": "Message",
+        "code": 95176
+    },
+    "Remove all unnecessary type casts.": {
+        "category": "Message",
+        "code": 95177
+    },
 
     "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
         "category": "Error",

src/compiler/types.ts

@@ -6713,6 +6713,7 @@ export interface CompilerOptions {
     noImplicitReturns?: boolean;
     noImplicitThis?: boolean;  // Always combine with strict property
     noStrictGenericChecks?: boolean;
+    noUnnecessaryCasts?: boolean;
     noUnusedLocals?: boolean;
     noUnusedParameters?: boolean;
     noImplicitUseStrict?: boolean;

src/services/_namespaces/ts.codefix.ts

@@ -66,6 +66,7 @@ export * from "../codefixes/wrapJsxInFragment";
 export * from "../codefixes/convertToMappedObjectType";
 export * from "../codefixes/removeAccidentalCallParentheses";
 export * from "../codefixes/removeUnnecessaryAwait";
+export * from "../codefixes/removeUnnecessaryCast";
 export * from "../codefixes/splitTypeOnlyImport";
 export * from "../codefixes/convertConstToLet";
 export * from "../codefixes/fixExpectedComma";

src/services/codefixes/removeUnnecessaryCast.ts

@@ -0,0 +1,44 @@
+import {
+    Diagnostics, findAncestor, isAssertionExpression, isInJSDoc, Node,
+    isParenthesizedExpression, ParenthesizedExpression, SourceFile, textChanges, TextSpan, tryCast, HasJSDoc, first, last, needsParentheses, isNonNullExpression, AssertionExpression, NonNullExpression,
+} from "../_namespaces/ts";
+import { codeFixAll, createCodeFixAction, findAncestorMatchingSpan, registerCodeFix } from "../_namespaces/ts.codefix";
+
+const fixId = "removeUnnecessaryCast";
+const errorCodes = [
+    Diagnostics.Type_cast_has_no_effect_on_the_type_of_this_expression.code,
+];
+
+registerCodeFix({
+    errorCodes,
+    getCodeActions: function getCodeActionsToRemoveUnnecessaryCast(context) {
+        const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, context.sourceFile, context.span));
+        if (changes.length > 0) {
+            return [createCodeFixAction(fixId, changes, Diagnostics.Remove_unnecessary_type_cast, fixId, Diagnostics.Remove_all_unnecessary_type_casts)];
+        }
+    },
+    fixIds: [fixId],
+    getAllCodeActions: context => {
+        return codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag));
+    },
+});
+
+function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, span: TextSpan) {
+    let node: Node | undefined = findAncestorMatchingSpan(sourceFile, span);
+    if (node && isInJSDoc(node)) {
+        node = findAncestor(node, isParenthesizedExpression);
+        if (node) {
+            changeTracker.deleteNodeRange(sourceFile, first((node as HasJSDoc).jsDoc!), last((node as HasJSDoc).jsDoc!));
+            if (!needsParentheses((node as ParenthesizedExpression).expression)) {
+                changeTracker.replaceNode(sourceFile, node, (node as ParenthesizedExpression).expression);
+            }
+        }
+        return;
+    }
+    const castExpr = tryCast(node, (n): n is AssertionExpression | NonNullExpression => isAssertionExpression(n) || isNonNullExpression(n));
+    if (!castExpr) {
+        return;
+    }
+
+    changeTracker.replaceNode(sourceFile, castExpr, castExpr.expression);
+}

tests/baselines/reference/api/tsserverlibrary.d.ts

@@ -7120,6 +7120,7 @@ declare namespace ts {
         noImplicitReturns?: boolean;
         noImplicitThis?: boolean;
         noStrictGenericChecks?: boolean;
+        noUnnecessaryCasts?: boolean;
         noUnusedLocals?: boolean;
         noUnusedParameters?: boolean;
         noImplicitUseStrict?: boolean;

tests/baselines/reference/api/typescript.d.ts

@@ -3182,6 +3182,7 @@ declare namespace ts {
         noImplicitReturns?: boolean;
         noImplicitThis?: boolean;
         noStrictGenericChecks?: boolean;
+        noUnnecessaryCasts?: boolean;
         noUnusedLocals?: boolean;
         noUnusedParameters?: boolean;
         noImplicitUseStrict?: boolean;

tests/baselines/reference/config/initTSConfig/Default initialized TSConfig/tsconfig.json

@@ -87,6 +87,7 @@
     // "alwaysStrict": true,                             /* Ensure 'use strict' is always emitted. */
     // "noUnusedLocals": true,                           /* Enable error reporting when local variables aren't read. */
     // "noUnusedParameters": true,                       /* Raise an error when a function parameter isn't read. */
+    // "noUnnecessaryCasts": true,                       /* Raise an error when a type cast does not affect the type of an expression. */
     // "exactOptionalPropertyTypes": true,               /* Interpret optional property types as written, rather than adding 'undefined'. */
     // "noImplicitReturns": true,                        /* Enable error reporting for codepaths that do not explicitly return in a function. */
     // "noFallthroughCasesInSwitch": true,               /* Enable error reporting for fallthrough cases in switch statements. */

tests/baselines/reference/config/initTSConfig/Initialized TSConfig with --help/tsconfig.json

@@ -87,6 +87,7 @@
     // "alwaysStrict": true,                             /* Ensure 'use strict' is always emitted. */
     // "noUnusedLocals": true,                           /* Enable error reporting when local variables aren't read. */
     // "noUnusedParameters": true,                       /* Raise an error when a function parameter isn't read. */
+    // "noUnnecessaryCasts": true,                       /* Raise an error when a type cast does not affect the type of an expression. */
     // "exactOptionalPropertyTypes": true,               /* Interpret optional property types as written, rather than adding 'undefined'. */
     // "noImplicitReturns": true,                        /* Enable error reporting for codepaths that do not explicitly return in a function. */
     // "noFallthroughCasesInSwitch": true,               /* Enable error reporting for fallthrough cases in switch statements. */

tests/baselines/reference/config/initTSConfig/Initialized TSConfig with --watch/tsconfig.json

@@ -87,6 +87,7 @@
     // "alwaysStrict": true,                             /* Ensure 'use strict' is always emitted. */
     // "noUnusedLocals": true,                           /* Enable error reporting when local variables aren't read. */
     // "noUnusedParameters": true,                       /* Raise an error when a function parameter isn't read. */
+    // "noUnnecessaryCasts": true,                       /* Raise an error when a type cast does not affect the type of an expression. */
     // "exactOptionalPropertyTypes": true,               /* Interpret optional property types as written, rather than adding 'undefined'. */
     // "noImplicitReturns": true,                        /* Enable error reporting for codepaths that do not explicitly return in a function. */
     // "noFallthroughCasesInSwitch": true,               /* Enable error reporting for fallthrough cases in switch statements. */

tests/baselines/reference/config/initTSConfig/Initialized TSConfig with advanced options/tsconfig.json

@@ -87,6 +87,7 @@
     // "alwaysStrict": true,                             /* Ensure 'use strict' is always emitted. */
     // "noUnusedLocals": true,                           /* Enable error reporting when local variables aren't read. */
     // "noUnusedParameters": true,                       /* Raise an error when a function parameter isn't read. */
+    // "noUnnecessaryCasts": true,                       /* Raise an error when a type cast does not affect the type of an expression. */
     // "exactOptionalPropertyTypes": true,               /* Interpret optional property types as written, rather than adding 'undefined'. */
     // "noImplicitReturns": true,                        /* Enable error reporting for codepaths that do not explicitly return in a function. */
     // "noFallthroughCasesInSwitch": true,               /* Enable error reporting for fallthrough cases in switch statements. */