Skip to content

Commit af07dd7

Browse files
Fix false positive warning when linking to asset files (#2225)
`build_extra_link` treats `.txt` (amongst other extensions) as a builtin extension and looks for it in the extras map. Asset files aren't extras, so linking to them from an extra page produces a false "does not exist" warning. After failing to match an extra, ExDoc now checks whether the path corresponds to an existing file in a configured asset directory before warning. Closes #2221
1 parent 9b35b38 commit af07dd7

File tree

3 files changed

+83
-6
lines changed

3 files changed

+83
-6
lines changed

lib/ex_doc/autolink.ex

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ defmodule ExDoc.Autolink do
6262
skip_code_autolink_to: &ExDoc.Formatter.Config.skip_code_autolink_to/1,
6363
force_module_prefix: nil,
6464
filtered_modules: [],
65+
assets: %{},
6566
warnings: :emit
6667
]
6768

@@ -217,17 +218,35 @@ defmodule ExDoc.Autolink do
217218
with %{scheme: nil, host: nil, path: path} = uri <- URI.parse(link),
218219
true <- is_binary(path) and path != "" and not (path =~ ref_regex()),
219220
true <- Path.extname(path) in @builtin_ext do
220-
if file = config.extras[Path.basename(path)] do
221-
append_fragment(file <> config.ext, uri.fragment)
222-
else
223-
maybe_warn(config, nil, nil, %{file_path: path, original_text: link})
224-
nil
221+
cond do
222+
file = config.extras[Path.basename(path)] ->
223+
append_fragment(file <> config.ext, uri.fragment)
224+
225+
asset_file?(path, config.assets) ->
226+
nil
227+
228+
true ->
229+
maybe_warn(config, nil, nil, %{file_path: path, original_text: link})
230+
nil
225231
end
226232
else
227233
_ -> nil
228234
end
229235
end
230236

237+
defp asset_file?(path, assets) do
238+
Enum.any?(assets, fn {source_dir, target_dir} ->
239+
prefix = String.trim_trailing(target_dir, "/") <> "/"
240+
241+
if String.starts_with?(path, prefix) do
242+
path
243+
|> String.trim_leading(prefix)
244+
|> Path.expand(source_dir)
245+
|> File.exists?()
246+
end
247+
end)
248+
end
249+
231250
defp maybe_remove_link(nil, :custom_link) do
232251
:remove_link
233252
end

lib/ex_doc/formatter.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,8 @@ defmodule ExDoc.Formatter do
196196
extras: extra_paths(extras),
197197
skip_undefined_reference_warnings_on: config.skip_undefined_reference_warnings_on,
198198
skip_code_autolink_to: config.skip_code_autolink_to,
199-
filtered_modules: filtered_nodes
199+
filtered_modules: filtered_nodes,
200+
assets: config.assets
200201
}
201202

202203
extras =

test/ex_doc/language/elixir_test.exs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,57 @@ defmodule ExDoc.Language.ElixirTest do
692692
~s|<code class="inline">String.upcase/9</code>|
693693
end
694694

695+
@tag :tmp_dir
696+
test "asset file warnings", %{tmp_dir: tmp_dir} do
697+
lic_dir = Path.join(tmp_dir, "src/licenses")
698+
File.mkdir_p!(lic_dir)
699+
File.write!(Path.join(lic_dir, "MIT.txt"), "")
700+
701+
assets_dir = Path.join(tmp_dir, "assets")
702+
File.mkdir_p!(Path.join(assets_dir, "sub"))
703+
File.write!(Path.join(assets_dir, "sub/file.txt"), "")
704+
705+
opts = [
706+
warnings: :send,
707+
extras: %{},
708+
assets: %{lic_dir => "LICENSES", assets_dir => "assets"}
709+
]
710+
711+
# Existing file — no warning
712+
refute_warn(fn ->
713+
assert autolink_doc("[MIT](LICENSES/MIT.txt)", opts) ==
714+
~s|<a href="LICENSES/MIT.txt">MIT</a>|
715+
end)
716+
717+
# Nested file — no warning
718+
refute_warn(fn -> autolink_doc("[License](assets/sub/file.txt)", opts) end)
719+
720+
# Source and target differ — link must use target dir
721+
assert warn(fn -> autolink_doc("[MIT](src/licenses/MIT.txt)", opts) end) =~
722+
"but it does not exist"
723+
724+
# Missing file in asset dir — warns
725+
assert warn(fn -> autolink_doc("[License](LICENSES/nonexistent.txt)", opts) end) =~
726+
"but it does not exist"
727+
728+
# Missing nested file — warns
729+
assert warn(fn -> autolink_doc("[License](assets/sub/missing.txt)", opts) end) =~
730+
"but it does not exist"
731+
732+
# Path not matching any target dir — warns
733+
assert warn(fn -> autolink_doc("[License](other/file.txt)", opts) end) =~
734+
"but it does not exist"
735+
736+
# Target dir prefix must match on "/" boundary
737+
assert warn(fn -> autolink_doc("[MIT](LICENSESEXTRA/MIT.txt)", opts) end) =~
738+
"but it does not exist"
739+
740+
# No assets configured — warns
741+
assert warn(fn ->
742+
autolink_doc("[MIT](LICENSES/MIT.txt)", warnings: :send, extras: %{}, assets: %{})
743+
end) =~ "but it does not exist"
744+
end
745+
695746
## Helpers
696747

697748
@default_options [
@@ -718,6 +769,12 @@ defmodule ExDoc.Language.ElixirTest do
718769
message
719770
end
720771

772+
defp refute_warn(fun) when is_function(fun, 0) do
773+
fun.()
774+
775+
refute_received {:warn, _, _}
776+
end
777+
721778
defp autolink_spec(spec, options \\ []) do
722779
config = struct!(ExDoc.Autolink, Keyword.merge(@default_options, options))
723780
ExDoc.Language.Elixir.autolink_spec(spec, config)

0 commit comments

Comments
 (0)