Using `filter_modules` doesn't produce warning referencing `hidden or private` docs

[WojciechBarczynski]

Using filter_modules in the mix.exs doesn't produce warnings indicating undocumented code reference.

Using @moduledoc false:

1. Code:

defmodule Foo do
  @moduledoc false
  # "Private" module

  @type foo :: integer()
end


defmodule Bar do
  @moduledoc """
  "Public" module
  """
  alias Foo

  @spec bar(Foo.foo()) :: nil
  def bar(num) do
    IO.inspect(num)
    nil
  end
end

2. Generates warring while trying to generate docs to notify users, that they are using type reference unavailable in the documentation:

$ mix docs
Generating docs...
warning: documentation references type "Foo.foo()" but it is hidden or private
  lib/membrane_template.ex:16: Bar.bar/1

View "html" docs at "doc/index.html"

Using filter_modules:

1. mix.exs

defmodule Mixfile do
  use Mix.Project

  def project do
    [
...
      # docs
      name: "Example",
      source_url: @github_url,
      docs: docs()
    ]
  end

...

  defp docs do
    [
      main: "readme",
      extras: ["README.md", "LICENSE"],
      formatters: ["html"],
      source_ref: "v#{@version}",
      filter_modules: ~r/Bar/,
    ]
  end
end

2. Code

defmodule Foo do
  @moduledoc """
  "Private" module excluded with `filter_modules`
  """

  @type foo :: integer()
end


defmodule Bar do
  @moduledoc """
  "Public" module
  """
  alias Foo

  @spec bar(Foo.foo()) :: nil
  def bar(num) do
    IO.inspect(num)
    nil
  end
end

3. Doesn't generate any warring, despite Foo.foo() being unavailable in docs

$ mix docs       
Generating docs...
View "html" docs at "doc/index.html"

4. Generated docs with Foo.foo() reference:

[viniciusmuller]

I gave this issue a try and I'm not sure what the correct fix would be.
When autolinking typespecs referenced by functions we only have a {Module, :type_name} tuple, if the user provides a regex/string as filter_modules, we can use the regex to match against the module name and warn if the module is being filtered.

But if the user provides a function that takes the module and its metadata, we don't have these values available in the typespec linking context

[josevalim]

@viniciusmuller could we somehow filter the modules and keep a list of all modules that were filtered out and pass the filtered modules list to autolink?

[viniciusmuller]

Do you think something like this could work?

Allowing the retriever only retrieve modules and then we can filter its results and get them already nicely separated.
My first attempt was to refactor the retriever as to return a {modules, filtered_modules} tuple but it broke its API (in case someone is using a custom retriever) and made the changes much more complex.

I think if we retrieve everything and then filter the resulting list we can use the list that passes the filter as usual and give the name of filtered ones directly to an autolink config, which I believe would then be enough for us to do:

if Enum.any?(filtered_module_names, & &1 == module_string) do 
   # warn
end

[josevalim]

@viniciusmuller could we perhaps change retriever to also return a list of all filtered modules?

[viniciusmuller]

I guess this would be possible, the only difference is that it will require a bigger refactor, I guess.

Also, I'm not sure if there cases of people using custom retrievers, since it seems possible but not documented, but if we change the retriever to return that we would break this API.

Also, if we apply the filter in the retriever results, filter_modules will work even with custom retrivers, untying the filtering from retrieval.

I think both should be doable, though, let me know what you find better

[josevalim]

Also, I'm not sure if there cases of people using custom retrievers

AFAIK it is only for tests. It is not public API.

I am fine either way. We can change the retriever as I think you should be able to change flat_map into a flat_map_reduce where the second argument is an accumulator of skipped modules. We can probably store all skipped modules in there.

The biggest question is the tests. I think moving this logic to the ExDoc module will actually make things harder to test, because the ExDoc entry points assumes IO, change global state, and other stuff. So pushing it to the retriever may be better.