Open kirkwaiblinger opened 1 month ago
This is already implemented (unflagged) in nightly. Playground
Oh! I should have checked that 😆
Playing around with it, it looks like it only reports a variable that's never assigned, is that right? I'd love to see something like this flagged too (playground):
export { }
function printFoo() {
console.log(foo.toLowerCase()); // would be nice if this were an error too
}
let foo: string;
if (Math.random() > 0.5) {
foo = 'bar'
}
printFoo(); // Uncaught TypeError: Cannot read properties of undefined (reading 'toLowerCase')
foo.toLowerCase(); // TS Error; Variable 'foo' is used before being assigned.
But I guess I'm probably too late to the party to bring that up. Do you know the issue/PR offhand where this was discussed?
Ok, I'm finding https://github.com/microsoft/TypeScript/pull/55887, https://github.com/microsoft/TypeScript/issues/23305.
I guess I still think there is value in having TS check that every reachable codepath is assigned, not just at least one (just like you would require in order to actually use the variable directly, rather than in a closure), and I'm not seeing that discussed explicitly in the PR or issues.
Is that a change you'd be open to? Or is this too late to revisit?
"every" implies a level of analysis that's not really possible. Like if you had
import { doSomething } from "./someother.js";
let foo: string;
doSomething();
foo = "";
export function readFoo(): string {
return foo;
}
if it turned out that someother
called readFoo
, you'd have a problem here, even though "all codepaths" initialize foo
. The rule you really need is to always initialize let
s, which can be accomplished with a lint rule.
It's not super high priority to move where in the gray zone the detected/not-detected line is.
if it turned out that
someother
calledreadFoo
, you'd have a problem here, even though "all codepaths" initializefoo
.
Completely agree and I agree this is out of scope to solve. Note that this exists with strictPropertyInitialization
too
class Foo {
bar: number
constructor() {
this.printFoo();
this.bar = Math.random();
}
printFoo(): void {
console.log(this.bar);
}
}
The rule you really need is to always initialize
let
s, which can be accomplished with a lint rule.
Well... temporarily leaving non-nullable let
s uninitialized is valuable, though. Like
declare const switchedVar: "a" | "b";
let foo: string;
switch (switchedVar) {
case 'a':
foo = 'falafel';
break;
case 'b':
foo = 'gyro';
break;
// etc
}
console.log(foo.toString());
or doing things where you want to trigger "evolving any
" behavior.
it's a bummer to lose some of those features 🤷♂️ (see also https://github.com/typescript-eslint/typescript-eslint/issues/9565#issuecomment-2228791848).
It's not super high priority to move where in the gray zone the detected/not-detected line is.
completely fair! And no hard feelings if you decide to close this issue!
My main point is just - it would help me as a user understand the feature better if it follows basically identical standards as set by strictPropertyInitialization
.
So, if SPI errors here
class Foo {
bar: number; // Property 'bar' has no initializer and is not definitely assigned in the constructor.
constructor() {
if (Math.random() > 0.5) {
this.bar = Math.random();
}
}
}
so should the new checks on let
s
export {}
let foo: number;
if (Math.random() > 0.5) {
foo = Math.random();
}
function printFoo() {
console.log(foo); // (suggested) Variable 'foo' is used before being assigned.
}
printFoo();
I'd agree that the "definitely not assigned" standard is the only definitely unambiguous check here. I just wish that we could set the detected/undetected line in the grey zone to the exact same position as SPI for the sake of user comprehension.
🔍 Search Terms
uninitialized variable, undefined, closure, strict property initialization,
✅ Viability Checklist
⭐ Suggestion
I would like to see a check that ensures that no variable whose type is not permitted to be
undefined
may remain uninitialized at the end of its scope.📃 Motivating Example
TypeScript allows unsafety that can and does catch users by surprise when using variables in a closure. Normally, TS won't let you access an uninitialized variable:
However, TypeScript optimistically assumes that variables are initialized when used in closures.
That's for good reason, but sometimes, as in the above case, this is provably unsafe, since
foo
is guaranteed not to be initialized.The new flag "strictVariableInitialization" ensures that a variable must be initialized by the end of all reachable codepaths in its scope.
Of course, variables whose type includes
undefined
are still permitted to be uninitialized.This check is highly analogous to strictPropertyInitialization for classes.
💻 Use Cases
See https://github.com/typescript-eslint/typescript-eslint/issues/9565 for a somewhat-wordier proposal in typescript-eslint, and https://github.com/typescript-eslint/typescript-eslint/issues/4513 and https://github.com/typescript-eslint/typescript-eslint/issues/10055#issuecomment-2374797860 for cases where this has caused confusion in the wild.
In short, people who have written code that does check for initialization of non-nullable variables become confused by the linter informing them that the check is unnecessary according to the types, even though they can see that it is necessary at runtime:
The code should be rewritten as