From aa447dc8e4cebad089a360b3b8cca61a581ca1ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulysse=20G=C3=A9rard?= Date: Tue, 26 Nov 2024 13:51:10 +0100 Subject: [PATCH 1/3] Add a test illustrating the issue --- tests/test-dirs/occurrences/no-def-mli-only.t | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/test-dirs/occurrences/no-def-mli-only.t diff --git a/tests/test-dirs/occurrences/no-def-mli-only.t b/tests/test-dirs/occurrences/no-def-mli-only.t new file mode 100644 index 000000000..a9f54fa9e --- /dev/null +++ b/tests/test-dirs/occurrences/no-def-mli-only.t @@ -0,0 +1,26 @@ + $ cat >noml.mli <<'EOF' + > type t = unit + > EOF + + $ cat >noml.ml <<'EOF' + > type t = unit + > EOF + + $ cat >main.ml <<'EOF' + > let x : Noml.t = () + > let y : Noml.t = () + > EOF + + $ $OCAMLC -c -bin-annot noml.mli noml.ml main.ml + +We remove the source file to mimick cases were generated source files are not +accessible to Merlin. + $ rm noml.ml + +FIXME: We still expect occurrences of definitions in hidden source files to work + $ $MERLIN single occurrences -identifier-at 2:13 -filename main.ml Date: Tue, 26 Nov 2024 13:53:23 +0100 Subject: [PATCH 2/3] Locate: don't discard results when the source file is not found Queries such as occurrences should work as long as a definition uid was found. --- src/analysis/locate.ml | 29 +++++++++++++------ src/analysis/locate.mli | 7 ++--- src/analysis/occurrences.ml | 6 ++-- src/frontend/query_commands.ml | 6 ++-- tests/test-dirs/occurrences/no-def-mli-only.t | 25 ++++++++++++++-- 5 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/analysis/locate.ml b/src/analysis/locate.ml index e1a06ee6a..5f6e3a654 100644 --- a/src/analysis/locate.ml +++ b/src/analysis/locate.ml @@ -643,7 +643,14 @@ let from_path ~config ~env ~local_defs ~decl path = log ~title:"find_source" "Found file: %s (%a)" file Logger.fmt (Fun.flip Location.print_loc location); `Found { uid; decl_uid = decl.uid; file; location; approximated } - | `File_not_found _ as otherwise -> otherwise) + | `File_not_found reason -> + `File_not_found + { uid; + decl_uid = decl.uid; + file = reason; + location = loc; + approximated + }) let from_longident ~config ~env ~local_defs nss ident = let str_ident = @@ -851,21 +858,25 @@ let get_doc ~config:mconfig ~env ~local_defs ~comments ~pos = let from_path = from_path ~config ~env ~local_defs ~namespace path in begin match from_path with - | `Found { uid; location = loc; _ } -> doc_from_uid ~config ~loc uid - | (`Builtin _ | `Not_in_env _ | `File_not_found _ | `Not_found _) as - otherwise -> otherwise + | `Found { uid; location = loc; _ } + | `File_not_found { uid; location = loc; _ } -> + doc_from_uid ~config ~loc uid + | (`Builtin _ | `Not_in_env _ | `Not_found _) as otherwise -> + otherwise end | `User_input path -> log ~title:"get_doc" "looking for the doc of '%s'" path; begin match from_string ~config ~env ~local_defs ~pos path with - | `Found { uid; location = loc; _ } -> doc_from_uid ~config ~loc uid + | `Found { uid; location = loc; _ } + | `File_not_found { uid; location = loc; _ } -> + doc_from_uid ~config ~loc uid | `At_origin -> `Found_loc { Location.loc_start = pos; loc_end = pos; loc_ghost = true } | `Missing_labels_namespace -> `No_documentation - | (`Builtin _ | `Not_in_env _ | `Not_found _ | `File_not_found _) as - otherwise -> otherwise + | (`Builtin _ | `Not_in_env _ | `Not_found _) as otherwise -> + otherwise end in match doc_from_uid_result with @@ -901,5 +912,5 @@ let get_doc ~config:mconfig ~env ~local_defs ~comments ~pos = | `User_input path -> `Builtin path | `Completion_entry (_, path, _) -> `Builtin (Path.name path) end - | (`File_not_found _ | `Not_found _ | `No_documentation | `Not_in_env _) as - otherwise -> otherwise + | (`Not_found _ | `No_documentation | `Not_in_env _) as otherwise -> + otherwise diff --git a/src/analysis/locate.mli b/src/analysis/locate.mli index 226e5d767..f685087ca 100644 --- a/src/analysis/locate.mli +++ b/src/analysis/locate.mli @@ -54,7 +54,7 @@ val from_path : local_defs:Mtyper.typedtree -> namespace:Env_lookup.Namespace.t -> Path.t -> - [> `File_not_found of string + [> `File_not_found of result | `Found of result | `Builtin of Shape.Uid.t * string | `Not_in_env of string @@ -67,7 +67,7 @@ val from_string : pos:Lexing.position -> ?namespaces:Env_lookup.Namespace.inferred_basic list -> string -> - [> `File_not_found of string + [> `File_not_found of result | `Found of result | `Builtin of Shape.Uid.t * string | `Missing_labels_namespace @@ -83,8 +83,7 @@ val get_doc : pos:Lexing.position -> [ `User_input of string | `Completion_entry of Env_lookup.Namespace.t * Path.t * Location.t ] -> - [> `File_not_found of string - | `Found of string + [> `Found of string | `Builtin of string | `Not_found of string * string option | `Not_in_env of string diff --git a/src/analysis/occurrences.ml b/src/analysis/occurrences.ml index 829c8146c..78dcc2d5c 100644 --- a/src/analysis/occurrences.ml +++ b/src/analysis/occurrences.ml @@ -151,7 +151,8 @@ let locs_of ~config ~env ~typer_result ~pos ~scope path = | _ -> scope in (node_uid_loc, scope) - | `Found { uid; location; approximated = false; _ } -> + | `Found { uid; location; approximated = false; _ } + | `File_not_found { uid; location; approximated = false; _ } -> log ~title:"locs_of" "Found definition uid using locate: %a " Logger.fmt (fun fmt -> Shape.Uid.print fmt uid); (* There is no way to distinguish uids from interfaces from uids of @@ -161,7 +162,8 @@ let locs_of ~config ~env ~typer_result ~pos ~scope path = are actually linked. *) let scope = if is_in_interface config location then `Buffer else scope in (Some (uid, location), scope) - | `Found { decl_uid; location; approximated = true; _ } -> + | `Found { decl_uid; location; approximated = true; _ } + | `File_not_found { decl_uid; location; approximated = true; _ } -> log ~title:"locs_of" "Approx. definition: %a " Logger.fmt (fun fmt -> Shape.Uid.print fmt decl_uid); (Some (decl_uid, location), `Buffer) diff --git a/src/frontend/query_commands.ml b/src/frontend/query_commands.ml index c36ecdaf1..6bd337099 100644 --- a/src/frontend/query_commands.ml +++ b/src/frontend/query_commands.ml @@ -349,7 +349,7 @@ let dispatch pipeline (type a) : a Query_protocol.t -> a = function | `Not_in_env _ as s -> s | `Not_found _ as s -> s | `Found { file; location; _ } -> `Found (Some file, location.loc_start) - | `File_not_found _ as s -> s) + | `File_not_found { file = reason; _ } -> `File_not_found reason) end | Complete_prefix (prefix, pos, kinds, with_doc, with_types) -> let pipeline, typer = for_completion pipeline pos in @@ -527,8 +527,8 @@ let dispatch pipeline (type a) : a Query_protocol.t -> a = function | `Builtin (_, s) -> Locate.log ~title:"result" "found builtin %s" s; `Builtin s - | (`Not_found _ | `At_origin | `Not_in_env _ | `File_not_found _) as - otherwise -> + | `File_not_found { file = reason; _ } -> `File_not_found reason + | (`Not_found _ | `At_origin | `Not_in_env _) as otherwise -> Locate.log ~title:"result" "not found"; otherwise end diff --git a/tests/test-dirs/occurrences/no-def-mli-only.t b/tests/test-dirs/occurrences/no-def-mli-only.t index a9f54fa9e..479cc1aa6 100644 --- a/tests/test-dirs/occurrences/no-def-mli-only.t +++ b/tests/test-dirs/occurrences/no-def-mli-only.t @@ -17,10 +17,31 @@ We remove the source file to mimick cases were generated source files are not accessible to Merlin. $ rm noml.ml -FIXME: We still expect occurrences of definitions in hidden source files to work +We still expect occurrences of definitions in hidden source files to work $ $MERLIN single occurrences -identifier-at 2:13 -filename main.ml Date: Tue, 26 Nov 2024 13:55:07 +0100 Subject: [PATCH 3/3] Add changelog entry for #1865 --- CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 0b46eaed4..6c9520aea 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,8 @@ unreleased - Fix jump to `fun` targets not working (#1863, fixes #1862) - Fix type-enclosing results instability. This reverts some overly aggressive deduplication that should be done on the client side. (#1864) + - Fix occurrences not working when the definition comes from a hidden source + file (#1865) merlin 5.2.1 ============