Refactor fetching callbacks

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
 

test/ex_doc/retriever_test.exs

@@ -109,26 +109,33 @@ 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
 
       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) == "<p>callback1/0 docs.</p>"
       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) == "<p>mod docs.</p>"
+      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) =~ "<p>function/0 docs.</p>"
+      assert DocAST.to_string(function.doc) =~ "function/0 docs."
     end
 
     test "module with no chunk", c do

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