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