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

Author: wojtekmach

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

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}

lib/ex_doc/language/elixir.ex

@@ -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)

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|<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
@@ -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(
@@ -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",

test/ex_doc/language/erlang_test.exs

@@ -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
@@ -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