Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Control flow analysis broken for never returning class methods #56812

Closed
gordonmleigh opened this issue Dec 17, 2023 · 3 comments
Closed

Control flow analysis broken for never returning class methods #56812

gordonmleigh opened this issue Dec 17, 2023 · 3 comments

Comments

@gordonmleigh
Copy link

πŸ”Ž Search Terms

"control flow analysis", "never return"

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about never

⏯ Playground Link

https://www.typescriptlang.org/play?#code/MYGwhgzhAEDC5WgbwLAChrQC4AsBOA9gO4CyAprgQCYAUAlAFzQB2ZAbmXsupproUWgAiAEZhmAcyEBuHtAC+6RWnQAzAK7NgWAJYFm2fMQBim7Xub0mrDl1QZDA4WMkyl6NWd37oeMgAdCAEYaMDwJJggsPB1JaAAfaE0qMlVYsio6bgcdVWgaAEIwiSz7XhYyQXhICHoAOn5ickpaOlkHZUw-LHU8A2K6kDJJXHblTy1vAz9AggAmUPDI6NiJBKTmFLTWTOzMXPyi8NK5PiMiU0mLenbMTt8KXv7wweGJUfc0IA

πŸ’» Code

class Class {
  throwMethod(): never {
    throw "bang";
  }
}

function throwFunction(): never {
  throw "bang";
}

function repro1(arg: string | undefined) {
  if (!arg) {
    new Class().throwMethod();
  }
  return arg.length;
}

function repro2(arg: string | undefined) {
  if (!arg) {
    throwFunction();
  }
  return arg.length;
}

πŸ™ Actual behavior

When a class method has a never return type (see repro1 function), the control flow analysis ignores this.

When a function method has a never return type (see repro2 function), the control flow analysis correctly identifies that this execution branch will end.

πŸ™‚ Expected behavior

I expected that the function and class method would be treated in the same way by the CFA, because I can't find an obvious reason why they should differ.

Additional information about the issue

No response

@MartinJohns
Copy link
Contributor

MartinJohns commented Dec 17, 2023

This is working as intended. For performance reasons only specific calls are analyzed. Check #32695 for details.

If you change your code according to the requirements it works:

 const c: Class = new Class();
 if (!arg) {
   c.throwMethod();
 }

@jcalz
Copy link
Contributor

jcalz commented Dec 17, 2023

Also duplicates #46254

@gordonmleigh
Copy link
Author

gordonmleigh commented Dec 17, 2023

@MartinJohns thanks for the quick reply.

Relevant quote from #32695 is:

A function call is analyzed as an assertion call or never-returning call when

  • the call occurs as a top-level expression statement, and
  • the call specifies a single identifier or a dotted sequence of identifiers for the function name, and
  • each identifier in the function name references an entity with an explicit type, and
  • the function name resolves to a function type with an asserts return type or an explicit never return type annotation.

Another workaround is as follows:

function repro1(arg: string | undefined): number {
  if (!arg) {
    // compatible with return type because the type is `never`, even if the CFA doesn't work
    return new Class().throwMethod();
  }
  return arg.length;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants