Repository navigation
Uninitialized variables work around strictNullChecks (follow-up to #13884) #23305
Description
Activity
For the time being you could use a lint rule.
I have a written a rule to detect variables that are never assigned, which would cover the simple examples you provided above: The rule
no-unassigned-variableis a core rule of my Fimbullinter project.The addition of
strictPropertyInitializationreally makes this quite in line with the otherstrict*flags and is sorely needed. There aren't other well-established, automated ways of catching this; ironically, the only lint rule that TSLint offers is the exact opposite of what we want hereno-unnecessary-initializer.Reacted by Martin VečeřaAccepting PRs. The rule here is that any variable that is read but never written to is an error if the variable doesn't have
undefinedin its type.Implementation note - this needs to be done not as a separate pass. Re-use the existing logic that finds unused locals and refactor it to provide read/write information instead of just read.
No new flag since this should only ever catch true errors -- this will be the default behavior for all
strictNullCheckscompilations.Reacted by Peter Flynn, Lev Izraelit and ZzzenReacted by Adrián Montesinos GonzálezReacted by Patrick Hulce, JasonS, Josh Ghoulberg 👻 and Osvi RamosReacted by Patrick Hulce, JasonS and Osvi RamosReacted by Martin VečeřaThat sounds like a fair approach, but it seems that it would only get us half way there:
let x: number; function use() { x.toString(); // Usage, therefore needs an assignment at some point. } function assign() { x = 5; // Assignment, so all is well. } use(); // Oops, this is out of order, but it's too late now. assign();
Maybe this is impossible to enforce without complete flow analysis.
Inlining the effects and requirements of every function call is out of scope in terms of our current flow control analysis architecture.
Nothing we can do will get us 100%, e.g.
// Does what it says declare function callInRandomOrder(fn1: () => void, fn2: () => void); let x: number; function use() { x.toString(); // Usage, therefore needs an assignment at some point. } function assign() { x = 5; // Assignment, so all is well. } callInRandomOrder(use, assign);
Reacted by Martin VečeřaReally excited about the progress here and openness from the team about addressing this thank you! ❤️
As we've established, the usage before initialization is an undecidable problem and doing any sort of flow analysis will have its limits. Is there opposition from the typescript team to also adding the proposed
strictLocalInitializationoption as one step of type safety further? It's a simple rule (No uninitialized declarations that do not haveundefinedas an allowable type) and is a surefire way to ensure these issues are handled.Reacted by Martin Večeřa and Adrián Montesinos GonzálezRyan Cavanaugh (@RyanCavanaugh) Right - in your example, since it cannot be proven that
assign()is called beforeuse(), I would expectxto be widened tonumber|undefinedinuse(). (Or the declaration itself would have to be rewritten asnumber|undefinedor with the definite assignment assertion).Totally understood that this is impossible with the current control flow analysis architecture. Just discussing ideals. In the meantime, what you proposed would certainly be a welcome addition.
Reacted by Peter FlynnYou can log another suggestion for
strictLocalInitialization(preferably after this one gets implemented) if you likeWait, how is Ryan Cavanaugh (@RyanCavanaugh)'s suggestion different from
strictLocalInitialization? Patrick Hulce (@patrickhulce)If I understood his suggestion and call for PRs correctly, the following code would compile fine
let foo: Foo foo = new Foo() foo.bar()
But with
strictLocalInitialization, it should fail on line 1 withType 'undefined' is not assignable to type 'Foo'. It's a much stricter and simpler assertion. Never allow a typedef oflet myVar: Tif the variable is not immediately initializedReacted by Martin VečeřaAh. I would argue that there shouldn't be an error in your example. It can be proved that
foois, indeed, a validFooat time of use.Agop Shirinian (@agopshi) sure in my example, I don't really care about the error; it was a trivial example.
It's just that previous attempts have been shot down because the generic problem is undecidable, and I recognize that fact. It'd be nice to have a surefire way to express that
let foo: Foo;is indeed a lie. At the moment that statement compiles just fine, I've declared thatfoois of typeFooyet it isundefined. I'd like a mode that prevents this contradiction from happening ever rather than silently allowing in the cases where flow analysis fails.Reacted by Josh Berry, MichaelAllenWarner and Adrián Montesinos GonzálezThere's surely a TSLint rule that enforces initializations in
letsRyan Cavanaugh (@RyanCavanaugh) the TSLint rule is actually the opposite :) it seems I will lose this debate yet again and perhaps I'd be better off adding a PR for a rule option to tslint instead, but I still hold that this seems like a fundamental responsibility of a type system.
Reacted by Alexandre Galays and Adrián Montesinos GonzálezI'm not following why this continuously was shot down. It seems clear that this should be handled by a compiler, not by a lint rule. The language sets uninitialized
lets toundefinedyet the compiler does not recognize it as such. Why?Reacted by Patrick Hulce, Alexandre Galays, Géry Ogam and Adrián Montesinos GonzálezI'm not following why this continuously was shot down.
Reacted by Zzzen and EbertReacted by Joseph LennoxWhile I appreciate the MS Paint work, I find it tough to have followed 3 different conversation on this topic and even just within this thread seen it flip to suggesting this is better as a lint rule. If there is not fundamental agreement that an uninitialized
letisundefinedper the language, any work to enforce that via the compiler would be wasted. The path forward needs to be clear for this to be worked on no?Reacted by Joseph Lennox and Adrián Montesinos GonzálezI would actually rather have the simpler
--strictLocalInitializationoption that flags any uninitialized declaration which does not have undefined as an allowable type, than a control-flow based feature that allows a subset of such constructs where assignment before use can be proven. And even more so if the control-flow based feature is going to permit some constructs where there is an assignment but it can't be proven to occur before use -- I think the majority of bugs people hope to catch with strictness like this would fall in this category.Reacted by Patrick Hulce, Várkonyi Zoltán, Robbie, kale riley, Joseph Lennox, Martin Večeřa, Adrián Montesinos González and Géry OgamI suppose the workaround for now is to use eslint to forbid uninitialized variables altogether (i.e., turn on
init-declarationsand turn offno-undef-init), though this requires cumbersome syntax likelet foo: string | undefined = undefined.I do like the idea of an optional TS flag that gives
foothestring | undefinedtype if it's declared aslet foo: string.It seems a bit weird to ask users to use separate program (tslint) to avoid shortcoming in compiler.
I believe there should be an option which forcesletto be initialized to a proper type in typescript compiler.So instead of
let x: Foo;you would have to writelet x: Foo | undefined;orlet x: Foo = undefined as Foo;Reacted by Carl Hamilton, Nittai-Cohen-TAU and Adrián Montesinos GonzálezAccepting PRs. The rule here is that any variable that is read but never written to is an error if the variable doesn't have
undefinedin its type.It should be an error even if it has
undefined,unknownoranyin its type.
This is a follow-up to #13884, where Ryan Cavanaugh (@RyanCavanaugh) asked me to open a new issue.
tl;dr: Now that there is
strictPropertyInitialization, it seems that there should also be an analogousstrictLocalInitialization.TypeScript Version: 2.8
Search Terms: uninitialized local variables, strict local variables, strictLocalInitialization, strictPropertyInitialization, strictNullChecks
Code
Slightly more interesting:
Expected behavior:
If an imaginary
--strictLocalInitializationflag is enabled, these snippets should each result in a compile-time error.To resolve such an error, one would need to:
A) Initialize the variable in a way that the TypeScript compiler understands (e.g. immediately, or before referencing it in a closure).
-OR-
B) Use a definite assignment assertion. It seems that this issue would come up naturally when discussing definite assignment assertions for local variables. Instead, the example provided in the blog feels contrived and wouldn't be a problem if it weren't for control flow analysis limitations.
-OR-
C) Explicitly mark the variable as potentially
undefined, forcing callers to acknowledge that it may not be set.Actual behavior:
No compile-time errors, but obvious run-time errors.
Playground Link:
Link (enable
strictNullChecks)Related Issues:
#13884, #9998
Understandably, this may not be as simple as
strictPropertyInitialization, but I feel it needs a discussion none the less. ThefooFactory()example above is analogous to thisFooclass, which correctly emits a compile-time error understrictPropertyInitialization: