Skip to content

Commit a099cb4

Browse files
committed
Investigate heuristics in jump to location.
There are a number of heuristics when getting a loc item from a text position. These heuristics are related to the behaviours of the JSX ppx and internal compiler behaviours that leak into the position of elements.
1 parent 8d32164 commit a099cb4

File tree

9 files changed

+94
-59
lines changed

9 files changed

+94
-59
lines changed

analysis/src/Cli.ml

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,18 +72,22 @@ let main () =
7272
~pos:(int_of_string line, int_of_string col)
7373
~currentFile
7474
| [_; "definition"; path; line; col] ->
75-
Commands.definition ~path ~line:(int_of_string line)
76-
~col:(int_of_string col)
75+
Commands.definition ~path
76+
~pos:(int_of_string line, int_of_string col)
77+
~debug:false
7778
| [_; "typeDefinition"; path; line; col] ->
78-
Commands.typeDefinition ~path ~line:(int_of_string line)
79-
~col:(int_of_string col)
79+
Commands.typeDefinition ~path
80+
~pos:(int_of_string line, int_of_string col)
81+
~debug:false
8082
| [_; "documentSymbol"; path] -> DocumentSymbol.command ~path
8183
| [_; "hover"; path; line; col; currentFile] ->
82-
Commands.hover ~path ~line:(int_of_string line) ~col:(int_of_string col)
84+
Commands.hover ~path
85+
~pos:(int_of_string line, int_of_string col)
8386
~currentFile ~debug:false
8487
| [_; "codeAction"; path; line; col; currentFile] ->
85-
Commands.codeAction ~path ~line:(int_of_string line)
86-
~col:(int_of_string col) ~currentFile
88+
Commands.codeAction ~path
89+
~pos:(int_of_string line, int_of_string col)
90+
~currentFile ~debug:false
8791
| _ :: "reanalyze" :: _ ->
8892
let len = Array.length Sys.argv in
8993
for i = 1 to len - 2 do
@@ -92,11 +96,13 @@ let main () =
9296
Sys.argv.(len - 1) <- "";
9397
Reanalyze.cli ()
9498
| [_; "references"; path; line; col] ->
95-
Commands.references ~path ~line:(int_of_string line)
96-
~col:(int_of_string col)
99+
Commands.references ~path
100+
~pos:(int_of_string line, int_of_string col)
101+
~debug:false
97102
| [_; "rename"; path; line; col; newName] ->
98-
Commands.rename ~path ~line:(int_of_string line) ~col:(int_of_string col)
99-
~newName
103+
Commands.rename ~path
104+
~pos:(int_of_string line, int_of_string col)
105+
~newName ~debug:false
100106
| [_; "semanticTokens"; currentFile] ->
101107
SemanticTokens.semanticTokens ~currentFile
102108
| [_; "createInterface"; path; cmiFile] ->

analysis/src/Commands.ml

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,18 @@ let completion ~debug ~path ~pos ~currentFile =
2828
|> List.map Protocol.stringifyCompletionItem
2929
|> Protocol.array)
3030

31-
let hover ~path ~line ~col ~currentFile ~debug =
31+
let hover ~path ~pos ~currentFile ~debug =
3232
let result =
3333
match Cmt.fullFromPath ~path with
3434
| None -> Protocol.null
3535
| Some full -> (
36-
match References.getLocItem ~full ~line ~col with
36+
match References.getLocItem ~full ~pos ~debug with
3737
| None -> (
3838
if debug then
3939
Printf.printf
4040
"Nothing at that position. Now trying to use completion.\n";
4141
let completions =
42-
getCompletions ~debug ~path ~pos:(line, col) ~currentFile
43-
~forHover:true
42+
getCompletions ~debug ~path ~pos ~currentFile ~forHover:true
4443
in
4544
match completions with
4645
| {kind = Label typString; docstring} :: _ ->
@@ -81,16 +80,16 @@ let hover ~path ~line ~col ~currentFile ~debug =
8180
in
8281
print_endline result
8382

84-
let codeAction ~path ~line ~col ~currentFile =
85-
Xform.extractCodeActions ~path ~pos:(line, col) ~currentFile
83+
let codeAction ~path ~pos ~currentFile ~debug =
84+
Xform.extractCodeActions ~path ~pos ~currentFile ~debug
8685
|> CodeActions.stringifyCodeActions |> print_endline
8786

88-
let definition ~path ~line ~col =
87+
let definition ~path ~pos ~debug =
8988
let locationOpt =
9089
match Cmt.fullFromPath ~path with
9190
| None -> None
9291
| Some full -> (
93-
match References.getLocItem ~full ~line ~col with
92+
match References.getLocItem ~full ~pos ~debug with
9493
| None -> None
9594
| Some locItem -> (
9695
match References.definitionForLocItem ~full locItem with
@@ -123,12 +122,12 @@ let definition ~path ~line ~col =
123122
| None -> Protocol.null
124123
| Some location -> location |> Protocol.stringifyLocation)
125124

126-
let typeDefinition ~path ~line ~col =
125+
let typeDefinition ~path ~pos ~debug =
127126
let maybeLocation =
128127
match Cmt.fullFromPath ~path with
129128
| None -> None
130129
| Some full -> (
131-
match References.getLocItem ~full ~line ~col with
130+
match References.getLocItem ~full ~pos ~debug with
132131
| None -> None
133132
| Some locItem -> (
134133
match References.typeDefinitionForLocItem ~full locItem with
@@ -143,12 +142,12 @@ let typeDefinition ~path ~line ~col =
143142
| None -> Protocol.null
144143
| Some location -> location |> Protocol.stringifyLocation)
145144

146-
let references ~path ~line ~col =
145+
let references ~path ~pos ~debug =
147146
let allLocs =
148147
match Cmt.fullFromPath ~path with
149148
| None -> []
150149
| Some full -> (
151-
match References.getLocItem ~full ~line ~col with
150+
match References.getLocItem ~full ~pos ~debug with
152151
| None -> []
153152
| Some locItem ->
154153
let allReferences = References.allReferencesForLocItem ~full locItem in
@@ -169,12 +168,12 @@ let references ~path ~line ~col =
169168
(if allLocs = [] then Protocol.null
170169
else "[\n" ^ (allLocs |> String.concat ",\n") ^ "\n]")
171170

172-
let rename ~path ~line ~col ~newName =
171+
let rename ~path ~pos ~newName ~debug =
173172
let result =
174173
match Cmt.fullFromPath ~path with
175174
| None -> Protocol.null
176175
| Some full -> (
177-
match References.getLocItem ~full ~line ~col with
176+
match References.getLocItem ~full ~pos ~debug with
178177
| None -> Protocol.null
179178
| Some locItem ->
180179
let allReferences = References.allReferencesForLocItem ~full locItem in
@@ -305,24 +304,24 @@ let test ~path =
305304
print_endline
306305
("Definition " ^ path ^ " " ^ string_of_int line ^ ":"
307306
^ string_of_int col);
308-
definition ~path ~line ~col
307+
definition ~path ~pos:(line, col) ~debug:true
309308
| "typ" ->
310309
print_endline
311310
("TypeDefinition " ^ path ^ " " ^ string_of_int line ^ ":"
312311
^ string_of_int col);
313-
typeDefinition ~path ~line ~col
312+
typeDefinition ~path ~pos:(line, col) ~debug:true
314313
| "hov" ->
315314
print_endline
316315
("Hover " ^ path ^ " " ^ string_of_int line ^ ":"
317316
^ string_of_int col);
318317
let currentFile = createCurrentFile () in
319-
hover ~path ~line ~col ~currentFile ~debug:true;
318+
hover ~path ~pos:(line, col) ~currentFile ~debug:true;
320319
Sys.remove currentFile
321320
| "ref" ->
322321
print_endline
323322
("References " ^ path ^ " " ^ string_of_int line ^ ":"
324323
^ string_of_int col);
325-
references ~path ~line ~col
324+
references ~path ~pos:(line, col) ~debug:true
326325
| "doc" ->
327326
print_endline ("DocumentSymbol " ^ path);
328327
DocumentSymbol.command ~path
@@ -333,7 +332,7 @@ let test ~path =
333332
("Rename " ^ path ^ " " ^ string_of_int line ^ ":"
334333
^ string_of_int col ^ " " ^ newName)
335334
in
336-
rename ~path ~line ~col ~newName
335+
rename ~path ~pos:(line, col) ~newName ~debug:true
337336
| "com" ->
338337
print_endline
339338
("Complete " ^ path ^ " " ^ string_of_int line ^ ":"
@@ -362,6 +361,7 @@ let test ~path =
362361
^ string_of_int col);
363362
let codeActions =
364363
Xform.extractCodeActions ~path ~pos:(line, col) ~currentFile:path
364+
~debug:true
365365
in
366366
codeActions
367367
|> List.iter (fun {Protocol.title; edit = {documentChanges}} ->

analysis/src/References.ml

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
open SharedTypes
22

33
let debugReferences = ref true
4+
45
let maybeLog m = if !debugReferences then Log.log ("[ref] " ^ m)
56

67
let checkPos (line, char)
@@ -16,10 +17,11 @@ let checkPos (line, char)
1617
let locItemsForPos ~extra pos =
1718
extra.locItems |> List.filter (fun {loc; locType = _} -> checkPos pos loc)
1819

19-
let lineColToCmtLoc ~line ~col = (line + 1, col)
20+
let lineColToCmtLoc ~pos:(line, col) = (line + 1, col)
2021

21-
let getLocItem ~full ~line ~col =
22-
let pos = lineColToCmtLoc ~line ~col in
22+
let getLocItem ~full ~pos ~debug =
23+
let log n msg = if debug then Printf.printf "getLocItem #%d: %s\n" n msg in
24+
let pos = lineColToCmtLoc ~pos in
2325
let locItems = locItemsForPos ~extra:full.extra pos in
2426
if !Log.verbose then
2527
print_endline
@@ -28,30 +30,33 @@ let getLocItem ~full ~line ~col =
2830
match locItems with
2931
| _ :: _ :: _ :: ({locType = Typed ("makeProps", _, _)} as li) :: _
3032
when full.file.uri |> Uri2.isInterface ->
31-
(* heuristic for makeProps in interface files *)
33+
log 1 "heuristic for makeProps in interface files";
3234
Some li
3335
| [
3436
{locType = Typed ("fragment", _, _)};
3537
{locType = Typed ("createElement", _, _)};
3638
] ->
37-
(* heuristic for </Comp> within a fragment *)
39+
log 2 "heuristic for </Comp> within a fragment";
3840
None
3941
| [
4042
{locType = Constant _};
4143
({locType = Typed ("createDOMElementVariadic", _, _)} as li2);
4244
] ->
43-
(* heuristic for <div> *)
45+
log 3 "heuristic for <div>";
4446
Some li2
4547
| {locType = Typed ("makeProps", _, _)}
4648
:: ({locType = Typed ("make", _, _)} as li2) :: _ ->
47-
(* heuristic for </Comp> within fragments: take make as makeProps does not work
48-
the type is not greatl but jump to definition works *)
49+
log 4
50+
"heuristic for </Comp> within fragments: take make as makeProps does not \
51+
work\n\
52+
the type is not great but jump to definition works";
4953
Some li2
5054
| [({locType = Typed (_, _, LocalReference _)} as li1); li3]
5155
when li1.loc = li3.loc ->
52-
(* JSX and compiler combined:
53-
~x becomes Props#x
54-
heuristic for: [Props, x], give loc of `x` *)
56+
log 5
57+
"heuristic for JSX and compiler combined:\n\
58+
~x becomes Props#x\n\
59+
heuristic for: [Props, x], give loc of `x`";
5560
Some li3
5661
| [
5762
({locType = Typed (_, _, LocalReference _)} as li1);
@@ -61,28 +66,33 @@ let getLocItem ~full ~line ~col =
6166
]
6267
(* For older compiler 9.0 or earlier *)
6368
when li1.loc = li2.loc && li2.loc = li3.loc ->
64-
(* JSX and compiler combined:
65-
~x becomes Js_OO.unsafe_downgrade(Props)#x
66-
heuristic for: [Props, unsafe_downgrade, x], give loc of `x` *)
69+
log 6
70+
"heuristic for JSX and compiler combined:\n\
71+
~x becomes Js_OO.unsafe_downgrade(Props)#x\n\
72+
heuristic for: [Props, unsafe_downgrade, x], give loc of `x`";
6773
Some li3
6874
| [
6975
{locType = Typed (_, _, LocalReference (_, Value))};
7076
({locType = Typed (_, _, Definition (_, Value))} as li2);
7177
] ->
72-
(* JSX on type-annotated labeled (~arg:t):
73-
(~arg:t) becomes Props#arg
74-
Props has the location range of arg:t
75-
arg has the location range of arg
76-
heuristic for: [Props, arg], give loc of `arg` *)
78+
log 7
79+
"heuristic for JSX on type-annotated labeled (~arg:t):\n\
80+
(~arg:t) becomes Props#arg\n\
81+
Props has the location range of arg:t\n\
82+
arg has the location range of arg\n\
83+
heuristic for: [Props, arg], give loc of `arg`";
7784
Some li2
7885
| [li1; li2; li3] when li1.loc = li2.loc && li2.loc = li3.loc ->
79-
(* JSX with at most one child
80-
heuristic for: [makeProps, make, createElement], give the loc of `make` *)
86+
log 8
87+
"heuristic for JSX with at most one child\n\
88+
heuristic for: [makeProps, make, createElement], give the loc of `make` ";
8189
Some li2
8290
| [li1; li2; li3; li4]
8391
when li1.loc = li2.loc && li2.loc = li3.loc && li3.loc = li4.loc ->
84-
(* JSX variadic, e.g. <C> {x} {y} </C>
85-
heuristic for: [makeProps , React.null, make, createElementVariadic], give the loc of `make` *)
92+
log 9
93+
"heuristic for JSX variadic, e.g. <C> {x} {y} </C>\n\
94+
heuristic for: [makeProps , React.null, make, createElementVariadic], \
95+
give the loc of `make`";
8696
Some li3
8797
| {locType = Typed (_, {desc = Tconstr (path, _, _)}, _)} :: li :: _
8898
when Utils.isUncurriedInternal path ->

analysis/src/Xform.ml

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -223,16 +223,14 @@ module AddTypeAnnotation = struct
223223
in
224224
{Ast_iterator.default_iterator with structure_item}
225225

226-
let xform ~path ~pos ~full ~structure ~codeActions =
227-
let line, col = pos in
228-
226+
let xform ~path ~pos ~full ~structure ~codeActions ~debug =
229227
let result = ref None in
230228
let iterator = mkIterator ~pos ~result in
231229
iterator.structure iterator structure;
232230
match !result with
233231
| None -> ()
234232
| Some annotation -> (
235-
match References.getLocItem ~full ~line ~col with
233+
match References.getLocItem ~full ~pos ~debug with
236234
| None -> ()
237235
| Some locItem -> (
238236
match locItem.locType with
@@ -297,14 +295,14 @@ let parse ~filename =
297295
in
298296
(structure, printExpr, printStructureItem)
299297

300-
let extractCodeActions ~path ~pos ~currentFile =
298+
let extractCodeActions ~path ~pos ~currentFile ~debug =
301299
match Cmt.fullFromPath ~path with
302300
| Some full when Filename.check_suffix currentFile ".res" ->
303301
let structure, printExpr, printStructureItem =
304302
parse ~filename:currentFile
305303
in
306304
let codeActions = ref [] in
307-
AddTypeAnnotation.xform ~path ~pos ~full ~structure ~codeActions;
305+
AddTypeAnnotation.xform ~path ~pos ~full ~structure ~codeActions ~debug;
308306
IfThenElse.xform ~pos ~codeActions ~printExpr ~path structure;
309307
AddBracesToFn.xform ~pos ~codeActions ~path ~printStructureItem structure;
310308
!codeActions

analysis/tests/src/expected/Div.res.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
Hover src/Div.res 0:10
2+
getLocItem #3: heuristic for <div>
23
{"contents": "```rescript\n(\n string,\n ~props: ReactDOMRe.domProps=?,\n array<React.element>,\n) => React.element\n```"}
34

45
Complete src/Div.res 3:17

analysis/tests/src/expected/Fragment.res.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
Hover src/Fragment.res 6:19
2+
getLocItem #4: heuristic for </Comp> within fragments: take make as makeProps does not work
3+
the type is not great but jump to definition works
24
{"contents": "```rescript\nReact.component<{\"children\": React.element}>\n```\n\n```rescript\ntype component<'props> = componentLike<'props, element>\n```"}
35

46
Hover src/Fragment.res 9:56
7+
getLocItem #2: heuristic for </Comp> within a fragment
58
Nothing at that position. Now trying to use completion.
69
posCursor:[9:56] posNoWhite:[9:55] Found expr:[9:10->9:67]
710
Pexp_construct :::[9:13->9:67] [9:13->9:67]

analysis/tests/src/expected/Hover.res.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,17 @@ Hover src/Hover.res 33:4
2020
{"contents": "```rescript\nunit => int\n```\n\nDoc comment for functionWithTypeAnnotation"}
2121

2222
Hover src/Hover.res 37:13
23+
getLocItem #5: SX and compiler combined:
24+
~x becomes Props#x
25+
heuristic for: [Props, x], give loc of `x`
2326
{"contents": "```rescript\nstring\n```"}
2427

2528
Hover src/Hover.res 41:13
29+
getLocItem #7: JSX on type-annotated labeled (~arg:t):
30+
(~arg:t) becomes Props#arg
31+
Props has the location range of arg:t
32+
arg has the location range of arg
33+
heuristic for: [Props, arg], give loc of `arg`
2634
{"contents": "```rescript\nstring\n```"}
2735

2836
Hover src/Hover.res 44:10
@@ -47,9 +55,13 @@ Hover src/Hover.res 75:7
4755
{"contents": "```rescript\nmodule A = {\n let x: int\n}\n```"}
4856

4957
Hover src/Hover.res 85:10
58+
getLocItem #9: JSX variadic, e.g. <C> {x} {y} </C>
59+
heuristic for: [makeProps , React.null, make, createElementVariadic], give the loc of `make`
5060
{"contents": "```rescript\nReact.component<{\"children\": React.element}>\n```\n\n```rescript\ntype component<'props> = componentLike<'props, element>\n```"}
5161

5262
Hover src/Hover.res 88:10
63+
getLocItem #9: JSX variadic, e.g. <C> {x} {y} </C>
64+
heuristic for: [makeProps , React.null, make, createElementVariadic], give the loc of `make`
5365
{"contents": "```rescript\nReact.component<{\"children\": React.element}>\n```\n\n```rescript\ntype component<'props> = componentLike<'props, element>\n```"}
5466

5567
Hover src/Hover.res 93:25

analysis/tests/src/expected/Jsx.res.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
Definition src/Jsx.res 5:9
2+
getLocItem #4: heuristic for </Comp> within fragments: take make as makeProps does not work
3+
the type is not great but jump to definition works
24
{"uri": "Jsx.res", "range": {"start": {"line": 2, "character": 6}, "end": {"line": 2, "character": 10}}}
35

46
Complete src/Jsx.res 8:15
@@ -196,6 +198,8 @@ Completable: Cjsx([M], k, [prop, k])
196198
}]
197199

198200
Definition src/Jsx.res 58:11
201+
getLocItem #4: heuristic for </Comp> within fragments: take make as makeProps does not work
202+
the type is not great but jump to definition works
199203
{"uri": "Component.res", "range": {"start": {"line": 1, "character": 4}, "end": {"line": 1, "character": 8}}}
200204

201205
Complete src/Jsx.res 68:10

0 commit comments

Comments
 (0)