Uninitialized variables work around strictNullChecks (follow-up to #13884)

[agopshi]

This is a follow-up to #13884, where @RyanCavanaugh asked me to open a new issue.

tl;dr: Now that there is strictPropertyInitialization, it seems that there should also be an analogous strictLocalInitialization.

TypeScript Version: 2.8

Search Terms: uninitialized local variables, strict local variables, strictLocalInitialization, strictPropertyInitialization, strictNullChecks

Code

let x: number[];  // No definite assignment assertion.
// x.toString();  // Compile-time error. Yay!
function foo() {
  x.toString();  // No compile-time error, but throws an exception. :(
}
foo();

Slightly more interesting:

function fooFactory() {
  let x: number;  // Inaccessible from anywhere but `foo()` below.
  return {
    foo() {
      // Guaranteed to throw an exception since `x` can't change.
      return x.toString();
    }
  };
}

Expected behavior:

If an imaginary --strictLocalInitialization flag 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. The fooFactory() example above is analogous to this Foo class, which correctly emits a compile-time error under strictPropertyInitialization:

class Foo {
  private x: number; // Compile-time error!
  foo() {
    // `x` isn't initialized! But it's OK, compile-time error above.
    return this.x.toString();
  }
}

[ajafff]

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-variable is a core rule of my Fimbullinter project.

[patrickhulce]

The addition of strictPropertyInitialization really makes this quite in line with the other strict* 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 here no-unnecessary-initializer.

[RyanCavanaugh]

Accepting PRs. The rule here is that any variable that is read but never written to is an error if the variable doesn't have undefined in 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 strictNullChecks compilations.

[agopshi]

That 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.

[RyanCavanaugh]

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);

[patrickhulce]

Really 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 strictLocalInitialization option as one step of type safety further? It's a simple rule (No uninitialized declarations that do not have undefined as an allowable type) and is a surefire way to ensure these issues are handled.

[agopshi]

@RyanCavanaugh Right - in your example, since it cannot be proven that assign() is called before use(), I would expect x to be widened to number|undefined in use(). (Or the declaration itself would have to be rewritten as number|undefined or 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.

[RyanCavanaugh]

You can log another suggestion for strictLocalInitialization (preferably after this one gets implemented) if you like

[agopshi]

Wait, how is @RyanCavanaugh's suggestion different from strictLocalInitialization? @patrickhulce

[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 with Type 'undefined' is not assignable to type 'Foo'. It's a much stricter and simpler assertion. Never allow a typedef of let myVar: T if the variable is not immediately initialized

[agopshi]

Ah. I would argue that there shouldn't be an error in your example. It can be proved that foo is, indeed, a valid Foo at time of use.

[patrickhulce]

@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 that foo is of type Foo yet it is undefined. I'd like a mode that prevents this contradiction from happening ever rather than silently allowing in the cases where flow analysis fails.

[RyanCavanaugh]

There's surely a TSLint rule that enforces initializations in lets

[patrickhulce]

@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.

[mgmolisani]

I'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 to undefined yet the compiler does not recognize it as such. Why?

[RyanCavanaugh]

I'm not following why this continuously was shot down.

[mgmolisani]

While 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 let is undefined per 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?

[jfirebaugh]

I would actually rather have the simpler --strictLocalInitialization option 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.

[MichaelAllenWarner]

I suppose the workaround for now is to use eslint to forbid uninitialized variables altogether (i.e., turn on init-declarations and turn off no-undef-init), though this requires cumbersome syntax like let foo: string | undefined = undefined.

I do like the idea of an optional TS flag that gives foo the string | undefined type if it's declared as let foo: string.

[Kagami]

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 forces let to be initialized to a proper type in typescript compiler.

So instead of let x: Foo; you would have to write let x: Foo | undefined; or let x: Foo = undefined as Foo;

[Zzzen]

Accepting PRs. The rule here is that any variable that is read but never written to is an error if the variable doesn't have undefined in its type.

It should be an error even if it has undefined, unknown or any in its type.