From 3540d399863dec7f88425779c5266adab58a39a9 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 2 Apr 2023 20:33:52 +0000 Subject: [PATCH 1/7] handle @editor.templateVariableName on types --- analysis/src/CompletionBackEnd.ml | 4 +++- analysis/src/ProcessAttributes.ml | 17 +++++++++++++++++ analysis/src/ProcessCmt.ml | 2 ++ analysis/src/SharedTypes.ml | 7 ++++++- analysis/src/TypeUtils.ml | 13 +++++++++++++ analysis/tests/src/CompletionExpressions.res | 10 ++++++++++ .../src/expected/CompletionExpressions.res.txt | 18 ++++++++++++++++++ 7 files changed, 69 insertions(+), 2 deletions(-) diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index 08e8f8d45..3853f84da 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -1355,7 +1355,9 @@ let rec completeTypedValue ~full ~prefix ~completionContext ~mode match Path.head p |> Ident.name with | "unit" -> "()" | "ReactEvent" | "JsxEvent" -> "event" - | _ -> "v" ^ indexText) + | _ -> + TypeUtils.templateVarNameForTyp ~env ~package:full.package argTyp + ^ indexText) in let mkFnArgs ~asSnippet = match args with diff --git a/analysis/src/ProcessAttributes.ml b/analysis/src/ProcessAttributes.ml index c610d1e79..f7cdae066 100644 --- a/analysis/src/ProcessAttributes.ml +++ b/analysis/src/ProcessAttributes.ml @@ -17,6 +17,23 @@ let rec findDocAttribute attributes = Some doc | _ :: rest -> findDocAttribute rest +let rec findTemplateVarNameAttribute attributes = + let open Parsetree in + match attributes with + | [] -> None + | ( {Asttypes.txt = "editor.templateVariableName"}, + PStr + [ + { + pstr_desc = + Pstr_eval + ({pexp_desc = Pexp_constant (Pconst_string (name, _))}, _); + }; + ] ) + :: _ -> + Some name + | _ :: rest -> findTemplateVarNameAttribute rest + let rec findDeprecatedAttribute attributes = let open Parsetree in match attributes with 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 df2a36e2f..831f54749 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 4911e4abb..589d6d941 100644 --- a/analysis/src/TypeUtils.ml +++ b/analysis/src/TypeUtils.ml @@ -202,6 +202,19 @@ let pathFromTypeExpr (t : Types.type_expr) = Some path | _ -> None +let rec templateVarNameForTyp ~env ~package (t : Types.type_expr) = + match t.desc with + | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> + templateVarNameForTyp ~env ~package t1 + | Tconstr (path, _, _) -> ( + match References.digConstructor ~env ~package path with + | Some (_env, {item = {attributes}}) -> ( + match ProcessAttributes.findTemplateVarNameAttribute attributes with + | None -> "v" + | Some name -> name) + | _ -> "v") + | _ -> "v" + let rec resolveTypeForPipeCompletion ~env ~package ~lhsLoc ~full (t : Types.type_expr) = let builtin = diff --git a/analysis/tests/src/CompletionExpressions.res b/analysis/tests/src/CompletionExpressions.res index 03d26c53d..f8b176c74 100644 --- a/analysis/tests/src/CompletionExpressions.res +++ b/analysis/tests/src/CompletionExpressions.res @@ -196,3 +196,13 @@ ignore(fff) // fff.someOpt // ^com + +@editor.templateVariableName("someTyp") +type someTyp = {test: bool} + +let takesCb = cb => { + cb({test: true}) +} + +// takesCb() +// ^com diff --git a/analysis/tests/src/expected/CompletionExpressions.res.txt b/analysis/tests/src/expected/CompletionExpressions.res.txt index cb80dfcb7..764769c18 100644 --- a/analysis/tests/src/expected/CompletionExpressions.res.txt +++ b/analysis/tests/src/expected/CompletionExpressions.res.txt @@ -951,3 +951,21 @@ Path fff "documentation": null }] +Complete src/CompletionExpressions.res 206:11 +posCursor:[206:11] posNoWhite:[206:10] Found expr:[206:3->206:12] +Pexp_apply ...[206:3->206:10] (...[206:11->206:12]) +Completable: Cexpression CArgument Value[takesCb]($0) +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 + }] + From 3585b4529f275f5f85fa70ce358a910f89abe0cd Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 3 Apr 2023 12:08:30 +0000 Subject: [PATCH 2/7] better logic for inferring template variable names --- analysis/src/CompletionBackEnd.ml | 18 +++++- analysis/src/TypeUtils.ml | 8 +-- analysis/src/Utils.ml | 9 +++ analysis/tests/src/CompletionExpressions.res | 32 ++++++++++ .../expected/CompletionExpressions.res.txt | 58 ++++++++++++++++++- 5 files changed, 116 insertions(+), 9 deletions(-) diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index 3853f84da..d1a93f216 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -1356,8 +1356,22 @@ let rec completeTypedValue ~full ~prefix ~completionContext ~mode | "unit" -> "()" | "ReactEvent" | "JsxEvent" -> "event" | _ -> - TypeUtils.templateVarNameForTyp ~env ~package:full.package argTyp - ^ indexText) + let txt = + match + TypeUtils.templateVarNameForTyp ~env ~package:full.package argTyp + with + | Some txt -> txt + | None -> ( + match p |> Utils.expandPath |> Utils.lastElements with + | [] | ["t"] -> "v" + | [someName; "t"] | [_; someName] | [someName] -> ( + match someName with + | "string" | "int" | "float" | "array" | "option" | "bool" -> + "v" + | someName -> someName |> Utils.lowercaseFirstChar) + | _ -> "v") + in + txt ^ indexText) in let mkFnArgs ~asSnippet = match args with diff --git a/analysis/src/TypeUtils.ml b/analysis/src/TypeUtils.ml index 589d6d941..b87a93334 100644 --- a/analysis/src/TypeUtils.ml +++ b/analysis/src/TypeUtils.ml @@ -209,11 +209,9 @@ let rec templateVarNameForTyp ~env ~package (t : Types.type_expr) = | Tconstr (path, _, _) -> ( match References.digConstructor ~env ~package path with | Some (_env, {item = {attributes}}) -> ( - match ProcessAttributes.findTemplateVarNameAttribute attributes with - | None -> "v" - | Some name -> name) - | _ -> "v") - | _ -> "v" + ProcessAttributes.findTemplateVarNameAttribute attributes) + | _ -> None) + | _ -> None let rec resolveTypeForPipeCompletion ~env ~package ~lhsLoc ~full (t : Types.type_expr) = diff --git a/analysis/src/Utils.ml b/analysis/src/Utils.ml index ff3e0fc43..ecac10709 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 f8b176c74..9a675e921 100644 --- a/analysis/tests/src/CompletionExpressions.res +++ b/analysis/tests/src/CompletionExpressions.res @@ -206,3 +206,35 @@ let takesCb = cb => { // 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 + +module RecordSourceSelectorProxy = { + @editor.templateVariableName("store") + 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 764769c18..d1741e400 100644 --- a/analysis/tests/src/expected/CompletionExpressions.res.txt +++ b/analysis/tests/src/expected/CompletionExpressions.res.txt @@ -845,13 +845,13 @@ ContextPath CArgument Value[fnTakingCallback]($3) ContextPath Value[fnTakingCallback] Path fnTakingCallback [{ - "label": "(~on, ~off=?, v1) => {}", + "label": "(~on, ~off=?, variant1) => {}", "kind": 12, "tags": [], "detail": "(~on: bool, ~off: bool=?, variant) => int", "documentation": null, "sortText": "A", - "insertText": "(~on, ~off=?, ${1:v1}) => {$0}", + "insertText": "(~on, ~off=?, ${1:variant1}) => {$0}", "insertTextFormat": 2 }] @@ -969,3 +969,57 @@ Path takesCb "insertTextFormat": 2 }] +Complete src/CompletionExpressions.res 217:12 +posCursor:[217:12] posNoWhite:[217:11] Found expr:[217:3->217:13] +Pexp_apply ...[217:3->217:11] (...[217:12->217:13]) +Completable: Cexpression CArgument Value[takesCb2]($0) +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 226:12 +posCursor:[226:12] posNoWhite:[226:11] Found expr:[226:3->226:13] +Pexp_apply ...[226:3->226:11] (...[226:12->226:13]) +Completable: Cexpression CArgument Value[takesCb3]($0) +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 238:30 +posCursor:[238:30] posNoWhite:[238:29] Found expr:[238:3->238:31] +Pexp_apply ...[238:3->238:20] (~updater238:22->238:29=...__ghost__[0:-1->0:-1]) +Completable: Cexpression CArgument Value[commitLocalUpdate](~updater) +ContextPath CArgument Value[commitLocalUpdate](~updater) +ContextPath Value[commitLocalUpdate] +Path commitLocalUpdate +[{ + "label": "store => {}", + "kind": 12, + "tags": [], + "detail": "RecordSourceSelectorProxy.t => unit", + "documentation": null, + "sortText": "A", + "insertText": "${1:store} => {$0}", + "insertTextFormat": 2 + }] + From 1799bc41535cbb72e280d8e4d95f2bd501cacba7 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 3 Apr 2023 12:23:28 +0000 Subject: [PATCH 3/7] restructure result some --- analysis/src/CompletionBackEnd.ml | 15 +++++++++------ .../src/expected/CompletionExpressions.res.txt | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index d1a93f216..2e1ed6d43 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -1356,22 +1356,25 @@ let rec completeTypedValue ~full ~prefix ~completionContext ~mode | "unit" -> "()" | "ReactEvent" | "JsxEvent" -> "event" | _ -> + let defaultVarName = "v" ^ indexText in let txt = match TypeUtils.templateVarNameForTyp ~env ~package:full.package argTyp with | Some txt -> txt | None -> ( - match p |> Utils.expandPath |> Utils.lastElements with - | [] | ["t"] -> "v" + match p |> Utils.expandPath |> List.rev |> Utils.lastElements with + | [] | ["t"] -> defaultVarName | [someName; "t"] | [_; someName] | [someName] -> ( match someName with | "string" | "int" | "float" | "array" | "option" | "bool" -> - "v" - | someName -> someName |> Utils.lowercaseFirstChar) - | _ -> "v") + defaultVarName + | someName when String.length someName < 30 -> + someName |> Utils.lowercaseFirstChar + | _ -> defaultVarName) + | _ -> defaultVarName) in - txt ^ indexText) + txt) in let mkFnArgs ~asSnippet = match args with diff --git a/analysis/tests/src/expected/CompletionExpressions.res.txt b/analysis/tests/src/expected/CompletionExpressions.res.txt index d1741e400..d34ed9822 100644 --- a/analysis/tests/src/expected/CompletionExpressions.res.txt +++ b/analysis/tests/src/expected/CompletionExpressions.res.txt @@ -845,13 +845,13 @@ ContextPath CArgument Value[fnTakingCallback]($3) ContextPath Value[fnTakingCallback] Path fnTakingCallback [{ - "label": "(~on, ~off=?, variant1) => {}", + "label": "(~on, ~off=?, variant) => {}", "kind": 12, "tags": [], "detail": "(~on: bool, ~off: bool=?, variant) => int", "documentation": null, "sortText": "A", - "insertText": "(~on, ~off=?, ${1:variant1}) => {$0}", + "insertText": "(~on, ~off=?, ${1:variant}) => {$0}", "insertTextFormat": 2 }] From 1e1f4ba7f9ccbc5088cf0bd61c354b05794627d9 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 3 Apr 2023 18:23:19 +0000 Subject: [PATCH 4/7] tweak logic some and break out functionality to own function --- analysis/src/CompletionBackEnd.ml | 43 +++---------------- analysis/src/CompletionExpressions.ml | 36 ++++++++++++++++ analysis/src/TypeUtils.ml | 20 ++++++++- analysis/src/Utils.ml | 2 +- analysis/tests/src/CompletionExpressions.res | 14 ++++++ .../expected/CompletionExpressions.res.txt | 42 ++++++++++++++++-- 6 files changed, 115 insertions(+), 42 deletions(-) diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index 2e1ed6d43..1cbd2c63d 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -1342,45 +1342,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" - | _ -> - let defaultVarName = "v" ^ indexText in - let txt = - match - TypeUtils.templateVarNameForTyp ~env ~package:full.package argTyp - with - | Some txt -> txt - | None -> ( - match p |> Utils.expandPath |> List.rev |> Utils.lastElements with - | [] | ["t"] -> defaultVarName - | [someName; "t"] | [_; someName] | [someName] -> ( - match someName with - | "string" | "int" | "float" | "array" | "option" | "bool" -> - defaultVarName - | someName when String.length someName < 30 -> - someName |> Utils.lowercaseFirstChar - | _ -> defaultVarName) - | _ -> defaultVarName) - in - txt) - 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 @@ -1395,7 +1363,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..7510f7e7a 100644 --- a/analysis/src/CompletionExpressions.ml +++ b/analysis/src/CompletionExpressions.ml @@ -216,3 +216,39 @@ 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 TypeUtils.templateVarNameForTyp ~env ~package:full.package argTyp with + | Some txt -> txt + | None -> ( + match argTyp |> TypeUtils.pathFromTypeExpr with + | None -> defaultVarName + | Some p -> ( + match p |> Utils.expandPath |> List.rev |> Utils.lastElements with + | [] | ["t"] -> defaultVarName + | ["unit"] -> "()" + (* Special treatment for JsxEvent, since that's a common enough thing + used in event handlers. *) + | ["JsxEvent"; "synthetic"] -> "event" + | ["synthetic"] when env.file.moduleName = "JsxEvent" -> "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 -> + (* 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/TypeUtils.ml b/analysis/src/TypeUtils.ml index b87a93334..87fa82788 100644 --- a/analysis/src/TypeUtils.ml +++ b/analysis/src/TypeUtils.ml @@ -208,11 +208,27 @@ let rec templateVarNameForTyp ~env ~package (t : Types.type_expr) = templateVarNameForTyp ~env ~package t1 | Tconstr (path, _, _) -> ( match References.digConstructor ~env ~package path with - | Some (_env, {item = {attributes}}) -> ( - ProcessAttributes.findTemplateVarNameAttribute attributes) + | Some (_env, {item = {attributes}}) -> + ProcessAttributes.findTemplateVarNameAttribute attributes | _ -> None) | _ -> 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 ecac10709..0ef9e3c7e 100644 --- a/analysis/src/Utils.ml +++ b/analysis/src/Utils.ml @@ -207,7 +207,7 @@ end let rec lastElements list = match list with - | ([_] | [_; _] | []) as res -> res + | ([_; _] | [_] | []) as res -> res | _ :: tl -> lastElements tl let lowercaseFirstChar s = diff --git a/analysis/tests/src/CompletionExpressions.res b/analysis/tests/src/CompletionExpressions.res index 9a675e921..22eb6dbdb 100644 --- a/analysis/tests/src/CompletionExpressions.res +++ b/analysis/tests/src/CompletionExpressions.res @@ -227,6 +227,20 @@ let takesCb3 = cb => { // takesCb3() // ^com +let takesCb4 = cb => { + cb(Some({hi: true})) +} + +// takesCb4() +// ^com + +let takesCb5 = cb => { + cb([Some({hi: true})]) +} + +// takesCb5() +// ^com + module RecordSourceSelectorProxy = { @editor.templateVariableName("store") type t diff --git a/analysis/tests/src/expected/CompletionExpressions.res.txt b/analysis/tests/src/expected/CompletionExpressions.res.txt index d34ed9822..67b51394f 100644 --- a/analysis/tests/src/expected/CompletionExpressions.res.txt +++ b/analysis/tests/src/expected/CompletionExpressions.res.txt @@ -1005,9 +1005,45 @@ Path takesCb3 "insertTextFormat": 2 }] -Complete src/CompletionExpressions.res 238:30 -posCursor:[238:30] posNoWhite:[238:29] Found expr:[238:3->238:31] -Pexp_apply ...[238:3->238:20] (~updater238:22->238:29=...__ghost__[0:-1->0:-1]) +Complete src/CompletionExpressions.res 233:12 +posCursor:[233:12] posNoWhite:[233:11] Found expr:[233:3->233:13] +Pexp_apply ...[233:3->233:11] (...[233:12->233:13]) +Completable: Cexpression CArgument Value[takesCb4]($0) +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 240:12 +posCursor:[240:12] posNoWhite:[240:11] Found expr:[240:3->240:13] +Pexp_apply ...[240:3->240:11] (...[240:12->240:13]) +Completable: Cexpression CArgument Value[takesCb5]($0) +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 252:30 +posCursor:[252:30] posNoWhite:[252:29] Found expr:[252:3->252:31] +Pexp_apply ...[252:3->252:20] (~updater252:22->252:29=...__ghost__[0:-1->0:-1]) Completable: Cexpression CArgument Value[commitLocalUpdate](~updater) ContextPath CArgument Value[commitLocalUpdate](~updater) ContextPath Value[commitLocalUpdate] From 31d576efd9e97896b5e89408fb7b7cfefbfef7ea Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 4 Apr 2023 10:27:33 +0000 Subject: [PATCH 5/7] clearer example --- analysis/tests/src/CompletionExpressions.res | 2 +- analysis/tests/src/expected/CompletionExpressions.res.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/analysis/tests/src/CompletionExpressions.res b/analysis/tests/src/CompletionExpressions.res index 22eb6dbdb..7262fc26d 100644 --- a/analysis/tests/src/CompletionExpressions.res +++ b/analysis/tests/src/CompletionExpressions.res @@ -197,7 +197,7 @@ ignore(fff) // fff.someOpt // ^com -@editor.templateVariableName("someTyp") +@editor.templateVariableName("otherNameForType") type someTyp = {test: bool} let takesCb = cb => { diff --git a/analysis/tests/src/expected/CompletionExpressions.res.txt b/analysis/tests/src/expected/CompletionExpressions.res.txt index 67b51394f..dbf4163dd 100644 --- a/analysis/tests/src/expected/CompletionExpressions.res.txt +++ b/analysis/tests/src/expected/CompletionExpressions.res.txt @@ -959,13 +959,13 @@ ContextPath CArgument Value[takesCb]($0) ContextPath Value[takesCb] Path takesCb [{ - "label": "someTyp => {}", + "label": "otherNameForType => {}", "kind": 12, "tags": [], "detail": "someTyp => 'a", "documentation": null, "sortText": "A", - "insertText": "${1:someTyp} => {$0}", + "insertText": "${1:otherNameForType} => {$0}", "insertTextFormat": 2 }] From 7a675305c2796e8ee7e9f8fedf39ae828487b055 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 27 Apr 2023 21:23:14 +0200 Subject: [PATCH 6/7] remove attribute for renaming template variable name --- analysis/src/CompletionExpressions.ml | 53 +++++++++--------- analysis/src/ProcessAttributes.ml | 17 ------ analysis/src/TypeUtils.ml | 11 ---- analysis/tests/src/CompletionExpressions.res | 2 - .../expected/CompletionExpressions.res.txt | 56 +++++++++++-------- .../src/expected/CompletionJsxProps.res.txt | 4 +- 6 files changed, 64 insertions(+), 79 deletions(-) diff --git a/analysis/src/CompletionExpressions.ml b/analysis/src/CompletionExpressions.ml index 7510f7e7a..f061a03fa 100644 --- a/analysis/src/CompletionExpressions.ml +++ b/analysis/src/CompletionExpressions.ml @@ -225,30 +225,33 @@ let prettyPrintFnTemplateArgName ?currentIndex ~env ~full | Some i -> string_of_int i in let defaultVarName = "v" ^ indexText in - let argTyp, suffix, env = + let argTyp, suffix, _env = TypeUtils.digToRelevantTemplateNameType ~env ~package:full.package argTyp in - match TypeUtils.templateVarNameForTyp ~env ~package:full.package argTyp with - | Some txt -> txt - | None -> ( - match argTyp |> TypeUtils.pathFromTypeExpr with - | None -> defaultVarName - | Some p -> ( - match p |> Utils.expandPath |> List.rev |> Utils.lastElements with - | [] | ["t"] -> defaultVarName - | ["unit"] -> "()" - (* Special treatment for JsxEvent, since that's a common enough thing - used in event handlers. *) - | ["JsxEvent"; "synthetic"] -> "event" - | ["synthetic"] when env.file.moduleName = "JsxEvent" -> "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 -> - (* 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)) + 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/ProcessAttributes.ml b/analysis/src/ProcessAttributes.ml index f7cdae066..c610d1e79 100644 --- a/analysis/src/ProcessAttributes.ml +++ b/analysis/src/ProcessAttributes.ml @@ -17,23 +17,6 @@ let rec findDocAttribute attributes = Some doc | _ :: rest -> findDocAttribute rest -let rec findTemplateVarNameAttribute attributes = - let open Parsetree in - match attributes with - | [] -> None - | ( {Asttypes.txt = "editor.templateVariableName"}, - PStr - [ - { - pstr_desc = - Pstr_eval - ({pexp_desc = Pexp_constant (Pconst_string (name, _))}, _); - }; - ] ) - :: _ -> - Some name - | _ :: rest -> findTemplateVarNameAttribute rest - let rec findDeprecatedAttribute attributes = let open Parsetree in match attributes with diff --git a/analysis/src/TypeUtils.ml b/analysis/src/TypeUtils.ml index 87fa82788..553359039 100644 --- a/analysis/src/TypeUtils.ml +++ b/analysis/src/TypeUtils.ml @@ -202,17 +202,6 @@ let pathFromTypeExpr (t : Types.type_expr) = Some path | _ -> None -let rec templateVarNameForTyp ~env ~package (t : Types.type_expr) = - match t.desc with - | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> - templateVarNameForTyp ~env ~package t1 - | Tconstr (path, _, _) -> ( - match References.digConstructor ~env ~package path with - | Some (_env, {item = {attributes}}) -> - ProcessAttributes.findTemplateVarNameAttribute attributes - | _ -> None) - | _ -> None - let rec digToRelevantTemplateNameType ~env ~package ?(suffix = "") (t : Types.type_expr) = match t.desc with diff --git a/analysis/tests/src/CompletionExpressions.res b/analysis/tests/src/CompletionExpressions.res index 7262fc26d..d61259101 100644 --- a/analysis/tests/src/CompletionExpressions.res +++ b/analysis/tests/src/CompletionExpressions.res @@ -197,7 +197,6 @@ ignore(fff) // fff.someOpt // ^com -@editor.templateVariableName("otherNameForType") type someTyp = {test: bool} let takesCb = cb => { @@ -242,7 +241,6 @@ let takesCb5 = cb => { // ^com module RecordSourceSelectorProxy = { - @editor.templateVariableName("store") type t } diff --git a/analysis/tests/src/expected/CompletionExpressions.res.txt b/analysis/tests/src/expected/CompletionExpressions.res.txt index dbf4163dd..d3cb9d304 100644 --- a/analysis/tests/src/expected/CompletionExpressions.res.txt +++ b/analysis/tests/src/expected/CompletionExpressions.res.txt @@ -951,28 +951,32 @@ Path fff "documentation": null }] -Complete src/CompletionExpressions.res 206:11 -posCursor:[206:11] posNoWhite:[206:10] Found expr:[206:3->206:12] -Pexp_apply ...[206:3->206:10] (...[206:11->206:12]) +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": "otherNameForType => {}", + "label": "someTyp => {}", "kind": 12, "tags": [], "detail": "someTyp => 'a", "documentation": null, "sortText": "A", - "insertText": "${1:otherNameForType} => {$0}", + "insertText": "${1:someTyp} => {$0}", "insertTextFormat": 2 }] -Complete src/CompletionExpressions.res 217:12 -posCursor:[217:12] posNoWhite:[217:11] Found expr:[217:3->217:13] -Pexp_apply ...[217:3->217:11] (...[217:12->217:13]) +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 @@ -987,10 +991,12 @@ Path takesCb2 "insertTextFormat": 2 }] -Complete src/CompletionExpressions.res 226:12 -posCursor:[226:12] posNoWhite:[226:11] Found expr:[226:3->226:13] -Pexp_apply ...[226:3->226:11] (...[226:12->226:13]) +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 @@ -1005,10 +1011,12 @@ Path takesCb3 "insertTextFormat": 2 }] -Complete src/CompletionExpressions.res 233:12 -posCursor:[233:12] posNoWhite:[233:11] Found expr:[233:3->233:13] -Pexp_apply ...[233:3->233:11] (...[233:12->233:13]) +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 @@ -1023,10 +1031,12 @@ Path takesCb4 "insertTextFormat": 2 }] -Complete src/CompletionExpressions.res 240:12 -posCursor:[240:12] posNoWhite:[240:11] Found expr:[240:3->240:13] -Pexp_apply ...[240:3->240:11] (...[240:12->240:13]) +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 @@ -1041,21 +1051,23 @@ Path takesCb5 "insertTextFormat": 2 }] -Complete src/CompletionExpressions.res 252:30 -posCursor:[252:30] posNoWhite:[252:29] Found expr:[252:3->252:31] -Pexp_apply ...[252:3->252:20] (~updater252:22->252:29=...__ghost__[0:-1->0:-1]) +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": "store => {}", + "label": "recordSourceSelectorProxy => {}", "kind": 12, "tags": [], "detail": "RecordSourceSelectorProxy.t => unit", "documentation": null, "sortText": "A", - "insertText": "${1:store} => {$0}", + "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 }] From ad14ad0df270952057ae2faa7f8de9459c57a13b Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 27 Apr 2023 21:24:12 +0200 Subject: [PATCH 7/7] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1e5a6240..3038a91a2 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 ## 1.16.0