Do not splice the type name into constructor and field references

CHANGES.md

@@ -4,6 +4,10 @@
 - Support for OxCaml zero alloc definitions (@Leonidas-from-XIV, #1422, #1444)
 - Remove requirement for ppx_expect in tests (@jonludlam, #1445)
 - Support for OxCaml modalities (@art-w, #1420)
+- Fix #372: do not splice the name of the type into the text a reference to a
+  constructor, a polymorphic variant tag or a record field is rendered as, so
+  that `{!M.Foo}` now renders as `M.Foo` rather than as the invalid `M.t.Foo`
+  (@MavenRain, #PLACEHOLDER_PR)
 
 # 3.2.1
 

src/document/comment.ml

@@ -40,12 +40,12 @@ module Reference = struct
         render_resolved (r :> t) ^ "." ^ ModuleTypeName.to_string s
     | `Type (r, s) -> render_resolved (r :> t) ^ "." ^ TypeName.to_string s
     | `Constructor (r, s) ->
-        render_resolved (r :> t) ^ "." ^ ConstructorName.to_string s
+        render_datatype_parent r ^ ConstructorName.to_string s
     | `PolyConstructor (r, s) ->
-        render_resolved (r :> t) ^ ".`" ^ ConstructorName.to_string s
-    | `Field (r, s) -> render_resolved (r :> t) ^ "." ^ FieldName.to_string s
+        render_datatype_parent r ^ "`" ^ ConstructorName.to_string s
+    | `Field (r, s) -> render_field_parent r ^ FieldName.to_string s
     | `UnboxedField (r, s) ->
-        render_resolved (r :> t) ^ "." ^ UnboxedFieldName.to_string s
+        render_datatype_parent r ^ UnboxedFieldName.to_string s
     | `Extension (r, s) ->
         render_resolved (r :> t) ^ "." ^ ExtensionName.to_string s
     | `ExtensionDecl (r, _, s) ->
@@ -63,6 +63,36 @@ module Reference = struct
         render_resolved (r :> t) ^ "." ^ InstanceVariableName.to_string s
     | `Label (_, s) -> LabelName.to_string s
 
+  (* The name of a type is not part of the OCaml path of its constructors and
+     of its fields: [M.t.Foo] is not valid syntax whereas [M.Foo] is. Render
+     the path of the parent of the type rather than the path of the type
+     itself. A polymorphic variant tag is not qualified in OCaml at all, but
+     the module prefix is kept for it too, as a locator rather than as a path.
+     See https://github.com/ocaml/odoc/issues/372 *)
+  and render_datatype_parent : Reference.Resolved.DataType.t -> string =
+   fun r ->
+    let open Reference.Resolved in
+    match r with
+    | `Identifier _ -> ""
+    | `Type (parent, _) -> render_resolved (parent :> t) ^ "."
+
+  and render_field_parent : Reference.Resolved.FieldParent.t -> string =
+   fun r ->
+    let open Reference.Resolved in
+    match r with
+    | `Identifier { iv = `Type _; _ } -> ""
+    | `Type (parent, _) -> render_resolved (parent :> t) ^ "."
+    (* The fields of an inline record in an extension constructor or in an
+       exception are parented by the enclosing signature rather than by a
+       type, so the parent is rendered as it was before. *)
+    | `Identifier
+        {
+          iv = `Root _ | `Module _ | `Parameter _ | `Result _ | `ModuleType _;
+          _;
+        }
+    | `Alias _ | `AliasModuleType _ | `Module _ | `Hidden _ | `ModuleType _ ->
+        render_resolved (r :> t) ^ "."
+
   let render_path (tag, cs) =
     let tag =
       match tag with

test/xref2/github_issue_372.t/a.mli

@@ -0,0 +1,54 @@
+type t = Alpha | Beta
+
+type r = { fld : int }
+
+(** The type is dropped whether or not the reference spelled it out:
+    {!Alpha}, {!constructor-Alpha}, {!t.Alpha}, {!t.constructor-Alpha},
+    {!fld}, {!field-fld} and {!r.fld}. *)
+
+module Bla : sig
+  type ha = Alpha | Beta
+
+  type ra = { fla : int }
+
+  type sw = [ `On | `Off ]
+
+  type ext = ..
+
+  type ext += Ext_a
+
+  exception Exn_a
+
+  module Inner : sig
+    type i = Gamma
+  end
+end
+
+(** The module path is kept: {!Bla.Alpha} and {!Bla.ha.Alpha} both render as
+    [Bla.Alpha]; {!Bla.fla} and {!Bla.ra.fla} both render as [Bla.fla].
+
+    Polymorphic constructors lose the type as well: {!Bla.sw.On} and
+    {!Bla.sw.`Off}.
+
+    Only the type component is dropped, not the enclosing modules:
+    {!Bla.Inner.Gamma} and {!Bla.Inner.i.Gamma}.
+
+    Parents that are not types are left alone: {!Bla.Ext_a}, {!Bla.Exn_a},
+    {!Bla.ha} and {!Bla.Inner.i}.
+
+    When the reference carries its own text, the rendered path becomes the
+    tooltip instead: {{!Bla.Alpha} the first case}. *)
+
+module Ambiguous : sig
+  type a = Same
+
+  type b = Same
+end
+
+(** Two types of a same module may share a constructor name; the rendered text
+    is then the same for both and only the anchor tells them apart:
+    {!Ambiguous.a.Same} and {!Ambiguous.b.Same}. *)
+
+class cls : object end
+
+(** An unresolved reference is still printed as it was written: {!cls.Alpha} *)

test/xref2/github_issue_372.t/run.t

@@ -0,0 +1,53 @@
+The name of a type is not part of the OCaml path of its constructors and of
+its fields, so it must not be spliced into the text a reference is rendered
+as: [M.t.Foo] is not valid syntax whereas [M.Foo] is.
+See https://github.com/ocaml/odoc/issues/372
+
+  $ ocamlc -c -bin-annot a.mli
+  $ odoc compile --warn-error -I . a.cmti
+
+Only the reference whose parent is a class fails to resolve:
+
+  $ odoc link a.odoc
+  File "a.mli", line 54, characters 64-76:
+  Warning: Failed to resolve reference unresolvedroot(cls).Alpha Couldn't find "Alpha"
+
+  $ odoc html-generate --output-dir html --indent a.odocl
+
+Constructors and fields are qualified by their module path only, never by the
+name of their type. Extensions, exceptions and types keep their parent, and
+the anchors are unaffected:
+
+  $ cat html/A/index.html | grep '<a href' | grep '<code>'
+       <a href="#type-t.Alpha"><code>Alpha</code></a>, 
+      <a href="#type-t.Alpha"><code>Alpha</code></a>, 
+      <a href="#type-t.Alpha"><code>Alpha</code></a>, 
+      <a href="#type-t.Alpha"><code>Alpha</code></a>, 
+      <a href="#type-r.fld"><code>fld</code></a>, 
+      <a href="#type-r.fld"><code>fld</code></a> and 
+      <a href="#type-r.fld"><code>fld</code></a>.
+      <a href="Bla/index.html#type-ha.Alpha"><code>Bla.Alpha</code></a>
+       and <a href="Bla/index.html#type-ha.Alpha"><code>Bla.Alpha</code></a>
+      <a href="Bla/index.html#type-ra.fla"><code>Bla.fla</code></a> and
+       <a href="Bla/index.html#type-ra.fla"><code>Bla.fla</code></a> both
+      <a href="Bla/index.html#type-sw.On"><code>Bla.`On</code></a> and 
+      <a href="Bla/index.html#type-sw.Off"><code>Bla.`Off</code></a>.
+      <a href="Bla/Inner/index.html#type-i.Gamma"><code>Bla.Inner.Gamma</code>
+      <a href="Bla/Inner/index.html#type-i.Gamma"><code>Bla.Inner.Gamma</code>
+      <a href="Bla/index.html#extension-Ext_a"><code>Bla.Ext_a</code></a>
+      , <a href="Bla/index.html#exception-Exn_a"><code>Bla.Exn_a</code></a>
+      , <a href="Bla/index.html#type-ha"><code>Bla.ha</code></a> and 
+      <a href="Bla/Inner/index.html#type-i"><code>Bla.Inner.i</code></a>
+      <a href="Ambiguous/index.html#type-a.Same"><code>Ambiguous.Same</code>
+      <a href="Ambiguous/index.html#type-b.Same"><code>Ambiguous.Same</code>
+
+With its own text, the reference is rendered as the tooltip:
+
+  $ cat html/A/index.html | grep -o 'title="[^"]*"'
+  title="Bla.Alpha"
+
+An unresolved reference is still printed exactly as it was written, and is
+not turned into a link:
+
+  $ cat html/A/index.html | grep 'cls.Alpha'
+      <code>cls.Alpha</code>

test/xref2/github_issue_447.t/run.t

@@ -18,9 +18,9 @@ Let's now check that the reference point to the right page/anchor:
 
   $ cat html/A/index.html | grep \# | grep Foo | grep -v anchor
      <p><a href="#type-u.Foo"><code>Foo</code></a> 
-      <a href="#type-u.Foo"><code>u.Foo</code></a> 
+      <a href="#type-u.Foo"><code>Foo</code></a> 
       <a href="#type-u.Foo"><code>Foo</code></a>
-     <p><a href="M/index.html#type-t.Foo"><code>M.t.Foo</code></a> and 
-      <a href="M/index.html#type-t.Foo"><code>M.t.Foo</code></a>
-     <p><a href="M/index.html#type-t.Foo"><code>M.t.Foo</code></a> and 
-      <a href="M/index.html#type-t.Foo"><code>M.t.Foo</code></a>
+     <p><a href="M/index.html#type-t.Foo"><code>M.Foo</code></a> and 
+      <a href="M/index.html#type-t.Foo"><code>M.Foo</code></a>
+     <p><a href="M/index.html#type-t.Foo"><code>M.Foo</code></a> and 
+      <a href="M/index.html#type-t.Foo"><code>M.Foo</code></a>

test/xref2/reference_to_polymorphic.t/run.t

@@ -12,14 +12,14 @@
 
   $ odoc html-generate -o html --indent main.odocl
   $ cat html/Main/index.html | grep -A3 "<li>"
-     <ul><li><a href="#type-switch.On"><code>switch.`On</code></a></li>
-      <li><a href="#type-switch.Off"><code>switch.`Off</code></a></li>
-      <li><a href="#type-switch.On"><code>switch.`On</code></a></li>
-      <li><a href="#type-switch.Off"><code>switch.`Off</code></a></li>
-      <li><a href="#type-switch.On"><code>switch.`On</code></a></li>
-      <li><a href="#type-switch.Off"><code>switch.`Off</code></a></li>
-      <li><a href="#type-switch.On"><code>switch.`On</code></a></li>
-      <li><a href="#type-switch.Off"><code>switch.`Off</code></a></li>
+     <ul><li><a href="#type-switch.On"><code>`On</code></a></li>
+      <li><a href="#type-switch.Off"><code>`Off</code></a></li>
+      <li><a href="#type-switch.On"><code>`On</code></a></li>
+      <li><a href="#type-switch.Off"><code>`Off</code></a></li>
+      <li><a href="#type-switch.On"><code>`On</code></a></li>
+      <li><a href="#type-switch.Off"><code>`Off</code></a></li>
+      <li><a href="#type-switch.On"><code>`On</code></a></li>
+      <li><a href="#type-switch.Off"><code>`Off</code></a></li>
      </ul><p>References in the environment don't work:</p>
      <ul><li><code>On</code></li><li><code>`On</code></li>
       <li><code>On</code></li><li><code>`On</code></li>