Skip to content

Commit

Permalink
Allow type errors to be 'pulled' from the daemon during file open and…
Browse files Browse the repository at this point in the history
… save

Summary:
This diff pulls type errors from the Pyre daemon on `didOpen` and `didSave` events. `didChange` events are already handled, so we can see how well Pyre works for providing type errors for unsaved changes.

Extra functionality has been added below (in a possibly hacky way) to gate this functionality to the codenav server only, since we don't want the persistent server to make multiple `publishDiagnostic` requests to VSCode. (i.e. without this, the persistent server will now perform [both type error flows](https://internalfb.com/excalidraw/EX127053) at the same)

**Justification**
I'm adding this functionality here and gating it with a check on the `PyreFlavor` for a few reasons. I'd prefer to avoid it, since this requires that the `PyreLanguageServer` knows about which server it's running instead of having abstractions that handle it, but there are a few issues with pushing that functionality into the `daemon_querier`.

To push these errors to the server, we need access to a `ClientTypeErrorHandler`, which the `PyreLanguageServer` has access to. Moving this functionality into the `daemon_querier` would add functionality to the querier that pushes information to the VSCode language client, blurring separation of concerns there.

Another approach could be to return these errors from the `PyreLanguageServer`'s `didOpen` and `didSave` events, but then we'd have to change the signature of these to return type errors, which also seems like the wrong approach. We'd also still have to use the `ClientTypeErrorHandler`, so to me this seems like the best location to perform this logic.

If we end up keeping it, we could add a new abstraction to handle functionality for calling `send_overlay_type_errors` depending on different approaches, but I think it's an approach I may implement in a follow up diff.

Either way, there is a TODO present to either remove this functionality if we end up not doing global lazy type check right now or delete this.

**Diff Stack Info**
This diff stack will be implementing (a gatekept) version of global, lazy, configurationless type checking to see if it improves user experiences for users with the worst type check projects.

Reviewed By: inseokhwang

Differential Revision: D49927253

fbshipit-source-id: 3ae83f793ff000af1c9ac4d209430fcc9cb540e4
  • Loading branch information
connernilsen authored and facebook-github-bot committed Oct 9, 2023
1 parent 8ac89fe commit 16cec0d
Showing 1 changed file with 17 additions and 1 deletion.
18 changes: 17 additions & 1 deletion client/commands/pyre_language_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
Union,
)

from .. import background_tasks, json_rpc, log, timer
from .. import background_tasks, identifiers, json_rpc, log, timer

from ..language_server import connections, daemon_connection, features, protocol as lsp
from . import commands, daemon_querier, find_symbols, server_state as state
Expand Down Expand Up @@ -432,6 +432,14 @@ async def process_open_request(
document_path, parameters.text_document.text
)

if (
self.get_language_server_features().type_errors.is_enabled()
# TODO (T165048078): hack to get this working only for codenav server
and self.server_state.server_options.flavor
== identifiers.PyreFlavor.CODE_NAVIGATION
):
await self.send_overlay_type_errors(document_path=document_path)

async def process_close_request(
self, parameters: lsp.DidCloseTextDocumentParameters
) -> None:
Expand Down Expand Up @@ -543,6 +551,14 @@ async def process_did_save_request(
pyre_code_updated=False,
)

if (
self.get_language_server_features().type_errors.is_enabled()
# TODO (T165048078): hack to get this working only for codenav server
and self.server_state.server_options.flavor
== identifiers.PyreFlavor.CODE_NAVIGATION
):
await self.send_overlay_type_errors(document_path=document_path)

await self.write_telemetry(
{
"type": "LSP",
Expand Down

0 comments on commit 16cec0d

Please sign in to comment.