Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert: remove reparsing of longident to index modules in paths #1772

Merged
merged 1 commit into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 5 additions & 37 deletions src/analysis/occurrences.ml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ let decl_of_path_or_lid env namespace path lid =
end
| _ -> Env_lookup.by_path path namespace env

let index_buffer_ ~config ~source ~current_buffer_path ~local_defs () =
let index_buffer_ ~current_buffer_path ~local_defs () =
let {Logger. log} = Logger.for_section "index" in
let defs = Hashtbl.create 64 in
let module Shape_reduce =
Expand Down Expand Up @@ -80,46 +80,14 @@ let index_buffer_ ~config ~source ~current_buffer_path ~local_defs () =
index_decl ()
end
in
let f ~namespace env path (lid : Longident.t Location.loc) =
(* The compiler lacks sufficient location information to precisely hihglight
modules in paths. This function hacks around that issue when looking for
occurrences in the current buffer only. *)
(* We rely on a custom re-parsing of the longidents that provide us with
location information and match these with the real path and longident. *)
let rec iter_on_path ~namespace (path : Path.t) lid reparsed =
log ~title:"iter_on_path" "Path %a, lid: %a"
Logger.fmt (Fun.flip Path.print path)
Logger.fmt (Fun.flip Pprintast.longident lid.Location.txt);
match path, lid.txt, reparsed with
| Pdot (path', n), (Ldot (_, s) | Lident s), _
when n <> s && String.lowercase_ascii n = n ->
iter_on_path ~namespace path' lid reparsed
| (Pdot _ | Pident _), Lident s, [{ Location.txt = name; loc}]
when name = s ->
log ~title:"iter_on_path" "Last %a, lid: %a"
Logger.fmt (Fun.flip Path.print path)
Logger.fmt (Fun.flip Pprintast.longident lid.Location.txt);
f ~namespace env path { lid with loc = loc }
| Pdot (path', _), Ldot (lid', s), { txt = name; loc} :: tl when name = s ->
let () = f ~namespace env path { lid with loc = loc } in
iter_on_path ~namespace:Module path' { lid with txt = lid' } tl
| Papply _, _, _ -> f ~namespace env path lid
| _, _, _ -> f ~namespace env path lid
in
let reparsed_lid =
Misc_utils.parse_identifier (config, source) lid.loc.loc_end
|> List.rev
in
iter_on_path ~namespace path lid reparsed_lid
in
Ast_iterators.iter_on_usages ~f local_defs;
defs

let index_buffer =
(* Right now, we only cache the last used index. We could do better by caching
the index for every known buffer. *)
let cache = ref None in
fun ~config ~source ~current_buffer_path ~stamp ~local_defs () ->
fun ~current_buffer_path ~stamp ~local_defs () ->
let {Logger. log} = Logger.for_section "index" in
match !cache with
| Some (path, stamp', value) when
Expand All @@ -131,7 +99,7 @@ let index_buffer =
| _ ->
log ~title:"index_cache" "No valid cache found, reindexing.";
let result =
index_buffer_ ~config ~source ~current_buffer_path ~local_defs ()
index_buffer_ ~current_buffer_path ~local_defs ()
in
cache := Some (current_buffer_path, stamp, result);
result
Expand Down Expand Up @@ -189,7 +157,7 @@ let comp_unit_of_uid = function
| Item { comp_unit; _ } -> Some comp_unit
| Internal | Predef _ -> None

let locs_of ~config ~source ~env ~typer_result ~pos path =
let locs_of ~config ~env ~typer_result ~pos path =
log ~title:"occurrences" "Looking for occurences of %s (pos: %s)"
path
(Lexing.print_position () pos);
Expand Down Expand Up @@ -233,7 +201,7 @@ let locs_of ~config ~source ~env ~typer_result ~pos path =
log ~title:"locs_of" "Indexing current buffer";
let buffer_index =
let stamp = Mtyper.get_stamp typer_result in
index_buffer ~config ~source ~current_buffer_path ~stamp ~local_defs ()
index_buffer ~current_buffer_path ~stamp ~local_defs ()
in
let buffer_locs = Hashtbl.find_opt buffer_index def_uid in
let locs = Option.value ~default:LidSet.empty buffer_locs in
Expand Down
1 change: 0 additions & 1 deletion src/analysis/occurrences.mli
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
val locs_of
: config:Mconfig.t
-> source:Msource.t
-> env:Env.t
-> typer_result:Mtyper.result
-> pos:Lexing.position
Expand Down
3 changes: 1 addition & 2 deletions src/frontend/query_commands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,6 @@ let dispatch pipeline (type a) : a Query_protocol.t -> a =

| Occurrences (`Ident_at pos, _) ->
let config = Mpipeline.final_config pipeline in
let source = Mpipeline.raw_source pipeline in
let typer_result = Mpipeline.typer_result pipeline in
let pos = Mpipeline.get_lexing_pos pipeline pos in
let env, _node = Mbrowse.leaf_node (Mtyper.node_at typer_result pos) in
Expand All @@ -810,7 +809,7 @@ let dispatch pipeline (type a) : a Query_protocol.t -> a =
path
in
let locs =
Occurrences.locs_of ~config ~source ~env ~typer_result ~pos path
Occurrences.locs_of ~config ~env ~typer_result ~pos path
|> Result.value ~default:[]
in
let loc_start l = l.Location.loc_start in
Expand Down
21 changes: 1 addition & 20 deletions tests/test-dirs/occurrences/mod-in-path-2.t
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
> | Mod.A -> ()
> EOF

FIXME: we could expect module appearing in paths to be highlighted
$ $MERLIN single occurrences -identifier-at 1:8 -filename test.ml <test.ml |
> jq '.value'
[
Expand All @@ -19,26 +20,6 @@
"line": 1,
"col": 10
}
},
{
"start": {
"line": 5,
"col": 8
},
"end": {
"line": 5,
"col": 11
}
},
{
"start": {
"line": 6,
"col": 4
},
"end": {
"line": 6,
"col": 7
}
}
]

Expand Down
21 changes: 1 addition & 20 deletions tests/test-dirs/occurrences/mod-in-path-3.t
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
> | Mod.A r -> r.lbl
> EOF

FIXME: we could expect module appearing in paths to be highlighted
$ $MERLIN single occurrences -identifier-at 4:9 -filename test.ml <test.ml |
> jq '.value'
[
Expand All @@ -20,26 +21,6 @@
"line": 1,
"col": 10
}
},
{
"start": {
"line": 4,
"col": 8
},
"end": {
"line": 4,
"col": 11
}
},
{
"start": {
"line": 7,
"col": 4
},
"end": {
"line": 7,
"col": 7
}
}
]

Expand Down
73 changes: 2 additions & 71 deletions tests/test-dirs/occurrences/mod-in-path.t
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
> let _ = let open Mod in Nod.x
> EOF

FIXME: we could expect module appearing in paths to be highlighted
$ $MERLIN single occurrences -identifier-at 6:13 -filename test.ml <test.ml |
> jq '.value'
[
Expand All @@ -25,50 +26,10 @@
"line": 2,
"col": 12
}
},
{
"start": {
"line": 6,
"col": 12
},
"end": {
"line": 6,
"col": 15
}
},
{
"start": {
"line": 7,
"col": 14
},
"end": {
"line": 7,
"col": 17
}
},
{
"start": {
"line": 9,
"col": 0
},
"end": {
"line": 9,
"col": 3
}
},
{
"start": {
"line": 12,
"col": 24
},
"end": {
"line": 12,
"col": 27
}
}
]


FIXME: we could expect module appearing in paths to be highlighted
$ $MERLIN single occurrences -identifier-at 12:18 -filename test.ml <test.ml |
> jq '.value'
[
Expand All @@ -82,36 +43,6 @@
"col": 10
}
},
{
"start": {
"line": 6,
"col": 8
},
"end": {
"line": 6,
"col": 11
}
},
{
"start": {
"line": 7,
"col": 8
},
"end": {
"line": 7,
"col": 11
}
},
{
"start": {
"line": 8,
"col": 8
},
"end": {
"line": 8,
"col": 11
}
},
{
"start": {
"line": 12,
Expand Down
Loading