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.

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

Code Lens #513

Merged
merged 10 commits into from
Jul 25, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#### :rocket: New Feature

- Inlay Hints (experimetal). `rescript.settings.inlayHints.enable: true`
- Code Lenses for functions (experimetal). `rescript.settings.codeLens: true`

## v1.4.2

#### :bug: Bug Fix
Expand Down
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,15 @@ rescript.settings.inlayHints.enable: true
rescript.settings.inlayHints.maxLength: 25
```

### Code Lens (experimental)

This tells the editor to add code lenses to function definitions, showing its full type as an annotation above the definition.

```jsonc
// Enable (experimental) code lens.
rescript.settings.codeLens: true
```

### Hide generated files

You can configure VSCode to collapse the JavaScript files ReScript generates under its source ReScript file. This will "hide" the generated files in the VSCode file explorer, but still leaving them accessible by expanding the source ReScript file they belong to.
Expand Down
8 changes: 7 additions & 1 deletion analysis/src/Cli.ml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ API examples:
./rescript-editor-analysis.exe references src/MyFile.res 10 2
./rescript-editor-analysis.exe rename src/MyFile.res 10 2 foo
./rescript-editor-analysis.exe diagnosticSyntax src/MyFile.res
/rescript-editor-analysis.exe inlayHint src/MyFile.res 0 3 25
./rescript-editor-analysis.exe inlayHint src/MyFile.res 0 3 25
./rescript-editor-analysis.exe codeLens src/MyFile.res

Dev-time examples:
./rescript-editor-analysis.exe dump src/MyFile.res src/MyFile2.res
Expand Down Expand Up @@ -70,6 +71,10 @@ Options:

./rescript-editor-analysis.exe inlayHint src/MyFile.res 0 3 25

codeLens: get all code lens entries for file src/MyFile.res

./rescript-editor-analysis.exe codeLens src/MyFile.res

test: run tests specified by special comments in file src/MyFile.res

./rescript-editor-analysis.exe test src/src/MyFile.res
Expand Down Expand Up @@ -98,6 +103,7 @@ let main () =
Commands.inlayhint ~path
~pos:(int_of_string line_start, int_of_string line_end)
~maxLength ~debug:false
| [_; "codeLens"; path] -> Commands.codeLens ~path ~debug:false
| [_; "codeAction"; path; line; col; currentFile] ->
Commands.codeAction ~path
~pos:(int_of_string line, int_of_string col)
Expand Down
7 changes: 7 additions & 0 deletions analysis/src/Commands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ let inlayhint ~path ~pos ~maxLength ~debug =
let result = Hint.inlay ~path ~pos ~maxLength ~debug |> Protocol.array in
print_endline result

let codeLens ~path ~debug =
let result = Hint.codeLens ~path ~debug |> Protocol.array in
print_endline result

let hover ~path ~pos ~currentFile ~debug =
let result =
match Cmt.fullFromPath ~path with
Expand Down Expand Up @@ -391,6 +395,9 @@ let test ~path =
let line_end = 6 in
print_endline ("Inlay Hint " ^ path ^ " " ^ string_of_int line_start ^ ":" ^ string_of_int line_end);
inlayhint ~path ~pos:(line_start, line_end) ~maxLength:"25" ~debug:false)
| "cle" ->
print_endline ("Code Lens " ^ path);
codeLens ~path ~debug:false
Comment on lines +398 to +400
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A "gotcha" that I needed some time to figure out is that these commands that you add to the code can only be 3 chars. I had more than 3 chars and couldn't figure out why it didn't pick them up in the tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Feel free to generalise that.

| _ -> ());
print_newline ())
in
Expand Down
56 changes: 55 additions & 1 deletion analysis/src/Hint.ml
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,58 @@ let inlay ~path ~pos ~maxLength ~debug =
| Some value ->
if String.length label > value then None else Some result
| None -> Some result)
| None -> None)))
| None -> None)))

let codeLens ~path ~debug =
let lenses = ref [] in
let push loc =
let range = Utils.cmtLocToRange loc in
lenses := range :: !lenses
in
(* Code lenses are only emitted for functions right now. So look for value bindings that are functions,
and use the loc of the value binding itself so we can look up the full function type for our code lens. *)
let value_binding (iterator : Ast_iterator.iterator)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This creates an AST iterator targeting value bindings, let whatever = .... Since we're only looking for function definitions, we can target value bindings only, since that's the only place they'll appear.

(vb : Parsetree.value_binding) =
(match vb with
| {
pvb_pat = {ppat_desc = Ppat_var _; ppat_loc};
pvb_expr = {pexp_desc = Pexp_fun _};
} ->
Comment on lines +135 to +138
Copy link
Collaborator Author

@zth zth Jul 25, 2022

Choose a reason for hiding this comment

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

This pattern matches the value binding to target 2 things:

  1. Ppat_var means it's a variable, like let whatever. Assignments can also be destructures let {something} = ...., but this ensures we only target variables.
  2. Pexp_fun means it's a function definition.

pvb_pat is the pattern, which is essentially the left hand side of let whatever = something. pvb_expr is the expression being assigned, so the right hand side after =. We ensure that it is indeed a variable being assigned Ppat_var, and that it's being assigned a function definition Pexp_fun.

Notice that we're also extracting ppat_loc from pvb_pat. That's the location of the variable name itself (whatever in let whatever = ....). That location is important, because we'll extract what type that location has, and that will lead us to the type definition for the function itself. Same logic as if you were to hover the variable holding the function - that's what'll produce the correct function type, not hovering the actual function itself.

push ppat_loc
| _ -> ());
Ast_iterator.default_iterator.value_binding iterator vb
in
let iterator = {Ast_iterator.default_iterator with value_binding} in
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This creates an iterator, and ensures that value_binding is the only thing we're overriding with our own logic. There's a ton of other things you can override as well if you'd want to.

(if Filename.check_suffix path ".res" then
let parser =
Res_driver.parsingEngine.parseImplementation ~forPrinter:false
in
let {Res_driver.parsetree = structure} = parser ~filename:path in
iterator.structure iterator structure |> ignore);
!lenses
|> List.filter_map (fun (range : Protocol.range) ->
match Cmt.fullFromPath ~path with
| None -> None
| Some full -> (
Comment on lines +154 to +156
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This here looks up the compiler artifact for path, which will be the file we're extracting code lenses from. The compiler artifacts are what holds the type information as the compiler has compiled successfully.

match
References.getLocItem ~full
~pos:(range.start.line, range.start.character + 1)
~debug
with
| Some {locType = Typed (_, typeExpr, _)} ->
Comment on lines +157 to +162
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We then use a helper to get the full type via the compiler artifact for the target file, using the positions where we found variables being assigned functions. This will lead us to the full, correct type definitions for all functions assigned to variables.

A type at a location can be a bunch of different things - we target Typed specifically here, because we know that's the only thing our function definition can be. In Typed, typeExpr is what we're interested in. typeExpr comes from the type checker, and we have helpers that can print those type definitions as strings.

Some
(Protocol.stringifyCodeLens
{
range;
command =
Some
{
(* Code lenses can run commands. An empty command string means we just want the editor
to print the text, not link to running a command. *)
command = "";
(* Print the type with a huge line width, because the code lens always prints on a
single line in the editor. *)
title = typeExpr |> Shared.typeToString ~lineWidth:400;
};
})
| _ -> None))
4 changes: 2 additions & 2 deletions analysis/src/PrintType.ml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
let printExpr typ =
let printExpr ?(lineWidth = 60) typ =
Printtyp.reset_names ();
Res_doc.toString ~width:60
Res_doc.toString ~width:lineWidth
(Res_outcome_printer.printOutTypeDoc (Printtyp.tree_of_typexp false typ))

let printDecl ~recStatus name decl =
Expand Down
18 changes: 18 additions & 0 deletions analysis/src/Protocol.ml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
type position = {line: int; character: int}
type range = {start: position; end_: position}
type markupContent = {kind: string; value: string}
type command = {title: string; command: string}
type codeLens = {range: range; command: command option}
type inlayHint = {
position: position;
label: string;
Expand Down Expand Up @@ -147,6 +149,22 @@ let stringifyHint hint =
(stringifyPosition hint.position)
(Json.escape hint.label) hint.kind hint.paddingLeft hint.paddingRight

let stringifyCommand (command : command) =
Printf.sprintf {|{"title": "%s", "command": "%s"}|}
(Json.escape command.title)
(Json.escape command.command)

let stringifyCodeLens (codeLens : codeLens) =
Printf.sprintf
{|{
"range": %s,
"command": %s
}|}
(stringifyRange codeLens.range)
(match codeLens.command with
| None -> ""
| Some command -> stringifyCommand command)

(* https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnostic *)
let stringifyDiagnostic d =
Printf.sprintf
Expand Down
4 changes: 2 additions & 2 deletions analysis/src/Shared.ml
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ let declToString ?(recStatus = Types.Trec_not) name t =
let cacheTypeToString = ref false
let typeTbl = Hashtbl.create 1

let typeToString (t : Types.type_expr) =
let typeToString ?(lineWidth = 60) (t : Types.type_expr) =
match
if !cacheTypeToString then Hashtbl.find_opt typeTbl (t.id, t) else None
with
| None ->
let s = PrintType.printExpr t in
let s = PrintType.printExpr ~lineWidth t in
Hashtbl.replace typeTbl (t.id, t) s;
s
| Some s -> s
11 changes: 11 additions & 0 deletions analysis/tests/src/CodeLens.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
let add = (x, y) => x + y

let foo = (~age, ~name) => name ++ string_of_int(age)

let ff = (~opt1=0, ~a, ~b, (), ~opt2=0, (), ~c) => a + b + c + opt1 + opt2

let compFF = Completion.ff

@react.component
let make = (~name) => React.string(name)
//^cle
15 changes: 15 additions & 0 deletions analysis/tests/src/expected/CodeLens.res.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Code Lens src/CodeLens.res
[{
"range": {"start": {"line": 9, "character": 4}, "end": {"line": 9, "character": 8}},
"command": {"title": "{\"name\": string} => React.element", "command": ""}
}, {
"range": {"start": {"line": 4, "character": 4}, "end": {"line": 4, "character": 6}},
"command": {"title": "(~opt1: int=?, ~a: int, ~b: int, unit, ~opt2: int=?, unit, ~c: int) => int", "command": ""}
}, {
"range": {"start": {"line": 2, "character": 4}, "end": {"line": 2, "character": 7}},
"command": {"title": "(~age: int, ~name: string) => string", "command": ""}
}, {
"range": {"start": {"line": 0, "character": 4}, "end": {"line": 0, "character": 7}},
"command": {"title": "(int, int) => int", "command": ""}
}]

2 changes: 1 addition & 1 deletion analysis/tests/src/expected/Dce.res.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
DCE src/Dce.res
issues:243
issues:249

3 changes: 2 additions & 1 deletion analysis/tests/src/expected/Debug.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ Dependencies: @rescript/react
Source directories: ./node_modules/@rescript/react/src ./node_modules/@rescript/react/src/legacy
Source files: ./node_modules/@rescript/react/src/React.res ./node_modules/@rescript/react/src/ReactDOM.res ./node_modules/@rescript/react/src/ReactDOMServer.res ./node_modules/@rescript/react/src/ReactDOMStyle.res ./node_modules/@rescript/react/src/ReactEvent.res ./node_modules/@rescript/react/src/ReactEvent.resi ./node_modules/@rescript/react/src/ReactTestUtils.res ./node_modules/@rescript/react/src/ReactTestUtils.resi ./node_modules/@rescript/react/src/RescriptReactErrorBoundary.res ./node_modules/@rescript/react/src/RescriptReactErrorBoundary.resi ./node_modules/@rescript/react/src/RescriptReactRouter.res ./node_modules/@rescript/react/src/RescriptReactRouter.resi ./node_modules/@rescript/react/src/legacy/ReactDOMRe.res ./node_modules/@rescript/react/src/legacy/ReasonReact.res
Source directories: ./src ./src/expected
Source files: ./src/Auto.res ./src/CompletePrioritize1.res ./src/CompletePrioritize2.res ./src/Completion.res ./src/Component.res ./src/Component.resi ./src/CreateInterface.res ./src/Cross.res ./src/Dce.res ./src/Debug.res ./src/Definition.res ./src/DefinitionWithInterface.res ./src/DefinitionWithInterface.resi ./src/Div.res ./src/DocumentSymbol.res ./src/Fragment.res ./src/Highlight.res ./src/Hover.res ./src/InlayHint.res ./src/Jsx.res ./src/Jsx.resi ./src/LongIdentTest.res ./src/Object.res ./src/Patterns.res ./src/RecModules.res ./src/RecordCompletion.res ./src/RecoveryOnProp.res ./src/References.res ./src/ReferencesWithInterface.res ./src/ReferencesWithInterface.resi ./src/Rename.res ./src/RenameWithInterface.res ./src/RenameWithInterface.resi ./src/TableclothMap.ml ./src/TableclothMap.mli ./src/TypeDefinition.res ./src/Xform.res
Source files: ./src/Auto.res ./src/CodeLens.res ./src/CompletePrioritize1.res ./src/CompletePrioritize2.res ./src/Completion.res ./src/Component.res ./src/Component.resi ./src/CreateInterface.res ./src/Cross.res ./src/Dce.res ./src/Debug.res ./src/Definition.res ./src/DefinitionWithInterface.res ./src/DefinitionWithInterface.resi ./src/Div.res ./src/DocumentSymbol.res ./src/Fragment.res ./src/Highlight.res ./src/Hover.res ./src/InlayHint.res ./src/Jsx.res ./src/Jsx.resi ./src/LongIdentTest.res ./src/Object.res ./src/Patterns.res ./src/RecModules.res ./src/RecordCompletion.res ./src/RecoveryOnProp.res ./src/References.res ./src/ReferencesWithInterface.res ./src/ReferencesWithInterface.resi ./src/Rename.res ./src/RenameWithInterface.res ./src/RenameWithInterface.resi ./src/TableclothMap.ml ./src/TableclothMap.mli ./src/TypeDefinition.res ./src/Xform.res
Impl cmt:./lib/bs/src/Auto.cmt res:./src/Auto.res
Impl cmt:./lib/bs/src/CodeLens.cmt res:./src/CodeLens.res
Impl cmt:./lib/bs/src/CompletePrioritize1.cmt res:./src/CompletePrioritize1.res
Impl cmt:./lib/bs/src/CompletePrioritize2.cmt res:./src/CompletePrioritize2.res
Impl cmt:./lib/bs/src/Completion.cmt res:./src/Completion.res
Expand Down
5 changes: 4 additions & 1 deletion client/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,10 @@ export function activate(context: ExtensionContext) {
// language client, and because of that requires a full restart.
context.subscriptions.push(
workspace.onDidChangeConfiguration(({ affectsConfiguration }) => {
if (affectsConfiguration("rescript.settings.inlayHints")) {
if (
affectsConfiguration("rescript.settings.inlayHints") ||
affectsConfiguration("rescript.settings.codeLens")
) {
commands.executeCommand("rescript-vscode.restart_language_server");
}
})
Expand Down
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@
],
"minimum": 0
},
"rescript.settings.codeLens.enable": {
"type": "boolean",
"default": false,
"description": "Enable (experimental) code lens for function definitions."
},
"rescript.settings.binaryPath": {
"type": ["string", "null"],
"default": null,
Expand Down
38 changes: 38 additions & 0 deletions server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
DidChangeConfigurationNotification,
InitializeParams,
InlayHintParams,
CodeLensParams,
} from "vscode-languageserver-protocol";
import * as utils from "./utils";
import * as codeActions from "./codeActions";
Expand All @@ -30,6 +31,7 @@ interface extensionConfiguration {
enable: boolean;
maxLength: number | null;
};
codeLens: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wondering whether extension config should go to its own file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean the type definition?

binaryPath: string | null;
}
let extensionConfiguration: extensionConfiguration = {
Expand All @@ -38,6 +40,7 @@ let extensionConfiguration: extensionConfiguration = {
enable: false,
maxLength: 25
},
codeLens: false,
binaryPath: null,
};
let pullConfigurationPeriodically: NodeJS.Timeout | null = null;
Expand Down Expand Up @@ -229,6 +232,9 @@ let compilerLogsWatcher = chokidar
if (extensionConfiguration.inlayHints.enable === true) {
sendInlayHintsRefresh();
}
if (extensionConfiguration.codeLens === true) {
sendCodeLensRefresh();
}
});
let stopWatchingCompilerLog = () => {
// TODO: cleanup of compilerLogs?
Expand Down Expand Up @@ -411,6 +417,27 @@ function sendInlayHintsRefresh() {
send(request);
}

function codeLens(msg: p.RequestMessage) {
const params = msg.params as p.CodeLensParams;
const filePath = fileURLToPath(params.textDocument.uri);

const response = utils.runAnalysisCommand(
filePath,
["codeLens", filePath],
msg
);
return response;
}

function sendCodeLensRefresh() {
let request: p.RequestMessage = {
jsonrpc: c.jsonrpcVersion,
method: p.CodeLensRefreshRequest.method,
id: serverSentRequestIdCounter++,
};
send(request);
}

function definition(msg: p.RequestMessage) {
// https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_definition
let params = msg.params as p.DefinitionParams;
Expand Down Expand Up @@ -1001,6 +1028,11 @@ function onMessage(msg: p.Message) {
full: true,
},
inlayHintProvider: extensionConfiguration.inlayHints.enable,
codeLensProvider: extensionConfiguration.codeLens
? {
workDoneProgress: false,
}
: undefined,
},
};
let response: p.ResponseMessage = {
Expand Down Expand Up @@ -1086,6 +1118,12 @@ function onMessage(msg: p.Message) {
if (extName === c.resExt) {
send(inlayHint(msg));
}
} else if (msg.method === p.CodeLensRequest.method) {
let params = msg.params as CodeLensParams;
let extName = path.extname(params.textDocument.uri);
if (extName === c.resExt) {
send(codeLens(msg));
}
} else {
let response: p.ResponseMessage = {
jsonrpc: c.jsonrpcVersion,
Expand Down