Skip to content

Commit 2f4de51

Browse files
committed
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.
1 parent ba532ac commit 2f4de51

File tree

5 files changed

+147
-15
lines changed

5 files changed

+147
-15
lines changed

Diff for: CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
- 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
2121
- Make pipe completion work more reliably after function calls. https://github.com/rescript-lang/rescript-vscode/pull/656
2222
- Make pipe completion work in pipe chains, not just on the first pipe. https://github.com/rescript-lang/rescript-vscode/pull/656
23+
- Make pipe completion work reliably when the path resolution needs to traverse submodules
2324

2425
#### :bug: Bug Fix
2526

Diff for: analysis/src/CompletionBackEnd.ml

+11-11
Original file line numberDiff line numberDiff line change
@@ -1348,21 +1348,21 @@ let rec getCompletionsForContextPath ~package ~opens ~rawOpens ~allFiles ~pos
13481348
| Some path -> Some path
13491349
| None -> (
13501350
match expandPath typePath with
1351-
| _ :: rest when rest <> [] ->
1352-
(* Assume a non-empty type path is coming from the compiler and
1353-
can be used as-is. *)
1354-
Some (List.rev rest)
1355-
| _ ->
1356-
(* Get the path from the comletion environment *)
1357-
let path = envFromCompletionItem.path in
1358-
if path = [] then None
1351+
| _ :: pathRev ->
1352+
(* type path is relative to the completion environment
1353+
express it from the root of the file *)
1354+
let pathFromEnv_ =
1355+
QueryEnv.pathFromEnv envFromCompletionItem (List.rev pathRev)
1356+
in
1357+
if pathFromEnv_ = [] then None
13591358
else
13601359
let pathFromEnv =
13611360
if env.file.moduleName = envFromCompletionItem.file.moduleName
1362-
then path
1363-
else envFromCompletionItem.file.moduleName :: path
1361+
then pathFromEnv_
1362+
else envFromCompletionItem.file.moduleName :: pathFromEnv_
13641363
in
1365-
Some pathFromEnv))
1364+
Some pathFromEnv
1365+
| _ -> None))
13661366
| None -> None
13671367
in
13681368
match completionPath with

Diff for: analysis/src/SharedTypes.ml

+34-4
Original file line numberDiff line numberDiff line change
@@ -238,15 +238,45 @@ module File = struct
238238
end
239239

240240
module QueryEnv : sig
241-
type t = private {file: File.t; exported: Exported.t; path: path}
241+
type t = private {
242+
file: File.t;
243+
exported: Exported.t;
244+
pathRev: path;
245+
parent: t option;
246+
}
242247
val fromFile : File.t -> t
243248
val enterStructure : t -> Module.structure -> t
249+
250+
(* Express a path starting from the module represented by the env.
251+
E.g. the env is at A.B.C and the path is D.
252+
The result is A.B.C.D if D is inside C.
253+
Or A.B.D or A.D or D if it's in one of its parents. *)
254+
val pathFromEnv : t -> path -> path
244255
end = struct
245-
type t = {file: File.t; exported: Exported.t; path: path}
256+
type t = {file: File.t; exported: Exported.t; pathRev: path; parent: t option}
257+
258+
let fromFile (file : File.t) =
259+
{file; exported = file.structure.exported; pathRev = []; parent = None}
260+
261+
(* Prune a path and find a parent environment that contains the module name *)
262+
let rec prunePath pathRev env name =
263+
if Exported.find env.exported Module name <> None then pathRev
264+
else
265+
match (pathRev, env.parent) with
266+
| _ :: rest, Some env -> prunePath rest env name
267+
| _ -> []
268+
269+
let pathFromEnv env path =
270+
match path with
271+
| [] -> env.pathRev |> List.rev
272+
| name :: _ ->
273+
let prunedPathRev = prunePath env.pathRev env name in
274+
List.rev_append prunedPathRev path
246275

247-
let fromFile file = {file; exported = file.structure.exported; path = []}
248276
let enterStructure env (structure : Module.structure) =
249-
{env with exported = structure.exported; path = structure.name :: env.path}
277+
let name = structure.name in
278+
let pathRev = name :: prunePath env.pathRev env name in
279+
{env with exported = structure.exported; pathRev; parent = Some env}
250280
end
251281

252282
module Completion = struct

Diff for: analysis/tests/src/CompletionPipeSubmodules.res

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
module A = {
2+
module B1 = {
3+
type b1 = B1
4+
let xx = B1
5+
}
6+
module B2 = {
7+
let yy = 20
8+
}
9+
type t = {v: B1.b1}
10+
let x = {v: B1.B1}
11+
}
12+
13+
// let _ = A.B1.xx->
14+
// ^com
15+
// b1 seen from B1 is A.B1.b1
16+
17+
// let _ = A.x.v->
18+
// ^com
19+
// B1.b1 seen from A is A.B1.b1
20+
21+
module C = {
22+
type t = C
23+
}
24+
25+
module D = {
26+
module C2 = {
27+
type t2 = C2
28+
}
29+
30+
type d = {v: C.t, v2: C2.t2}
31+
let d = {v: C.C, v2: C2.C2}
32+
}
33+
34+
module E = {
35+
type e = {v: D.d}
36+
let e = {v: D.d}
37+
}
38+
39+
// let _ = E.e.v.v->
40+
// ^com
41+
// C.t seen from D is C.t
42+
43+
// let _ = E.e.v.v2->
44+
// ^com
45+
// C2.t2 seen from D is D.C2.t2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
Complete src/CompletionPipeSubmodules.res 12:20
2+
posCursor:[12:20] posNoWhite:[12:19] Found expr:[12:11->20:8]
3+
Completable: Cpath Value[A, B1, xx]->
4+
[{
5+
"label": "A.B1.xx",
6+
"kind": 12,
7+
"tags": [],
8+
"detail": "b1",
9+
"documentation": null
10+
}, {
11+
"label": "A.B1.B1",
12+
"kind": 4,
13+
"tags": [],
14+
"detail": "B1\n\ntype b1 = B1",
15+
"documentation": null
16+
}]
17+
18+
Complete src/CompletionPipeSubmodules.res 16:18
19+
posCursor:[16:18] posNoWhite:[16:17] Found expr:[16:11->20:8]
20+
Completable: Cpath Value[A, x].v->
21+
[{
22+
"label": "A.B1.xx",
23+
"kind": 12,
24+
"tags": [],
25+
"detail": "b1",
26+
"documentation": null
27+
}, {
28+
"label": "A.B1.B1",
29+
"kind": 4,
30+
"tags": [],
31+
"detail": "B1\n\ntype b1 = B1",
32+
"documentation": null
33+
}]
34+
35+
Complete src/CompletionPipeSubmodules.res 38:20
36+
posCursor:[38:20] posNoWhite:[38:19] Found expr:[38:11->0:-1]
37+
Completable: Cpath Value[E, e].v.v->
38+
[{
39+
"label": "C.C",
40+
"kind": 4,
41+
"tags": [],
42+
"detail": "C\n\ntype t = C",
43+
"documentation": null
44+
}]
45+
46+
Complete src/CompletionPipeSubmodules.res 42:21
47+
posCursor:[42:21] posNoWhite:[42:20] Found expr:[42:11->0:-1]
48+
Completable: Cpath Value[E, e].v.v2->
49+
[{
50+
"label": "D.C2.C2",
51+
"kind": 4,
52+
"tags": [],
53+
"detail": "C2\n\ntype t2 = C2",
54+
"documentation": null
55+
}]
56+

0 commit comments

Comments
 (0)