Auto link extras: fail on bad path

Author: ftes

lib/ex_doc.ex

@@ -249,6 +249,10 @@ defmodule ExDoc do
     * `:title` - The title of the extra page. If not provided, the title will be inferred from the extra name.
     * `:url` - The external url to link to from the sidebar.
 
+  Bare filenames such as `[Intro](intro.md)` use the legacy filename-based lookup against the flattened output.
+  Links with a directory component, such as `[Intro](guides/intro.md)`, `[Intro](../guides/intro.md)`, or
+  `[Intro](/guides/intro.md)`, are resolved against the extra source path (or project root for `/`).
+
   ### Customizing search data
 
   It is possible to fully customize the way a given extra is indexed, both in autocomplete and in search.

lib/ex_doc/autolink.ex

@@ -52,7 +52,7 @@ defmodule ExDoc.Autolink do
     :language,
     file: "nofile",
     apps: [],
-    extras: [],
+    extras: %{},
     deps: [],
     ext: ".html",
     current_kfa: nil,
@@ -214,20 +214,86 @@ defmodule ExDoc.Autolink do
   @builtin_ext [".livemd", ".cheatmd", ".md", ".txt", ""]
 
   defp build_extra_link(link, config) do
-    with %{scheme: nil, host: nil, path: path} = uri <- URI.parse(link),
-         true <- is_binary(path) and path != "" and not (path =~ ref_regex()),
-         true <- Path.extname(path) in @builtin_ext do
-      if file = config.extras[Path.basename(path)] do
-        append_fragment(file <> config.ext, uri.fragment)
-      else
+    case extra_link_target(link, config) do
+      {:ok, target, fragment} ->
+        append_fragment(target <> config.ext, fragment)
+
+      {:missing, path} ->
         maybe_warn(config, nil, nil, %{file_path: path, original_text: link})
         nil
-      end
+
+      :ignore ->
+        nil
+    end
+  end
+
+  defp extra_link_target(link, config) do
+    case parse_extra_link(link) do
+      {:ok, path, fragment} ->
+        extra_link_target(path, fragment, config)
+
+      {:error, :invalid} ->
+        :ignore
+    end
+  end
+
+  defp extra_link_target(path, fragment, config) do
+    case resolve_extra_target(path, config) do
+      nil -> {:missing, path}
+      target -> {:ok, target, fragment}
+    end
+  end
+
+  defp parse_extra_link(link) do
+    case URI.parse(link) do
+      %{scheme: nil, host: nil, path: path, fragment: fragment} when is_binary(path) ->
+        if valid_extra_link_path?(path) do
+          {:ok, path, fragment}
+        else
+          {:error, :invalid}
+        end
+
+      _ ->
+        {:error, :invalid}
+    end
+  end
+
+  defp valid_extra_link_path?(path) do
+    path != "" and not (path =~ ref_regex()) and Path.extname(path) in @builtin_ext
+  end
+
+  defp resolve_extra_target(path, config) do
+    if path_qualified_link?(path) do
+      config.extras[normalize_extra_link_path(path, config.file)]
     else
-      _ -> nil
+      config.extras[Path.basename(path)]
     end
   end
 
+  defp path_qualified_link?(path), do: path != Path.basename(path)
+
+  defp normalize_extra_link_path("/" <> path, _current_file) do
+    normalize_extra_link_path_from(path, File.cwd!())
+  end
+
+  defp normalize_extra_link_path(path, current_file) do
+    normalize_extra_link_path_from(path, extra_link_base_dir(current_file))
+  end
+
+  defp normalize_extra_link_path_from(path, base_dir) do
+    path
+    |> Path.expand(base_dir)
+    |> Path.relative_to(File.cwd!())
+  end
+
+  defp extra_link_base_dir(file) when is_binary(file) do
+    file
+    |> Path.expand(File.cwd!())
+    |> Path.dirname()
+  end
+
+  defp extra_link_base_dir(_), do: File.cwd!()
+
   defp maybe_remove_link(nil, :custom_link) do
     :remove_link
   end

lib/ex_doc/formatter.ex

@@ -313,11 +313,21 @@ defmodule ExDoc.Formatter do
         acc
 
       %ExDoc.ExtraNode{source_path: source_path, id: id}, acc when is_binary(source_path) ->
-        base = Path.basename(source_path)
-        Map.put(acc, base, id)
+        path = normalize_extra_path(source_path)
+        base = Path.basename(path)
+
+        acc
+        |> Map.put(path, id)
+        |> Map.put(base, id)
 
       _extra, acc ->
         acc
     end)
   end
+
+  defp normalize_extra_path(path) do
+    path
+    |> Path.relative_to(File.cwd!())
+    |> String.replace_leading("./", "")
+  end
 end

test/ex_doc/language/elixir_test.exs

@@ -259,7 +259,9 @@ defmodule ExDoc.Language.ElixirTest do
 
     test "extras" do
       opts = [
+        file: "guides/current.md",
         extras: %{
+          "guide/Foo Bar.md" => "foo-bar",
           "Foo Bar.md" => "foo-bar",
           "Bar Baz.livemd" => "bar-baz",
           "Bar Baz.cheatmd" => "bar-baz"
@@ -286,6 +288,48 @@ defmodule ExDoc.Language.ElixirTest do
       assert autolink_doc("[Foo](#baz)", opts) == ~s|<a href="#baz">Foo</a>|
     end
 
+    test "extras relative paths use the extra source path" do
+      opts = [
+        file: "guides/current.md",
+        extras: %{"guide/Foo Bar.md" => "foo-bar", "Foo Bar.md" => "foo-bar"}
+      ]
+
+      assert autolink_doc("[Foo](../guide/Foo Bar.md)", opts) ==
+               ~s|<a href="foo-bar.html">Foo</a>|
+
+      assert autolink_doc("[Foo](/guide/Foo Bar.md)", opts) ==
+               ~s|<a href="foo-bar.html">Foo</a>|
+    end
+
+    test "bare filename links use legacy lookup but directory paths use source paths" do
+      opts = [
+        file: "guides/current.md",
+        extras: %{"guides/Foo Bar.md" => "relative-foo", "Foo Bar.md" => "legacy-foo"}
+      ]
+
+      assert autolink_doc("[Foo](Foo Bar.md)", opts) ==
+               ~s|<a href="legacy-foo.html">Foo</a>|
+
+      assert autolink_doc("[Foo](./Foo Bar.md)", opts) ==
+               ~s|<a href="relative-foo.html">Foo</a>|
+
+      assert autolink_doc("[Foo](../guides/Foo Bar.md)", opts) ==
+               ~s|<a href="relative-foo.html">Foo</a>|
+    end
+
+    test "extras with bad directories warn instead of silently matching by basename" do
+      opts = [
+        warnings: :send,
+        file: "guides/current.md",
+        extras: %{"guide/Foo Bar.md" => "foo-bar", "Foo Bar.md" => "foo-bar"}
+      ]
+
+      assert warn(fn ->
+               assert autolink_doc("[Foo](/bad_dir/Foo Bar.md)", opts) ==
+                        ~s|<a href="/bad_dir/Foo Bar.md">Foo</a>|
+             end) =~ ~s|documentation references file "/bad_dir/Foo Bar.md" but it does not exist|
+    end
+
     test "special case links" do
       assert autolink_doc("`//2`") ==
                ~s|<a href="https://hexdocs.pm/elixir/Kernel.html#//2"><code class="inline">//2</code></a>|

test/ex_doc/language/erlang_test.exs

@@ -669,6 +669,16 @@ defmodule ExDoc.Language.ErlangTest do
       extras: %{"Foo Bar.md" => "foo-bar", "Bar Baz.livemd" => "bar-baz"}
     ]
 
+    @relative_opts [
+      file: "guides/current.md",
+      extras: %{
+        "guide/Foo Bar.md" => "foo-bar",
+        "guide/Bar Baz.livemd" => "bar-baz",
+        "Foo Bar.md" => "foo-bar",
+        "Bar Baz.livemd" => "bar-baz"
+      }
+    ]
+
     test "extras", c do
       assert autolink_doc("[Foo](Foo Bar.md)", c, @opts) ==
                ~s|<a href="foo-bar.html">Foo</a>|
@@ -690,7 +700,7 @@ defmodule ExDoc.Language.ErlangTest do
     end
 
     test "extras relative", c do
-      assert autolink_doc("[Foo](../guide/Foo Bar.md)", c, @opts) ==
+      assert autolink_doc("[Foo](../guide/Foo Bar.md)", c, @relative_opts) ==
                ~s|<a href="foo-bar.html">Foo</a>|
     end
   end