Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(lsp): Add Inlay types #2005

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat(lsp): Add Inlay types #2005

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3cc4ffe
feat(lsp): Add Inlay types
spotandjake Feb 2, 2024
1a5872a
chore: Simplify `Function` inlays
spotandjake Feb 2, 2024
848978c
feat(lsp): Implement Inlays For parameters
spotandjake Jul 29, 2024
217b7e5
Do not show hints for top level statements
spotandjake Jul 29, 2024
da038be
chore: Disable type hints for functions.
spotandjake Aug 2, 2024
ffc8709
chore: Add flag to toggle the type hints
spotandjake Aug 2, 2024
04048de
chore: Apply suggestions from code review
spotandjake Aug 2, 2024
d281379
chore: Apply suggestions from oscar code review
spotandjake Aug 5, 2024
e86c6f2
chore: Remove cmdline toggle
spotandjake Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: Add flag to toggle the type hints
  • Loading branch information
@spotandjake
spotandjake committed Oct 4, 2024
commit ffc870956fcb611eccc16972d81b406ca2323905
4 changes: 4 additions & 0 deletions cli/bin/grain.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,10 @@ program

program
.command("lsp")
.forwardOption(
"--disable-inlay-types",
"Disable's the language server's inlay type hints"
)
.description("start the Grain LSP server")
.action(exec.grainlsp);

Expand Down
2 changes: 2 additions & 0 deletions compiler/grainlsp/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
(:include ./config/flags.sexp)))
(libraries cmdliner grain_utils grain_language_server grain_formatting
dune-build-info)
(preprocess
(pps ppx_deriving_cmdliner))
(js_of_ocaml
(flags --no-sourcemap --no-extern-fs --quiet)
(javascript_files hacks.js)))
21 changes: 16 additions & 5 deletions compiler/grainlsp/grainlsp.re
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,26 @@ open Cmdliner;
open Grain_utils;
open Grain_language_server;

let run = () => {
[@deriving cmdliner]
type params = {
/**
A feature flag to disable inlay type hints.
*/
[@name "disable-inlay-types"]
disable_inlay_types: bool,
};

let run = opts => {
set_binary_mode_in(stdin, true);
set_binary_mode_out(stdout, true);

let rec read_stdin = () => {
switch (Protocol.request()) {
| Error(err) => Trace.log("Failed to read message: " ++ err)
| Ok(msg) =>
switch (Driver.process(msg)) {
switch (
Driver.process(~toggle_type_hints=!opts.disable_inlay_types, msg)
) {
| Exit(code) => exit(code)
| Break => ()
| Reading => read_stdin()
Expand All @@ -20,8 +31,8 @@ let run = () => {
read_stdin();
};

let lsp = () =>
try(run()) {
let lsp = opts =>
try(run(opts)) {
| exn =>
Format.eprintf("@[%s@]@.", Printexc.to_string(exn));
exit(2);
Expand All @@ -39,7 +50,7 @@ let cmd = {

Cmd.v(
Cmd.info(Sys.argv[0], ~version, ~doc),
Grain_utils.Config.with_cli_options(lsp) $ const(),
Grain_utils.Config.with_cli_options(lsp) $ params_cmdliner_term(),
);
};

Expand Down
10 changes: 8 additions & 2 deletions compiler/src/language_server/driver.re
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ let compiled_code: Hashtbl.t(Protocol.uri, code) = Hashtbl.create(128);
let is_initialized = ref(false);
let is_shutting_down = ref(false);

let process = msg => {
let process = (~toggle_type_hints, msg) => {
switch (Message.of_request(msg)) {
| Initialize(id, params) =>
Trace.log("initializing");
Expand All @@ -26,7 +26,13 @@ let process = msg => {
Hover.process(~id, ~compiled_code, ~documents, params);
Reading;
| TextDocumentInlayHint(id, params) when is_initialized^ =>
Inlayhint.process(~id, ~compiled_code, ~documents, params);
Inlayhint.process(
~id,
~compiled_code,
~documents,
~toggle_type_hints,
params,
);
Reading;
| TextDocumentCodeLens(id, params) when is_initialized^ =>
Lenses.process(~id, ~compiled_code, ~documents, params);
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/language_server/driver.rei
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ type status =
| Break
| Exit(int);

let process: Protocol.request_message => status;
let process: (~toggle_type_hints: bool, Protocol.request_message) => status;
124 changes: 65 additions & 59 deletions compiler/src/language_server/inlayhint.re
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,81 +56,86 @@ let is_func_typ = (typ: Types.type_expr) => {
};
};

let find_hints = program => {
let find_hints = (~toggle_type_hints: bool, program) => {
let hints = ref([]);
open Typedtree;
open Protocol;
module Iterator =
TypedtreeIter.MakeIterator({
include TypedtreeIter.DefaultIteratorArgument;

let enter_expression = ({exp_desc, exp_type}: expression) => {
switch (exp_desc) {
| TExpLambda(bindings, _) =>
List.iter(
({mb_pat, mb_loc}: match_branch) => {
// Get Parameters
switch (mb_pat.pat_desc) {
| TPatTuple(args) =>
switch (resolve_typ(exp_type).desc) {
| TTyArrow(typ_args, _, _) =>
// Iterate For Types
let argument_typs =
List.map(
((arg, typ: Types.type_expr)) =>
switch (arg) {
| Default(_) => None
| _ => Some(typ)
let enter_expression = ({exp_desc, exp_type}: expression) =>
// Function parameter type hints
spotandjake marked this conversation as resolved.
Show resolved Hide resolved
if (toggle_type_hints) {
switch (exp_desc) {
| TExpLambda(bindings, _) =>
List.iter(
({mb_pat, mb_loc}: match_branch) => {
// Get Parameters
spotandjake marked this conversation as resolved.
Show resolved Hide resolved
switch (mb_pat.pat_desc) {
| TPatTuple(args) =>
switch (resolve_typ(exp_type).desc) {
| TTyArrow(typ_args, _, _) =>
// Iterate For Types
spotandjake marked this conversation as resolved.
Show resolved Hide resolved
let argument_typs =
List.map(
((arg, typ: Types.type_expr)) =>
switch (arg) {
| Default(_) => None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you expand on this? I'm seeing in your screenshot that the default argument has an inline hint on it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default hint comes from the binding pass, if this is enabled it shows : Option<_> after the value.

| _ => Some(typ)
},
typ_args,
);
// Make Hints
spotandjake marked this conversation as resolved.
Show resolved Hide resolved
if (List.length(argument_typs) == List.length(args)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this ever be false?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so but I also think it makes sense to ensure we don't make a change somewhere else in the compiler and break this. I am going to add a failwith("Impossible") though if it is the not equal.

List.iter(
((arg: pattern, typ: option(Types.type_expr))) => {
switch (arg.pat_desc, typ) {
| (TPatVar(_, _), Some(typ)) =>
let bind_end = arg.pat_loc.loc_end;
let p: Protocol.position = {
line: bind_end.pos_lnum - 1,
character: bind_end.pos_cnum - bind_end.pos_bol,
};
let typeSignature = string_of_typ(typ);
hints :=
[build_hint(p, typeSignature), ...hints^];
| _ => ()
}
},
typ_args,
);
// Make Hints
if (List.length(argument_typs) == List.length(args)) {
List.iter(
((arg: pattern, typ: option(Types.type_expr))) => {
switch (arg.pat_desc, typ) {
| (TPatVar(_, _), Some(typ)) =>
let bind_end = arg.pat_loc.loc_end;
let p: Protocol.position = {
line: bind_end.pos_lnum - 1,
character: bind_end.pos_cnum - bind_end.pos_bol,
};
let typeSignature = string_of_typ(typ);
hints := [build_hint(p, typeSignature), ...hints^];
| _ => ()
}
},
List.combine(args, argument_typs),
);
};
List.combine(args, argument_typs),
);
};
| _ => ()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would never be possible. Should we do failwith("Impossible: ...")?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That case can be hit if we are omitting the annotation because typ is None

}
| _ => ()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this ever be possible? If not should we do failwith("Impossible: ...")?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems as though there is a case where it can be a constructor.

}
| _ => ()
}
},
bindings,
)
| _ => ()
},
bindings,
)
| _ => ()
};
};
};

let enter_binding = ({vb_pat, vb_expr}: value_binding, toplevel: bool) =>
if (!toplevel) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: cut down on the condition nesting, perhaps if (!toplevel && toggle_type_hints) and then

switch (vb_pat) {
| {pat_extra: [], pat_desc: TPatVar(_, {loc}) => ...
| _ => ()
}

switch (vb_pat.pat_extra) {
| [] =>
switch (vb_pat.pat_desc) {
| TPatVar(_, {loc}) =>
let bind_end = loc.loc_end;
let p: Protocol.position = {
line: bind_end.pos_lnum - 1,
character: bind_end.pos_cnum - bind_end.pos_bol,
};
let typ = vb_pat.pat_type;
if (!is_func_typ(typ)) {
let typeSignature = string_of_typ(typ);
hints := [build_hint(p, typeSignature), ...hints^];
if (toggle_type_hints) {
switch (vb_pat.pat_desc) {
| TPatVar(_, {loc}) =>
let bind_end = loc.loc_end;
let p: Protocol.position = {
line: bind_end.pos_lnum - 1,
character: bind_end.pos_cnum - bind_end.pos_bol,
};
let typ = vb_pat.pat_type;
if (!is_func_typ(typ)) {
let typeSignature = string_of_typ(typ);
hints := [build_hint(p, typeSignature), ...hints^];
};
| _ => ()
};
| _ => ()
}
| _ => ()
};
Expand All @@ -145,13 +150,14 @@ let process =
~id: Protocol.message_id,
~compiled_code: Hashtbl.t(Protocol.uri, code),
~documents: Hashtbl.t(Protocol.uri, string),
~toggle_type_hints: bool,
params: RequestParams.t,
) => {
Trace.log("Inlay hint request received");
switch (Hashtbl.find_opt(compiled_code, params.text_document.uri)) {
| None => send_no_result(~id)
| Some({program, sourcetree}) =>
let hints = find_hints(program);
let hints = find_hints(~toggle_type_hints, program);
Protocol.response(~id, ResponseResult.to_yojson(hints));
};
};
1 change: 1 addition & 0 deletions compiler/src/language_server/inlayhint.rei
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ let process:
~id: Protocol.message_id,
~compiled_code: Hashtbl.t(Protocol.uri, Lsp_types.code),
~documents: Hashtbl.t(Protocol.uri, string),
~toggle_type_hints: bool,
RequestParams.t
) =>
unit;