Skip to content

Fix Erlang Scanning & Warnings #1818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Changes from all commits
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
44 changes: 18 additions & 26 deletions lib/ex_doc/autolink.ex
Original file line number Diff line number Diff line change
@@ -7,6 +7,9 @@ defmodule ExDoc.Autolink do
# * `:current_module` - the module that the docs are being generated for. Used to link local
# calls and see if remote calls are in the same app.
#
# * `:current_kfa` - the kind, function, arity that the docs are being generated for. Is nil
# if there is no such thing. Used to generate more accurate warnings.
#
# * `:module_id` - id of the module being documented (e.g.: `"String"`)
#
# * `:file` - source file location
@@ -48,6 +51,7 @@ defmodule ExDoc.Autolink do
extras: [],
deps: [],
ext: ".html",
current_kfa: nil,
siblings: [],
skip_undefined_reference_warnings_on: [],
skip_code_autolink_to: [],
@@ -371,29 +375,6 @@ defmodule ExDoc.Autolink do
end
end

# There are special forms that are forbidden by the tokenizer
def parse_function("__aliases__"), do: {:function, :__aliases__}
def parse_function("__block__"), do: {:function, :__block__}
def parse_function("%"), do: {:function, :%}

def parse_function(string) do
case Code.string_to_quoted("& #{string}/0", warnings: false) do
{:ok, {:&, _, [{:/, _, [{:__aliases__, _, [function]}, 0]}]}} when is_atom(function) ->
## When function starts with capital letter
{:function, function}

## When function is 'nil'
{:ok, {:&, _, [{:/, _, [nil, 0]}]}} ->
{:function, nil}

{:ok, {:&, _, [{:/, _, [{function, _, _}, 0]}]}} when is_atom(function) ->
{:function, function}

_ ->
:error
end
end

def kind("c:" <> rest), do: {:callback, rest}
def kind("t:" <> rest), do: {:type, rest}
## \\ does not work for :custom_url as Earmark strips the \...
@@ -432,7 +413,7 @@ defmodule ExDoc.Autolink do
{:type, _visibility} ->
case config.language.try_builtin_type(name, arity, mode, config, original_text) do
nil ->
if mode == :custom_link do
if mode == :custom_link or config.language == ExDoc.Language.Erlang do
Copy link
Member

Choose a reason for hiding this comment

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

Is this check here to avoid warning in Elixir? If so, you can go ahead and warn. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is, but if I remove it a lot (~20) of tests break (or not break, but they start to emit warnings to stdout).

Copy link
Member

Choose a reason for hiding this comment

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

Ok, @wojtekmach please look into it afterwards then. :)

Ideally we want to avoid hardcoding the language here, an option would be better, but since this is temporary, it is fine by me. :)

maybe_warn(config, ref, visibility, %{original_text: original_text})
end

@@ -501,7 +482,9 @@ defmodule ExDoc.Autolink do

nil

{:regular_link, _module_visibility, :undefined} when not same_module? ->
{:regular_link, _module_visibility, :undefined}
when not same_module? and
(config.language != ExDoc.Language.Erlang or kind == :function) ->
Copy link
Member

Choose a reason for hiding this comment

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

Same here, feel free to let it warn on Elixir, assuming it is one of m:, t: or c:.

nil

{_mode, _module_visibility, visibility} ->
@@ -518,7 +501,16 @@ defmodule ExDoc.Autolink do
# TODO: Remove on Elixir v1.14
stacktrace_info =
if unquote(Version.match?(System.version(), ">= 1.14.0")) do
[file: config.file, line: config.line]
f =
case config.current_kfa do
{:function, f, a} ->
[function: {f, a}]

_ ->
[]
end

[file: config.file, line: config.line, module: config.current_module] ++ f
else
[]
end
19 changes: 17 additions & 2 deletions lib/ex_doc/formatter/html.ex
Original file line number Diff line number Diff line change
@@ -92,7 +92,15 @@ defmodule ExDoc.Formatter.HTML do
docs =
for child_node <- node.docs do
id = id(node, child_node)
autolink_opts = autolink_opts ++ [id: id, line: child_node.doc_line]

autolink_opts =
autolink_opts ++
[
id: id,
line: child_node.doc_line,
current_kfa: {:function, child_node.name, child_node.arity}
]

specs = Enum.map(child_node.specs, &language.autolink_spec(&1, autolink_opts))
child_node = %{child_node | specs: specs}
render_doc(child_node, language, autolink_opts, opts)
@@ -101,7 +109,14 @@ defmodule ExDoc.Formatter.HTML do
typespecs =
for child_node <- node.typespecs do
id = id(node, child_node)
autolink_opts = autolink_opts ++ [id: id, line: child_node.doc_line]

autolink_opts =
autolink_opts ++
[
id: id,
line: child_node.doc_line,
current_kfa: {child_node.type, child_node.name, child_node.arity}
]

child_node = %{
child_node
19 changes: 17 additions & 2 deletions lib/ex_doc/language/elixir.ex
Original file line number Diff line number Diff line change
@@ -267,7 +267,7 @@ defmodule ExDoc.Language.Elixir do
def parse_module_function(string) do
case string |> String.split(".") |> Enum.reverse() do
[string] ->
with {:function, function} <- Autolink.parse_function(string) do
with {:function, function} <- parse_function(string) do
{:local, function}
end

@@ -298,12 +298,27 @@ defmodule ExDoc.Language.Elixir do
module_string = rest |> Enum.reverse() |> Enum.join(".")

with {:module, module} <- parse_module(module_string, :custom_link),
{:function, function} <- Autolink.parse_function(function_string) do
{:function, function} <- parse_function(function_string) do
{:remote, module, function}
end
end
end

# There are special forms that are forbidden by the tokenizer
defp parse_function("__aliases__"), do: {:function, :__aliases__}
defp parse_function("__block__"), do: {:function, :__block__}
defp parse_function("%"), do: {:function, :%}

defp parse_function(string) do
case Code.string_to_quoted("& #{string}/0", warnings: false) do
{:ok, {:&, _, [{:/, _, [{function, _, _}, 0]}]}} when is_atom(function) ->
{:function, function}

_ ->
:error
end
end

@impl true
def parse_module(<<first>> <> _ = string, _mode) when first in ?A..?Z do
if string =~ ~r/^[A-Za-z0-9_.]+$/ do
51 changes: 34 additions & 17 deletions lib/ex_doc/language/erlang.ex
Original file line number Diff line number Diff line change
@@ -262,6 +262,8 @@ defmodule ExDoc.Language.Erlang do
end

defp walk_doc({:code, attrs, [code], meta} = ast, config) when is_binary(code) do
config = %{config | line: meta[:line]}

case Autolink.url(code, :regular_link, config) do
url when is_binary(url) ->
code = remove_prefix(code)
@@ -279,11 +281,11 @@ defmodule ExDoc.Language.Erlang do

case String.split(url, ":") do
[module] ->
walk_doc({:a, [href: "`m:#{module}#{fragment}`"], inner, meta}, config)
walk_doc({:a, [href: "`m:#{maybe_quote(module)}#{fragment}`"], inner, meta}, config)

[app, module] ->
inner = strip_app(inner, app)
walk_doc({:a, [href: "`m:#{module}#{fragment}`"], inner, meta}, config)
walk_doc({:a, [href: "`m:#{maybe_quote(module)}#{fragment}`"], inner, meta}, config)

_ ->
warn_ref(attrs[:href], config)
@@ -333,7 +335,7 @@ defmodule ExDoc.Language.Erlang do
walk_doc({:a, [href: "`t:#{fixup(type)}`"], inner, meta}, config)

"https://erlang.org/doc/link/" <> see ->
warn_ref(attrs[:href] <> " (#{see})", config)
warn_ref(attrs[:href] <> " (#{see})", %{config | id: nil})
inner

_ ->
@@ -372,13 +374,21 @@ defmodule ExDoc.Language.Erlang do
end

defp fixup(mfa) do
case String.split(mfa, "#") do
["", mfa] ->
mfa
{m, fa} =
case String.split(mfa, "#") do
["", mfa] ->
{"", mfa}

[m, fa] ->
m <> ":" <> fa
end
[m, fa] ->
{"#{maybe_quote(m)}:", fa}
end

[f, a] = String.split(fa, "/")
m <> maybe_quote(f) <> "/" <> a
end

defp maybe_quote(m) do
to_string(:io_lib.write_atom(String.to_atom(m)))
end

defp strip_app([{:code, attrs, [code], meta}], app) do
@@ -413,12 +423,12 @@ defmodule ExDoc.Language.Erlang do
case String.split(string, ":") do
[module_string, function_string] ->
with {:module, module} <- parse_module_string(module_string, :custom_link),
{:function, function} <- Autolink.parse_function(function_string) do
{:function, function} <- parse_function(function_string) do
{:remote, module, function}
end

[function_string] ->
with {:function, function} <- Autolink.parse_function(function_string) do
with {:function, function} <- parse_function(function_string) do
{:local, function}
end

@@ -427,6 +437,16 @@ defmodule ExDoc.Language.Erlang do
end
end

defp parse_function(string) do
with {:ok, toks, _} <- :erl_scan.string(String.to_charlist("fun #{string}/0.")),
{:ok, [{:fun, _, {:function, name, _arity}}]} <- :erl_parse.parse_exprs(toks) do
{:function, name}
else
_ ->
:error
end
end

@impl true
def try_autoimported_function(name, arity, mode, config, original_text) do
if :erl_internal.bif(name, arity) do
@@ -464,12 +484,9 @@ defmodule ExDoc.Language.Erlang do
end
end

def parse_module_string(string, _mode) do
case Code.string_to_quoted(":'#{string}'",
warn_on_unnecessary_quotes: false,
emit_warnings: false
) do
{:ok, module} when is_atom(module) ->
defp parse_module_string(string, _mode) do
case :erl_scan.string(String.to_charlist(string)) do
{:ok, [{:atom, _, module}], _} when is_atom(module) ->
{:module, module}

_ ->
Loading