Skip to content

Warn when referencing functions/callbacks/types from filtered out modules #1824

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 1 commit into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* Support `m:SomeModule` for explicitly linking to a module
* Add `noindex` meta tag to 404 and Search pages
* Move search to the main content so we can display more results
* Warn when referencing functions, types, and callbacks from filtered out modules

* Bug fixes
* Fix search for words with hyphens in them
Expand Down
22 changes: 20 additions & 2 deletions lib/ex_doc/autolink.ex
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ defmodule ExDoc.Autolink do
nil
end

defp string_app_module_url(string, tool, module, anchor, config) do
if Enum.any?(config.filtered_modules, &(&1.module == module)) do
# TODO: Remove on Elixir v1.14
prefix =
if unquote(Version.match?(System.version(), ">= 1.14.0")) do
""
else
~s|"#{string}" |
end

warn(config, prefix <> "reference to a filtered module")
nil
else
app_module_url(tool, module, anchor, config)
end
end

# TODO: make more generic
@doc false
def ex_doc_app_url(module, config, path, ext, suffix) do
Expand Down Expand Up @@ -260,7 +277,7 @@ defmodule ExDoc.Autolink do

case {mode, Refs.get_visibility(ref)} do
{_link_type, visibility} when visibility in [:public, :limited] ->
app_module_url(tool(module, config), module, anchor, config)
string_app_module_url(string, tool(module, config), module, anchor, config)

{:regular_link, :undefined} ->
nil
Expand Down Expand Up @@ -468,7 +485,8 @@ defmodule ExDoc.Autolink do
if same_module? do
fragment(tool, kind, name, arity)
else
app_module_url(tool, module, config) <> fragment(tool, kind, name, arity)
url = string_app_module_url(original_text, tool, module, nil, config)
url && url <> fragment(tool, kind, name, arity)
end

{:regular_link, module_visibility, :undefined}
Expand Down
6 changes: 1 addition & 5 deletions lib/ex_doc/language/elixir.ex
Original file line number Diff line number Diff line change
Expand Up @@ -663,16 +663,12 @@ defmodule ExDoc.Language.Elixir do
(\(.*\)) # Arguments <rest />
}x

Regex.replace(regex, string, fn all, call_string, module_string, name_string, rest ->
Regex.replace(regex, string, fn _all, call_string, module_string, name_string, rest ->
module = string_to_module(module_string)
name = String.to_atom(name_string)
arity = count_args(rest, 0, 0)
original_text = call_string <> "()"

if Enum.any?(config.filtered_modules, &(&1.id == module_string)) do
Autolink.warn(config, "typespec references filtered module: #{all}")
end

url =
if module do
Autolink.remote_url({:type, module, name, arity}, config, original_text)
Expand Down
47 changes: 30 additions & 17 deletions test/ex_doc/language/elixir_test.exs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
defmodule ExDoc.Language.ElixirTest do
# ExDoc.Refs is global
use ExUnit.Case, async: false

doctest ExDoc.Autolink

describe "autolink_doc/2" do
Expand Down Expand Up @@ -36,21 +35,14 @@ defmodule ExDoc.Language.ElixirTest do
end

test "project-local module" do
ExDoc.Refs.insert([
{{:module, AutolinkTest.Foo}, :public}
])

assert autolink_doc("`AutolinkTest.Foo`") ==
~s|<a href="AutolinkTest.Foo.html"><code class="inline">AutolinkTest.Foo</code></a>|
assert autolink_doc("`ExDoc.Markdown`") ==
~s|<a href="ExDoc.Markdown.html"><code class="inline">ExDoc.Markdown</code></a>|

assert autolink_doc("`m:AutolinkTest.Foo`") ==
~s|<a href="AutolinkTest.Foo.html"><code class="inline">AutolinkTest.Foo</code></a>|
assert autolink_doc("`m:ExDoc.Markdown`") ==
~s|<a href="ExDoc.Markdown.html"><code class="inline">ExDoc.Markdown</code></a>|

assert autolink_doc("`String`", apps: [:elixir]) ==
~s|<a href="String.html"><code class="inline">String</code></a>|

assert autolink_doc("`AutolinkTest.Foo`", current_module: AutolinkTest.Foo) ==
~s|<a href="AutolinkTest.Foo.html#content"><code class="inline">AutolinkTest.Foo</code></a>|
end

test "remote function" do
Expand Down Expand Up @@ -564,14 +556,35 @@ defmodule ExDoc.Language.ElixirTest do
assert autolink_doc("[text](`t:supervisor.child_spec/0`)", opts) == "text"
end) =~ ~s|documentation references "t:supervisor.child_spec/0" but it is invalid|

## filtered_modules

assert warn(fn ->
autolink_spec(quote(do: t() :: String.bad()), opts)
end) =~ ~s|documentation references type "String.bad()"|
opts = opts ++ [filtered_modules: [%ExDoc.ModuleNode{module: String}]]

assert autolink_doc("`String`", opts) ==
~s|<code class="inline">String</code>|
end) =~ "reference to a filtered module"

assert warn(fn ->
opts = opts ++ [filtered_modules: [%ExDoc.ModuleNode{id: "String"}]]
opts = opts ++ [filtered_modules: [%ExDoc.ModuleNode{module: String}]]

assert autolink_doc("`String.upcase/1`", opts) ==
~s|<code class="inline">String.upcase/1</code>|
end) =~ "reference to a filtered module"

assert warn(fn ->
opts = opts ++ [filtered_modules: [%ExDoc.ModuleNode{module: String}]]

assert autolink_doc("`t:String.t/0`", opts) ==
~s|<code class="inline">t:String.t/0</code>|
end) =~ "reference to a filtered module"

assert warn(fn ->
opts = opts ++ [filtered_modules: [%ExDoc.ModuleNode{module: String}]]
autolink_spec(quote(do: t() :: String.t()), opts)
end) == "typespec references filtered module: String.t()"
end) =~ "reference to a filtered module"

## typespecs

assert warn(fn ->
autolink_spec(
Expand Down Expand Up @@ -640,7 +653,7 @@ defmodule ExDoc.Language.ElixirTest do
## Helpers

@default_options [
apps: [:myapp],
apps: [:ex_doc],
current_module: MyModule,
module_id: "MyModule",
file: "nofile",
Expand Down
36 changes: 36 additions & 0 deletions test/ex_doc/language/erlang_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,24 @@ defmodule ExDoc.Language.ErlangTest do
assert autolink_edoc("{@link //foo}", c) == ~s|<code>//foo</code>|
end) =~ ~s|invalid reference: foo:index|
end

test "filtered module", c do
opts = [filtered_modules: [%ExDoc.ModuleNode{module: :lists, id: "lists"}]]

assert warn(fn ->
assert autolink_edoc("{@link lists}", c, opts) ==
~s|<code>lists</code>|
end) == "reference to a filtered module"
end

test "filtered module function", c do
opts = [filtered_modules: [%ExDoc.ModuleNode{module: :lists, id: "lists"}]]

assert warn(fn ->
assert autolink_edoc("{@link lists:all/2}", c, opts) ==
~s|<code>lists:all/2</code>|
end) == "reference to a filtered module"
end
end

describe "autolink_doc/2 for markdown" do
Expand Down Expand Up @@ -426,6 +444,24 @@ defmodule ExDoc.Language.ErlangTest do
line: nil
) =~ ~r/documentation references "e:extra.md" but it is invalid/
end

test "filtered module", c do
opts = [filtered_modules: [%ExDoc.ModuleNode{module: :lists, id: "lists"}]]

assert warn(fn ->
assert autolink_doc("`m:lists`", c, opts) ==
~s|<code class="inline">m:lists</code>|
end) =~ "reference to a filtered module"
end

test "filtered module callback", c do
opts = [filtered_modules: [%ExDoc.ModuleNode{module: :gen_server, id: "gen_server"}]]

assert warn(fn ->
assert autolink_doc("`c:gen_server:handle_call/3`", c, opts) ==
~s|<code class="inline">c:gen_server:handle_call/3</code>|
end) =~ "reference to a filtered module"
end
end

describe "autolink_doc/2 for extra" do
Expand Down