Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error on variables that are used but never initialized #55887

Merged
merged 28 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ecf36a7
error on variables that are used but never uninitialized
Zzzen Sep 27, 2023
8416f98
format
Zzzen Sep 27, 2023
54183fd
Merge remote-tracking branch 'origin/main' into check-uninitialized-v…
Zzzen Sep 27, 2023
ff025b3
fix tests
Zzzen Sep 27, 2023
7bb240f
fix ForInOrOfStatement
Zzzen Sep 28, 2023
8d814cd
reuse `isAssigned` flag
Zzzen Sep 28, 2023
e897198
Merge branch 'main' into check-uninitialized-variables
DanielRosenwasser Apr 5, 2024
5fbd69d
Merge remote-tracking branch 'origin/main' into check-uninitialized-v…
Zzzen Jul 17, 2024
e1262a6
add tests
Zzzen Jul 18, 2024
2942ade
fix compoud assignment
Zzzen Jul 19, 2024
23be105
update error message
Zzzen Jul 19, 2024
8e4e266
Merge remote-tracking branch 'origin/main' into check-uninitialized-v…
Zzzen Jul 20, 2024
d32f696
update baseline
Zzzen Jul 20, 2024
b3edbf0
ignore var declarations
Zzzen Jul 21, 2024
775670c
fix self check
Zzzen Jul 21, 2024
158d0f3
cache isDefinitelyAssigned
Zzzen Jul 23, 2024
6bfa970
Merge remote-tracking branch 'origin/main' into check-uninitialized-v…
Zzzen Jul 23, 2024
66f2210
update error code
Zzzen Jul 23, 2024
44ce876
format
Zzzen Jul 23, 2024
f9a8207
update comments and ignore constant variables
Zzzen Jul 26, 2024
46b735c
Merge remote-tracking branch 'origin/main' into check-uninitialized-v…
Zzzen Jul 27, 2024
1aabd49
reset impl
Zzzen Jul 27, 2024
45c0011
refactor
Zzzen Jul 28, 2024
f0b637f
narrow function scoped variables
Zzzen Jul 28, 2024
bf4bb51
inline variables
Zzzen Aug 4, 2024
10e2cf8
ignore var variables
Zzzen Aug 8, 2024
6c85362
update test
Zzzen Aug 9, 2024
b930faa
update code
Zzzen Aug 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 33 additions & 13 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29504,7 +29504,15 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
node.kind === SyntaxKind.PropertyDeclaration)!;
}

gabritto marked this conversation as resolved.
Show resolved Hide resolved
// Check if a parameter or catch variable is assigned anywhere
// Check if a parameter, catch variable, or mutable local variable is assigned anywhere definitely
gabritto marked this conversation as resolved.
Show resolved Hide resolved
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);
}
Expand All @@ -29523,7 +29531,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
Expand Down Expand Up @@ -29557,12 +29565,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 isDefiniteAssignment = 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;
gabritto marked this conversation as resolved.
Show resolved Hide resolved
}
if (isDefiniteAssignment && symbol.lastAssignmentPos > 0) {
symbol.lastAssignmentPos = -symbol.lastAssignmentPos;
gabritto marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
return;
Expand All @@ -29572,7 +29587,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;
Expand Down Expand Up @@ -29622,13 +29638,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const declaration = symbol.valueDeclaration && getRootDeclaration(symbol.valueDeclaration);
return !!declaration && (
isParameter(declaration) ||
isVariableDeclaration(declaration) && (isCatchClause(declaration.parent) || isMutableLocalVariableDeclaration(declaration))
isVariableDeclaration(declaration) && (isCatchClause(declaration.parent) || isMutableLocalVariableDeclaration(symbol, declaration))
);
}

function isMutableLocalVariableDeclaration(declaration: VariableDeclaration) {
function isMutableLocalVariableDeclaration(symbol: Symbol, declaration: VariableDeclaration) {
// Return true if symbol is a non-exported and non-global `let` variable
return !!(declaration.parent.flags & NodeFlags.Let) && !(
return !!(symbol.flags & SymbolFlags.FunctionScopedVariable || (declaration.parent.flags & NodeFlags.Let)) && !(
gabritto marked this conversation as resolved.
Show resolved Hide resolved
getCombinedModifierFlags(declaration) & ModifierFlags.Export ||
declaration.parent.parent.kind === SyntaxKind.VariableStatement && isGlobalSourceFile(declaration.parent.parent.parent)
);
Expand Down Expand Up @@ -30316,6 +30332,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
Expand Down Expand Up @@ -30405,7 +30422,9 @@ 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 assumeInitialized = isParameter || isAlias ||
(isOuterVariable && !((immediateDeclaration && isVariableDeclaration(immediateDeclaration) && !immediateDeclaration.initializer && !immediateDeclaration.exclamationToken && isMutableLocalVariableDeclaration(symbol, immediateDeclaration)) && !isSymbolAssignedDefinitely(symbol))) ||
gabritto marked this conversation as resolved.
Show resolved Hide resolved
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 ||
Expand Down Expand Up @@ -43264,7 +43283,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);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/classFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ export function transformClassFields(context: TransformationContext): (x: Source
context.onEmitNode = onEmitNode;

let shouldTransformPrivateStaticElementsInFile = false;
let enabledSubstitutions: ClassPropertySubstitutionFlags;
let enabledSubstitutions!: ClassPropertySubstitutionFlags;

let classAliases: Identifier[];

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/es2015.ts
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,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;
gabritto marked this conversation as resolved.
Show resolved Hide resolved

return chainBundle(context, transformSourceFile);

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/es2017.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,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;

/**
* This keeps track of containers where `super` is valid, for use with
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/es2018.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export function transformES2018(context: TransformationContext): (x: SourceFile
context.onSubstituteNode = onSubstituteNode;

let exportedVariableStatement = false;
let enabledSubstitutions: ESNextSubstitutionFlags;
let enabledSubstitutions!: ESNextSubstitutionFlags;
let enclosingFunctionFlags: FunctionFlags;
let parametersWithPrecedingObjectRestOrSpread: Set<ParameterDeclaration> | undefined;
let enclosingSuperContainerFlags: NodeCheckFlags = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,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;

/**
* Keeps track of whether we are within any containing namespaces when performing
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5944,7 +5944,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
}
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ import {
forEachChild,
forEachChildRecursively,
ForInOrOfStatement,
ForInStatement,
ForOfStatement,
ForStatement,
FunctionBody,
FunctionDeclaration,
Expand Down Expand Up @@ -7905,6 +7907,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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// for (<|var { [|property1|]: p1 } of elems|>) {
// }
// var p2;
// for (<|{ [|{| isWriteAccess: true |}property1|] : p2 } of elems|>) {
// for (<|{ [|property1|] : p2 } of elems|>) {
Zzzen marked this conversation as resolved.
Show resolved Hide resolved
// }

// === Definitions ===
Expand Down Expand Up @@ -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 ===
Expand Down Expand Up @@ -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 ===
Expand Down Expand Up @@ -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 ===
Expand Down Expand Up @@ -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 ===
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,12 @@ narrowingPastLastAssignment.ts(90,20): error TS7005: Variable 'x' implicitly has
}
values.forEach(v => foo.push(v));
}


function f13() {
var foo: string | undefined;
gabritto marked this conversation as resolved.
Show resolved Hide resolved
foo = '';

return () => {
foo.toLocaleLowerCase();
}
}
16 changes: 16 additions & 0 deletions tests/baselines/reference/narrowingPastLastAssignment.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -363,3 +363,19 @@ function f12() {
>v : Symbol(v, Decl(narrowingPastLastAssignment.ts, 151, 19))
}

function f13() {
>f13 : Symbol(f13, Decl(narrowingPastLastAssignment.ts, 152, 1))

var foo: string | undefined;
>foo : Symbol(foo, Decl(narrowingPastLastAssignment.ts, 155, 7))

foo = '';
>foo : Symbol(foo, Decl(narrowingPastLastAssignment.ts, 155, 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, 155, 7))
>toLocaleLowerCase : Symbol(String.toLocaleLowerCase, Decl(lib.es5.d.ts, --, --), Decl(lib.es2020.string.d.ts, --, --))
}
}
31 changes: 31 additions & 0 deletions tests/baselines/reference/narrowingPastLastAssignment.types
Original file line number Diff line number Diff line change
Expand Up @@ -731,3 +731,34 @@ function f12() {
> : ^^^^^^
}

function f13() {
>f13 : () => () => void
> : ^^^^^^^^^^^^^^^^

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
> : ^^^^^^
>toLocaleLowerCase : { (locales?: string | string[]): string; (locales?: Intl.LocalesArgument): string; }
> : ^^^ ^^^ ^^^ ^^^ ^^^ ^^^ ^^^
}
}
Loading