diff --git a/CHANGELOG.md b/CHANGELOG.md index 72788e11b..73ce577e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ #### :rocket: New Feature - Docstring template Code Action. https://github.com/rescript-lang/rescript-vscode/pull/764 +- Improve unlabelled argument names in completion function templates. https://github.com/rescript-lang/rescript-vscode/pull/754 - Add `Some(fieldName)` case when completing in a pattern with an option on a record field. https://github.com/rescript-lang/rescript-vscode/pull/766 #### :bug: Bug Fix diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index f5316319d..c20b6fe26 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -1355,26 +1355,13 @@ let rec completeTypedValue ~full ~prefix ~completionContext ~mode ] else [] | Tfunction {env; typ; args} when prefix = "" && mode = Expression -> - let prettyPrintArgTyp ?currentIndex (argTyp : Types.type_expr) = - let indexText = - match currentIndex with - | None -> "" - | Some i -> string_of_int i - in - match argTyp |> TypeUtils.pathFromTypeExpr with - | None -> "v" ^ indexText - | Some p -> ( - (* Pretty print a few common patterns. *) - match Path.head p |> Ident.name with - | "unit" -> "()" - | "ReactEvent" | "JsxEvent" -> "event" - | _ -> "v" ^ indexText) - in let mkFnArgs ~asSnippet = match args with | [(Nolabel, argTyp)] when TypeUtils.typeIsUnit argTyp -> "()" | [(Nolabel, argTyp)] -> - let varName = prettyPrintArgTyp argTyp in + let varName = + CompletionExpressions.prettyPrintFnTemplateArgName ~env ~full argTyp + in if asSnippet then "${1:" ^ varName ^ "}" else varName | _ -> let currentUnlabelledIndex = ref 0 in @@ -1389,7 +1376,10 @@ let rec completeTypedValue ~full ~prefix ~completionContext ~mode else ( currentUnlabelledIndex := !currentUnlabelledIndex + 1; let num = !currentUnlabelledIndex in - let varName = prettyPrintArgTyp typ ~currentIndex:num in + let varName = + CompletionExpressions.prettyPrintFnTemplateArgName + ~currentIndex:num ~env ~full typ + in if asSnippet then "${" ^ string_of_int num ^ ":" ^ varName ^ "}" else varName)) diff --git a/analysis/src/CompletionExpressions.ml b/analysis/src/CompletionExpressions.ml index 1c91dad11..f061a03fa 100644 --- a/analysis/src/CompletionExpressions.ml +++ b/analysis/src/CompletionExpressions.ml @@ -216,3 +216,42 @@ and traverseExprTupleItems tupleItems ~nextExprPath ~resultFromFoundItemNum ~pos if pos >= Loc.start e.Parsetree.pexp_loc then posNum := index); if !posNum > -1 then Some ("", resultFromFoundItemNum !posNum) else None | v, _ -> v + +let prettyPrintFnTemplateArgName ?currentIndex ~env ~full + (argTyp : Types.type_expr) = + let indexText = + match currentIndex with + | None -> "" + | Some i -> string_of_int i + in + let defaultVarName = "v" ^ indexText in + let argTyp, suffix, _env = + TypeUtils.digToRelevantTemplateNameType ~env ~package:full.package argTyp + in + match argTyp |> TypeUtils.pathFromTypeExpr with + | None -> defaultVarName + | Some p -> ( + let trailingElementsOfPath = + p |> Utils.expandPath |> List.rev |> Utils.lastElements + in + match trailingElementsOfPath with + | [] | ["t"] -> defaultVarName + | ["unit"] -> "()" + (* Special treatment for JsxEvent, since that's a common enough thing + used in event handlers. *) + | ["JsxEvent"; "synthetic"] -> "event" + | ["synthetic"] -> "event" + (* Ignore `t` types, and go for its module name instead. *) + | [someName; "t"] | [_; someName] | [someName] -> ( + match someName with + | "string" | "int" | "float" | "array" | "option" | "bool" -> + defaultVarName + | someName when String.length someName < 30 -> + if someName = "synthetic" then + Printf.printf "synthetic! %s\n" + (trailingElementsOfPath |> SharedTypes.ident); + (* We cap how long the name can be, so we don't end up with super + long type names. *) + (someName |> Utils.lowercaseFirstChar) ^ suffix + | _ -> defaultVarName) + | _ -> defaultVarName) diff --git a/analysis/src/ProcessCmt.ml b/analysis/src/ProcessCmt.ml index 329af53fc..c7ecd8525 100644 --- a/analysis/src/ProcessCmt.ml +++ b/analysis/src/ProcessCmt.ml @@ -65,6 +65,7 @@ let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t) ~item: { Type.decl; + attributes = type_attributes; name = name.txt; kind = (match type_kind with @@ -172,6 +173,7 @@ let forTypeDeclaration ~env ~(exported : Exported.t) ~item: { Type.decl = typ_type; + attributes = typ_attributes; name = name.txt; kind = (match typ_kind with diff --git a/analysis/src/SharedTypes.ml b/analysis/src/SharedTypes.ml index 13c64d59f..b47343f89 100644 --- a/analysis/src/SharedTypes.ml +++ b/analysis/src/SharedTypes.ml @@ -59,7 +59,12 @@ module Type = struct | Record of field list | Variant of Constructor.t list - type t = {kind: kind; decl: Types.type_declaration; name: string} + type t = { + kind: kind; + decl: Types.type_declaration; + name: string; + attributes: Parsetree.attributes; + } end module Exported = struct diff --git a/analysis/src/TypeUtils.ml b/analysis/src/TypeUtils.ml index 43377fdcf..bf1da267f 100644 --- a/analysis/src/TypeUtils.ml +++ b/analysis/src/TypeUtils.ml @@ -202,6 +202,22 @@ let pathFromTypeExpr (t : Types.type_expr) = Some path | _ -> None +let rec digToRelevantTemplateNameType ~env ~package ?(suffix = "") + (t : Types.type_expr) = + match t.desc with + | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> + digToRelevantTemplateNameType ~suffix ~env ~package t1 + | Tconstr (Path.Pident {name = "option"}, [t1], _) -> + digToRelevantTemplateNameType ~suffix ~env ~package t1 + | Tconstr (Path.Pident {name = "array"}, [t1], _) -> + digToRelevantTemplateNameType ~suffix:"s" ~env ~package t1 + | Tconstr (path, _, _) -> ( + match References.digConstructor ~env ~package path with + | Some (env, {item = {decl = {type_manifest = Some typ}}}) -> + digToRelevantTemplateNameType ~suffix ~env ~package typ + | _ -> (t, suffix, env)) + | _ -> (t, suffix, env) + let rec resolveTypeForPipeCompletion ~env ~package ~lhsLoc ~full (t : Types.type_expr) = let builtin = diff --git a/analysis/src/Utils.ml b/analysis/src/Utils.ml index ff3e0fc43..0ef9e3c7e 100644 --- a/analysis/src/Utils.ml +++ b/analysis/src/Utils.ml @@ -204,3 +204,12 @@ module Option = struct | None -> None | Some v -> f v end + +let rec lastElements list = + match list with + | ([_; _] | [_] | []) as res -> res + | _ :: tl -> lastElements tl + +let lowercaseFirstChar s = + if String.length s = 0 then s + else String.mapi (fun i c -> if i = 0 then Char.lowercase_ascii c else c) s diff --git a/analysis/tests/src/CompletionExpressions.res b/analysis/tests/src/CompletionExpressions.res index 03d26c53d..d61259101 100644 --- a/analysis/tests/src/CompletionExpressions.res +++ b/analysis/tests/src/CompletionExpressions.res @@ -196,3 +196,57 @@ ignore(fff) // fff.someOpt // ^com + +type someTyp = {test: bool} + +let takesCb = cb => { + cb({test: true}) +} + +// takesCb() +// ^com + +module Environment = { + type t = {hello: bool} +} + +let takesCb2 = cb => { + cb({Environment.hello: true}) +} + +// takesCb2() +// ^com + +type apiCallResult = {hi: bool} + +let takesCb3 = cb => { + cb({hi: true}) +} + +// takesCb3() +// ^com + +let takesCb4 = cb => { + cb(Some({hi: true})) +} + +// takesCb4() +// ^com + +let takesCb5 = cb => { + cb([Some({hi: true})]) +} + +// takesCb5() +// ^com + +module RecordSourceSelectorProxy = { + type t +} + +@val +external commitLocalUpdate: (~updater: RecordSourceSelectorProxy.t => unit) => unit = + "commitLocalUpdate" + +// commitLocalUpdate(~updater=) +// ^com diff --git a/analysis/tests/src/expected/CompletionExpressions.res.txt b/analysis/tests/src/expected/CompletionExpressions.res.txt index 89c0c3013..2be6098fd 100644 --- a/analysis/tests/src/expected/CompletionExpressions.res.txt +++ b/analysis/tests/src/expected/CompletionExpressions.res.txt @@ -848,13 +848,13 @@ ContextPath CArgument Value[fnTakingCallback]($3) ContextPath Value[fnTakingCallback] Path fnTakingCallback [{ - "label": "(~on, ~off=?, v1) => {}", + "label": "(~on, ~off=?, variant) => {}", "kind": 12, "tags": [], "detail": "(~on: bool, ~off: bool=?, variant) => int", "documentation": null, "sortText": "A", - "insertText": "(~on, ~off=?, ${1:v1}) => {$0}", + "insertText": "(~on, ~off=?, ${1:variant}) => {$0}", "insertTextFormat": 2 }] @@ -954,3 +954,123 @@ Path fff "documentation": null }] +Complete src/CompletionExpressions.res 205:11 +posCursor:[205:11] posNoWhite:[205:10] Found expr:[205:3->205:12] +Pexp_apply ...[205:3->205:10] (...[205:11->205:12]) +Completable: Cexpression CArgument Value[takesCb]($0) +Package opens Pervasives.JsxModules.place holder +Resolved opens 1 pervasives +ContextPath CArgument Value[takesCb]($0) +ContextPath Value[takesCb] +Path takesCb +[{ + "label": "someTyp => {}", + "kind": 12, + "tags": [], + "detail": "someTyp => 'a", + "documentation": null, + "sortText": "A", + "insertText": "${1:someTyp} => {$0}", + "insertTextFormat": 2 + }] + +Complete src/CompletionExpressions.res 216:12 +posCursor:[216:12] posNoWhite:[216:11] Found expr:[216:3->216:13] +Pexp_apply ...[216:3->216:11] (...[216:12->216:13]) +Completable: Cexpression CArgument Value[takesCb2]($0) +Package opens Pervasives.JsxModules.place holder +Resolved opens 1 pervasives +ContextPath CArgument Value[takesCb2]($0) +ContextPath Value[takesCb2] +Path takesCb2 +[{ + "label": "environment => {}", + "kind": 12, + "tags": [], + "detail": "Environment.t => 'a", + "documentation": null, + "sortText": "A", + "insertText": "${1:environment} => {$0}", + "insertTextFormat": 2 + }] + +Complete src/CompletionExpressions.res 225:12 +posCursor:[225:12] posNoWhite:[225:11] Found expr:[225:3->225:13] +Pexp_apply ...[225:3->225:11] (...[225:12->225:13]) +Completable: Cexpression CArgument Value[takesCb3]($0) +Package opens Pervasives.JsxModules.place holder +Resolved opens 1 pervasives +ContextPath CArgument Value[takesCb3]($0) +ContextPath Value[takesCb3] +Path takesCb3 +[{ + "label": "apiCallResult => {}", + "kind": 12, + "tags": [], + "detail": "apiCallResult => 'a", + "documentation": null, + "sortText": "A", + "insertText": "${1:apiCallResult} => {$0}", + "insertTextFormat": 2 + }] + +Complete src/CompletionExpressions.res 232:12 +posCursor:[232:12] posNoWhite:[232:11] Found expr:[232:3->232:13] +Pexp_apply ...[232:3->232:11] (...[232:12->232:13]) +Completable: Cexpression CArgument Value[takesCb4]($0) +Package opens Pervasives.JsxModules.place holder +Resolved opens 1 pervasives +ContextPath CArgument Value[takesCb4]($0) +ContextPath Value[takesCb4] +Path takesCb4 +[{ + "label": "apiCallResult => {}", + "kind": 12, + "tags": [], + "detail": "option => 'a", + "documentation": null, + "sortText": "A", + "insertText": "${1:apiCallResult} => {$0}", + "insertTextFormat": 2 + }] + +Complete src/CompletionExpressions.res 239:12 +posCursor:[239:12] posNoWhite:[239:11] Found expr:[239:3->239:13] +Pexp_apply ...[239:3->239:11] (...[239:12->239:13]) +Completable: Cexpression CArgument Value[takesCb5]($0) +Package opens Pervasives.JsxModules.place holder +Resolved opens 1 pervasives +ContextPath CArgument Value[takesCb5]($0) +ContextPath Value[takesCb5] +Path takesCb5 +[{ + "label": "apiCallResults => {}", + "kind": 12, + "tags": [], + "detail": "array> => 'a", + "documentation": null, + "sortText": "A", + "insertText": "${1:apiCallResults} => {$0}", + "insertTextFormat": 2 + }] + +Complete src/CompletionExpressions.res 250:30 +posCursor:[250:30] posNoWhite:[250:29] Found expr:[250:3->250:31] +Pexp_apply ...[250:3->250:20] (~updater250:22->250:29=...__ghost__[0:-1->0:-1]) +Completable: Cexpression CArgument Value[commitLocalUpdate](~updater) +Package opens Pervasives.JsxModules.place holder +Resolved opens 1 pervasives +ContextPath CArgument Value[commitLocalUpdate](~updater) +ContextPath Value[commitLocalUpdate] +Path commitLocalUpdate +[{ + "label": "recordSourceSelectorProxy => {}", + "kind": 12, + "tags": [], + "detail": "RecordSourceSelectorProxy.t => unit", + "documentation": null, + "sortText": "A", + "insertText": "${1:recordSourceSelectorProxy} => {$0}", + "insertTextFormat": 2 + }] + diff --git a/analysis/tests/src/expected/CompletionJsxProps.res.txt b/analysis/tests/src/expected/CompletionJsxProps.res.txt index e65d45841..475fbb2cf 100644 --- a/analysis/tests/src/expected/CompletionJsxProps.res.txt +++ b/analysis/tests/src/expected/CompletionJsxProps.res.txt @@ -185,13 +185,13 @@ ContextPath CJsxPropValue [div] onMouseEnter Path ReactDOM.domProps Path Pervasives.JsxDOM.domProps [{ - "label": "v => {}", + "label": "event => {}", "kind": 12, "tags": [], "detail": "JsxEventC.Mouse.t => unit", "documentation": null, "sortText": "A", - "insertText": "{${1:v} => {$0}}", + "insertText": "{${1:event} => {$0}}", "insertTextFormat": 2 }]