Limit naming-table tendrils 2/2
commit0ec8e9fb9d929fa4534284cd1360a2411301a791
authorLucian Wischik <ljw@meta.com>
Tue, 6 Dec 2022 05:47:45 +0000 (5 21:47 -0800)
committerFacebook GitHub Bot <facebook-github-bot@users.noreply.github.com>
Tue, 6 Dec 2022 05:47:45 +0000 (5 21:47 -0800)
treec190d841109837edd3538b0848fe7dedebfe81fe
parenta986bec2760cb2f3a15b0e481cf5c04f387c6a1c
Limit naming-table tendrils 2/2

Summary:
As described in the previous diff, my goal is to limit the tendrils of the typechecker into the naming-table, and to avoid the need to cache the naming table.

My approach is a dummy diff D41254732 which raises an exception if the naming-table is ever accessed in a tendril-like way that I don't endorse.

This diff represents my next discovery of tendrils:
```
Naming_provider.get_module_full_pos
Naming_provider.get_type_full_pos
Naming_provider.get_const_full_pos
Naming_provider.get_fun_full_pos
```
The functionality is all the same. Given filename, name_type and name, these functions retrieve the line+column position of the symbol.
* In hh_server they do this by parsing the file into the AST-cache, then scanning its top-level declarations for the matching one
* In re-architecture they do this by relying on the decl_provider to have included line+col, but they substitute in the filename they're given. (actually, in the current implementation of Pos_or_decl, all decls already include filename+line+col, and they really ignore the filename they're given).

Where are they used?
1. In Naming_global, which is how hh_server updates the naming-table and reports duplicate-name errors. (This is done entirely differently in the re-architecture, via hh_decl UPDATE action, so I don't care about these uses)
2. In symbolIndexCore, which is used to build a file+line+col search index. I'm not sure what this is used for. It might be to index disk changes so we can provide symbol-search and go-to-def and autocomplete?
3. In unbound_name_check, in case of error, we use Naming_provider.get_type_full_pos to formulate the error text.

## How to read this diff

This diff doesn't change any functionality. It just shuffles these functions around a little bit. I thought they belonged more properly inside Decl_service_client, since they are a decl-service-specific technique for obtaining Pos.t from a cached decl.

## End result of my project to "limit naming-table tendrils"

The dummy diff D41254732 raises an exception if the naming-table is accessed in a tendril-like way that I don't endorse. The result of running it on WWW shows there are no unaccounted-for tendrils. Let me spell out the accounting:
* **Naming_global.ml** calls into naming-table in order to update the naming table and report duplicate name errors. (this is okay because the re-architecture doesn't use Naming_global.ml; it instead does naming-table updates via hh_decl.UPDATE, and it doesn't yet have an implementation for duplicate name errors, but when it does you can be sure it will be satisfactory!)
* **Naming_provider.ml** hh_server calls into naming-table for `{const,fun,module}_exists`, `get_type_kind`, `get_typedef_path`. But all of these use non-tendril alternatives when backed by hh_decl.
* **Decl_provider.ml** hh_server calls into naming-table for `get_{fun,typedef,gconst,module}`. But all of these use non-tendril alternatives when backed by hh_decl.
* **Direct_decl_utils.parse_and_cache_decls** hh_server calls into the naming-table to avoid caching decls that aren't winners; it uses the naming-table to determine winners. There are a lot of callers to this function, but the function raises an exception when hacked by hh_decl, and I've not seen the exception raised, so fingers crossed it's not needed.
* **Unbound_name_check.ml** calls into the naming-table in case of an unbound name, to see if there is a canonical capitalization of the name, and if so what is its exact position. You'd think this would never be called for code that typechecks clean, but it currently is in 144 files in WWW, each one suppressed with `HH_FIXME[2049]`. I think hh_decl can live with doing 144 naming-sqlite lookups for these. Eventually I think they'll be replaced with the sound-dynamic version `$fun = HH\dynamic_fun('HH\Facts\ensure_updated')` which will no longer need the unbound name check.
* **Typing_check_service.ml** hh_server calls into the naming-table in case of deferred decl, so it knows which filename the deferred-decl came from and can enqueue it later. The re-architecture won't use deferred decls, so I'm okay with this.

Reviewed By: CatherineGasnier

Differential Revision: D41314228

fbshipit-source-id: df4f0307a97ceba34bc84aecdb6f7983752be5fb
hphp/hack/src/providers/naming_provider.ml
hphp/hack/src/stubs/decl_service_client.ml