error on variables that are used but never initialized (#55887)

Co-authored-by: Daniel Rosenwasser <[email protected]>
microsoft · Aug 20, 2024 · 533ed3d · 533ed3d
1 parent 627fbcb
commit 533ed3d
Show file tree

Hide file tree

Showing 19 changed files with 1,970 additions and 23 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -29602,7 +29602,15 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             node.kind === SyntaxKind.PropertyDeclaration)!;
     }
 
-    // Check if a parameter or catch variable is assigned anywhere
+    // Check if a parameter, catch variable, or mutable local variable is assigned anywhere definitely
+    function isSymbolAssignedDefinitely(symbol: Symbol) {
+        if (symbol.lastAssignmentPos !== undefined) {
+            return symbol.lastAssignmentPos < 0;
+        }
+        return isSymbolAssigned(symbol) && symbol.lastAssignmentPos !== undefined && symbol.lastAssignmentPos < 0;
+    }
+
+    // Check if a parameter, catch variable, or mutable local variable is assigned anywhere
     function isSymbolAssigned(symbol: Symbol) {
         return !isPastLastAssignment(symbol, /*location*/ undefined);
     }
@@ -29621,7 +29629,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 markNodeAssignments(parent);
             }
         }
-        return !symbol.lastAssignmentPos || location && symbol.lastAssignmentPos < location.pos;
+        return !symbol.lastAssignmentPos || location && Math.abs(symbol.lastAssignmentPos) < location.pos;
     }
 
     // Check if a parameter or catch variable (or their bindings elements) is assigned anywhere
@@ -29655,12 +29663,19 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
     function markNodeAssignments(node: Node) {
         switch (node.kind) {
             case SyntaxKind.Identifier:
-                if (isAssignmentTarget(node)) {
+                const assigmentTarget = getAssignmentTargetKind(node);
+                if (assigmentTarget !== AssignmentKind.None) {
                     const symbol = getResolvedSymbol(node as Identifier);
-                    if (isParameterOrMutableLocalVariable(symbol) && symbol.lastAssignmentPos !== Number.MAX_VALUE) {
-                        const referencingFunction = findAncestor(node, isFunctionOrSourceFile);
-                        const declaringFunction = findAncestor(symbol.valueDeclaration, isFunctionOrSourceFile);
-                        symbol.lastAssignmentPos = referencingFunction === declaringFunction ? extendAssignmentPosition(node, symbol.valueDeclaration!) : Number.MAX_VALUE;
+                    const hasDefiniteAssignment = assigmentTarget === AssignmentKind.Definite || (symbol.lastAssignmentPos !== undefined && symbol.lastAssignmentPos < 0);
+                    if (isParameterOrMutableLocalVariable(symbol)) {
+                        if (symbol.lastAssignmentPos === undefined || Math.abs(symbol.lastAssignmentPos) !== Number.MAX_VALUE) {
+                            const referencingFunction = findAncestor(node, isFunctionOrSourceFile);
+                            const declaringFunction = findAncestor(symbol.valueDeclaration, isFunctionOrSourceFile);
+                            symbol.lastAssignmentPos = referencingFunction === declaringFunction ? extendAssignmentPosition(node, symbol.valueDeclaration!) : Number.MAX_VALUE;
+                        }
+                        if (hasDefiniteAssignment && symbol.lastAssignmentPos > 0) {
+                            symbol.lastAssignmentPos *= -1;
+                        }
                     }
                 }
                 return;
@@ -29670,7 +29685,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 if (!(node as ExportSpecifier).isTypeOnly && !exportDeclaration.isTypeOnly && !exportDeclaration.moduleSpecifier && name.kind !== SyntaxKind.StringLiteral) {
                     const symbol = resolveEntityName(name, SymbolFlags.Value, /*ignoreErrors*/ true, /*dontResolveAlias*/ true);
                     if (symbol && isParameterOrMutableLocalVariable(symbol)) {
-                        symbol.lastAssignmentPos = Number.MAX_VALUE;
+                        const sign = symbol.lastAssignmentPos !== undefined && symbol.lastAssignmentPos < 0 ? -1 : 1;
+                        symbol.lastAssignmentPos = sign * Number.MAX_VALUE;
                     }
                 }
                 return;
@@ -30414,6 +30430,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
 
         const localOrExportSymbol = getExportSymbolOfValueSymbolIfExported(symbol);
         let declaration = localOrExportSymbol.valueDeclaration;
+        const immediateDeclaration = declaration;
 
         // If the identifier is declared in a binding pattern for which we're currently computing the implied type and the
         // reference occurs with the same binding pattern, return the non-inferrable any type. This for example occurs in
@@ -30503,7 +30520,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         // 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).
-        const assumeInitialized = isParameter || isAlias || isOuterVariable || isSpreadDestructuringAssignmentTarget || isModuleExports || isSameScopedBindingElement(node, declaration) ||
+        const isNeverInitialized = immediateDeclaration && isVariableDeclaration(immediateDeclaration) && !immediateDeclaration.initializer && !immediateDeclaration.exclamationToken && isMutableLocalVariableDeclaration(immediateDeclaration) && !isSymbolAssignedDefinitely(symbol);
+        const assumeInitialized = isParameter || isAlias ||
+            (isOuterVariable && !isNeverInitialized) ||
+            isSpreadDestructuringAssignmentTarget || isModuleExports || isSameScopedBindingElement(node, declaration) ||
             type !== autoType && type !== autoArrayType && (!strictNullChecks || (type.flags & (TypeFlags.AnyOrUnknown | TypeFlags.Void)) !== 0 ||
                     isInTypeQuery(node) || isInAmbientOrTypeNode(node) || node.parent.kind === SyntaxKind.ExportSpecifier) ||
             node.parent.kind === SyntaxKind.NonNullExpression ||
@@ -43495,7 +43515,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 case SyntaxKind.MethodDeclaration:
                 case SyntaxKind.GetAccessor:
                 case SyntaxKind.SetAccessor:
-                    if (node.body) { // Don't report unused parameters in overloads
+                    // Only report unused parameters on the implementation, not overloads.
+                    if (node.body) {
                         checkUnusedLocalsAndParameters(node, addDiagnostic);
                     }
                     checkUnusedTypeParameters(node, addDiagnostic);

diff --git a/src/compiler/transformers/classFields.ts b/src/compiler/transformers/classFields.ts
@@ -228,6 +228,7 @@ import {
 } from "../_namespaces/ts.js";
 
 const enum ClassPropertySubstitutionFlags {
+    None = 0,
     /**
      * Enables substitutions for class expressions with static fields
      * which have initializers that reference the class name.
@@ -401,7 +402,7 @@ export function transformClassFields(context: TransformationContext): (x: Source
     context.onEmitNode = onEmitNode;
 
     let shouldTransformPrivateStaticElementsInFile = false;
-    let enabledSubstitutions: ClassPropertySubstitutionFlags;
+    let enabledSubstitutions = ClassPropertySubstitutionFlags.None;
 
     let classAliases: Identifier[];
 

diff --git a/src/compiler/transformers/es2015.ts b/src/compiler/transformers/es2015.ts
@@ -218,6 +218,7 @@ import {
 } from "../_namespaces/ts.js";
 
 const enum ES2015SubstitutionFlags {
+    None = 0,
     /** Enables substitutions for captured `this` */
     CapturedThis = 1 << 0,
     /** Enables substitutions for block-scoped bindings. */
@@ -523,7 +524,7 @@ export function transformES2015(context: TransformationContext): (x: SourceFile
      * They are persisted between each SourceFile transformation and should not
      * be reset.
      */
-    let enabledSubstitutions: ES2015SubstitutionFlags;
+    let enabledSubstitutions = ES2015SubstitutionFlags.None;
 
     return chainBundle(context, transformSourceFile);
 

diff --git a/src/compiler/transformers/es2017.ts b/src/compiler/transformers/es2017.ts
@@ -104,6 +104,7 @@ import {
 type SuperContainer = ClassDeclaration | MethodDeclaration | GetAccessorDeclaration | SetAccessorDeclaration | ConstructorDeclaration;
 
 const enum ES2017SubstitutionFlags {
+    None = 0,
     /** Enables substitutions for async methods with `super` calls. */
     AsyncMethodsWithSuper = 1 << 0,
 }
@@ -132,7 +133,7 @@ export function transformES2017(context: TransformationContext): (x: SourceFile
      * Keeps track of whether expression substitution has been enabled for specific edge cases.
      * They are persisted between each SourceFile transformation and should not be reset.
      */
-    let enabledSubstitutions: ES2017SubstitutionFlags;
+    let enabledSubstitutions = ES2017SubstitutionFlags.None;
 
     /**
      * This keeps track of containers where `super` is valid, for use with

diff --git a/src/compiler/transformers/es2018.ts b/src/compiler/transformers/es2018.ts
@@ -113,6 +113,7 @@ import {
 } from "../_namespaces/ts.js";
 
 const enum ESNextSubstitutionFlags {
+    None = 0,
     /** Enables substitutions for async methods with `super` calls. */
     AsyncMethodsWithSuper = 1 << 0,
 }
@@ -170,7 +171,7 @@ export function transformES2018(context: TransformationContext): (x: SourceFile
     context.onSubstituteNode = onSubstituteNode;
 
     let exportedVariableStatement = false;
-    let enabledSubstitutions: ESNextSubstitutionFlags;
+    let enabledSubstitutions = ESNextSubstitutionFlags.None;
     let enclosingFunctionFlags: FunctionFlags;
     let parametersWithPrecedingObjectRestOrSpread: Set<ParameterDeclaration> | undefined;
     let enclosingSuperContainerFlags: NodeCheckFlags = 0;

diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts
@@ -208,6 +208,7 @@ import {
 const USE_NEW_TYPE_METADATA_FORMAT = false;
 
 const enum TypeScriptSubstitutionFlags {
+    None = 0,
     /** Enables substitutions for namespace exports. */
     NamespaceExports = 1 << 1,
     /* Enables substitutions for unqualified enum members */
@@ -270,7 +271,7 @@ export function transformTypeScript(context: TransformationContext) {
      * Keeps track of whether expression substitution has been enabled for specific edge cases.
      * They are persisted between each SourceFile transformation and should not be reset.
      */
-    let enabledSubstitutions: TypeScriptSubstitutionFlags;
+    let enabledSubstitutions = TypeScriptSubstitutionFlags.None;
 
     /**
      * Keeps track of whether we are within any containing namespaces when performing

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -5970,7 +5970,7 @@ export interface Symbol {
     /** @internal */ exportSymbol?: Symbol; // Exported symbol associated with this symbol
     /** @internal */ constEnumOnlyModule: boolean | undefined; // True if module contains only const enums or other modules with only const enums
     /** @internal */ isReferenced?: SymbolFlags; // True if the symbol is referenced elsewhere. Keeps track of the meaning of a reference in case a symbol is both a type parameter and parameter.
-    /** @internal */ lastAssignmentPos?: number; // Source position of last node that assigns value to symbol
+    /** @internal */ lastAssignmentPos?: number; // Source position of last node that assigns value to symbol. Negative if it is assigned anywhere definitely
     /** @internal */ isReplaceableByMethod?: boolean; // Can this Javascript class property be replaced by a method symbol?
     /** @internal */ assignmentDeclarationMembers?: Map<number, Declaration>; // detected late-bound assignment declarations associated with the symbol
 }

diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -153,6 +153,8 @@ import {
     forEachChild,
     forEachChildRecursively,
     ForInOrOfStatement,
+    ForInStatement,
+    ForOfStatement,
     ForStatement,
     FunctionBody,
     FunctionDeclaration,
@@ -7906,6 +7908,9 @@ function accessKind(node: Node): AccessKind {
             return node === (parent as ShorthandPropertyAssignment).objectAssignmentInitializer ? AccessKind.Read : accessKind(parent.parent);
         case SyntaxKind.ArrayLiteralExpression:
             return accessKind(parent);
+        case SyntaxKind.ForInStatement:
+        case SyntaxKind.ForOfStatement:
+            return node === (parent as ForInStatement | ForOfStatement).initializer ? AccessKind.Write : AccessKind.Read;
         default:
             return AccessKind.Read;
     }

diff --git a/tests/baselines/reference/findAllRefsObjectBindingElementPropertyName06.baseline.jsonc b/tests/baselines/reference/findAllRefsObjectBindingElementPropertyName06.baseline.jsonc
@@ -13,7 +13,7 @@
 // for (<|var { [|property1|]: p1 } of elems|>) {
 // }
 // var p2;
-// for (<|{ [|{| isWriteAccess: true |}property1|] : p2 } of elems|>) {
+// for (<|{ [|property1|] : p2 } of elems|>) {
 // }
 
   // === Definitions ===
@@ -94,7 +94,7 @@
 // for (<|var { [|property1|]: p1 } of elems|>) {
 // }
 // var p2;
-// for (<|{ [|{| isWriteAccess: true |}property1|] : p2 } of elems|>) {
+// for (<|{ [|property1|] : p2 } of elems|>) {
 // }
 
   // === Definitions ===
@@ -180,7 +180,7 @@
 // for (<|var { /*FIND ALL REFS*/[|property1|]: p1 } of elems|>) {
 // }
 // var p2;
-// for (<|{ [|{| isWriteAccess: true |}property1|] : p2 } of elems|>) {
+// for (<|{ [|property1|] : p2 } of elems|>) {
 // }
 
   // === Definitions ===
@@ -270,7 +270,7 @@
 // for (<|var { [|property1|]: p1 } of elems|>) {
 // }
 // var p2;
-// for (<|{ /*FIND ALL REFS*/[|{| isWriteAccess: true, isDefinition: true |}property1|] : p2 } of elems|>) {
+// for (<|{ /*FIND ALL REFS*/[|{| isDefinition: true |}property1|] : p2 } of elems|>) {
 // }
 
   // === Definitions ===
@@ -358,7 +358,7 @@
 // for (<|var { [|{| defId: 0 |}property1|]: p1 } of elems|>) {
 // }
 // var p2;
-// for (<|{ [|{| defId: 0, isWriteAccess: true |}property1|] : p2 } of elems|>) {
+// for (<|{ [|{| defId: 0 |}property1|] : p2 } of elems|>) {
 // }
 
   // === Definitions ===

diff --git a/tests/baselines/reference/narrowingPastLastAssignment.errors.txt b/tests/baselines/reference/narrowingPastLastAssignment.errors.txt
@@ -1,8 +1,9 @@
 narrowingPastLastAssignment.ts(88,9): error TS7034: Variable 'x' implicitly has type 'any' in some locations where its type cannot be determined.
 narrowingPastLastAssignment.ts(90,20): error TS7005: Variable 'x' implicitly has an 'any' type.
+narrowingPastLastAssignment.ts(161,9): error TS18048: 'foo' is possibly 'undefined'.
 
 
-==== narrowingPastLastAssignment.ts (2 errors) ====
+==== narrowingPastLastAssignment.ts (3 errors) ====
     function action(f: Function) {}
 
     // Narrowings are preserved in closures created past last assignment
@@ -160,4 +161,15 @@ narrowingPastLastAssignment.ts(90,20): error TS7005: Variable 'x' implicitly has
         }
         values.forEach(v => foo.push(v));
     }
-
+
+    function f13() {
+        // Test for captured 'var' declaration (as opposed to parameters, let, const).
+        var foo: string | undefined;
+        foo = '';
+
+        return () => {
+            foo.toLocaleLowerCase();
+            ~~~
+!!! error TS18048: 'foo' is possibly 'undefined'.
+        }
+    }
diff --git a/tests/baselines/reference/narrowingPastLastAssignment.symbols b/tests/baselines/reference/narrowingPastLastAssignment.symbols
@@ -363,3 +363,20 @@ function f12() {
 >v : Symbol(v, Decl(narrowingPastLastAssignment.ts, 151, 19))
 }
 
+function f13() {
+>f13 : Symbol(f13, Decl(narrowingPastLastAssignment.ts, 152, 1))
+
+    // Test for captured 'var' declaration (as opposed to parameters, let, const).
+    var foo: string | undefined;
+>foo : Symbol(foo, Decl(narrowingPastLastAssignment.ts, 156, 7))
+
+    foo = '';
+>foo : Symbol(foo, Decl(narrowingPastLastAssignment.ts, 156, 7))
+
+    return () => {
+        foo.toLocaleLowerCase();
+>foo.toLocaleLowerCase : Symbol(String.toLocaleLowerCase, Decl(lib.es5.d.ts, --, --), Decl(lib.es2020.string.d.ts, --, --))
+>foo : Symbol(foo, Decl(narrowingPastLastAssignment.ts, 156, 7))
+>toLocaleLowerCase : Symbol(String.toLocaleLowerCase, Decl(lib.es5.d.ts, --, --), Decl(lib.es2020.string.d.ts, --, --))
+    }
+}
diff --git a/tests/baselines/reference/narrowingPastLastAssignment.types b/tests/baselines/reference/narrowingPastLastAssignment.types
@@ -731,3 +731,35 @@ function f12() {
 >  : ^^^^^^
 }
 
+function f13() {
+>f13 : () => () => void
+>    : ^^^^^^^^^^^^^^^^
+
+    // Test for captured 'var' declaration (as opposed to parameters, let, const).
+    var foo: string | undefined;
+>foo : string | undefined
+>    : ^^^^^^^^^^^^^^^^^^
+
+    foo = '';
+>foo = '' : ""
+>         : ^^
+>foo : string | undefined
+>    : ^^^^^^^^^^^^^^^^^^
+>'' : ""
+>   : ^^
+
+    return () => {
+>() => {        foo.toLocaleLowerCase();    } : () => void
+>                                             : ^^^^^^^^^^
+
+        foo.toLocaleLowerCase();
+>foo.toLocaleLowerCase() : string
+>                        : ^^^^^^
+>foo.toLocaleLowerCase : { (locales?: string | string[]): string; (locales?: Intl.LocalesArgument): string; }
+>                      : ^^^       ^^^                 ^^^      ^^^       ^^^                    ^^^      ^^^
+>foo : string | undefined
+>    : ^^^^^^^^^^^^^^^^^^
+>toLocaleLowerCase : { (locales?: string | string[]): string; (locales?: Intl.LocalesArgument): string; }
+>                  : ^^^       ^^^                 ^^^      ^^^       ^^^                    ^^^      ^^^
+    }
+}