From 2745e7f8fd9c3ce73a1030e214ab9a9c6dcd86dc Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Jan 2023 22:58:57 +0100 Subject: [PATCH 1/4] add broken test for pipe completion on aliased types --- analysis/tests/src/CompletionPipeChain.res | 11 ++++++++ .../src/expected/CompletionPipeChain.res.txt | 28 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/analysis/tests/src/CompletionPipeChain.res b/analysis/tests/src/CompletionPipeChain.res index f9bcd2345..813bc1a85 100644 --- a/analysis/tests/src/CompletionPipeChain.res +++ b/analysis/tests/src/CompletionPipeChain.res @@ -62,3 +62,14 @@ let _ = [123]->Js.Array2.forEach(v => Js.log(v)) let _ = [123]->Belt.Array.reduce(0, (acc, curr) => acc + curr) // ->t // ^com + +type aliasedType = CompletionSupport.Test.t + +let aliased: aliasedType = {name: 123} +let notAliased: CompletionSupport.Test.t = {name: 123} + +// aliased-> +// ^com + +// notAliased-> +// ^com diff --git a/analysis/tests/src/expected/CompletionPipeChain.res.txt b/analysis/tests/src/expected/CompletionPipeChain.res.txt index c7cff492f..b9e219f6c 100644 --- a/analysis/tests/src/expected/CompletionPipeChain.res.txt +++ b/analysis/tests/src/expected/CompletionPipeChain.res.txt @@ -256,3 +256,31 @@ Completable: Cpath Value[Belt, Array, reduce](Nolabel, Nolabel, Nolabel)->t "documentation": {"kind": "markdown", "value": "\n Converts a given `int` to a `float`.\n\n ```res example\n Js.log(Belt.Int.toFloat(1) === 1.0) /* true */\n ```\n"} }] +Complete src/CompletionPipeChain.res 70:12 +posCursor:[70:12] posNoWhite:[70:11] Found expr:[70:3->0:-1] +Completable: Cpath Value[aliased]-> +[] + +Complete src/CompletionPipeChain.res 73:15 +posCursor:[73:15] posNoWhite:[73:14] Found expr:[73:3->0:-1] +Completable: Cpath Value[notAliased]-> +[{ + "label": "CompletionSupport.Test.add", + "kind": 12, + "tags": [], + "detail": "t => int", + "documentation": null + }, { + "label": "CompletionSupport.Test.addSelf", + "kind": 12, + "tags": [], + "detail": "t => t", + "documentation": null + }, { + "label": "CompletionSupport.Test.make", + "kind": 12, + "tags": [], + "detail": "int => t", + "documentation": null + }] + From 10a3077b0e5582154e104790a451fb31bba16f06 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 17 Jan 2023 18:56:54 +0100 Subject: [PATCH 2/4] one step forward (and one backward) in resolving aliases in pipe completion --- analysis/src/CompletionBackEnd.ml | 4 ++++ analysis/src/TypeUtils.ml | 13 ++++++++++++ .../tests/src/expected/Completion.res.txt | 14 +------------ .../src/expected/CompletionPipeChain.res.txt | 20 ++++++++++++++++++- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index fab77ce18..20de55fe6 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -720,6 +720,10 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env |> completionsGetTypeEnv with | Some (typ, envFromCompletionItem) -> ( + let env, typ = + typ |> TypeUtils.resolveTypeForPipeCompletion ~env ~package + in + (* If the type we're completing on is a type parameter, we won't be able to do completion unless we know what that type parameter is compiled as. This attempts to look up the compiled type for that type parameter by looking diff --git a/analysis/src/TypeUtils.ml b/analysis/src/TypeUtils.ml index c94041cfa..4424e34a7 100644 --- a/analysis/src/TypeUtils.ml +++ b/analysis/src/TypeUtils.ml @@ -148,6 +148,19 @@ let rec extractType ~env ~package (t : Types.type_expr) = Some (Tpolyvariant {env; constructors; typeExpr = t}) | _ -> None +let rec resolveTypeForPipeCompletion ~env ~package (t : Types.type_expr) = + match t.desc with + | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> + resolveTypeForPipeCompletion ~env ~package t1 + (* Don't descend into types named "t". Type t is a convention in the ReScript ecosystem. *) + | Tconstr (path, _, _) when path |> Path.last = "t" -> (env, t) + | Tconstr (path, _, _) -> ( + match References.digConstructor ~env ~package path with + | Some (env, {item = {decl = {type_manifest = Some typ}}}) -> + resolveTypeForPipeCompletion ~env ~package typ + | _ -> (env, t)) + | _ -> (env, t) + (** This moves through a nested path via a set of instructions, trying to resolve the type at the end of the path. *) let rec resolveNested (typ : completionType) ~env ~package ~nested = match nested with diff --git a/analysis/tests/src/expected/Completion.res.txt b/analysis/tests/src/expected/Completion.res.txt index 7907834ab..ba25f9391 100644 --- a/analysis/tests/src/expected/Completion.res.txt +++ b/analysis/tests/src/expected/Completion.res.txt @@ -1745,17 +1745,5 @@ posCursor:[425:8] posNoWhite:[425:7] Found expr:[425:3->425:8] Completable: Cpath Value[ok]->g Raw opens: 2 Shadow.B.place holder ... Shadow.A.place holder Resolved opens 2 Completion.res Completion.res -[{ - "label": "Belt.Result.getExn", - "kind": 12, - "tags": [], - "detail": "t<'a, 'b> => 'a", - "documentation": {"kind": "markdown", "value": "\n `getExn(res)`: when `res` is `Ok(n)`, returns `n` when `res` is `Error(m)`, raise an exception\n\n ```res example\n Belt.Result.getExn(Belt.Result.Ok(42)) == 42\n\n Belt.Result.getExn(Belt.Result.Error(\"Invalid data\")) /* raises exception */\n ```\n"} - }, { - "label": "Belt.Result.getWithDefault", - "kind": 12, - "tags": [], - "detail": "(t<'a, 'b>, 'a) => 'a", - "documentation": {"kind": "markdown", "value": "\n `getWithDefault(res, defaultValue)`: If `res` is `Ok(n)`, returns `n`,\n otherwise `default`\n\n ```res example\n Belt.Result.getWithDefault(Ok(42), 0) == 42\n\n Belt.Result.getWithDefault(Error(\"Invalid Data\"), 0) == 0\n ```\n"} - }] +[] diff --git a/analysis/tests/src/expected/CompletionPipeChain.res.txt b/analysis/tests/src/expected/CompletionPipeChain.res.txt index b9e219f6c..c149770fe 100644 --- a/analysis/tests/src/expected/CompletionPipeChain.res.txt +++ b/analysis/tests/src/expected/CompletionPipeChain.res.txt @@ -259,7 +259,25 @@ Completable: Cpath Value[Belt, Array, reduce](Nolabel, Nolabel, Nolabel)->t Complete src/CompletionPipeChain.res 70:12 posCursor:[70:12] posNoWhite:[70:11] Found expr:[70:3->0:-1] Completable: Cpath Value[aliased]-> -[] +[{ + "label": "CompletionSupport.Test.add", + "kind": 12, + "tags": [], + "detail": "t => int", + "documentation": null + }, { + "label": "CompletionSupport.Test.addSelf", + "kind": 12, + "tags": [], + "detail": "t => t", + "documentation": null + }, { + "label": "CompletionSupport.Test.make", + "kind": 12, + "tags": [], + "detail": "int => t", + "documentation": null + }] Complete src/CompletionPipeChain.res 73:15 posCursor:[73:15] posNoWhite:[73:14] Found expr:[73:3->0:-1] From 7343fcadbd5ec558d753a78889a5695381f81f00 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 18 Jan 2023 15:10:20 +0100 Subject: [PATCH 3/4] refactor pipe completion to handle builtins properly in new approach when resolving aliased types --- analysis/src/CompletionBackEnd.ml | 161 ++++++++---------- analysis/src/TypeUtils.ml | 104 ++++++++--- .../tests/src/expected/Completion.res.txt | 14 +- 3 files changed, 164 insertions(+), 115 deletions(-) diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index 20de55fe6..914866c68 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -719,93 +719,52 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env ~exact:true ~scope |> completionsGetTypeEnv with + | None -> [] | Some (typ, envFromCompletionItem) -> ( let env, typ = - typ |> TypeUtils.resolveTypeForPipeCompletion ~env ~package - in - - (* If the type we're completing on is a type parameter, we won't be able to do - completion unless we know what that type parameter is compiled as. This - attempts to look up the compiled type for that type parameter by looking - for compiled information at the loc of that expression. *) - let typ = - match typ with - | {Types.desc = Tvar _} -> ( - match - TypeUtils.findReturnTypeOfFunctionAtLoc lhsLoc ~env ~full - ~debug:false - with - | None -> typ - | Some typFromLoc -> typFromLoc) - | _ -> typ - in - let { - arrayModulePath; - optionModulePath; - stringModulePath; - intModulePath; - floatModulePath; - promiseModulePath; - listModulePath; - resultModulePath; - } = - package.builtInCompletionModules - in - let getBuiltinTypePath path = - match path with - | Path.Pident id when Ident.name id = "array" -> Some arrayModulePath - | Path.Pident id when Ident.name id = "option" -> Some optionModulePath - | Path.Pident id when Ident.name id = "string" -> Some stringModulePath - | Path.Pident id when Ident.name id = "int" -> Some intModulePath - | Path.Pident id when Ident.name id = "float" -> Some floatModulePath - | Path.Pident id when Ident.name id = "promise" -> - Some promiseModulePath - | Path.Pident id when Ident.name id = "list" -> Some listModulePath - | Path.Pident id when Ident.name id = "result" -> Some resultModulePath - | Path.Pident id when Ident.name id = "lazy_t" -> Some ["Lazy"] - | Path.Pident id when Ident.name id = "char" -> Some ["Char"] - | Pdot (Pident id, "result", _) when Ident.name id = "Pervasives" -> - Some resultModulePath - | _ -> None - in - let rec expandPath (path : Path.t) = - match path with - | Pident id -> [Ident.name id] - | Pdot (p, s, _) -> s :: expandPath p - | Papply _ -> [] - in - let getTypePath typ = - match typ.Types.desc with - | Tconstr (path, _typeArgs, _) - | Tlink {desc = Tconstr (path, _typeArgs, _)} - | Tsubst {desc = Tconstr (path, _typeArgs, _)} - | Tpoly ({desc = Tconstr (path, _typeArgs, _)}, []) -> - Some path - | _ -> None - in - let rec removeRawOpen rawOpen modulePath = - match (rawOpen, modulePath) with - | [_], _ -> Some modulePath - | s :: inner, first :: restPath when s = first -> - removeRawOpen inner restPath - | _ -> None - in - let rec removeRawOpens rawOpens modulePath = - match rawOpens with - | rawOpen :: restOpens -> ( - let newModulePath = removeRawOpens restOpens modulePath in - match removeRawOpen rawOpen newModulePath with - | None -> newModulePath - | Some mp -> mp) - | [] -> modulePath + typ + |> TypeUtils.resolveTypeForPipeCompletion ~env ~package ~full ~lhsLoc in let completionPath = - match getTypePath typ with - | Some typePath -> ( - match getBuiltinTypePath typePath with - | Some path -> Some path - | None -> ( - match expandPath typePath with + match typ with + | Builtin (builtin, _) -> + let { + arrayModulePath; + optionModulePath; + stringModulePath; + intModulePath; + floatModulePath; + promiseModulePath; + listModulePath; + resultModulePath; + } = + package.builtInCompletionModules + in + Some + (match builtin with + | Array -> arrayModulePath + | Option -> optionModulePath + | String -> stringModulePath + | Int -> intModulePath + | Float -> floatModulePath + | Promise -> promiseModulePath + | List -> listModulePath + | Result -> resultModulePath + | Lazy -> ["Lazy"] + | Char -> ["Char"]) + | TypExpr t -> ( + let rec expandPath (path : Path.t) = + match path with + | Pident id -> [Ident.name id] + | Pdot (p, s, _) -> s :: expandPath p + | Papply _ -> [] + in + match t.Types.desc with + | Tconstr (path, _typeArgs, _) + | Tlink {desc = Tconstr (path, _typeArgs, _)} + | Tsubst {desc = Tconstr (path, _typeArgs, _)} + | Tpoly ({desc = Tconstr (path, _typeArgs, _)}, []) -> ( + match expandPath path with | _ :: pathRev -> (* type path is relative to the completion environment express it from the root of the file *) @@ -820,11 +779,27 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env else envFromCompletionItem.file.moduleName :: pathFromEnv_ in Some pathFromEnv - | _ -> None)) - | None -> None + | _ -> None) + | _ -> None) in match completionPath with | Some completionPath -> ( + let rec removeRawOpen rawOpen modulePath = + match (rawOpen, modulePath) with + | [_], _ -> Some modulePath + | s :: inner, first :: restPath when s = first -> + removeRawOpen inner restPath + | _ -> None + in + let rec removeRawOpens rawOpens modulePath = + match rawOpens with + | rawOpen :: restOpens -> ( + let newModulePath = removeRawOpens restOpens modulePath in + match removeRawOpen rawOpen newModulePath with + | None -> newModulePath + | Some mp -> mp) + | [] -> modulePath + in let completionPathMinusOpens = completionPath |> removeRawOpens package.opens @@ -852,17 +827,16 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env (* We add React element functions to the completion if we're in a JSX context *) let forJsxCompletion = if inJsx then - match getTypePath typ with - | Some (Path.Pident id) when Ident.name id = "int" -> Some "int" - | Some (Path.Pident id) when Ident.name id = "float" -> Some "float" - | Some (Path.Pident id) when Ident.name id = "string" -> - Some "string" - | Some (Path.Pident id) when Ident.name id = "array" -> Some "array" + match typ with + | Builtin (Int, t) -> Some ("int", t) + | Builtin (Float, t) -> Some ("float", t) + | Builtin (String, t) -> Some ("string", t) + | Builtin (Array, t) -> Some ("array", t) | _ -> None else None in match forJsxCompletion with - | Some builtinNameToComplete + | Some (builtinNameToComplete, typ) when Utils.checkName builtinNameToComplete ~prefix:funNamePrefix ~exact:false -> [ @@ -878,8 +852,7 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env ] @ completions | _ -> completions) - | None -> []) - | None -> []) + | None -> [])) | CTuple ctxPaths -> (* Turn a list of context paths into a list of type expressions. *) let typeExrps = diff --git a/analysis/src/TypeUtils.ml b/analysis/src/TypeUtils.ml index 4424e34a7..a94b71cf2 100644 --- a/analysis/src/TypeUtils.ml +++ b/analysis/src/TypeUtils.ml @@ -148,18 +148,90 @@ let rec extractType ~env ~package (t : Types.type_expr) = Some (Tpolyvariant {env; constructors; typeExpr = t}) | _ -> None -let rec resolveTypeForPipeCompletion ~env ~package (t : Types.type_expr) = - match t.desc with - | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> - resolveTypeForPipeCompletion ~env ~package t1 - (* Don't descend into types named "t". Type t is a convention in the ReScript ecosystem. *) - | Tconstr (path, _, _) when path |> Path.last = "t" -> (env, t) - | Tconstr (path, _, _) -> ( - match References.digConstructor ~env ~package path with - | Some (env, {item = {decl = {type_manifest = Some typ}}}) -> - resolveTypeForPipeCompletion ~env ~package typ - | _ -> (env, t)) - | _ -> (env, t) +let findReturnTypeOfFunctionAtLoc loc ~(env : QueryEnv.t) ~full ~debug = + match References.getLocItem ~full ~pos:(loc |> Loc.end_) ~debug with + | Some {locType = Typed (_, typExpr, _)} -> ( + match extractFunctionType ~env ~package:full.package typExpr with + | args, tRet when args <> [] -> Some tRet + | _ -> None) + | _ -> None + +type builtinType = + | Array + | Option + | String + | Int + | Float + | Promise + | List + | Result + | Lazy + | Char + +type pipeCompletionType = + | Builtin of builtinType * Types.type_expr + | TypExpr of Types.type_expr + +let getBuiltinFromTypePath path = + match path with + | Path.Pident id when Ident.name id = "array" -> Some Array + | Path.Pident id when Ident.name id = "option" -> Some Option + | Path.Pident id when Ident.name id = "string" -> Some String + | Path.Pident id when Ident.name id = "int" -> Some Int + | Path.Pident id when Ident.name id = "float" -> Some Float + | Path.Pident id when Ident.name id = "promise" -> Some Promise + | Path.Pident id when Ident.name id = "list" -> Some List + | Path.Pident id when Ident.name id = "result" -> Some Result + | Path.Pident id when Ident.name id = "lazy_t" -> Some Lazy + | Path.Pident id when Ident.name id = "char" -> Some Char + | Pdot (Pident id, "result", _) when Ident.name id = "Pervasives" -> + Some Result + | _ -> None + +let rec resolveTypeForPipeCompletion ~env ~package ~lhsLoc ~full + (t : Types.type_expr) = + let builtin = + match t.desc with + | Tconstr (path, _typeArgs, _) + | Tlink {desc = Tconstr (path, _typeArgs, _)} + | Tsubst {desc = Tconstr (path, _typeArgs, _)} + | Tpoly ({desc = Tconstr (path, _typeArgs, _)}, []) -> + path |> getBuiltinFromTypePath + | _ -> None + in + match builtin with + | Some builtin -> (env, Builtin (builtin, t)) + | None -> ( + (* If the type we're completing on is a type parameter, we won't be able to + do completion unless we know what that type parameter is compiled as. + This attempts to look up the compiled type for that type parameter by + looking for compiled information at the loc of that expression. *) + let typFromLoc = + match t with + | {Types.desc = Tvar _} -> ( + match findReturnTypeOfFunctionAtLoc lhsLoc ~env ~full ~debug:false with + | None -> None + | Some typFromLoc -> Some typFromLoc) + | _ -> None + in + match typFromLoc with + | Some typFromLoc -> + typFromLoc |> resolveTypeForPipeCompletion ~lhsLoc ~env ~package ~full + | None -> + let rec digToRelevantType ~env ~package (t : Types.type_expr) = + match t.desc with + | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> + digToRelevantType ~env ~package t1 + (* Don't descend into types named "t". Type t is a convention in the ReScript ecosystem. *) + | Tconstr (path, _, _) when path |> Path.last = "t" -> (env, TypExpr t) + | Tconstr (path, _, _) -> ( + match References.digConstructor ~env ~package path with + | Some (env, {item = {decl = {type_manifest = Some typ}}}) -> + digToRelevantType ~env ~package typ + | _ -> (env, TypExpr t)) + | _ -> (env, TypExpr t) + in + digToRelevantType ~env ~package t) (** This moves through a nested path via a set of instructions, trying to resolve the type at the end of the path. *) let rec resolveNested (typ : completionType) ~env ~package ~nested = @@ -224,14 +296,6 @@ let rec resolveNested (typ : completionType) ~env ~package ~nested = TypeExpr typ |> resolveNested ~env ~package ~nested | _ -> None) -let findReturnTypeOfFunctionAtLoc loc ~(env : QueryEnv.t) ~full ~debug = - match References.getLocItem ~full ~pos:(loc |> Loc.end_) ~debug with - | Some {locType = Typed (_, typExpr, _)} -> ( - match extractFunctionType ~env ~package:full.package typExpr with - | args, tRet when args <> [] -> Some tRet - | _ -> None) - | _ -> None - let getArgs ~env (t : Types.type_expr) ~full = let rec getArgsLoop ~env (t : Types.type_expr) ~full ~currentArgumentPosition = diff --git a/analysis/tests/src/expected/Completion.res.txt b/analysis/tests/src/expected/Completion.res.txt index ba25f9391..7907834ab 100644 --- a/analysis/tests/src/expected/Completion.res.txt +++ b/analysis/tests/src/expected/Completion.res.txt @@ -1745,5 +1745,17 @@ posCursor:[425:8] posNoWhite:[425:7] Found expr:[425:3->425:8] Completable: Cpath Value[ok]->g Raw opens: 2 Shadow.B.place holder ... Shadow.A.place holder Resolved opens 2 Completion.res Completion.res -[] +[{ + "label": "Belt.Result.getExn", + "kind": 12, + "tags": [], + "detail": "t<'a, 'b> => 'a", + "documentation": {"kind": "markdown", "value": "\n `getExn(res)`: when `res` is `Ok(n)`, returns `n` when `res` is `Error(m)`, raise an exception\n\n ```res example\n Belt.Result.getExn(Belt.Result.Ok(42)) == 42\n\n Belt.Result.getExn(Belt.Result.Error(\"Invalid data\")) /* raises exception */\n ```\n"} + }, { + "label": "Belt.Result.getWithDefault", + "kind": 12, + "tags": [], + "detail": "(t<'a, 'b>, 'a) => 'a", + "documentation": {"kind": "markdown", "value": "\n `getWithDefault(res, defaultValue)`: If `res` is `Ok(n)`, returns `n`,\n otherwise `default`\n\n ```res example\n Belt.Result.getWithDefault(Ok(42), 0) == 42\n\n Belt.Result.getWithDefault(Error(\"Invalid Data\"), 0) == 0\n ```\n"} + }] From 3c9d46fc52741920c5772bf3fe027542dacb07f4 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 18 Jan 2023 15:12:55 +0100 Subject: [PATCH 4/4] changleog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d0efa108..4705df6d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ - Highlight `catch` like a keyword https://github.com/rescript-lang/rescript-vscode/pull/677 - Make signature help work in calls nested inside of other calls. https://github.com/rescript-lang/rescript-vscode/pull/687 +- Fix pipe completion to work on aliased types. https://github.com/rescript-lang/rescript-vscode/pull/700 ## v1.10.0