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

Code reachability contains False Positive in simple "always true" return condition #58455

Closed
kungfooman opened this issue May 7, 2024 · 8 comments
Labels
Duplicate An existing issue was already created

Comments

@kungfooman
Copy link

kungfooman commented May 7, 2024

🔎 Search Terms

  • Unreachable Code Detection Too Weak
  • Control flow analysis does not analyze well enough
  • Available control flow information is not exploited as much as it could be
  • Code reachability contains False Positive

🕗 Version & Regression Information

I don't know if this ever worked and it clearly doesn't work on v5.5.0-beta

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.5.0-beta&filetype=ts#code/GYVwdgxgLglg9mABFApgZygCgJSIN4BQiiECGiAhgDYDuFAnmgCoBOIKiAvMmygNxFEMYIkzU6jVu1yFixFiiggWYAcQC+g0mDRwqKAHRU4Ac0wAiJgAsYaRACo0VuCCoATe4gBGHEywYoboguUAbm2GpCIphQvDKC8orKqoKaxNq6+kamFta2DraefgFBIWERBJqoGDh8QA

💻 Code

function test() {
  const alwaysTrue = true;
  if (alwaysTrue) {
    return;
  }
  console.log("This *should* be grayed out.");
  if (true) {
    return;
  }
  console.log("This *is* grayed out.");
}
test();

🙁 Actual behavior

image

🙂 Expected behavior

Correctly identify unreachable code from the variable.

Additional information about the issue

No response

@whzx5byb
Copy link

whzx5byb commented May 7, 2024

Duplicate of #29323.

@kungfooman
Copy link
Author

Thank you @whzx5byb, that issue is from 2019, is it still a design limitation? I want to use some advanced type programming which figures out true/false values and have unreachability detection of it.

@whzx5byb
Copy link

whzx5byb commented May 7, 2024

@kungfooman I'm only a passerby so I can't tell. You have to wait for a TS team member to answer it.

@fatcerberus
Copy link

Quote from the other issue, emphasis mine:

Background: The control flow graph creator can't resolve the types of expressions (doing so would create a circular dependency between passes of the compiler), but does have some logic for detecting if (true) {. Basically, TS thinks if (value === null) { might not always execute, because during the construction of the control flow graph, it is not yet known that __TRUE__ is true.

That's a pretty fundamental design constraint, and I highly doubt anything has changed in that regard.

Then again, TS can do control flow for asserts T and never returns as long as there's an explicit type annotation on the function, so who knows...

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label May 8, 2024
@RyanCavanaugh
Copy link
Member

Everything is of course in-principle solvable, but there aren't a lot of cases where people intentionally write if (expr) where expr is a complex expression which statically provable to be true (and then need the corresponding code to change color).

@kungfooman
Copy link
Author

@RyanCavanaugh I investigated type checks on ranges a bit through type programming doing arithmetic, e.g. a function could return a random number from 20 to 100. If later there is an condition which checks that this number is bigger than... lets say 200, it can only be false (and ideally be pointed out through dead-code detection).

You already planned range based type flow in another PR and I just wanted to see if it's possible already without any extra PR's. But this issue makes range based type checks not too helpful as-is.

OTOH it seems to already exist, but with an extra linting step: https://typescript-eslint.io/rules/no-unnecessary-condition/

@RyanCavanaugh
Copy link
Member

The lint rule has a lot of false positives, e.g.

const n = [1, 2];
if (n[4] !== undefined) { // <- thinks this is always true
  
}

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants