-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Typechecker does not identify errors in unreachable code #9013
Comments
Variables being typed as Treating dead code as an error would result in a lot of undesirable behavior. Given a function fun() with a return type annotation of nullable int and the following code. $x = fun();
if ($x is int) return $x;
// Create a default value and return it. If I know that fun() only every returns ints and I make the return type int instead of nullable int, Hack would report errors on this code. This would discourage tightening return types, since every time you do, the typechecker makes it look like you broke everything. In fact, you didn't break anything. Your code has not changed its behavior. |
I don't really know enough about type systems to discuss intelligently what should be inferred for unreachable code— Running UnreachableCodeLinter—which is limited to much less sophisticated analysis than the typechecker—on Slack's codebase, found ~20 unreachable blocks, several of which represented logic bugs. In your example I disagree that identifying the errors would be undesirable. It's already pretty common for changing or constraining type annotations to produce new typechecker errors. I think it would be useful to be alerted that code you thought might execute at some point will never run. Nothing is broken in the sense that the runtime will error, but not all bugs manifest as runtime errors. |
Describe the bug
Errors in code after a return statement are not identified.
Standalone code, or other way to reproduce the problem
Expected behavior
Typechecker error about the undefined variable or error about unreachable code.
Actual behavior
No errors.
Environment
Additional context
This is related to #8660, where @Wilfred writes:
The typechecker is working here in the goal of preventing runtime errors—the dead code can't produce an error. But this is a failure in the related goal of avoiding bugs; the existence of unreachable code itself is often a bug. I found this issue because of a refactor that introduced an early return and missed some important behavior.
The text was updated successfully, but these errors were encountered: