Skip to content

Commit 56cff12

Browse files
cknittclaude
cknitt
and
claude
committed
Name extension constructors relative to where completion runs
Codex review on #9085: extension constructors were named relative to the root of their file and to the file of the completed value. Inside the declaring module that inserted `Sub.Flag`, which is not in scope there, and for a value imported from another file the constructors were not qualified with its module, while extensions in the edited file were missed. A completion request now records the edited file and the module path at the cursor; constructors of that file drop the module path shared with the cursor, constructors of other files get their module's name, and extensions are collected from the edited file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Signed-off-by: Christoph Knittel <ck@cca.io>
1 parent f678bb3 commit 56cff12

6 files changed

Lines changed: 143 additions & 19 deletions

File tree

‎analysis/src/completion_back_end.ml‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2105,8 +2105,8 @@ let rec complete_typed_value ?(type_arg_context : type_arg_context option)
21052105

21062106
module String_set = Set.Make (String)
21072107

2108-
let rec process_completable ~state ~debug ~full ~scope ~env ~pos ~for_hover
2109-
completable =
2108+
let rec process_completable_in_scope ~state ~debug ~full ~scope ~env ~pos
2109+
~for_hover completable =
21102110
if debug then
21112111
Printf.printf "Completable: %s\n" (Completable.to_string completable);
21122112
let package = full.package in
@@ -2412,7 +2412,8 @@ let rec process_completable ~state ~debug ~full ~scope ~env ~pos ~for_hover
24122412
match (fallback, items) with
24132413
| Some fallback, (None | Some []) ->
24142414
fallback
2415-
|> process_completable ~state ~debug ~full ~scope ~env ~pos ~for_hover
2415+
|> process_completable_in_scope ~state ~debug ~full ~scope ~env ~pos
2416+
~for_hover
24162417
| _, Some items -> items
24172418
| None, None -> []
24182419
in
@@ -2647,3 +2648,32 @@ let rec process_completable ~state ~debug ~full ~scope ~env ~pos ~for_hover
26472648
~env ~insert_text:"todo(\"${0:TODO}\")";
26482649
]
26492650
else []
2651+
2652+
(* The names of the modules around [pos] in [file], outermost first. *)
2653+
let module_path_at ~(full : full) ~pos =
2654+
let rec in_items (items : Module.item list) =
2655+
match
2656+
items
2657+
|> List.find_opt (fun (item : Module.item) ->
2658+
Loc.has_pos ~pos item.loc
2659+
&&
2660+
match item.kind with
2661+
| Module _ -> true
2662+
| _ -> false)
2663+
with
2664+
| Some {name; kind = Module {type_}} -> name :: in_module type_
2665+
| _ -> []
2666+
and in_module (m : Module.t) =
2667+
match m with
2668+
| Structure {items} -> in_items items
2669+
| Constraint (implementation, _) -> in_module implementation
2670+
| Ident _ -> []
2671+
in
2672+
in_items full.file.structure.items
2673+
2674+
let process_completable ~state ~debug ~full ~scope ~env ~pos ~for_hover
2675+
completable =
2676+
Type_utils.with_completion_scope ~file:full.file
2677+
~module_path:(module_path_at ~full ~pos) (fun () ->
2678+
process_completable_in_scope ~state ~debug ~full ~scope ~env ~pos
2679+
~for_hover completable)

‎analysis/src/type_utils.ml‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,17 @@ let extract_function_type2 ?type_arg_context ~state ~env ~package typ =
361361
in
362362
loop ?type_arg_context ~env [] typ
363363

364+
(* The file being edited and the module path at the cursor, while a
365+
completion request runs. Extension constructors are named relative to
366+
it: unlike ordinary constructors they can't be found from the expected
367+
type, so they have to be written so that they are in scope there. *)
368+
let completion_scope : (File.t * string list) option ref = ref None
369+
370+
let with_completion_scope ~file ~module_path f =
371+
let previous = !completion_scope in
372+
completion_scope := Some (file, module_path);
373+
Fun.protect ~finally:(fun () -> completion_scope := previous) f
374+
364375
(* The stamp under which [typ] is declared in [file]. *)
365376
let type_stamp (file : File.t) (typ : Type.t) =
366377
let found = ref None in
@@ -377,6 +388,11 @@ let type_stamp (file : File.t) (typ : Type.t) =
377388
constructor can't be found from the expected type alone. *)
378389
let extension_constructors ~state ~package ~(current_file : File.t)
379390
~(type_file : File.t) ~type_stamp ~type_name ~decl =
391+
let current_file, scope_path =
392+
match !completion_scope with
393+
| Some (file, module_path) -> (file, module_path)
394+
| None -> (current_file, [])
395+
in
380396
let extends (file : File.t) (ext_decl : Types.type_declaration) =
381397
match (type_stamp, ext_decl.type_manifest) with
382398
| None, _ -> true
@@ -424,8 +440,16 @@ let extension_constructors ~state ~package ~(current_file : File.t)
424440
let path =
425441
Module_path.to_path declared.module_path declared.item.cname.txt
426442
in
443+
(* Inside a module, its own name and those of the modules around it
444+
are not in scope: drop the module path shared with the cursor. *)
445+
let rec relative path scope =
446+
match (path, scope) with
447+
| p :: (_ :: _ as path), s :: scope when p = s -> relative path scope
448+
| _ -> path
449+
in
427450
let path =
428-
if file.uri = current_file.uri then path else file.module_name :: path
451+
if file.uri = current_file.uri then relative path scope_path
452+
else file.module_name :: path
429453
in
430454
{
431455
declared.item with

‎tests/analysis_tests/tests/src/CompletionExtensibleVariant.res‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ type attr += Str(string) | Num(int)
33

44
module Sub = {
55
type attr += Flag
6+
// Inside the declaring module the constructor is not qualified
7+
// let inside: attr = Fl
8+
// ^com
69
}
710

811
// let a: attr = Nu

‎tests/analysis_tests/tests/src/CompletionExtensibleVariantUse.res‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
type CompletionExtensibleVariant.attr += Local
2+
3+
let imported = CompletionExtensibleVariant.value
4+
5+
// switch importe
6+
// ^com

‎tests/analysis_tests/tests/src/expected/CompletionExtensibleVariant.res.txt‎

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,40 @@
1-
Complete src/CompletionExtensibleVariant.res 7:19
1+
Complete src/CompletionExtensibleVariant.res 6:26
2+
XXX Not found!
3+
Completable: Cexpression Type[attr]=Fl
4+
Package opens Stdlib.place holder Pervasives.JsxModules.place holder
5+
Resolved opens 1 Stdlib
6+
ContextPath Type[attr]
7+
Path attr
8+
[
9+
{
10+
"detail": "Flag",
11+
"documentation": {
12+
"kind": "markdown",
13+
"value": "```rescript\nFlag\n```\n\n```rescript\ntype attr = ..\n```"
14+
},
15+
"insertText": "Flag",
16+
"insertTextFormat": 2,
17+
"kind": 4,
18+
"label": "Flag",
19+
"sortText": "A Flag",
20+
"tags": []
21+
},
22+
{
23+
"detail": "module Float32Array",
24+
"kind": 9,
25+
"label": "Float32Array",
26+
"tags": []
27+
},
28+
{
29+
"detail": "module Float64Array",
30+
"kind": 9,
31+
"label": "Float64Array",
32+
"tags": []
33+
},
34+
{ "detail": "module Float", "kind": 9, "label": "Float", "tags": [] }
35+
]
36+
37+
Complete src/CompletionExtensibleVariant.res 10:19
238
XXX Not found!
339
Completable: Cexpression Type[attr]=Nu
440
Package opens Stdlib.place holder Pervasives.JsxModules.place holder
@@ -23,17 +59,17 @@ Path attr
2359
{ "detail": "module Null", "kind": 9, "label": "Null", "tags": [] }
2460
]
2561

26-
Hover src/CompletionExtensibleVariant.res 10:19
62+
Hover src/CompletionExtensibleVariant.res 13:19
2763
{
2864
"contents": {
2965
"kind": "markdown",
3066
"value": "```rescript\nattr\n```\n\n---\n\n```\n \n```\n```rescript\ntype attr = ..\n```\nGo to: [Type definition](CompletionExtensibleVariant.res#L1,1)\n"
3167
}
3268
}
3369

34-
Complete src/CompletionExtensibleVariant.res 13:21
35-
pos_cursor:[13:21] pos_no_white:[13:20] Found pattern:[13:20->13:21]
36-
Ppat_construct S:[13:20->13:21]
70+
Complete src/CompletionExtensibleVariant.res 16:21
71+
pos_cursor:[16:21] pos_no_white:[16:20] Found pattern:[16:20->16:21]
72+
Ppat_construct S:[16:20->16:21]
3773
Completable: Cpattern Value[value]=S
3874
Package opens Stdlib.place holder Pervasives.JsxModules.place holder
3975
Resolved opens 1 Stdlib
@@ -66,7 +102,7 @@ Path value
66102
}
67103
]
68104

69-
Complete src/CompletionExtensibleVariant.res 16:14
105+
Complete src/CompletionExtensibleVariant.res 19:14
70106
XXX Not found!
71107
Completable: CexhaustiveSwitch Value[valu]
72108
Package opens Stdlib.place holder Pervasives.JsxModules.place holder
@@ -86,9 +122,9 @@ Path valu
86122
}
87123
]
88124

89-
Complete src/CompletionExtensibleVariant.res 32:17
90-
pos_cursor:[32:17] pos_no_white:[32:16] Found pattern:[32:16->32:17]
91-
Ppat_construct M:[32:16->32:17]
125+
Complete src/CompletionExtensibleVariant.res 35:17
126+
pos_cursor:[35:17] pos_no_white:[35:16] Found pattern:[35:16->35:17]
127+
Ppat_construct M:[35:16->35:17]
92128
Completable: Cpattern Value[a]=M
93129
Package opens Stdlib.place holder Pervasives.JsxModules.place holder
94130
Resolved opens 1 Stdlib
@@ -109,9 +145,9 @@ Path a
109145
}
110146
]
111147

112-
Complete src/CompletionExtensibleVariant.res 35:17
113-
pos_cursor:[35:17] pos_no_white:[35:16] Found pattern:[35:16->35:17]
114-
Ppat_construct F:[35:16->35:17]
148+
Complete src/CompletionExtensibleVariant.res 38:17
149+
pos_cursor:[38:17] pos_no_white:[38:16] Found pattern:[38:16->38:17]
150+
Ppat_construct F:[38:16->38:17]
115151
Completable: Cpattern Value[a]=F
116152
Package opens Stdlib.place holder Pervasives.JsxModules.place holder
117153
Resolved opens 1 Stdlib
@@ -132,9 +168,9 @@ Path a
132168
}
133169
]
134170

135-
Complete src/CompletionExtensibleVariant.res 38:22
136-
pos_cursor:[38:22] pos_no_white:[38:21] Found pattern:[38:20->38:22]
137-
Ppat_construct Fl:[38:20->38:22]
171+
Complete src/CompletionExtensibleVariant.res 41:22
172+
pos_cursor:[41:22] pos_no_white:[41:21] Found pattern:[41:20->41:22]
173+
Ppat_construct Fl:[41:20->41:22]
138174
Completable: Cpattern Value[value]=Fl
139175
Package opens Stdlib.place holder Pervasives.JsxModules.place holder
140176
Resolved opens 1 Stdlib

‎tests/analysis_tests/tests/src/expected/CompletionExtensibleVariantUse.res.txt‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
Complete src/CompletionExtensibleVariantUse.res 4:17
2+
XXX Not found!
3+
Completable: CexhaustiveSwitch Value[importe]
4+
Package opens Stdlib.place holder Pervasives.JsxModules.place holder
5+
Resolved opens 1 Stdlib
6+
ContextPath Value[importe]
7+
Path importe
8+
[
9+
{
10+
"detail": "CompletionExtensibleVariant.attr",
11+
"kind": 12,
12+
"label": "imported",
13+
"tags": []
14+
},
15+
{
16+
"detail": "insert exhaustive switch for value",
17+
"filterText": "imported",
18+
"insertText": "imported {\n | CompletionExtensibleVariant.Str(_) => ${1:%todo}\n | CompletionExtensibleVariant.Num(_) => ${2:%todo}\n | CompletionExtensibleVariant.Sub.Flag => ${3:%todo}\n | Local => ${4:%todo}\n | _ => ${5:%todo}\n }",
19+
"insertTextFormat": 2,
20+
"kind": 15,
21+
"label": "imported (exhaustive switch)",
22+
"tags": []
23+
}
24+
]
25+

0 commit comments

Comments
 (0)