microsoft / language-server-protocol

Defines a common protocol for language servers.
https://microsoft.github.io/language-server-protocol/
Creative Commons Attribution 4.0 International
11.22k stars 798 forks source link

Discussion: as-you-type squiggles vs build-time squiggles #510

Open ljw1004 opened 6 years ago

ljw1004 commented 6 years ago

In Visual Studio, it has two kinds of error squiggles: blue squiggles come from when you build a project (using the Project>Build menu item) and are obtained by scraping the output of the build tool; red squiggles come "as-you-type" from the language service and are only ever generated for the file you're currently editing, and even then only up to a limit (a few hundred?). It will be common that a red squiggle and a blue squiggle coincide (because the same error is reported both by the project-build and by the language service) - in this case the red one replaces the blue one.

The idea is that (1) you can't reasonably expect the full set of errors for a project to be recalculated every keystroke; (2) the user deserves some indication that the build-time squiggles are "stale" and require a manual step to generate.

How do folks feel about this in VSCode and LSP? (I'm wanting to do this for my language service).

OPTION 1 This is a question that should be unrelated to LSP. LSP should solely be for the "red" squiggles. The editor itself should figure out when to invoke a command-line build, and scrape blue squiggles from it, and display them, and the editor should figure out how to de-dupe red and blue squiggles.

OPTION 2 This functionality should be rolled into LSP. The "publishDiagnostics" message from the LSP server should indicate whether a given squiggle is blue or red. There should be an additional LSP method to "kick off a build".

jvican commented 6 years ago

How do folks feel about this in VSCode and LSP? (I'm wanting to do this for my language service).

What you describe may be done with the Build Server Protocol (currently developed at the Scala Center and JetBrains). The idea is that the language server connects to the build tool to fetch compiler diagnostics directly from it and then it handles them. Then, LSP can combine them with the diagnostics coming from LSP in whatever way it prefers depending on things like correctness, last compiler results, etc.

Currently, BSP diagnostics have the same structure as LSP diagnostics. In the future, we'd like to enrich these diagnostics so that the build tool can give us structured build messages that the clients/editors know how to display (a message could be composed of different fields; one short summary, one long summary, a code snippet with a suggestion/actionable feedback, etc).

ShaneDelmore commented 6 years ago

^ It would be great if the structured messages could include references to code actions in the cases that an automated fix is available.

jvican commented 6 years ago

I have opened a ticket with my thoughts on how we could support this in the build tool side in https://github.com/scalacenter/bsp/issues/29, but LSP support would be required as well so that language servers can forward our more structured data to editors.

@dbaeumer I think that there's value in allowing diagnostics to provide the following fields in LSP:

  1. Summary.
  2. Long description of the diagnostic.
  3. Link to doc/reference.

For a description of how I think this information is visually important for the user, see the linked ticket. The major advantage of this approach is that language servers can get linter output directly from the build tool -- no need for custom linter integrations in language servers, the integrations in the build tools are reused.

(Note that there's no need to support code actions/fixes since those are already supported by workspace/codeAction and a language server can interpret them from the BSP diagnostics.)

EDIT: Judging from https://github.com/Microsoft/language-server-protocol/issues/500#issuecomment-398825990, it looks like any change to diagnostics is cumbersome because they flow in both ways. I think it would make sense to add this in LSP 4.0 -- we're not rushed in our side to implement rich diagnostics. Another option would be to add an independent endpoint for rich diagnostics and enable it via capabilities.

matklad commented 4 years ago

Kind of an annoying +1, but we could really use this for Rust :)

Our language server is pretty nascent at the moment, and can show only few errors. In contrast, errors from the compiler are outright stunning in quality.

We currently union the two sets of diagnostics on the server side, but this is awkward to implement, and awkward for the user. It would be cool if:

ljw1004 commented 4 years ago

@matklad I wonder... wouldn't you also want on-the-fly ranges to be automatically shifted when the user types?

matklad commented 4 years ago

@ljw1004 yes, but that's not nearly as important, as they are updated more or less instantly by the server anyway.

dbaeumer commented 4 years ago

We had this discussion in VS Code as well and one solution is to differentiate between builder and reconcile problems. See https://github.com/microsoft/vscode/issues/61140#issuecomment-496217510

But we so far carefully state away in the LSP to talk about position adjustment. So I would like to leave it that way.

There is also the build protocol: https://build-server-protocol.github.io/docs/specification