uid_of_path can use the decl_id if shapes fail

locate: fallback after failing to load the uid_to_loc table (or if there is no appropriate entre in the table)
ocaml · Nov 13, 2023 · 5b191db · 5b191db
1 parent bce00ec
commit 5b191db
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 53 deletions.
diff --git a/src/analysis/locate.ml b/src/analysis/locate.ml
@@ -361,22 +361,27 @@ let uid_of_path ~config ~env ~ml_or_mli ~decl_uid path namespace =
         ~namespace:Shape.Sig_component_kind.Module env (Pident id)
     end)
   in
-  match ml_or_mli with
-  | `MLI ->
-    let uid = scrape_alias ~fallback_uid:decl_uid ~env ~namespace path in
-    log ~title:"uid_of_path" "Declaration uid: %a"
-      Logger.fmt (fun fmt -> Shape.Uid.print fmt decl_uid);
-    log ~title:"uid_of_path" "Alias scrapped: %a"
+  let unalias fallback_uid =
+    let uid = scrape_alias ~fallback_uid ~env ~namespace path in
+    log ~title:"uid_of_path" "Unaliasing uid: %a -> %a"
+      Logger.fmt (fun fmt -> Shape.Uid.print fmt fallback_uid)
       Logger.fmt (fun fmt -> Shape.Uid.print fmt uid);
-    Some uid
+    uid
+  in
+  match ml_or_mli with
+  | `MLI -> unalias decl_uid
   | `ML ->
     let shape = Env.shape_of_path ~namespace env path in
     log ~title:"shape_of_path" "initial: %a"
       Logger.fmt (fun fmt -> Shape.print fmt shape);
     let r = Shape_reduce.weak_reduce env shape in
     log ~title:"shape_of_path" "reduced: %a"
       Logger.fmt (fun fmt -> Shape.print fmt r);
-    r.uid
+    match r.uid with
+    | Some uid -> uid
+    | None ->
+      log ~title:"shape_of_path" "No uid found; fallbacking to declaration uid";
+      unalias decl_uid
 
 let from_uid ~config ~ml_or_mli uid loc path =
   let loc_of_comp_unit comp_unit =
@@ -389,61 +394,48 @@ let from_uid ~config ~ml_or_mli uid loc path =
   in
   let title = "from_uid" in
   match uid with
-  | Some (Shape.Uid.Item { comp_unit; _ } as uid) ->
+  | Shape.Uid.Item { comp_unit; _ } ->
     let locopt =
-      if Env.get_unit_name () = comp_unit then begin
-        log ~title "We look for %a in the current compilation unit."
+      let log_and_return msg = log ~title msg; None in
+      let uid_to_loc_tbl =
+        if Env.get_unit_name () = comp_unit then begin
+          log ~title "We look for %a in the current compilation unit."
+            Logger.fmt (fun fmt -> Shape.Uid.print fmt uid);
+          Some (Env.get_uid_to_loc_tbl ())
+        end else begin
+          log ~title "Loading the cmt for unit %S" comp_unit;
+          match load_cmt ~config comp_unit ml_or_mli with
+          | Ok (_pos_fname, cmt) -> Some cmt.cmt_uid_to_loc
+          | Error () -> log_and_return "Failed to load the cmt file."
+        end
+      in
+      Option.bind uid_to_loc_tbl ~f:(fun tbl ->
+        log ~title "Looking for %a in the uid_to_loc table"
           Logger.fmt (fun fmt -> Shape.Uid.print fmt uid);
-        let tbl = Env.get_uid_to_loc_tbl () in
         match Shape.Uid.Tbl.find_opt tbl uid with
         | Some loc ->
           log ~title "Found location: %a"
             Logger.fmt (fun fmt -> Location.print_loc fmt loc);
           Some (uid, loc)
-        | None ->
-          log ~title
-            "Uid not found in the local table.\
-            Fallbacking to the node's location: %a"
-          Logger.fmt (fun fmt -> Location.print_loc fmt loc);
-          Some (uid, loc)
-      end else begin
-        log ~title "Loading the shapes for unit %S" comp_unit;
-        match load_cmt ~config comp_unit ml_or_mli with
-        | Ok (_pos_fname, cmt) ->
-          log ~title "Shapes successfully loaded, looking for %a"
-            Logger.fmt (fun fmt -> Shape.Uid.print fmt uid);
-          begin match Shape.Uid.Tbl.find_opt cmt.cmt_uid_to_loc uid with
-            | Some loc ->
-              log ~title "Found location: %a"
-                Logger.fmt (fun fmt -> Location.print_loc fmt loc);
-              Some (uid, loc)
-            | None ->
-              log ~title "Uid not found in the cmt table. \
-                Fallbacking to the node's location: %a"
-                Logger.fmt (fun fmt -> Location.print_loc fmt loc);
-              Some (uid, loc)
-          end
-        | _ ->
-          log ~title "Failed to load the shapes";
-          None
-      end
+        | None -> log_and_return  "Uid not found in the table.")
     in
     begin match locopt with
     | Some (uid, loc) -> `Found (Some uid, loc)
-    | None -> `Not_found (Path.name path, None)
+    | None ->
+      log ~title "Fallbacking to lookup location: %a"
+        Logger.fmt (fun fmt -> Location.print_loc fmt loc);
+      `Found (Some uid, loc)
     end
-  | Some (Compilation_unit comp_unit as uid) ->
+  | Compilation_unit comp_unit ->
     begin
       log ~title "Got the uid of a compilation unit: %a"
         Logger.fmt (fun fmt -> Shape.Uid.print fmt uid);
       match loc_of_comp_unit comp_unit with
       | Some loc -> `Found (Some uid, loc)
-      | _ -> log ~title "Failed to load the shapes";
+      | _ -> log ~title "Failed to load the CU's cmt";
         `Not_found (Path.name path, None)
     end
-  | Some (Predef _ | Internal) -> assert false
-  | None -> log ~title "No UID found, fallbacking to lookup location.";
-      `Found (None, loc)
+  | Predef _ | Internal -> assert false
 
 let locate ~config ~env ~ml_or_mli decl_uid loc path ns =
   let uid = uid_of_path ~config ~env ~ml_or_mli ~decl_uid path ns in

diff --git a/tests/test-dirs/document/issue1513.t b/tests/test-dirs/document/issue1513.t
@@ -20,8 +20,8 @@ FIXME: We should not rely on "fallbacking". This requires a compiler change.
   $ $MERLIN single document -position 1:13 \
   > -log-file - -log-section locate \
   > -filename main.ml <main.ml 2>&1 | 
-  > grep "Uid not found in the cmt table"
-  Uid not found in the cmt table. Fallbacking to the node's location: File "naux.ml", line 2, characters 2-5
+  > grep "Uid not found in the table."
+  Uid not found in the table.
 
 FIXME: expected "B Comment"
   $ $MERLIN single document -position 2:13 \

diff --git a/tests/test-dirs/locate/local-build-scheme.t b/tests/test-dirs/locate/local-build-scheme.t
@@ -37,5 +37,11 @@
   > -build-path experimental -build-path unix \
   > -source-path . -source-path unix -source-path experimental \
   > -filename hack.ml <hack.ml | jq '.value'
-  "Several source files in your path have the same name, and merlin doesn't know which is the right one: $TESTCASE_ROOT/unix/m_intf.ml, $TESTCASE_ROOT/experimental/m_intf.ml"
+  {
+    "file": "$TESTCASE_ROOT/experimental/m_intf.ml",
+    "pos": {
+      "line": 1,
+      "col": 20
+    }
+  }
 
diff --git a/tests/test-dirs/locate/non-local/ignore-kept-locs.t/run.t b/tests/test-dirs/locate/non-local/ignore-kept-locs.t/run.t
@@ -20,8 +20,8 @@ available:
   }
 
   $ grep -A1 from_uid log | grep -v from_uid | sed '/^--$/d'
-  Loading the shapes for unit "A"
-  Shapes successfully loaded, looking for A.0
+  Loading the cmt for unit "A"
+  Looking for A.0 in the uid_to_loc table
   Found location: File "a.ml", line 1, characters 4-9
 
   $ rm log
@@ -41,8 +41,8 @@ available:
   }
 
   $ grep -A1 from_uid log | grep -v from_uid | sed '/^--$/d'
-  Loading the shapes for unit "A"
-  Shapes successfully loaded, looking for A.0
+  Loading the cmt for unit "A"
+  Looking for A.0 in the uid_to_loc table
   Found location: File "a.ml", line 1, characters 4-9
 
   $ rm log
@@ -66,6 +66,9 @@ In the absence of cmt though, fallbacking to the cmi loc makes sense:
   }
 
   $ grep -A1 from_uid log | grep -v from_uid
-  No UID found, fallbacking to lookup location.
+  Loading the cmt for unit "A"
+  --
+  Failed to load the cmt file.
+  Fallbacking to lookup location: File "a.ml", line 1, characters 4-9
 
   $ rm log