Skip to content

Commit ddc3ae1

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 05a1393 commit ddc3ae1

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 https://github.com/rescript-lang/rescript-vscode/pull/663
2324

2425
#### :bug: Bug Fix
2526

Diff for: analysis/src/CompletionBackEnd.ml

+11-11
Original file line numberDiff line numberDiff line change
@@ -1371,21 +1371,21 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env
13711371
| Some path -> Some path
13721372
| None -> (
13731373
match expandPath typePath with
1374-
| _ :: rest when rest <> [] ->
1375-
(* Assume a non-empty type path is coming from the compiler and
1376-
can be used as-is. *)
1377-
Some (List.rev rest)
1378-
| _ ->
1379-
(* Get the path from the comletion environment *)
1380-
let path = envFromCompletionItem.path in
1381-
if path = [] then None
1374+
| _ :: pathRev ->
1375+
(* type path is relative to the completion environment
1376+
express it from the root of the file *)
1377+
let pathFromEnv_ =
1378+
QueryEnv.pathFromEnv envFromCompletionItem (List.rev pathRev)
1379+
in
1380+
if pathFromEnv_ = [] then None
13821381
else
13831382
let pathFromEnv =
13841383
if env.file.moduleName = envFromCompletionItem.file.moduleName
1385-
then path
1386-
else envFromCompletionItem.file.moduleName :: path
1384+
then pathFromEnv_
1385+
else envFromCompletionItem.file.moduleName :: pathFromEnv_
13871386
in
1388-
Some pathFromEnv))
1387+
Some pathFromEnv
1388+
| _ -> None))
13891389
| None -> None
13901390
in
13911391
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)