Skip to content

Warn if typespec references filtered module #1761

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions lib/ex_doc.ex
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ defmodule ExDoc do
ExDoc.Markdown.put_markdown_processor(processor)
end

docs = config.retriever.docs_from_dir(config.source_beam, config)
find_formatter(config.formatter).run(docs, config)
{module_nodes, filtered_nodes} = config.retriever.docs_from_dir(config.source_beam, config)
find_formatter(config.formatter).run({module_nodes, filtered_nodes}, config)
end

# Short path for programmatic interface
Expand Down
5 changes: 4 additions & 1 deletion lib/ex_doc/autolink.ex
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ defmodule ExDoc.Autolink do
# * `:skip_undefined_reference_warnings_on` - list of modules to skip the warning on
#
# * `:skip_code_autolink_to` - list of terms that will be skipped when autolinking (e.g: "PrivateModule")
#
# * `:filtered_modules` - A list of module nodes that were filtered by the retriever

defstruct [
:current_module,
Expand All @@ -35,7 +37,8 @@ defmodule ExDoc.Autolink do
ext: ".html",
siblings: [],
skip_undefined_reference_warnings_on: [],
skip_code_autolink_to: []
skip_code_autolink_to: [],
filtered_modules: []
]

@hexdocs "https://hexdocs.pm/"
Expand Down
9 changes: 7 additions & 2 deletions lib/ex_doc/formatter/epub.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ defmodule ExDoc.Formatter.EPUB do
Generate EPUB documentation for the given modules.
"""
@spec run(list, ExDoc.Config.t()) :: String.t()
def run(project_nodes, config) when is_map(config) do
def run(project_nodes, config) when is_list(project_nodes) and is_map(config) do
run({project_nodes, []}, config)
end

def run({project_nodes, filtered_modules}, config) when is_map(config) do
parent = config.output
config = normalize_config(config)

Expand All @@ -19,7 +23,8 @@ defmodule ExDoc.Formatter.EPUB do
&create_output_dir(&1, config)
)

project_nodes = HTML.render_all(project_nodes, ".xhtml", config, highlight_tag: "samp")
project_nodes =
HTML.render_all(project_nodes, filtered_modules, ".xhtml", config, highlight_tag: "samp")

nodes_map = %{
modules: HTML.filter_list(:module, project_nodes),
Expand Down
20 changes: 15 additions & 5 deletions lib/ex_doc/formatter/html.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,24 @@ defmodule ExDoc.Formatter.HTML do
@doc """
Generate HTML documentation for the given modules.
"""
@spec run(list, ExDoc.Config.t()) :: String.t()
def run(project_nodes, config) when is_map(config) do
@spec run(
[ExDoc.ModuleNode.t()]
| {[ExDoc.ModuleNode.t()], [ExDoc.ModuleNode.t()]},
ExDoc.Config.t()
) :: String.t()

def run(project_nodes, config) when is_list(project_nodes) and is_map(config) do
run({project_nodes, []}, config)
end

def run({project_nodes, filtered_modules}, config) when is_map(config) do
config = normalize_config(config)
config = %{config | output: Path.expand(config.output)}

build = Path.join(config.output, ".build")
setup_output(config.output, &cleanup_output_dir(&1, config), &create_output_dir(&1, config))

project_nodes = render_all(project_nodes, ".html", config, [])
project_nodes = render_all(project_nodes, filtered_modules, ".html", config, [])
extras = build_extras(config, ".html")

# Generate search early on without api reference in extras
Expand Down Expand Up @@ -64,14 +73,15 @@ defmodule ExDoc.Formatter.HTML do
@doc """
Autolinks and renders all docs.
"""
def render_all(project_nodes, ext, config, opts) do
def render_all(project_nodes, filtered_modules, ext, config, opts) do
base = [
apps: config.apps,
deps: config.deps,
ext: ext,
extras: extra_paths(config),
skip_undefined_reference_warnings_on: config.skip_undefined_reference_warnings_on,
skip_code_autolink_to: config.skip_code_autolink_to
skip_code_autolink_to: config.skip_code_autolink_to,
filtered_modules: filtered_modules
]

project_nodes
Expand Down
6 changes: 5 additions & 1 deletion lib/ex_doc/language/elixir.ex
Original file line number Diff line number Diff line change
Expand Up @@ -783,12 +783,16 @@ 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
warn("Typespec references filtered module: #{all}", {config.file, config.line}, config.id)
end

url =
if module do
remote_url({:type, module, name, arity}, config, original_text)
Expand Down
6 changes: 4 additions & 2 deletions lib/ex_doc/nodes.ex
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ defmodule ExDoc.ModuleNode do
source_url: nil,
type: nil,
language: nil,
annotations: []
annotations: [],
metadata: nil

@typep annotation :: atom()

Expand All @@ -46,7 +47,8 @@ defmodule ExDoc.ModuleNode do
source_url: String.t() | nil,
type: atom(),
language: module(),
annotations: [annotation()]
annotations: [annotation()],
metadata: map()
}
end

Expand Down
39 changes: 30 additions & 9 deletions lib/ex_doc/retriever.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ defmodule ExDoc.Retriever do

@doc """
Extract documentation from all modules in the specified directory or directories.

Returns a tuple containing `{modules, filtered}`, using `config.filter_modules`
as a filter criteria.
"""
@spec docs_from_dir(Path.t() | [Path.t()], ExDoc.Config.t()) :: [ExDoc.ModuleNode.t()]
@spec docs_from_dir(Path.t() | [Path.t()], ExDoc.Config.t()) ::
{[ExDoc.ModuleNode.t()], [ExDoc.ModuleNode.t()]}
def docs_from_dir(dir, config) when is_binary(dir) do
files = Path.wildcard(Path.expand("*.beam", dir))

Expand All @@ -28,15 +32,32 @@ defmodule ExDoc.Retriever do
end

@doc """
Extract documentation from all modules in the list `modules`
Extract documentation from all modules and returns a tuple containing
`{modules, filtered}`, two lists of modules that were extracted and filtered
by `config.filter_modules`, respectively.
"""
@spec docs_from_modules([atom], ExDoc.Config.t()) :: [ExDoc.ModuleNode.t()]
@spec docs_from_modules([atom], ExDoc.Config.t()) ::
{[ExDoc.ModuleNode.t()], [ExDoc.ModuleNode.t()]}
def docs_from_modules(modules, config) when is_list(modules) do
modules
|> Enum.flat_map(&get_module(&1, config))
|> Enum.reduce({[], []}, fn module_name, {modules, filtered} = acc ->
case get_module(module_name, config) do
{:error, _module} ->
acc

{:ok, module_node} ->
if config.filter_modules.(module_node.module, module_node.metadata),
do: {[module_node | modules], filtered},
else: {modules, [module_node | filtered]}
end
end)
|> sort_modules(config)
end

defp sort_modules({modules, filtered}, config) do
{sort_modules(modules, config), sort_modules(filtered, config)}
end

defp sort_modules(modules, config) when is_list(modules) do
Enum.sort_by(modules, fn module ->
{GroupMatcher.group_index(config.groups_for_modules, module.group), module.nested_context,
Expand All @@ -50,14 +71,13 @@ defmodule ExDoc.Retriever do
end

defp get_module(module, config) do
with {:docs_v1, _, language, _, _, metadata, _} = docs_chunk <- docs_chunk(module),
true <- config.filter_modules.(module, metadata),
with {:docs_v1, _, language, _, _, _metadata, _} = docs_chunk <- docs_chunk(module),
{:ok, language} <- ExDoc.Language.get(language, module),
%{} = module_data <- language.module_data(module, docs_chunk, config) do
[generate_node(module, module_data, config)]
{:ok, generate_node(module, module_data, config)}
else
_ ->
[]
{:error, module}
end
end

Expand Down Expand Up @@ -142,7 +162,8 @@ defmodule ExDoc.Retriever do
source_path: source_path,
source_url: source_link(source, module_data.line),
language: module_data.language,
annotations: List.wrap(metadata[:tags])
annotations: List.wrap(metadata[:tags]),
metadata: metadata
}
end

Expand Down
4 changes: 2 additions & 2 deletions test/ex_doc/formatter/epub/templates_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ defmodule ExDoc.Formatter.EPUB.TemplatesTest do

defp get_module_page(names, config \\ []) do
config = doc_config(config)
mods = ExDoc.Retriever.docs_from_modules(names, config)
[mod | _] = HTML.render_all(mods, ".xhtml", config, highlight_tag: "samp")
{mods, []} = ExDoc.Retriever.docs_from_modules(names, config)
[mod | _] = HTML.render_all(mods, [], ".xhtml", config, highlight_tag: "samp")
Templates.module_page(config, mod)
end

Expand Down
2 changes: 1 addition & 1 deletion test/ex_doc/formatter/html/search_data_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ defmodule ExDoc.Formatter.HTML.SearchDataTest do
end

defp search_data(modules, config) do
modules = ExDoc.Retriever.docs_from_modules(modules, config)
{modules, []} = ExDoc.Retriever.docs_from_modules(modules, config)

ExDoc.Formatter.HTML.run(modules, config)
[path] = Path.wildcard(Path.join([config.output, "dist", "search_data-*.js"]))
Expand Down
18 changes: 9 additions & 9 deletions test/ex_doc/formatter/html/templates_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ defmodule ExDoc.Formatter.HTML.TemplatesTest do

defp get_module_page(names, context, config \\ []) do
config = doc_config(context, config)
mods = ExDoc.Retriever.docs_from_modules(names, config)
[mod | _] = HTML.render_all(mods, ".html", config, [])
{mods, []} = ExDoc.Retriever.docs_from_modules(names, config)
[mod | _] = HTML.render_all(mods, [], ".html", config, [])
Templates.module_page(mod, @empty_nodes_map, config)
end

Expand Down Expand Up @@ -264,7 +264,7 @@ defmodule ExDoc.Formatter.HTML.TemplatesTest do

test "outputs listing for the given nodes", context do
names = [CompiledWithDocs, CompiledWithDocs.Nested]
nodes = ExDoc.Retriever.docs_from_modules(names, doc_config(context))
{nodes, []} = ExDoc.Retriever.docs_from_modules(names, doc_config(context))

assert [
%{
Expand Down Expand Up @@ -293,7 +293,7 @@ defmodule ExDoc.Formatter.HTML.TemplatesTest do

test "outputs deprecated: true if node is deprecated", context do
names = [CompiledWithDocs]
nodes = ExDoc.Retriever.docs_from_modules(names, doc_config(context))
{nodes, []} = ExDoc.Retriever.docs_from_modules(names, doc_config(context))

path = ["modules", Access.at!(0), "nodeGroups", Access.at!(0), "nodes"]
sidebar_functions = get_in(create_sidebar_items(%{modules: nodes}, []), path)
Expand All @@ -306,7 +306,7 @@ defmodule ExDoc.Formatter.HTML.TemplatesTest do

test "outputs deprecated: true if module is deprecated", context do
names = [Warnings]
nodes = ExDoc.Retriever.docs_from_modules(names, doc_config(context))
{nodes, []} = ExDoc.Retriever.docs_from_modules(names, doc_config(context))

assert Enum.any?(
create_sidebar_items(%{modules: nodes}, [])["modules"],
Expand All @@ -315,7 +315,7 @@ defmodule ExDoc.Formatter.HTML.TemplatesTest do
end

test "outputs nodes grouped based on metadata", context do
nodes =
{nodes, []} =
ExDoc.Retriever.docs_from_modules(
[CompiledWithDocs, CompiledWithDocs.Nested],
doc_config(context,
Expand Down Expand Up @@ -361,7 +361,7 @@ defmodule ExDoc.Formatter.HTML.TemplatesTest do
test "outputs module groups for the given nodes", context do
names = [CompiledWithDocs, CompiledWithDocs.Nested]
group_mapping = [groups_for_modules: [Group: [CompiledWithDocs]]]
nodes = ExDoc.Retriever.docs_from_modules(names, doc_config(context, group_mapping))
{nodes, []} = ExDoc.Retriever.docs_from_modules(names, doc_config(context, group_mapping))

assert [
%{"group" => ""},
Expand Down Expand Up @@ -420,8 +420,8 @@ defmodule ExDoc.Formatter.HTML.TemplatesTest do
test "builds sections out of moduledocs", context do
names = [CompiledWithDocs, CompiledWithoutDocs, DuplicateHeadings]
config = doc_config(context)
nodes = ExDoc.Retriever.docs_from_modules(names, config)
nodes = HTML.render_all(nodes, ".html", config, [])
{nodes, []} = ExDoc.Retriever.docs_from_modules(names, config)
nodes = HTML.render_all(nodes, [], ".html", config, [])

[compiled_with_docs, compiled_without_docs, duplicate_headings] =
create_sidebar_items(%{modules: nodes}, [])["modules"]
Expand Down
10 changes: 10 additions & 0 deletions test/ex_doc/language/elixir_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,16 @@ 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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like some guidance here, I was able to check the fix with a local project but I'm not sure what would be the best way to test this.

Also, maybe we should create a new test case for this, as it's a bit more complex than the other cases that are based only on autolink

end

test "warnings" do
ExDoc.Refs.insert([
{{:module, AutolinkTest.Foo}, :public},
Expand Down
Loading