Fix resolution involving deeply nested substitutions

Fixes #690

There has been logic to fix this sort of issue for a long time, but it
broke at some point.

Mostly the fix is that when calculating the signatures of `With`
module expressions, we always recalculate rather than using any
previously calculated signature - this is because the previously
calculated signature makes a round-trip from Component.* to Lang.* and
Lang.ModuleType.simple_signature doesn't preserve removed items as the
Components layer does.

By itself this would not have fixed #690 though - The way the rest of
the fix works is by ensuring that at each level of the expansion we
have enough to figure out that a replacement is necessary and what it
should be. For example, in issue #690, @Octachron provides the
following example:

```ocaml
module type S = sig
  module M: sig
    type t
  end
  type t = M.t
end
module type T = S with type M.t := int
```

Before this fix, the expansion of `T` would look like this:

```ocaml
module M : sig end
type t = M.t
```

Clearly the `M.t` can't be resolved from this. Instead, this fix
works by ensuring that the expansion of `T` looks like this:

```ocaml
module M: sig type t end with type t := int
type t = M.t
```

From this we have enough information to replace the `M.t` with an
`int` at the point we do the resolution of the RHS of type `t`.

We don't really want the output to show
`sig type t end with type t := int` however, so in the document
layer these expressions are replaced with `sig ... end` as we had
before.

Author: jonludlam

src/document/generator.ml

@@ -1430,14 +1430,22 @@ module Make (Syntax : SYNTAX) = struct
           ++ Link.from_path (m :> Paths.Path.t)
           ++ O.txt " " ++ O.keyword "end"
 
+    and is_elidable_with_u : Odoc_model.Lang.ModuleType.U.expr -> bool = function
+      | Path _ -> false
+      | Signature _ -> true
+      | With (_, expr) -> is_elidable_with_u expr
+      | TypeOf _ -> false
+
     and umty : Odoc_model.Lang.ModuleType.U.expr -> text =
      fun m ->
       match m with
       | Path p -> Link.from_path (p :> Paths.Path.t)
+      | Signature _ ->
+        Syntax.Mod.open_tag ++ O.txt " ... " ++ Syntax.Mod.close_tag
+      | With (_, expr) when is_elidable_with_u expr ->
+        Syntax.Mod.open_tag ++ O.txt " ... " ++ Syntax.Mod.close_tag
       | With (subs, expr) -> mty_with subs expr
       | TypeOf { t_desc; _ } -> mty_typeof t_desc
-      | Signature _ ->
-          Syntax.Mod.open_tag ++ O.txt " ... " ++ Syntax.Mod.close_tag
 
     and mty : Odoc_model.Lang.ModuleType.expr -> text =
      fun m ->
@@ -1470,6 +1478,8 @@ module Make (Syntax : SYNTAX) = struct
                  ++ mty arg_expr ++ O.txt ")" ++ O.txt " " ++ Syntax.Type.arrow
                  )
             ++ O.txt " " ++ mty expr
+        | With { w_expr ; _ } when is_elidable_with_u w_expr ->
+          Syntax.Mod.open_tag ++ O.txt " ... " ++ Syntax.Mod.close_tag
         | With { w_substitutions; w_expr; _ } -> mty_with w_substitutions w_expr
         | TypeOf { t_desc; _ } -> mty_typeof t_desc
         | Signature _ ->

src/xref2/compile.ml

@@ -580,20 +580,7 @@ and u_module_type_expr :
           | None -> subs
         in
         let result : ModuleType.U.expr = With (subs', expr') in
-        match expr' with
-        | Signature _ -> (
-            (* Explicitly handle 'sig ... end with ...' - replace with a plain signature.
-               See equivalent in [module_type_expr] *)
-            let cu =
-              Component.Of_Lang.(u_module_type_expr empty (With (subs, expr')))
-            in
-            let expansion =
-              Expand_tools.aux_expansion_of_u_module_type_expr env cu
-            in
-            match expansion with
-            | Ok sg -> Signature Lang_of.(signature id empty sg)
-            | _ -> result)
-        | _ -> result)
+        result)
     | TypeOf { t_desc; t_expansion } ->
         let t_desc =
           match t_desc with
@@ -634,17 +621,11 @@ and module_type_expr :
   | With { w_substitutions; w_expansion; w_expr } as e -> (
       let w_expansion = get_expansion w_expansion e in
       let w_expr = u_module_type_expr env id w_expr in
-      match (w_expr, w_expansion) with
-      | Signature _, Some (Signature sg) ->
-          (* Explicitly handle 'sig ... end with ...' - replace with a plain signature.
-             See equivalent in [u_module_type_expr] *)
-          Signature sg
-      | _, _ -> (
-          let cexpr = Component.Of_Lang.(u_module_type_expr empty w_expr) in
-          let subs' = module_type_map_subs env id cexpr w_substitutions in
-          match subs' with
-          | None -> With { w_substitutions; w_expansion; w_expr }
-          | Some s -> With { w_substitutions = s; w_expansion; w_expr }))
+      let cexpr = Component.Of_Lang.(u_module_type_expr empty w_expr) in
+      let subs' = module_type_map_subs env id cexpr w_substitutions in
+      match subs' with
+      | None -> With { w_substitutions; w_expansion; w_expr }
+      | Some s -> With { w_substitutions = s; w_expansion; w_expr })
   | Functor (param, res) ->
       let param' = functor_parameter env param in
       let env' = Env.add_functor_parameter param env in

src/xref2/test.md

@@ -739,7 +739,7 @@ now we can ask for the signature of this module:
 # let sg = get_ok @@ Tools.signature_of_module env (Component.Delayed.get m);;
 val sg : Component.Signature.t =
   {Odoc_xref2.Component.Signature.items =
-    [Odoc_xref2.Component.Signature.Module (`LModule (M, 39),
+    [Odoc_xref2.Component.Signature.Module (`LModule (M, 45),
       Odoc_model.Lang.Signature.Ordinary,
       {Odoc_xref2.Component.Delayed.v =
         Some
@@ -751,7 +751,7 @@ val sg : Component.Signature.t =
             None);
           canonical = None; hidden = false};
        get = None});
-     Odoc_xref2.Component.Signature.Module (`LModule (N, 40),
+     Odoc_xref2.Component.Signature.Module (`LModule (N, 46),
       Odoc_model.Lang.Signature.Ordinary,
       {Odoc_xref2.Component.Delayed.v =
         Some
@@ -760,7 +760,7 @@ val sg : Component.Signature.t =
            Odoc_xref2.Component.Module.ModuleType
             (Odoc_xref2.Component.ModuleType.Path
               {Odoc_xref2.Component.ModuleType.p_expansion = None;
-               p_path = `Dot (`Local (`LModule (M, 39), false), "S")});
+               p_path = `Dot (`Local (`LModule (M, 45), false), "S")});
           canonical = None; hidden = false};
        get = None})];
    compiled = false; removed = []; doc = []}
@@ -793,7 +793,7 @@ val m : Component.Module.t Component.Delayed.t =
 # get_ok @@ Tools.signature_of_module env (Component.Delayed.get m);;
 - : Component.Signature.t =
 {Odoc_xref2.Component.Signature.items =
-  [Odoc_xref2.Component.Signature.Type (`LType (t, 47),
+  [Odoc_xref2.Component.Signature.Type (`LType (t, 53),
     Odoc_model.Lang.Signature.Ordinary,
     {Odoc_xref2.Component.Delayed.v =
       Some

src/xref2/tools.ml

@@ -1233,8 +1233,12 @@ and signature_of_module_type_expr :
       | Ok (_, mt) -> signature_of_module_type env mt
       | Error e -> Error (`UnresolvedPath (`ModuleType (p_path, e))))
   | Component.ModuleType.Signature s -> Ok s
-  | Component.ModuleType.With { w_expansion = Some e; _ } ->
+  (* | Component.ModuleType.With { w_expansion = Some e; _ } ->
       Ok (signature_of_simple_expansion e)
+      
+      Recalculate 'With' expressions always, as we need to know which
+      items have been removed 
+      *)
   | Component.ModuleType.With { w_substitutions; w_expr; _ } ->
       signature_of_u_module_type_expr ~mark_substituted env w_expr >>= fun sg ->
       handle_signature_with_subs ~mark_substituted env sg w_substitutions

test/xref2/deep_substitution.t/m.mli

@@ -0,0 +1,8 @@
+module type S = sig
+  module M: sig
+    type t
+  end
+  type t = M.t
+end
+module type T = S with type M.t := int
+

test/xref2/deep_substitution.t/run.t

@@ -0,0 +1,46 @@
+Test to check that when doing a deep substitution inside a signature,
+that items that reference that element are still correctly resolved
+even if they're in a different path.
+
+  $ cat m.mli
+  module type S = sig
+    module M: sig
+      type t
+    end
+    type t = M.t
+  end
+  module type T = S with type M.t := int
+  
+
+In this, we want to check that the type `t` in module type `T` has
+its RHS correctly replaced with an `int`
+
+  $ ocamlc -c -bin-annot m.mli
+  $ odoc compile m.cmti
+  $ odoc link m.odoc
+  $ odoc html-generate m.odocl --indent -o .
+  $ odoc_print m.odocl -r T.t
+  {
+    "id": {
+      "`Type": [
+        { "`ModuleType": [ { "`Root": [ "None", "M" ] }, "T" ] },
+        "t"
+      ]
+    },
+    "doc": [],
+    "equation": {
+      "params": [],
+      "private_": "false",
+      "manifest": {
+        "Some": {
+          "Constr": [
+            { "`Resolved": { "`Identifier": { "`CoreType": "int" } } },
+            []
+          ]
+        }
+      },
+      "constraints": []
+    },
+    "representation": "None"
+  }
+ 

test/xref2/with.t/run.t

@@ -44,10 +44,10 @@ Let's check which module type `.content.Module.items[0].ModuleType` refers to:
     ]
   }
 
-And it ought to be a simple Signature after compiling and linking.
+And it ought to still be a With after compiling and linking.
 
   $ odoc_print test.odocl | jq '.content.Module.items[0].ModuleType.expr.Some | keys'
   [
-    "Signature"
+    "With"
   ]