Clean up references: represent explicitly references to toplevel modules.

Author: cristianoc

analysis/src/Commands.ml

@@ -126,7 +126,12 @@ let references ~path ~line ~col =
         let allLocs =
           allReferences
           |> List.fold_left
-               (fun acc {References.uri = uri2; loc} ->
+               (fun acc {References.uri = uri2; locOpt} ->
+                 let loc =
+                   match locOpt with
+                   | Some loc -> loc
+                   | None -> Utils.topLoc (Uri2.toPath uri2)
+                 in
                  Protocol.stringifyLocation
                    {uri = Uri2.toString uri2; range = Utils.cmtLocToRange loc}
                  :: acc)
@@ -182,16 +187,20 @@ let rename ~path ~line ~col ~newName =
       | None -> Protocol.null
       | Some locItem ->
         let allReferences = References.allReferencesForLocItem ~full locItem in
-        let referencesToToplevelModules, referencesToItems =
+        let referencesToToplevelModules =
           allReferences
-          |> List.fold_left
-               (fun acc {References.uri = uri2; loc} -> (uri2, loc) :: acc)
-               []
-          |> List.partition (fun (_, loc) -> Utils.isTopLoc loc)
+          |> Utils.filterMap (fun {References.uri = uri2; locOpt} ->
+                 if locOpt = None then Some uri2 else None)
+        in
+        let referencesToItems =
+          allReferences
+          |> Utils.filterMap (function
+               | {References.uri = uri2; locOpt = Some loc} -> Some (uri2, loc)
+               | {locOpt = None} -> None)
         in
         let fileRenames =
           referencesToToplevelModules
-          |> List.map (fun (uri, _) ->
+          |> List.map (fun uri ->
                  let path = Uri2.toPath uri in
                  let dir = Filename.dirname path in
                  let newPath =

analysis/src/References.ml

@@ -379,7 +379,10 @@ let rec pathFromVisibility visibilityPath current =
 let pathFromVisibility visibilityPath tipName =
   pathFromVisibility visibilityPath (Tip tipName)
 
-type references = {uri : Uri2.t; loc : Location.t}
+type references = {
+  uri : Uri2.t;
+  locOpt : Location.t option; (* None: reference to a toplevel module *)
+}
 
 let forLocalStamp ~full:{file; extra; package} stamp tip =
   let env = QueryEnv.fromFile file in
@@ -419,7 +422,9 @@ let forLocalStamp ~full:{file; extra; package} stamp tip =
                   with
                   | None -> []
                   | Some locs ->
-                    locs |> List.map (fun loc -> {uri = file.uri; loc})))
+                    locs
+                    |> List.map (fun loc -> {uri = file.uri; locOpt = Some loc})
+                  ))
               (* if this file has a corresponding interface or implementation file
                  also find the references in that file *)
             in
@@ -450,8 +455,9 @@ let forLocalStamp ~full:{file; extra; package} stamp tip =
                                       if p = path && t = tip then Some locs
                                       else None)
                              in
-                             locs |> List.map (fun loc -> {uri = file.uri; loc})
-                           )))
+                             locs
+                             |> List.map (fun loc ->
+                                    {uri = file.uri; locOpt = Some loc}))))
                 |> List.concat
               in
               alternativeReferences @ externals)
@@ -460,7 +466,7 @@ let forLocalStamp ~full:{file; extra; package} stamp tip =
             [])
       in
       List.append
-        (locs |> List.map (fun loc -> {uri = file.uri; loc}))
+        (locs |> List.map (fun loc -> {uri = file.uri; locOpt = Some loc}))
         externals)
 
 let allReferencesForLocItem ~full:({file; package} as full) locItem =
@@ -480,21 +486,15 @@ let allReferencesForLocItem ~full:({file; package} as full) locItem =
                |> List.map (fun loc ->
                       {
                         uri = Uri2.fromPath loc.Location.loc_start.pos_fname;
-                        loc;
+                        locOpt = Some loc;
                       }))
       |> List.flatten
     in
     let targetModuleReferences =
       match Hashtbl.find_opt package.pathsForModule moduleName with
       | None -> []
       | Some paths ->
-        let moduleSrcToRef src =
-          {
-            uri = Uri2.fromPath src;
-            (* XXX TODO use different runtime representation *)
-            loc = Utils.topLoc src;
-          }
-        in
+        let moduleSrcToRef src = {uri = Uri2.fromPath src; locOpt = None} in
         getSrc paths |> List.map moduleSrcToRef
     in
     List.append targetModuleReferences otherModulesReferences

analysis/src/Utils.ml

@@ -7,12 +7,6 @@ let topLoc fname =
     loc_ghost = false;
   }
 
-let isTopLoc (loc : Warnings.loc) =
-  let isTopPos (pos : Lexing.position) =
-    pos.pos_lnum = 1 && pos.pos_bol = 0 && pos.pos_cnum = 0
-  in
-  isTopPos loc.loc_start && isTopPos loc.loc_end && loc.loc_ghost = false
-
 (**
  * `startsWith(string, prefix)`
  * true if the string starts with the prefix
@@ -32,15 +26,11 @@ let endsWith s suffix =
 
 let protocolLineColToCmtLoc ~line ~col = (line + 1, col)
 
-let cmtPosToPosition {Lexing.pos_lnum; pos_cnum; pos_bol} = Protocol.{
-  line = pos_lnum - 1;
-  character = pos_cnum - pos_bol;
-}
+let cmtPosToPosition {Lexing.pos_lnum; pos_cnum; pos_bol} =
+  Protocol.{line = pos_lnum - 1; character = pos_cnum - pos_bol}
 
-let cmtLocToRange {Location.loc_start; loc_end} = Protocol.{
-  start = cmtPosToPosition loc_start;
-  end_ = cmtPosToPosition loc_end;
-}
+let cmtLocToRange {Location.loc_start; loc_end} =
+  Protocol.{start = cmtPosToPosition loc_start; end_ = cmtPosToPosition loc_end}
 
 let locWithinLoc inner outer =
   let open Location in

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

@@ -21,24 +21,24 @@ Rename tests/src/Cross.res 21:13 RenameWithInterfacePrime
 [
 {
   "kind": "rename",
-  "oldUri": "RenameWithInterface.res",
-  "newUri": "RenameWithInterfacePrime.res"
+  "oldUri": "RenameWithInterface.resi",
+  "newUri": "RenameWithInterfacePrime.resi"
 },
 {
   "kind": "rename",
-  "oldUri": "RenameWithInterface.resi",
-  "newUri": "RenameWithInterfacePrime.resi"
+  "oldUri": "RenameWithInterface.res",
+  "newUri": "RenameWithInterfacePrime.res"
 },
 {
   "textDocument": {
   "version": null,
   "uri": "Cross.res"
   },
   "edits": [{
-  "range": {"start": {"line": 24, "character": 8}, "end": {"line": 24, "character": 27}},
+  "range": {"start": {"line": 21, "character": 8}, "end": {"line": 21, "character": 27}},
   "newText": "RenameWithInterfacePrime"
   }, {
-  "range": {"start": {"line": 21, "character": 8}, "end": {"line": 21, "character": 27}},
+  "range": {"start": {"line": 24, "character": 8}, "end": {"line": 24, "character": 27}},
   "newText": "RenameWithInterfacePrime"
   }]
   }
@@ -72,10 +72,10 @@ Rename tests/src/Cross.res 24:28 xPrime
   "uri": "Cross.res"
   },
   "edits": [{
-  "range": {"start": {"line": 24, "character": 28}, "end": {"line": 24, "character": 29}},
+  "range": {"start": {"line": 21, "character": 28}, "end": {"line": 21, "character": 29}},
   "newText": "xPrime"
   }, {
-  "range": {"start": {"line": 21, "character": 28}, "end": {"line": 21, "character": 29}},
+  "range": {"start": {"line": 24, "character": 28}, "end": {"line": 24, "character": 29}},
   "newText": "xPrime"
   }]
   }

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

@@ -6,13 +6,13 @@ Rename tests/src/Rename.res 0:4 y
   "uri": "Rename.res"
   },
   "edits": [{
-  "range": {"start": {"line": 7, "character": 8}, "end": {"line": 7, "character": 9}},
+  "range": {"start": {"line": 0, "character": 4}, "end": {"line": 0, "character": 5}},
   "newText": "y"
   }, {
   "range": {"start": {"line": 3, "character": 8}, "end": {"line": 3, "character": 9}},
   "newText": "y"
   }, {
-  "range": {"start": {"line": 0, "character": 4}, "end": {"line": 0, "character": 5}},
+  "range": {"start": {"line": 7, "character": 8}, "end": {"line": 7, "character": 9}},
   "newText": "y"
   }]
   }
@@ -26,10 +26,10 @@ Rename tests/src/Rename.res 9:19 yy
   "uri": "Rename.res"
   },
   "edits": [{
-  "range": {"start": {"line": 9, "character": 19}, "end": {"line": 9, "character": 21}},
+  "range": {"start": {"line": 9, "character": 12}, "end": {"line": 9, "character": 14}},
   "newText": "yy"
   }, {
-  "range": {"start": {"line": 9, "character": 12}, "end": {"line": 9, "character": 14}},
+  "range": {"start": {"line": 9, "character": 19}, "end": {"line": 9, "character": 21}},
   "newText": "yy"
   }]
   }

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

@@ -26,10 +26,10 @@ Rename tests/src/RenameWithInterface.res 0:4 y
   "uri": "Cross.res"
   },
   "edits": [{
-  "range": {"start": {"line": 24, "character": 28}, "end": {"line": 24, "character": 29}},
+  "range": {"start": {"line": 21, "character": 28}, "end": {"line": 21, "character": 29}},
   "newText": "y"
   }, {
-  "range": {"start": {"line": 21, "character": 28}, "end": {"line": 21, "character": 29}},
+  "range": {"start": {"line": 24, "character": 28}, "end": {"line": 24, "character": 29}},
   "newText": "y"
   }]
   }

analysis/tests/src/expected/RenameWithInterface.resi.txt

@@ -26,10 +26,10 @@ Rename tests/src/RenameWithInterface.resi 0:4 y
   "uri": "Cross.res"
   },
   "edits": [{
-  "range": {"start": {"line": 24, "character": 28}, "end": {"line": 24, "character": 29}},
+  "range": {"start": {"line": 21, "character": 28}, "end": {"line": 21, "character": 29}},
   "newText": "y"
   }, {
-  "range": {"start": {"line": 21, "character": 28}, "end": {"line": 21, "character": 29}},
+  "range": {"start": {"line": 24, "character": 28}, "end": {"line": 24, "character": 29}},
   "newText": "y"
   }]
   }