Skip to content

Commit

Permalink
[flow] Setup types for extra data for detailedly rendered diagonistics
Browse files Browse the repository at this point in the history
Summary:
In this diff, we setup LSP types for the extra information needed to render detailed errors in vscode. We attach the information in the `data` field of [diagnostic](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnostic), which is guaranteed to not be inspected by standard LSP client, but is something our vscode extension can inspect and do something meaningful.

The rendering protocol will introduce a tight coupling between our language server and client, so I also introduced some version scheme. For now, as I am prototyping, I will start with a simple schema that just dumps the CLI error in plaintext form. Therefore, the schema for our data field is:

```
type Data =
  | null | void
  | { version: 0, rendered: string }
  // | ... to be extended in the future
```

We can continue iterating on the v0 design, since it's still unreleased.

Changelog: [internal]

Reviewed By: panagosg7

Differential Revision: D57169091

fbshipit-source-id: b4dd8ba1031f1384c8aa9e3a44e303b8f53068e4
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed May 14, 2024
1 parent b9770f9 commit 8894f2d
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 12 deletions.
1 change: 1 addition & 0 deletions src/common/flow_lsp_conversions.ml
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ let diagnostics_of_flow_errors =
message = error.Flow_errors_utils.Lsp_output.message;
tags = [];
relatedInformation;
data = Lsp.PublishDiagnostics.NoExtraDetailedDiagnostic;
}
)
| Error _ -> None
Expand Down
5 changes: 5 additions & 0 deletions src/hack_forked/utils/lsp/lsp.ml
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@ module PublishDiagnostics = struct
message: string; (** the diagnostic's message *)
tags: DiagnosticTag.t list;
relatedInformation: diagnosticRelatedInformation list;
data: extraDetailedDiagnostic;
}

and diagnosticCode =
Expand All @@ -760,6 +761,10 @@ module PublishDiagnostics = struct
relatedLocation: Location.t; (** wire: just "location" *)
relatedMessage: string; (** wire: just "message" *)
}

and extraDetailedDiagnostic =
| NoExtraDetailedDiagnostic
| ExtraDetailedDiagnosticV0 of string
end

(** DidOpenTextDocument notification, method="textDocument/didOpen" *)
Expand Down
5 changes: 5 additions & 0 deletions src/hack_forked/utils/lsp/lsp.mli
Original file line number Diff line number Diff line change
Expand Up @@ -592,12 +592,17 @@ module PublishDiagnostics : sig
message: string;
tags: DiagnosticTag.t list;
relatedInformation: diagnosticRelatedInformation list;
data: extraDetailedDiagnostic;
}

and diagnosticRelatedInformation = {
relatedLocation: Location.t;
relatedMessage: string;
}

and extraDetailedDiagnostic =
| NoExtraDetailedDiagnostic
| ExtraDetailedDiagnosticV0 of string
end

module DidOpen : sig
Expand Down
29 changes: 19 additions & 10 deletions src/hack_forked/utils/lsp/lsp_fmt.ml
Original file line number Diff line number Diff line change
Expand Up @@ -542,16 +542,22 @@ let print_diagnostic (diagnostic : PublishDiagnostics.diagnostic) : json =
]
in
Jprint.object_opt
[
("range", Some (print_range diagnostic.range));
("severity", Base.Option.map diagnostic.severity ~f:print_diagnosticSeverity);
("code", print_diagnosticCode diagnostic.code);
("source", Base.Option.map diagnostic.source ~f:string_);
("message", Some (JSON_String diagnostic.message));
( "relatedInformation",
Some (JSON_Array (Base.List.map diagnostic.relatedInformation ~f:print_related))
);
]
([
("range", Some (print_range diagnostic.range));
("severity", Base.Option.map diagnostic.severity ~f:print_diagnosticSeverity);
("code", print_diagnosticCode diagnostic.code);
("source", Base.Option.map diagnostic.source ~f:string_);
("message", Some (JSON_String diagnostic.message));
( "relatedInformation",
Some (JSON_Array (Base.List.map diagnostic.relatedInformation ~f:print_related))
);
]
@
match diagnostic.data with
| PublishDiagnostics.NoExtraDetailedDiagnostic -> []
| PublishDiagnostics.ExtraDetailedDiagnosticV0 str ->
[("data", Some (JSON_Object [("version", JSON_Number "0"); ("rendered", JSON_String str)]))]
)
)

let print_diagnostic_list (ds : PublishDiagnostics.diagnostic list) : json =
Expand Down Expand Up @@ -611,6 +617,9 @@ let parse_diagnostic (j : json option) : PublishDiagnostics.diagnostic =
tags = Jget.array_d j "tags" ~default:[] |> Base.List.filter_map ~f:DiagnosticTagFmt.of_json;
relatedInformation =
Jget.array_d j "relatedInformation" ~default:[] |> Base.List.map ~f:parse_info;
(* The parsing is for recovering diagnostics in the code action. The extra detailed one is
* not relevant, since it contains the same information as the rest of the diagnostic. *)
data = PublishDiagnostics.NoExtraDetailedDiagnostic;
}
)

Expand Down
22 changes: 20 additions & 2 deletions src/hack_forked/utils/lsp/lsp_mapper.ml
Original file line number Diff line number Diff line change
Expand Up @@ -239,14 +239,32 @@ let default_mapper =
(fun _mapper { ConnectionStatus.isConnected } -> { ConnectionStatus.isConnected });
of_diagnostic =
(fun mapper
{ PublishDiagnostics.range; severity; code; source; message; tags; relatedInformation } ->
{
PublishDiagnostics.range;
severity;
code;
source;
message;
tags;
relatedInformation;
data;
} ->
let range = mapper.of_range mapper range in
let map_related_info { PublishDiagnostics.relatedLocation; relatedMessage } =
let relatedLocation = mapper.of_location mapper relatedLocation in
{ PublishDiagnostics.relatedLocation; relatedMessage }
in
let relatedInformation = Base.List.map ~f:map_related_info relatedInformation in
{ PublishDiagnostics.range; severity; code; source; message; tags; relatedInformation });
{
PublishDiagnostics.range;
severity;
code;
source;
message;
tags;
relatedInformation;
data;
});
of_definition_params = (fun mapper t -> mapper.of_text_document_position_params mapper t);
of_definition_result =
(fun mapper results -> Base.List.map ~f:(mapper.of_location mapper) results);
Expand Down
1 change: 1 addition & 0 deletions src/lsp/__tests__/lspErrors_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ let mk_diagnostic { uri = _; kind; msg } =
message = msg;
tags = [];
relatedInformation = [];
data = NoExtraDetailedDiagnostic;
}

(* Take the json output and convert it back into a list of errors *)
Expand Down
1 change: 1 addition & 0 deletions src/lsp/flowLsp.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,7 @@ let diagnostic_of_parse_error (loc, parse_error) : PublishDiagnostics.diagnostic
tags = [];
message = Parse_error.PP.error parse_error;
relatedInformation = [];
data = PublishDiagnostics.NoExtraDetailedDiagnostic;
}

let live_syntax_errors_enabled (state : server_state) =
Expand Down
1 change: 1 addition & 0 deletions src/lsp/lspErrors.ml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ let limit_errors errors =
tags = [];
message;
relatedInformation = [];
data = PublishDiagnostics.NoExtraDetailedDiagnostic;
}
in

Expand Down

0 comments on commit 8894f2d

Please sign in to comment.