Skip to content

Add test for warnings if referencing filtered typespec #1764

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Add test for warnings if referencing filtered typespec #1764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion lib/ex_doc/language/elixir.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ defmodule ExDoc.Language.Elixir do
original_text = call_string <> "()"

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

url =
Expand Down
10 changes: 10 additions & 0 deletions test/ex_doc/formatter/html_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,15 @@ defmodule ExDoc.Formatter.HTMLTest do
assert output =~ ~r"Warnings.bar/0.*\n test/fixtures/warnings.ex:8: t:Warnings.t/0"
end

test "warns when referencing typespec on filtered module", context do
output =
capture_io(:stderr, fn ->
generate_docs(doc_config(context, filter_modules: ~r/ReferencesTypespec/))
end)

assert output =~ "typespec references filtered module: TypesAndSpecs.public(integer())"
end
Comment on lines +144 to +151
Copy link
Member

@wojtekmach wojtekmach Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to get away from fixtures as these are hard to maintain. Some time ago we have introduced TestHelper.elixirc and .erlc functions to generate modules and docs "inline". We don't have to rewrite all tests but once we change existing or add new tests, ideally we'd move towards that.

My suggestion is to rewrite this test to something like the following:

  test "warns when referencing typespec on filtered module", context do
    TestHelper.elixirc(context, ~S'''
    defmodule FilteredOutModule do
      @type t() :: term()
    end

    defmodule PublicModule do
      @type t() :: FilteredOutModule.t()
    end
    ''')

    output =
      capture_io(:stderr, fn ->
        generate_docs(
          doc_config(context,
            source_beam: "#{context.tmp_dir}/ebin",
            filter_modules: ~r/PublicModule/
          )
        )
      end)

    assert output =~ "typespec references filtered module: FilteredOutModule.t()"
  end

btw, as I alluded to in the module name, I wonder if the warning should say:

typespec references filtered out module (...)
                             ^^^

but not sure, perhaps it's ok as is.

I believe I haven't considered filter_modules when redesigning autolink warnings. In other words, we shouldn't warn on just typespecs like in https://github.com/elixir-lang/ex_doc/pull/1761/files#diff-9fbe8daa2950d6163a320e9b4ce2b1667fab4185f4593dafb1f1bb672ddee0e1R793 but I believe we need a more general solution, to warn when referencing filtered out modules as well as their functions and callbacks. This should produce a warning but currently does not:

    TestHelper.elixirc(context, ~S'''
    defmodule FilteredOutModule do
      def foo do
      end
    end

    defmodule PublicModule do
      @moduledoc """
      See `FilteredOutModule.foo/0`.
      """
    end
    ''')

    output =
      capture_io(:stderr, fn ->
        generate_docs(
          doc_config(context,
            source_beam: "#{context.tmp_dir}/ebin",
            filter_modules: ~r/PublicModule/
          )
        )
      end)

We build a list of references and whether they are visible in the ExDoc.Autolink.Refs. I think when retriever parses the module from the current project it inserts its references into Refs. So that might be a good place to filter out things, mark the whole module as hidden and then I think referencing its functions, types, etc would warn.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we do something like setting the module visibility to :filtered and then show a "module filtered" message when we reference a filtered module, kinda like it's done currently for the hidden modules?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good to me


test "generates headers for index.html and module pages", %{tmp_dir: tmp_dir} = context do
generate_docs(doc_config(context, main: "RandomError"))
content_index = File.read!(tmp_dir <> "/html/index.html")
Expand Down Expand Up @@ -221,6 +230,7 @@ defmodule ExDoc.Formatter.HTMLTest do
%{"id" => "CustomProtocol"},
%{"id" => "DuplicateHeadings"},
%{"id" => "OverlappingDefaults"},
%{"id" => "ReferencesTypespec"},
%{"id" => "TypesAndSpecs"},
%{"id" => "TypesAndSpecs.Sub"},
%{"id" => "Warnings"},
Expand Down
10 changes: 0 additions & 10 deletions test/ex_doc/language/elixir_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,16 +439,6 @@ defmodule ExDoc.Language.ElixirTest do
warn(~m"`c:InMemory.unknown/0`")
end

test "warning if typespec references filtered module" do
ExDoc.Refs.insert([
{{:module, AutolinkTest.Keep}, :public},
{{:function, AutolinkTest.Filtered}, :public},
{{:type, AutolinkTest.Filtered, :type, 0}, :public}
])

# TODO: testing
end

test "warnings" do
ExDoc.Refs.insert([
{{:module, AutolinkTest.Foo}, :public},
Expand Down
10 changes: 10 additions & 0 deletions test/fixtures/references_typespec.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
defmodule ReferencesTypespec do
@moduledoc """
references public typespecs
"""

@spec a() :: TypesAndSpecs.public(integer())
def a do
10
end
end