Skip to content

Commit

Permalink
Warn on invalid references in links (#1722)
Browse files Browse the repository at this point in the history
  • Loading branch information
viniciusmuller committed Jun 18, 2023
1 parent 27f5603 commit 3fdde46
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 9 deletions.
58 changes: 52 additions & 6 deletions lib/ex_doc/language/elixir.ex
Original file line number Diff line number Diff line change
Expand Up @@ -444,15 +444,43 @@ defmodule ExDoc.Language.Elixir do
case Keyword.fetch(attrs, :href) do
{:ok, href} ->
case Regex.scan(@ref_regex, href) do
[[_, custom_link]] -> url(custom_link, :custom_link, config)
[] -> build_extra_link(href, config)
[[_, custom_link]] ->
custom_link
|> url(:custom_link, config)
|> remove_and_warn_if_invalid(custom_link, config)

[] ->
build_extra_link(href, config)
end

_ ->
nil
end
end

defp remove_and_warn_if_invalid(nil, reference, config) do
warn(
~s[documentation references "#{reference}" but it is invalid],
{config.file, config.line},
config.id
)

:remove_link
end

defp remove_and_warn_if_invalid(result, _, _), do: result

defp warn(message, {file, line}, id) do
warning = IO.ANSI.format([:yellow, "warning: ", :reset])

stacktrace =
" #{file}" <>
if(line, do: ":#{line}", else: "") <>
if(id, do: ": #{id}", else: "")

IO.puts(:stderr, [warning, message, ?\n, stacktrace, ?\n])
end

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),
Expand Down Expand Up @@ -522,8 +550,13 @@ defmodule ExDoc.Language.Elixir do
timeout: 0
]

defp url(string = "mix help " <> name, mode, config), do: mix_task(name, string, mode, config)
defp url(string = "mix " <> name, mode, config), do: mix_task(name, string, mode, config)
defp url(string = "mix help " <> name, mode, config) do
name |> mix_task(string, mode, config) |> maybe_remove_link(mode)
end

defp url(string = "mix " <> name, mode, config) do
name |> mix_task(string, mode, config) |> maybe_remove_link(mode)
end

defp url(string, mode, config) do
case Regex.run(~r{^(.+)/(\d+)$}, string) do
Expand All @@ -533,10 +566,14 @@ defmodule ExDoc.Language.Elixir do

case parse_module_function(rest) do
{:local, function} ->
local_url(kind, function, arity, config, string, mode: mode)
kind
|> local_url(function, arity, config, string, mode: mode)
|> maybe_remove_link(mode)

{:remote, module, function} ->
remote_url({kind, module, function, arity}, config, string, mode: mode)
{kind, module, function, arity}
|> remote_url(config, string, mode: mode)
|> maybe_remove_link(mode)

:error ->
nil
Expand All @@ -560,6 +597,15 @@ defmodule ExDoc.Language.Elixir do
end
end

# Remove link when we fail to parse reference so we don't warn twice
defp maybe_remove_link(nil, :custom_link) do
:remove_link
end

defp maybe_remove_link(result, _mode) do
result
end

defp kind("c:" <> rest), do: {:callback, rest}
defp kind("t:" <> rest), do: {:type, rest}
defp kind(rest), do: {:function, rest}
Expand Down
25 changes: 22 additions & 3 deletions test/ex_doc/language/elixir_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,22 @@ defmodule ExDoc.Language.ElixirTest do

warn(~m"`t:Calendar.date/9`")

assert warn(fn ->
assert autolink_doc(~m"[text](`fakefunction`)") == ~m"text"
end) =~ ~s[documentation references "fakefunction" but it is invalid]

assert warn(fn ->
assert autolink_doc(~m"[text](`some.function`)") == ~m"text"
end) =~ ~s[documentation references "some.function" but it is invalid]

assert warn(fn ->
assert autolink_doc(~m"[text](`Enum.map()`)") == ~m"text"
end) =~ ~s[documentation references "Enum.map()" but it is invalid]

assert warn(fn ->
assert autolink_doc(~m"[text](`t:supervisor.child_spec/0`)") == ~m"text"
end) =~ ~s[documentation references "t:supervisor.child_spec/0" but it is invalid]

assert warn(fn ->
autolink_spec(quote(do: t() :: String.bad()))
end) =~ ~s[documentation references type "String.bad()"]
Expand Down Expand Up @@ -499,12 +515,15 @@ defmodule ExDoc.Language.ElixirTest do
~m"It is Unknown"
end) =~ ~s[documentation references module "Unknown" but it is undefined]

assert warn(~m"[Foo task](`mix foo`)", []) =~
~s[documentation references "mix foo" but it is undefined]
assert warn(fn ->
autolink_doc(~m"[Foo task](`mix foo`)", [])
end) =~ ~s[documentation references "mix foo" but it is undefined]

assert_unchanged(~m"`mix foo`")

assert warn(~m"[bad](`String.upcase/9`)", extras: []) =~
assert warn(fn ->
autolink_doc(~m"[bad](`String.upcase/9`)", extras: [])
end) =~
~s[documentation references function "String.upcase/9" but it is undefined or private]

assert_unchanged(~m"`Unknown`")
Expand Down

0 comments on commit 3fdde46

Please sign in to comment.