From b59a20094cb362de00226fe228475398c674dd72 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sat, 9 Mar 2024 13:03:13 +0100 Subject: [PATCH 1/3] extend signature help to also work on variant constructor payloads --- analysis/bin/main.ml | 8 +- analysis/src/Commands.ml | 10 +- analysis/src/SignatureHelp.ml | 242 +++++++++++++++++- analysis/tests/src/SignatureHelp.res | 26 ++ .../tests/src/expected/SignatureHelp.res.txt | 66 +++++ package.json | 5 + server/src/config.ts | 24 +- server/src/server.ts | 5 +- 8 files changed, 361 insertions(+), 25 deletions(-) diff --git a/analysis/bin/main.ml b/analysis/bin/main.ml index 9d44434b4..e857e6af2 100644 --- a/analysis/bin/main.ml +++ b/analysis/bin/main.ml @@ -136,10 +136,16 @@ let main () = (match supportsMarkdownLinks with | "true" -> true | _ -> false) - | [_; "signatureHelp"; path; line; col; currentFile] -> + | [ + _; "signatureHelp"; path; line; col; currentFile; allowForConstructorPayloads; + ] -> Commands.signatureHelp ~path ~pos:(int_of_string line, int_of_string col) ~currentFile ~debug + ~allowForConstructorPayloads: + (match allowForConstructorPayloads with + | "true" -> true + | _ -> false) | [_; "inlayHint"; path; line_start; line_end; maxLength] -> Commands.inlayhint ~path ~pos:(int_of_string line_start, int_of_string line_end) diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index bfb9dc3c2..e4470be40 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -71,9 +71,12 @@ let hover ~path ~pos ~currentFile ~debug ~supportsMarkdownLinks = in print_endline result -let signatureHelp ~path ~pos ~currentFile ~debug = +let signatureHelp ~path ~pos ~currentFile ~debug ~allowForConstructorPayloads = let result = - match SignatureHelp.signatureHelp ~path ~pos ~currentFile ~debug with + match + SignatureHelp.signatureHelp ~path ~pos ~currentFile ~debug + ~allowForConstructorPayloads + with | None -> {Protocol.signatures = []; activeSignature = None; activeParameter = None} | Some res -> res @@ -345,7 +348,8 @@ let test ~path = ("Signature help " ^ path ^ " " ^ string_of_int line ^ ":" ^ string_of_int col); let currentFile = createCurrentFile () in - signatureHelp ~path ~pos:(line, col) ~currentFile ~debug:true; + signatureHelp ~path ~pos:(line, col) ~currentFile ~debug:true + ~allowForConstructorPayloads:true; Sys.remove currentFile | "int" -> print_endline ("Create Interface " ^ path); diff --git a/analysis/src/SignatureHelp.ml b/analysis/src/SignatureHelp.ml index 9227b293f..881c4f6f7 100644 --- a/analysis/src/SignatureHelp.ml +++ b/analysis/src/SignatureHelp.ml @@ -172,7 +172,22 @@ let docsForLabel typeExpr ~file ~package ~supportsMarkdownLinks = in typeString :: typeDefinitions |> String.concat "\n" -let signatureHelp ~path ~pos ~currentFile ~debug = +let findConstructorArgs ~full ~env ~constructorName loc = + match + References.getLocItem ~debug:false ~full + ~pos:(Pos.ofLexing loc.Location.loc_end) + with + | None -> None + | Some {locType = Typed (_, typExpr, _)} -> ( + match TypeUtils.extractType ~env ~package:full.package typExpr with + | Some (Tvariant {constructors}, _) -> + constructors + |> List.find_opt (fun (c : Constructor.t) -> + c.cname.txt = constructorName) + | _ -> None) + | _ -> None + +let signatureHelp ~path ~pos ~currentFile ~debug ~allowForConstructorPayloads = let textOpt = Files.readFile currentFile in match textOpt with | None | Some "" -> None @@ -187,8 +202,18 @@ let signatureHelp ~path ~pos ~currentFile ~debug = Some text.[offsetNoWhite] else None in + let locHasCursor loc = + loc |> CursorPosition.locHasCursor ~pos:posBeforeCursor + in let supportsMarkdownLinks = true in let foundFunctionApplicationExpr = ref None in + let foundConstructorExpr = ref None in + let setFoundConstructor r = + if allowForConstructorPayloads then + match !foundConstructorExpr with + | None -> foundConstructorExpr := Some r + | Some _ -> () + in let setFound r = (* Because we want to handle both piped and regular function calls, and in the case of piped calls the iterator will process both the pipe and the @@ -216,7 +241,7 @@ let signatureHelp ~path ~pos ~currentFile ~debug = let currentUnlabelledArgCount = !unlabelledArgCount in unlabelledArgCount := currentUnlabelledArgCount + 1; (* An argument without a label is just the expression, so we can use that. *) - if arg.exp.pexp_loc |> Loc.hasPos ~pos:posBeforeCursor then + if locHasCursor arg.exp.pexp_loc then Some (Unlabelled currentUnlabelledArgCount) else ( (* If this unlabelled arg doesn't have the cursor, record @@ -286,9 +311,7 @@ let signatureHelp ~path ~pos ~currentFile ~debug = } ); ] ); } - when pexp_loc - |> CursorPosition.classifyLoc ~pos:posBeforeCursor - == HasCursor -> + when locHasCursor pexp_loc -> let argAtCursor, extractedArgs = searchForArgWithCursor ~isPipeExpr:true ~args in @@ -298,13 +321,17 @@ let signatureHelp ~path ~pos ~currentFile ~debug = pexp_desc = Pexp_apply (({pexp_desc = Pexp_ident _} as exp), args); pexp_loc; } - when pexp_loc - |> CursorPosition.classifyLoc ~pos:posBeforeCursor - == HasCursor -> + when locHasCursor pexp_loc -> let argAtCursor, extractedArgs = searchForArgWithCursor ~isPipeExpr:false ~args in setFound (argAtCursor, exp, extractedArgs) + | {pexp_desc = Pexp_construct (lid, Some payloadExp); pexp_loc} + when locHasCursor payloadExp.pexp_loc + || CompletionExpressions.isExprHole payloadExp + && locHasCursor pexp_loc -> + (* Constructor payloads *) + setFoundConstructor (lid, payloadExp) | _ -> ()); Ast_iterator.default_iterator.expr iterator expr in @@ -314,6 +341,7 @@ let signatureHelp ~path ~pos ~currentFile ~debug = in let {Res_driver.parsetree = structure} = parser ~filename:currentFile in iterator.structure iterator structure |> ignore; + (* Handle function application, if found *) match !foundFunctionApplicationExpr with | Some (argAtCursor, exp, _extractedArgs) -> ( (* Not looking for the cursor position after this, but rather the target function expression's loc. *) @@ -395,4 +423,200 @@ let signatureHelp ~path ~pos ~currentFile ~debug = | activeParameter -> activeParameter); } | _ -> None) - | _ -> None)) + | None -> ( + (* Handle constructor payload if we had no function application *) + match !foundConstructorExpr with + | Some (lid, expr) -> ( + if Debug.verbose () then + Printf.printf "[signature_help] Found constructor expr!\n"; + match Cmt.loadFullCmtFromPath ~path with + | None -> + if Debug.verbose () then + Printf.printf "[signature_help] Could not load cmt\n"; + None + | Some full -> ( + let {file} = full in + let env = QueryEnv.fromFile file in + let constructorName = Longident.last lid.txt in + match + findConstructorArgs ~full ~env ~constructorName + {lid.loc with loc_start = lid.loc.loc_end} + with + | None -> + if Debug.verbose () then + Printf.printf "[signature_help] Did not find constructor '%s'\n" + constructorName; + None + | Some constructor -> + let argParts = + match constructor.args with + | Args [] -> None + | InlineRecord fields -> + let offset = ref 0 in + Some + (`InlineRecord + (fields + |> List.map (fun (field : field) -> + let startOffset = !offset in + let argText = + Printf.sprintf "%s%s: %s" field.fname.txt + (if field.optional then "?" else "") + (Shared.typeToString + (if field.optional then + Utils.unwrapIfOption field.typ + else field.typ)) + in + let endOffset = + startOffset + String.length argText + in + offset := endOffset + String.length ", "; + (argText, field, (startOffset, endOffset))))) + | Args [(typ, _)] -> + Some + (`SingleArg + ( typ |> Shared.typeToString, + docsForLabel ~file:full.file ~package:full.package + ~supportsMarkdownLinks typ )) + | Args args -> + let offset = ref 0 in + Some + (`TupleArg + (args + |> List.map (fun (typ, _) -> + let startOffset = !offset in + let argText = typ |> Shared.typeToString in + let endOffset = + startOffset + String.length argText + in + offset := endOffset + String.length ", "; + ( argText, + docsForLabel ~file:full.file + ~package:full.package ~supportsMarkdownLinks + typ, + (startOffset, endOffset) )))) + in + let label = + constructor.cname.txt ^ "(" + ^ (match argParts with + | None -> "" + | Some (`InlineRecord fields) -> + "{" + ^ (fields + |> List.map (fun (argText, _, _) -> argText) + |> String.concat ", ") + ^ "}" + | Some (`SingleArg (arg, _)) -> arg + | Some (`TupleArg items) -> + items + |> List.map (fun (argText, _, _) -> argText) + |> String.concat ", ") + ^ ")" + in + let activeParameter = + match expr with + | {pexp_desc = Pexp_tuple items} -> ( + let idx = ref 0 in + let tupleItemWithCursor = + items + |> List.find_map (fun (item : Parsetree.expression) -> + let currentIndex = !idx in + idx := currentIndex + 1; + if locHasCursor item.pexp_loc then Some currentIndex + else None) + in + match tupleItemWithCursor with + | None -> -1 + | Some i -> i) + | {pexp_desc = Pexp_record (fields, _)} -> ( + let fieldNameWithCursor = + fields + |> List.find_map + (fun + (({loc; txt}, expr) : + Longident.t Location.loc * Parsetree.expression) + -> + if + posBeforeCursor >= Pos.ofLexing loc.loc_start + && posBeforeCursor + <= Pos.ofLexing expr.pexp_loc.loc_end + then Some (Longident.last txt) + else None) + in + match (fieldNameWithCursor, argParts) with + | Some fieldName, Some (`InlineRecord fields) -> + let idx = ref 0 in + let fieldIndex = ref (-1) in + fields + |> List.iter (fun (_, field, _) -> + idx := !idx + 1; + let currentIndex = !idx in + if fieldName = field.fname.txt then + fieldIndex := currentIndex + else ()); + !fieldIndex + | _ -> -1) + | _ when locHasCursor expr.pexp_loc -> 0 + | _ -> -1 + in + + let constructorNameLength = String.length constructor.cname.txt in + let params = + match argParts with + | None -> [] + | Some (`SingleArg (_, docstring)) -> + [ + { + Protocol.label = + (constructorNameLength + 1, String.length label - 1); + documentation = + {Protocol.kind = "markdown"; value = docstring}; + }; + ] + | Some (`InlineRecord fields) -> + (* Account for leading '({' *) + let baseOffset = constructorNameLength + 2 in + { + Protocol.label = (0, 0); + documentation = {Protocol.kind = "markdown"; value = ""}; + } + :: (fields + |> List.map (fun (_, field, (start, end_)) -> + { + Protocol.label = + (baseOffset + start, baseOffset + end_); + documentation = + { + Protocol.kind = "markdown"; + value = field.docstring |> String.concat "\n"; + }; + })) + | Some (`TupleArg items) -> + (* Account for leading '(' *) + let baseOffset = constructorNameLength + 1 in + items + |> List.map (fun (_, docstring, (start, end_)) -> + { + Protocol.label = + (baseOffset + start, baseOffset + end_); + documentation = + {Protocol.kind = "markdown"; value = docstring}; + }) + in + Some + { + Protocol.signatures = + [ + { + label; + parameters = params; + documentation = + (match List.nth_opt constructor.docstring 0 with + | None -> None + | Some docs -> + Some {Protocol.kind = "markdown"; value = docs}); + }; + ]; + activeSignature = Some 0; + activeParameter = Some activeParameter; + })) + | None -> None))) diff --git a/analysis/tests/src/SignatureHelp.res b/analysis/tests/src/SignatureHelp.res index 41d2b8cfb..f7cf8bedd 100644 --- a/analysis/tests/src/SignatureHelp.res +++ b/analysis/tests/src/SignatureHelp.res @@ -74,3 +74,29 @@ let fn = (age: int, name: string, year: int) => { // let _ = fn({ iAmSoSpecial({ someFunc() }) }) // ^she + +/** This is my own special thing. */ +type mySpecialThing = string + +type t = + | /** One is cool. */ One({miss?: bool, hit?: bool, stuff?: string}) + | /** Two is fun! */ Two(mySpecialThing) + | /** Three is... three */ Three(mySpecialThing, array>) + +let _one = One({}) +// ^she + +let _one = One({miss: true}) +// ^she + +let _one = One({hit: true, miss: true}) +// ^she + +let two = Two("true") +// ^she + +let three = Three("", []) +// ^she + +let three2 = Three("", []) +// ^she diff --git a/analysis/tests/src/expected/SignatureHelp.res.txt b/analysis/tests/src/expected/SignatureHelp.res.txt index 2a7811df6..664affdcb 100644 --- a/analysis/tests/src/expected/SignatureHelp.res.txt +++ b/analysis/tests/src/expected/SignatureHelp.res.txt @@ -386,3 +386,69 @@ extracted params: "activeParameter": 0 } +Signature help src/SignatureHelp.res 85:16 +{ + "signatures": [{ + "label": "One({miss?: bool, hit?: bool, stuff?: string})", + "parameters": [{"label": [0, 0], "documentation": {"kind": "markdown", "value": ""}}, {"label": [5, 16], "documentation": {"kind": "markdown", "value": ""}}, {"label": [18, 28], "documentation": {"kind": "markdown", "value": ""}}, {"label": [30, 44], "documentation": {"kind": "markdown", "value": ""}}], + "documentation": {"kind": "markdown", "value": " One is cool. "} + }], + "activeSignature": 0, + "activeParameter": -1 +} + +Signature help src/SignatureHelp.res 88:18 +{ + "signatures": [{ + "label": "One({miss?: bool, hit?: bool, stuff?: string})", + "parameters": [{"label": [0, 0], "documentation": {"kind": "markdown", "value": ""}}, {"label": [5, 16], "documentation": {"kind": "markdown", "value": ""}}, {"label": [18, 28], "documentation": {"kind": "markdown", "value": ""}}, {"label": [30, 44], "documentation": {"kind": "markdown", "value": ""}}], + "documentation": {"kind": "markdown", "value": " One is cool. "} + }], + "activeSignature": 0, + "activeParameter": 1 +} + +Signature help src/SignatureHelp.res 91:23 +{ + "signatures": [{ + "label": "One({miss?: bool, hit?: bool, stuff?: string})", + "parameters": [{"label": [0, 0], "documentation": {"kind": "markdown", "value": ""}}, {"label": [5, 16], "documentation": {"kind": "markdown", "value": ""}}, {"label": [18, 28], "documentation": {"kind": "markdown", "value": ""}}, {"label": [30, 44], "documentation": {"kind": "markdown", "value": ""}}], + "documentation": {"kind": "markdown", "value": " One is cool. "} + }], + "activeSignature": 0, + "activeParameter": 2 +} + +Signature help src/SignatureHelp.res 94:15 +{ + "signatures": [{ + "label": "Two(mySpecialThing)", + "parameters": [{"label": [4, 18], "documentation": {"kind": "markdown", "value": "```rescript\nmySpecialThing\n```\n```rescript\ntype mySpecialThing = string\n```\nGo to: [Type definition](command:rescript-vscode.go_to_location?%5B%22SignatureHelp.res%22%2C78%2C0%5D)"}}], + "documentation": {"kind": "markdown", "value": " Two is fun! "} + }], + "activeSignature": 0, + "activeParameter": 0 +} + +Signature help src/SignatureHelp.res 97:19 +{ + "signatures": [{ + "label": "Three(mySpecialThing, array>)", + "parameters": [{"label": [6, 20], "documentation": {"kind": "markdown", "value": "```rescript\nmySpecialThing\n```\n```rescript\ntype mySpecialThing = string\n```\nGo to: [Type definition](command:rescript-vscode.go_to_location?%5B%22SignatureHelp.res%22%2C78%2C0%5D)"}}, {"label": [22, 43], "documentation": {"kind": "markdown", "value": "```rescript\narray>\n```"}}], + "documentation": {"kind": "markdown", "value": " Three is... three "} + }], + "activeSignature": 0, + "activeParameter": 0 +} + +Signature help src/SignatureHelp.res 100:24 +{ + "signatures": [{ + "label": "Three(mySpecialThing, array>)", + "parameters": [{"label": [6, 20], "documentation": {"kind": "markdown", "value": "```rescript\nmySpecialThing\n```\n```rescript\ntype mySpecialThing = string\n```\nGo to: [Type definition](command:rescript-vscode.go_to_location?%5B%22SignatureHelp.res%22%2C78%2C0%5D)"}}, {"label": [22, 43], "documentation": {"kind": "markdown", "value": "```rescript\narray>\n```"}}], + "documentation": {"kind": "markdown", "value": " Three is... three "} + }], + "activeSignature": 0, + "activeParameter": 1 +} + diff --git a/package.json b/package.json index fecc29e22..6ef2cb0a7 100644 --- a/package.json +++ b/package.json @@ -176,6 +176,11 @@ "default": true, "description": "Enable signature help for function calls." }, + "rescript.settings.signatureHelp.forConstructorPayloads": { + "type": "boolean", + "default": true, + "description": "Enable signature help for variant constructor payloads." + }, "rescript.settings.incrementalTypechecking.enabled": { "type": "boolean", "default": false, diff --git a/server/src/config.ts b/server/src/config.ts index 57ec3c88c..9949d6bde 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -3,22 +3,23 @@ import { Message } from "vscode-languageserver-protocol"; export type send = (msg: Message) => void; export interface extensionConfiguration { - allowBuiltInFormatter: boolean; - askToStartBuild: boolean; + allowBuiltInFormatter?: boolean; + askToStartBuild?: boolean; inlayHints?: { - enable: boolean; - maxLength: number | null; + enable?: boolean; + maxLength?: number | null; }; - codeLens: boolean; - binaryPath: string | null; - platformPath: string | null; + codeLens?: boolean; + binaryPath?: string | null; + platformPath?: string | null; signatureHelp?: { - enabled: boolean; + enabled?: boolean; + forConstructorPayloads?: boolean; }; incrementalTypechecking?: { - enabled: boolean; - acrossFiles: boolean; - debugLogging: boolean; + enabled?: boolean; + acrossFiles?: boolean; + debugLogging?: boolean; }; } @@ -37,6 +38,7 @@ let config: { extensionConfiguration: extensionConfiguration } = { platformPath: null, signatureHelp: { enabled: true, + forConstructorPayloads: true, }, incrementalTypechecking: { enabled: false, diff --git a/server/src/server.ts b/server/src/server.ts index c407b9dba..a1864209a 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -469,6 +469,9 @@ function signatureHelp(msg: p.RequestMessage) { params.position.line, params.position.character, tmpname, + config.extensionConfiguration.signatureHelp?.forConstructorPayloads + ? "true" + : "false", ], msg ); @@ -745,7 +748,7 @@ function format(msg: p.RequestMessage): Array { bscExeBinaryPath, filePath, code, - config.extensionConfiguration.allowBuiltInFormatter + Boolean(config.extensionConfiguration.allowBuiltInFormatter) ); if (formattedResult.kind === "success") { let max = code.length; From 6474ca540495d402f6d9427d79e55d0b1f3653d9 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sat, 9 Mar 2024 13:06:11 +0100 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cde953555..661991ce4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ ## master +#### :rocket: New Feature + +- Extend signature help to work on constructor payloads as well. Can be turned off if wanted through settings. + ## 1.48.0 #### :bug: Bug Fix From 7aed8447b9ac569757c0519b1d747c0c6a402ff4 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sat, 9 Mar 2024 13:06:31 +0100 Subject: [PATCH 3/3] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 661991ce4..549ea5b6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ #### :rocket: New Feature -- Extend signature help to work on constructor payloads as well. Can be turned off if wanted through settings. +- Extend signature help to work on constructor payloads as well. Can be turned off if wanted through settings. https://github.com/rescript-lang/rescript-vscode/pull/947 ## 1.48.0