[5.0.2 regression] unused type guard result erroneously changing inferred type of variable

[seansfkelley]

Bug Report

🔎 Search Terms

control flow analysis type guard narrowing widening no-op

🕗 Version & Regression Information

• This changed between versions 4.9.5 and 5.0.2

⏯ Playground Link

Playground Link

💻 Code

type FooNode = {
    kind: 'foo';
    children: Node[];
};

type BarNode = {
    kind: 'bar';
}

type Node = FooNode | BarNode;

type Document = {
    kind: 'document';
    children: Node[];
};

declare function isNode(node: unknown): node is Node;
declare function isBar(node: Node): node is BarNode;

export function visitNodes<T extends Node>(
  node: Document | Node,
  predicate: (testNode: Node) => testNode is T,
): void {
  // Spooky action at a distance: this no-op statement causes the compiler to reject the assignment
  // guarded by the conditional, below. Comment it out to fix the failure.
  isNode(node) && predicate(node);

  if (!isNode(node) || !isBar(node)) {
    // This line should compile fine!
    const nodes: Node[] = node.children;
  }
}

The original code that turned this issue up was a bit more indirect and looked like:

const shouldYield = isNode(node) && predicate(node);
if (shouldYield) {
  yield node;
}

const hasChildren = !isNode(node) || !isBar(node);
if (hasChildren) {
  yield* yieldAll(node.children);
}

I was able to work around the issue by inlining the guards directly into the if rather than using an intermediate value.

🙁 Actual behavior

The assignment const nodes: Node[] = node.children; fails, complaining that

Property 'children' does not exist on type 'FooNode | Document | T'.
  Property 'children' does not exist on type 'T'.(2339)

but only if the unused type guard at the beginning of the function is present. Commenting the dead code out fixes the issue, and the compiler correctly narrows the type of node to just FooNode | Document.

🙂 Expected behavior

The unused type guard/dead code has no effect on the inferred type of the value.

[RyanCavanaugh]

A good workaround would be to write

export function visitNodes<T>(node: Document | Node, predicate: (testNode: Node) => testNode is (T & Node)): void {
  isNode(node) && predicate(node);

  if (!isNode(node) || !isBar(node)) {
    const nodes: Node[] = node.children; // ok
  }
}

(you may also keep the constraint if desired)

[ahejlsberg]

This one is interesting. The issue is actually a pre-existing defect in subtype reduction that happens to be revealed by the 5.0 improvements to narrowing with user defined type predicates.

In the code

export function visitNodes<T extends Node>(node: Document | Node, predicate: (n: Node) => n is T): void {
    node;  // Document | Node
    isNode(node) && predicate(node);
    node;  // Document | Node | T
}

the type of node isn't properly reduced back to Document | Node following the type predicate calls, even though T is a subtype of Document | Node. This is the case in both 4.9 and 5.0. The reason this reduction doesn't happen is that subtype reduction only removes union constituents that are subtypes of another individual constituent. This potentially misses type parameters with union constraints because they can be subtypes of some union, but not subtypes of the individual constituents of that union. For example, a type parameter T extends A | B is a is subtype of A | B, but not a subtype of just A or just B. In the issue here, T is a subtype of FooNode | BarNode, but not a subtype of just FooNode or just BarNode, and subtype reduction therefore doesn't remove T from Document | Node | T. We need to fix subtype reduction to handle such cases.

[ahejlsberg]

The following is a short example that demonstrates the subtype reduction defect:

type A = { a: string };
type B = { b: string };

function f1<T extends A | B>(t: T, ab: A | B) {
    const a = [t, ab];  // (A | B | T)[], but should be (A | B)[]
}

[fatcerberus]

What I’m curious about is how a statement like foo(x) && bar(x); affects the type of x in the first place. Neither one is an asserts call and there is no if so my expectation would be that no CFA node is created at all.

[ahejlsberg]

@fatcerberus The statement foo(x) && bar(x); is really no different than an if statement with an empty body from a CFA point of view. There may be no effects to x after the statement, but there are most certainly effects on x within the statement (i.e. foo(x) narrows the x observed in bar(x)). The control flow analyzer doesn't really reason in terms of statements, rather it looks at flows of control through the possible conditional branches of the code and tracks their effects on the types of variables. When branches join (e.g. following an if statement) we union the observed types and that's where subtype reduction sometimes comes in.

Now, it so happens that the issue reported here only shows up when the type predicate calls occur in an expression statement (and not an empty if statement), but that's for unrelated reasons having to do with the exact shape of the control flow graphs.