Improve HH error type messages in runtimenightly-2022.04.01
commit9afc31e10d8ebd671432fbdc37c277acd4d889d7
authorJames Wu <jjwu@fb.com>
Fri, 1 Apr 2022 02:07:17 +0000 (31 19:07 -0700)
committerFacebook GitHub Bot <facebook-github-bot@users.noreply.github.com>
Fri, 1 Apr 2022 02:07:17 +0000 (31 19:07 -0700)
treeff5408c60985253c6cee99eb408936e68be55d94
parenta4f9cf2902bc9a61822a8bfdc6510aa3cd03f47e
Improve HH error type messages in runtime

Summary:
The code I'm fixing is kind of just working by accident, and is an attempt at partially cleaning up some tech debt. Here's what we did before to `hh` errors in the runtime if we ever raised one (i.e. during lowering with `raise_hh_error`):
1. If the error was hh_fixme'd, we'd ignore it
2. If the error was one of some specific set of codes, we'd ignore it
3. Otherwise, we'd raise a compiler parse error with no position and no message.

By complete accident, the existing errors that we raise with `raise_hh_error` either already cause parse errors, are only raised when `env.is_typechecker` is true, or are part of the specific set of error codes hard coded in step 2.

This is obviously double confusing to the user and to us debugging, so this diff changes this behavior to fix 1 and 3:

1. Change it so that we *never* ignore HH_FIXME'd errors in the runtime. HH_FIXMEs should have no influence on runtime behavior. This means raising an hh_error during parsing, by default, will also cause a parse error in the runtime.
2. Make it so that we error with an actual message and a position.

To fix the specific set of error codes being ignored, I recommend we update the UserError object to include a "runtime_enforced" flag. Unfortunately, this would involve adding a boolean to every error in the typechecker as well, since UserError is derived from the oxidized typechecker definition. It doesn't feel right to add a field there that the typechecker itself doesn't care about at all, so we'll probably want to introduce an intermediate structure between UserError and the one the compiler uses. I may tackle this in a later diff if I have time, but for now the first two fixes are worth doing.

Once cleaned up, this will allow us to create richer error messages with multiple positions for the typechecker while simultaneously still allowing them to error in the runtime with a similar error message, which is useful for many runtime enforced features.

Reviewed By: hgoldstein

Differential Revision: D35266152

fbshipit-source-id: 7a322c9ed8688bbc2bc252329097f2d0592f8c0b
hphp/hack/src/hackc/compile/compile.rs
hphp/hack/src/oxidized/manual/errors_impl.rs
hphp/hack/src/oxidized_by_ref/manual/errors_impl.rs
hphp/test/slow/error_message_shape_hint_invalid.php [new file with mode: 0644]
hphp/test/slow/error_message_shape_hint_invalid.php.expectf [new file with mode: 0644]
hphp/test/slow/error_message_shape_hint_invalid_through_fixme.php [new file with mode: 0644]
hphp/test/slow/error_message_shape_hint_invalid_through_fixme.php.expectf [new file with mode: 0644]