Skip to content

Commit

Permalink
Warn when referencing functions/callbacks/types from filtered out mod…
Browse files Browse the repository at this point in the history
…ules
  • Loading branch information
wojtekmach committed Nov 21, 2023
1 parent 7fd5238 commit 86b2d7a
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 25 deletions.
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
52 changes: 34 additions & 18 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,17 @@ 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:AutolinkTest.Foo`") ==
# ~s|<a href="AutolinkTest.Foo.html"><code class="inline">AutolinkTest.Foo</code></a>|

assert autolink_doc("`String`", apps: [:elixir]) ==
~s|<a href="String.html"><code class="inline">String</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>|
# 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 +559,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 +656,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

0 comments on commit 86b2d7a

Please sign in to comment.