From 5ec676ab1efb16724695ce06edb6c7b4484d6779 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Thu, 4 Feb 2021 14:07:54 +0100 Subject: [PATCH] Refactor fetching callbacks --- lib/ex_doc/retriever.ex | 26 +++++++++++----- test/ex_doc/retriever_test.exs | 20 ++++++++---- test/test_helper.exs | 57 +++++++++++++++++++++------------- 3 files changed, 68 insertions(+), 35 deletions(-) diff --git a/lib/ex_doc/retriever.ex b/lib/ex_doc/retriever.ex index 74f64c6f5..791fe6424 100644 --- a/lib/ex_doc/retriever.ex +++ b/lib/ex_doc/retriever.ex @@ -170,6 +170,7 @@ defmodule ExDoc.Retriever do type: get_type(module), specs: get_specs(module), impls: get_impls(module), + callbacks: get_callbacks(module), abst_code: get_abstract_code(module), docs: docs_chunk } @@ -309,7 +310,6 @@ defmodule ExDoc.Retriever do doc: doc_ast, doc_line: doc_line, defaults: Enum.sort_by(defaults, fn {name, arity} -> sort_key(name, arity) end), - # TODO: Enum.join(signature, ""), signature: Enum.join(signature, " "), specs: specs, source_path: source.path, @@ -382,14 +382,24 @@ defmodule ExDoc.Retriever do {{kind, name, arity}, anno, _, doc, metadata} = callback actual_def = actual_def(name, arity, kind) doc_line = anno_line(anno) - annotations = annotations_from_metadata(metadata) - {:attribute, anno, :callback, {^actual_def, specs}} = - Enum.find(module_data.abst_code, &match?({:attribute, _, :callback, {^actual_def, _}}, &1)) + specs = + case Map.fetch(module_data.callbacks, actual_def) do + {:ok, specs} -> + specs + + :error -> + # gen_statem:state_name/3 is stored as StateName/3 in the docs chunk + name = name |> Atom.to_string() |> Macro.underscore() |> String.to_atom() + Map.fetch!(module_data.callbacks, {name, arity}) + end + {:type, anno, _, _} = hd(specs) line = anno_line(anno) specs = Enum.map(specs, &Code.Typespec.spec_to_quoted(name, &1)) + annotations = annotations_from_metadata(metadata) + annotations = if actual_def in optional_callbacks, do: ["optional" | annotations], else: annotations @@ -424,15 +434,15 @@ defmodule ExDoc.Retriever do # Returns a map of {name, arity} => behaviour. defp get_impls(module) do for behaviour <- behaviours_implemented_by(module), - callback <- callbacks_defined_by(behaviour), + callback <- Map.keys(get_callbacks(behaviour)), do: {callback, behaviour}, into: %{} end - defp callbacks_defined_by(module) do + defp get_callbacks(module) do case Code.Typespec.fetch_callbacks(module) do - {:ok, callbacks} -> Enum.map(callbacks, &elem(&1, 0)) - :error -> [] + {:ok, callbacks} -> Map.new(callbacks) + :error -> %{} end end diff --git a/test/ex_doc/retriever_test.exs b/test/ex_doc/retriever_test.exs index d92e98494..d5d37648b 100644 --- a/test/ex_doc/retriever_test.exs +++ b/test/ex_doc/retriever_test.exs @@ -109,7 +109,8 @@ defmodule ExDoc.RetrieverTest do end """) - [mod] = Retriever.docs_from_modules([Mod], %ExDoc.Config{}) + config = %ExDoc.Config{source_url_pattern: "%{path}:%{line}"} + [mod] = Retriever.docs_from_modules([Mod], config) assert mod.type == :behaviour [callback1, macrocallback1, optional_callback1] = mod.docs @@ -117,18 +118,24 @@ defmodule ExDoc.RetrieverTest do assert callback1.id == "callback1/0" assert callback1.type == :callback assert callback1.annotations == [] + assert callback1.doc_line == 2 + assert Path.basename(callback1.source_url) == "nofile:3" assert DocAST.to_string(callback1.doc) == "

callback1/0 docs.

" assert Macro.to_string(callback1.specs) == "[callback1() :: :ok]" assert optional_callback1.id == "optional_callback1/0" assert optional_callback1.type == :callback assert optional_callback1.annotations == ["optional"] + assert optional_callback1.doc_line == 5 + assert Path.basename(optional_callback1.source_url) == "nofile:5" refute optional_callback1.doc assert Macro.to_string(optional_callback1.specs) == "[optional_callback1() :: :ok]" assert macrocallback1.id == "macrocallback1/0" assert macrocallback1.type == :macrocallback assert macrocallback1.annotations == [] + assert macrocallback1.doc_line == 9 + assert Path.basename(macrocallback1.source_url) == "nofile:9" refute macrocallback1.doc assert Macro.to_string(macrocallback1.specs) == "[macrocallback1(term()) :: :ok]" @@ -341,7 +348,7 @@ defmodule ExDoc.RetrieverTest do %ExDoc.ModuleNode{ deprecated: nil, - doc_line: 0, + doc_line: _, docs: [function], function_groups: ["Functions"], group: nil, @@ -357,26 +364,27 @@ defmodule ExDoc.RetrieverTest do typespecs: [] } = mod - assert DocAST.to_string(mod.doc) == "

mod docs.

" + assert DocAST.to_string(mod.doc) =~ "mod docs." %ExDoc.FunctionNode{ annotations: [], arity: 0, defaults: [], deprecated: nil, - doc_line: 0, + doc_line: _, group: "Functions", id: "function/0", name: :function, rendered_doc: nil, - signature: "function() -> term()\n", + # TODO: assert when edoc is fixed + signature: _, source_path: _, source_url: nil, specs: [], type: :function } = function - assert DocAST.to_string(function.doc) =~ "

function/0 docs.

" + assert DocAST.to_string(function.doc) =~ "function/0 docs." end test "module with no chunk", c do diff --git a/test/test_helper.exs b/test/test_helper.exs index f529e38d5..4b6632574 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1,6 +1,7 @@ exclude = [ earmark: not ExDoc.Markdown.Earmark.available?(), - otp23: System.otp_release() < "23" + otp23: System.otp_release() < "23", + otp24: System.otp_release() < "24" ] ExUnit.start(exclude: Enum.filter(exclude, &elem(&1, 1))) @@ -66,25 +67,40 @@ defmodule TestHelper do :ok end - def edoc_to_chunk(module) do - source_path = module.module_info(:compile)[:source] - beam_path = :code.which(module) - dir = :filename.dirname(source_path) - xml_path = '#{dir}/#{module}.xml' - chunk_path = '#{dir}/#{module}.chunk' - - docgen_dir = :code.lib_dir(:erl_docgen) - cmd!("escript #{docgen_dir}/priv/bin/xml_from_edoc.escript -dir #{dir} #{source_path}") - - :docgen_xml_to_chunk.main(["app", xml_path, beam_path, "", chunk_path]) - docs_chunk = File.read!(chunk_path) - {:ok, ^module, chunks} = :beam_lib.all_chunks(beam_path) - {:ok, beam} = :beam_lib.build_module([{'Docs', docs_chunk} | chunks]) - File.write!(beam_path, beam) - end - - defp cmd!(command) do - 0 = Mix.shell().cmd(command) + if Code.ensure_loaded?(:edoc_doclet_chunks) do + def edoc_to_chunk(module) do + source_path = module.module_info(:compile)[:source] + dir = :filename.dirname(source_path) + + :ok = + :edoc.files([source_path], + preprocess: true, + doclet: :edoc_doclet_chunks, + layout: :edoc_layout_chunks, + dir: dir ++ '/doc' + ) + end + else + def edoc_to_chunk(module) do + source_path = module.module_info(:compile)[:source] + beam_path = :code.which(module) + dir = :filename.dirname(source_path) + xml_path = '#{dir}/#{module}.xml' + chunk_path = '#{dir}/#{module}.chunk' + + docgen_dir = :code.lib_dir(:erl_docgen) + cmd!("escript #{docgen_dir}/priv/bin/xml_from_edoc.escript -dir #{dir} #{source_path}") + + :docgen_xml_to_chunk.main(["app", xml_path, beam_path, "", chunk_path]) + docs_chunk = File.read!(chunk_path) + {:ok, ^module, chunks} = :beam_lib.all_chunks(beam_path) + {:ok, beam} = :beam_lib.build_module([{'Docs', docs_chunk} | chunks]) + File.write!(beam_path, beam) + end + + defp cmd!(command) do + 0 = Mix.shell().cmd(command) + end end # TODO: replace with ExUnit @tag :tmp_dir feature when we require Elixir v1.11. @@ -94,7 +110,6 @@ defmodule TestHelper do dir = Path.join(["tmp", module, name]) File.rm_rf!(dir) File.mkdir_p!(dir) - Map.put(context, :tmp_dir, dir) end