From ddc3ae18b42f2233d55d03d7270034d87164ebca Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 22 Dec 2022 13:46:16 +0100 Subject: [PATCH] Make pipe completion work reliably across submodules. Before this, a type path of non-zero length was assumed to come from the compiler, otherwise the path of the completion environment was taken verbatim. Neither case is correct. Now there's a single case, where one takes into account 2 inputs: - the current environment which gives context to the type path - the type path, which is interpreted as starting within that environment Since submodules can refer to types into other submodules, the path needs to be resolved to determine the actual path starting from the root of the file. This requires keeping track of where a module is defined. It could be defined in the current env, or in a parent one. The current env is walked backwards toward the root until the required module is found, and the resulting paths combined. --- CHANGELOG.md | 1 + analysis/src/CompletionBackEnd.ml | 22 ++++---- analysis/src/SharedTypes.ml | 38 +++++++++++-- .../tests/src/CompletionPipeSubmodules.res | 45 +++++++++++++++ .../expected/CompletionPipeSubmodules.res.txt | 56 +++++++++++++++++++ 5 files changed, 147 insertions(+), 15 deletions(-) create mode 100644 analysis/tests/src/CompletionPipeSubmodules.res create mode 100644 analysis/tests/src/expected/CompletionPipeSubmodules.res.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index d97d24d61..28b849eb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ - Accept both `@ns.doc` and the new `@res.doc` for the internal representation of doc comments. And both `@ns.optional` and `@res.optional` for the optional fields. https://github.com/rescript-lang/rescript-vscode/pull/642 - Make pipe completion work more reliably after function calls. https://github.com/rescript-lang/rescript-vscode/pull/656 - Make pipe completion work in pipe chains, not just on the first pipe. https://github.com/rescript-lang/rescript-vscode/pull/656 +- Make pipe completion work reliably when the path resolution needs to traverse submodules https://github.com/rescript-lang/rescript-vscode/pull/663 #### :bug: Bug Fix diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index 2b588784a..0e463e91e 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -1371,21 +1371,21 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env | Some path -> Some path | None -> ( match expandPath typePath with - | _ :: rest when rest <> [] -> - (* Assume a non-empty type path is coming from the compiler and - can be used as-is. *) - Some (List.rev rest) - | _ -> - (* Get the path from the comletion environment *) - let path = envFromCompletionItem.path in - if path = [] then None + | _ :: pathRev -> + (* type path is relative to the completion environment + express it from the root of the file *) + let pathFromEnv_ = + QueryEnv.pathFromEnv envFromCompletionItem (List.rev pathRev) + in + if pathFromEnv_ = [] then None else let pathFromEnv = if env.file.moduleName = envFromCompletionItem.file.moduleName - then path - else envFromCompletionItem.file.moduleName :: path + then pathFromEnv_ + else envFromCompletionItem.file.moduleName :: pathFromEnv_ in - Some pathFromEnv)) + Some pathFromEnv + | _ -> None)) | None -> None in match completionPath with diff --git a/analysis/src/SharedTypes.ml b/analysis/src/SharedTypes.ml index ac56eed2f..3d4878583 100644 --- a/analysis/src/SharedTypes.ml +++ b/analysis/src/SharedTypes.ml @@ -238,15 +238,45 @@ module File = struct end module QueryEnv : sig - type t = private {file: File.t; exported: Exported.t; path: path} + type t = private { + file: File.t; + exported: Exported.t; + pathRev: path; + parent: t option; + } val fromFile : File.t -> t val enterStructure : t -> Module.structure -> t + + (* Express a path starting from the module represented by the env. + E.g. the env is at A.B.C and the path is D. + The result is A.B.C.D if D is inside C. + Or A.B.D or A.D or D if it's in one of its parents. *) + val pathFromEnv : t -> path -> path end = struct - type t = {file: File.t; exported: Exported.t; path: path} + type t = {file: File.t; exported: Exported.t; pathRev: path; parent: t option} + + let fromFile (file : File.t) = + {file; exported = file.structure.exported; pathRev = []; parent = None} + + (* Prune a path and find a parent environment that contains the module name *) + let rec prunePath pathRev env name = + if Exported.find env.exported Module name <> None then pathRev + else + match (pathRev, env.parent) with + | _ :: rest, Some env -> prunePath rest env name + | _ -> [] + + let pathFromEnv env path = + match path with + | [] -> env.pathRev |> List.rev + | name :: _ -> + let prunedPathRev = prunePath env.pathRev env name in + List.rev_append prunedPathRev path - let fromFile file = {file; exported = file.structure.exported; path = []} let enterStructure env (structure : Module.structure) = - {env with exported = structure.exported; path = structure.name :: env.path} + let name = structure.name in + let pathRev = name :: prunePath env.pathRev env name in + {env with exported = structure.exported; pathRev; parent = Some env} end module Completion = struct diff --git a/analysis/tests/src/CompletionPipeSubmodules.res b/analysis/tests/src/CompletionPipeSubmodules.res new file mode 100644 index 000000000..a3ee0af8d --- /dev/null +++ b/analysis/tests/src/CompletionPipeSubmodules.res @@ -0,0 +1,45 @@ +module A = { + module B1 = { + type b1 = B1 + let xx = B1 + } + module B2 = { + let yy = 20 + } + type t = {v: B1.b1} + let x = {v: B1.B1} +} + +// let _ = A.B1.xx-> +// ^com +// b1 seen from B1 is A.B1.b1 + +// let _ = A.x.v-> +// ^com +// B1.b1 seen from A is A.B1.b1 + +module C = { + type t = C +} + +module D = { + module C2 = { + type t2 = C2 + } + + type d = {v: C.t, v2: C2.t2} + let d = {v: C.C, v2: C2.C2} +} + +module E = { + type e = {v: D.d} + let e = {v: D.d} +} + +// let _ = E.e.v.v-> +// ^com +// C.t seen from D is C.t + +// let _ = E.e.v.v2-> +// ^com +// C2.t2 seen from D is D.C2.t2 diff --git a/analysis/tests/src/expected/CompletionPipeSubmodules.res.txt b/analysis/tests/src/expected/CompletionPipeSubmodules.res.txt new file mode 100644 index 000000000..a699eb8e6 --- /dev/null +++ b/analysis/tests/src/expected/CompletionPipeSubmodules.res.txt @@ -0,0 +1,56 @@ +Complete src/CompletionPipeSubmodules.res 12:20 +posCursor:[12:20] posNoWhite:[12:19] Found expr:[12:11->20:8] +Completable: Cpath Value[A, B1, xx]-> +[{ + "label": "A.B1.xx", + "kind": 12, + "tags": [], + "detail": "b1", + "documentation": null + }, { + "label": "A.B1.B1", + "kind": 4, + "tags": [], + "detail": "B1\n\ntype b1 = B1", + "documentation": null + }] + +Complete src/CompletionPipeSubmodules.res 16:18 +posCursor:[16:18] posNoWhite:[16:17] Found expr:[16:11->20:8] +Completable: Cpath Value[A, x].v-> +[{ + "label": "A.B1.xx", + "kind": 12, + "tags": [], + "detail": "b1", + "documentation": null + }, { + "label": "A.B1.B1", + "kind": 4, + "tags": [], + "detail": "B1\n\ntype b1 = B1", + "documentation": null + }] + +Complete src/CompletionPipeSubmodules.res 38:20 +posCursor:[38:20] posNoWhite:[38:19] Found expr:[38:11->0:-1] +Completable: Cpath Value[E, e].v.v-> +[{ + "label": "C.C", + "kind": 4, + "tags": [], + "detail": "C\n\ntype t = C", + "documentation": null + }] + +Complete src/CompletionPipeSubmodules.res 42:21 +posCursor:[42:21] posNoWhite:[42:20] Found expr:[42:11->0:-1] +Completable: Cpath Value[E, e].v.v2-> +[{ + "label": "D.C2.C2", + "kind": 4, + "tags": [], + "detail": "C2\n\ntype t2 = C2", + "documentation": null + }] +