From 96b134295ae5ec09c92955c756c836e598d7c19e Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Tue, 21 Nov 2023 12:15:07 +0100 Subject: [PATCH] Warn when referencing functions/callbacks/types from filtered out modules --- CHANGELOG.md | 1 + lib/ex_doc/autolink.ex | 22 +++++++++++-- lib/ex_doc/language/elixir.ex | 6 +--- test/ex_doc/language/elixir_test.exs | 47 ++++++++++++++++++---------- test/ex_doc/language/erlang_test.exs | 36 +++++++++++++++++++++ 5 files changed, 88 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc27bf6ed..2f90b9b1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/ex_doc/autolink.ex b/lib/ex_doc/autolink.ex index 74aa91cf3..811dba370 100644 --- a/lib/ex_doc/autolink.ex +++ b/lib/ex_doc/autolink.ex @@ -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 @@ -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 @@ -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} diff --git a/lib/ex_doc/language/elixir.ex b/lib/ex_doc/language/elixir.ex index ec03875d8..4ebcba741 100644 --- a/lib/ex_doc/language/elixir.ex +++ b/lib/ex_doc/language/elixir.ex @@ -663,16 +663,12 @@ defmodule ExDoc.Language.Elixir do (\(.*\)) # Arguments }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) diff --git a/test/ex_doc/language/elixir_test.exs b/test/ex_doc/language/elixir_test.exs index 97d7a065a..d0bb1c392 100644 --- a/test/ex_doc/language/elixir_test.exs +++ b/test/ex_doc/language/elixir_test.exs @@ -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 @@ -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|AutolinkTest.Foo| + assert autolink_doc("`ExDoc.Markdown`") == + ~s|ExDoc.Markdown| - assert autolink_doc("`m:AutolinkTest.Foo`") == - ~s|AutolinkTest.Foo| + assert autolink_doc("`m:ExDoc.Markdown`") == + ~s|ExDoc.Markdown| assert autolink_doc("`String`", apps: [:elixir]) == ~s|String| - - assert autolink_doc("`AutolinkTest.Foo`", current_module: AutolinkTest.Foo) == - ~s|AutolinkTest.Foo| end test "remote function" do @@ -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|String| + 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|String.upcase/1| + 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|t:String.t/0| + 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( @@ -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", diff --git a/test/ex_doc/language/erlang_test.exs b/test/ex_doc/language/erlang_test.exs index b7f127508..ccf3ceeb0 100644 --- a/test/ex_doc/language/erlang_test.exs +++ b/test/ex_doc/language/erlang_test.exs @@ -192,6 +192,24 @@ defmodule ExDoc.Language.ErlangTest do assert autolink_edoc("{@link //foo}", c) == ~s|//foo| 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|lists| + 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|lists:all/2| + end) == "reference to a filtered module" + end end describe "autolink_doc/2 for markdown" do @@ -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|m:lists| + 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|c:gen_server:handle_call/3| + end) =~ "reference to a filtered module" + end end describe "autolink_doc/2 for extra" do