Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions lib/ex_doc/extras.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ defmodule ExDoc.Extras do
Build a list of `ExDoc.ExtraNode` and `ExDoc.URLNode`.
"""
def build(extras_input, config) do
validate_no_duplicate_extras!(extras_input)
groups = config.groups_for_extras

extras =
Expand All @@ -23,6 +24,55 @@ defmodule ExDoc.Extras do
|> Enum.sort_by(fn extra -> Config.index(groups, extra.group) end)
end

# Detects duplicate extras by normalizing each entry to {source, resolved_id}
# and checking for collisions on that pair. The resolved_id is the :filename
# option if provided, otherwise derived from the source path (same logic as
# build_extra). Only the :filename option affects the output file ID — other
# options like :title, :source, and :search_data do not. So:
#
# - "readme.md" and "readme.md" are duplicates
#
# - {"readme.md", title: "A"} and {"readme.md", title: "B"} are duplicates
# (same source file, same resolved ID "readme")
#
# - "readme.md" and {"readme.md", filename: "readme"} are duplicates
# (the explicit filename matches the derived one)
#
# - {"readme.md", filename: "a"} and {"readme.md", filename: "b"} are NOT duplicates
# (explicitly different resolved IDs)
#
# - foo/README.md and bar/README.md are NOT duplicates — they have different
# content and disambiguate_id/2 handles their ID collision
defp validate_no_duplicate_extras!(extras_input) do
duplicates =
extras_input
|> Enum.map(&extra_duplicate_key/1)
|> Enum.frequencies()
|> Enum.filter(fn {_, count} -> count > 1 end)

if duplicates != [] do
entries = Enum.map_join(duplicates, ", ", fn {{source, _}, _} -> inspect(source) end)

raise ArgumentError,
"duplicate extras: #{entries}"
end
end

defp extra_duplicate_key(input) when is_binary(input),
do: {input, resolve_id(input, nil)}

defp extra_duplicate_key({input, opts}) when is_list(opts),
do: extra_duplicate_key({input, Map.new(opts)})

defp extra_duplicate_key({input, %{} = opts}) do
input = to_string(input)

{input, resolve_id(input, opts[:filename])}
end

defp resolve_id(_input, filename) when is_binary(filename), do: filename
defp resolve_id(input, _filename), do: input |> filename_to_title() |> Utils.text_to_id()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is doing a bit of duplicate work compareed to build_extra. Can't we run this validation after we call build_extra, so we work with well-defined structs? Also, keep in mind that we have URL nodes (and those likely shouldn't be part of this validation).

Copy link
Copy Markdown
Contributor Author

@eksperimental eksperimental Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. That simplified things a WHOLE LOT. Thank you for the advice.


defp disambiguate_id(extra, discriminator) do
Map.put(extra, :id, "#{extra.id}-#{discriminator}")
end
Expand Down
63 changes: 63 additions & 0 deletions test/ex_doc/extras_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,69 @@ defmodule ExDoc.ExtrasTest do
end
end

test "raises on duplicate extras", %{tmp_dir: tmp_dir} do
File.write!("#{tmp_dir}/readme.md", "# README")

assert_raise ArgumentError, ~r/duplicate extras/, fn ->
extras = ["#{tmp_dir}/readme.md", "#{tmp_dir}/readme.md"]
Extras.build(extras, config())
end
end

test "raises on same file with different non-filename options", %{tmp_dir: tmp_dir} do
File.write!("#{tmp_dir}/readme.md", "# README")

assert_raise ArgumentError, ~r/duplicate extras/, fn ->
extras = [
{"#{tmp_dir}/readme.md", [title: "A"]},
{"#{tmp_dir}/readme.md", [title: "B"]}
]

Extras.build(extras, config())
end
end

test "raises when explicit filename matches derived id", %{tmp_dir: tmp_dir} do
File.write!("#{tmp_dir}/readme.md", "# README")

assert_raise ArgumentError, ~r/duplicate extras/, fn ->
extras = [
"#{tmp_dir}/readme.md",
{"#{tmp_dir}/readme.md", [filename: "readme"]}
]

Extras.build(extras, config())
end
end

test "allows same file with different filename options", %{tmp_dir: tmp_dir} do
File.write!("#{tmp_dir}/readme.md", "# README")

extras = [
{"#{tmp_dir}/readme.md", [filename: "foo"]},
{"#{tmp_dir}/readme.md", [filename: "bar"]}
]

result = Extras.build(extras, config())
assert length(result) == 2
end

test "allows different files with the same resolved ID", %{tmp_dir: tmp_dir} do
File.mkdir_p!("#{tmp_dir}/foo")
File.write!("#{tmp_dir}/foo/readme.md", "# README foo")

File.mkdir_p!("#{tmp_dir}/bar")
File.write!("#{tmp_dir}/bar/readme.md", "# README bar")

extras = [
"#{tmp_dir}/foo/readme.md",
{"#{tmp_dir}/bar/readme.md", [filename: "readme"]}
]

result = Extras.build(extras, config())
assert length(result) == 2
end

test "raises when title option is not a string", %{tmp_dir: tmp_dir} do
File.write!("#{tmp_dir}/page.md", "# Page")

Expand Down
Loading