Skip to content

Control Flow Analysis for Destructured Discriminated Unions doesn't work for let variables #48071

Open
Open
Control Flow Analysis for Destructured Discriminated Unions doesn't work for let variables#48071
Labels
Needs InvestigationThis issue needs a team member to investigate its status.
Milestone
Backlog

Description

@kasperpeulen
Kasper Peulen (kasperpeulen)
opened on Mar 1, 2022

Bug Report

🔎 Search Terms

Control flow analysis destructuring assignment

🕗 Version & Regression Information

This was already there, but I thought it might have been fixed with the introduction of this feature in 4.6.

⏯ Playground Link

Playground link with relevant code

💻 Code

export const reduce = <S, T extends S>(self: Iterable<T>, operation: (acc: S, t: T) => S): S => {
  const iter = self[Symbol.iterator]();
  let { value, done }: IteratorResult<T, unknown> = iter.next();
  if (done) throw new Error("Empty iterable can't be reduced.");
  let acc: S = value; // Type 'unknown' is not assignable to type 'S'.
  while (!done) {
    acc = operation(acc, value);
    ({ value, done } = iter.next());
  }
  return acc;
};

Note that if you don't upcast iter.next(), you won't get errors, but not for good reasons:

let {value, done} = iter.next(); // type is IteratorResult<T, any>
if (done) throw new Error("Empty iterable can't be reduced.");
// value is any and can be assigned to anything

A smaller repo would be:

export function foo(iter: Iterator<number, string>) {
  let { value, done } = iter.next();
  if (done) return;
  let acc: number = value; // Type 'string | number' is not assignable to type 'number'. Type 'string' is not assignable to type 'number'.
  return acc;
}

🙁 Actual behavior

value above in the last example is inferred as number | string, it should be number

🙂 Expected behavior

I expect the same inference as without destructuring:

export const reduce2 = <S, T extends S>(self: Iterable<T>, operation: (acc: S, t: T) => S): S => {
  const iter = self[Symbol.iterator]();
  let result: IteratorResult<T, unknown> = iter.next();
  if (result.done) throw new Error("Empty iterable can't be reduced.");
  let acc: S = result.value; // result.value is T (extends S)
  while (!result.done) {
    acc = operation(acc, result.value);
    result = iter.next();
  }
  return acc;
};

Activity

  1. MartinJohns commented on Mar 2, 2022

    @MartinJohns
    Martin Johns (MartinJohns)
    on Mar 2, 2022
    Contributor
    1. You forgot to fill out the issue template for bug reports.
    2. CFA for destructured types requires the type to be inferred, but you're explicitly setting the type. Remove the : IteratorResult<T, unknown> part from your variable declaration and it works.

  2. kasperpeulen commented on Mar 3, 2022

    @kasperpeulen
    Kasper Peulen (kasperpeulen)
    on Mar 3, 2022
    Author

    Martin Johns (@MartinJohns) Okay, I'm sorry, I will fill it out.

    But, the bug is not fixed by leaving out IteratorResult<T, unknown>, see my last comment, it just won't give an error, because it is inferred as any, instead of unknown (second generic parameter is any by default).

    You can also try:

    function foo(iter: Iterator<number, unknown>) {
  let { value, done } = iter.next();
  if (done) throw new Error("Empty iterable can't be reduced.");
  let acc = value; // unknown, not number
}
  3. Ryan Cavanaugh (RyanCavanaugh)
    added
    Needs InvestigationThis issue needs a team member to investigate its status.
    on Mar 3, 2022
  4. Ryan Cavanaugh (RyanCavanaugh)
    added this to the Backlog milestone on Mar 3, 2022
  5. Kasper Peulen (kasperpeulen)
    changed the title [-]Control Flow Analysis for Destructured Discriminated Unions doesn't work for IteratorResult[/-] [+]Control Flow Analysis for Destructured Discriminated Unions doesn't work for let variables[/+] on Apr 22, 2022

  6. kasperpeulen commented on Apr 22, 2022

    @kasperpeulen
    Kasper Peulen (kasperpeulen)
    on Apr 22, 2022
    Author

    I have another example:

    export const last = <T>(self: Iterable<T>): T => {
  let { found, last }: { found: false; last?: undefined } | { found: true; last: T } = {
    found: false,
  };
  for (const it of self) {
    ({ found, last } = { found: false, last: it });
  }
  if (!found) throw new NoSuchElementException("Iterable contains no element matching the predicate.");
  // can not infer last to be T
  return last;
};

    Ryan Cavanaugh (@RyanCavanaugh) Martin Johns (@MartinJohns)

  7. whzx5byb commented on Aug 6, 2022

    @whzx5byb
    whzx5byb
    on Aug 6, 2022

    A shorter const vs let. Example taken from #46266.

    type Action =
    | { kind: 'A', payload: number }
    | { kind: 'B', payload: string };

function f11(action: Action) {
    const { kind, payload } = action; // works
    if (kind === 'A') {
        payload.toFixed();
    }
    if (kind === 'B') {
        payload.toUpperCase();
    }
}

function _f11(action: Action) {
    let { kind, payload } = action; // doesn't work
    if (kind === 'A') {
        payload.toFixed();
    }
    if (kind === 'B') {
        payload.toUpperCase();
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Needs InvestigationThis issue needs a team member to investigate its status.

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions