Improve error messages when traversing source

Author: josevalim

lib/ex_doc/language.ex

@@ -23,7 +23,6 @@ defmodule ExDoc.Language do
     * `:source_file` - the source file the module code is located, defmodule in Elixir, or -module in Erlang
 
     * `:source_basedir` - the absolute directory where the Elixir/Erlang compiler was run.
-      See `ExDoc.Language.Source.get_basedir/2` for more details.
 
     * `:callback_types` - a list of types that are considered callbacks
 

lib/ex_doc/language/elixir.ex

@@ -39,8 +39,10 @@ defmodule ExDoc.Language.Elixir do
       abst_code = Source.get_abstract_code(module) ->
         title = module_title(module, type)
 
-        source_basedir = Source.get_basedir(abst_code, module)
-        {source_file, source_line} = Source.get_module_location(abst_code, source_basedir, module)
+        source_basedir = Source.fetch_basedir!(abst_code, module)
+
+        {source_file, source_line} =
+          Source.fetch_module_location!(abst_code, source_basedir, module)
 
         optional_callbacks = Source.get_optional_callbacks(module, type)
 
@@ -184,7 +186,7 @@ defmodule ExDoc.Language.Elixir do
     {{_kind, name, arity}, _anno, _signature, _doc, _metadata} = entry
 
     %{type: type, spec: spec, source_file: source, source_line: line} =
-      Source.get_type_from_module_data(module_data, name, arity)
+      Source.fetch_type!(module_data, name, arity)
 
     quoted = spec |> Code.Typespec.type_to_quoted() |> process_type_ast(type)
     signature = [get_typespec_signature(quoted, arity)]
@@ -530,7 +532,7 @@ defmodule ExDoc.Language.Elixir do
   end
 
   defp find_function_line(module_data, na) do
-    {_source, line} = Source.get_function_location(module_data, na)
+    {_source, line} = Source.fetch_function_location!(module_data, na)
     line
   end
 

lib/ex_doc/language/erlang.ex

@@ -26,8 +26,11 @@ defmodule ExDoc.Language.Erlang do
   def module_data(module, docs_chunk, _config) do
     if abst_code = Source.get_abstract_code(module) do
       id = Atom.to_string(module)
-      source_basedir = Source.get_basedir(abst_code, module)
-      {source_file, source_line} = Source.get_module_location(abst_code, source_basedir, module)
+      source_basedir = Source.fetch_basedir!(abst_code, module)
+
+      {source_file, source_line} =
+        Source.fetch_module_location!(abst_code, source_basedir, module)
+
       type = module_type(module)
 
       %{
@@ -115,7 +118,7 @@ defmodule ExDoc.Language.Erlang do
           end
       end
 
-    {file, line} = Source.get_function_location(module_data, {name, arity})
+    {file, line} = Source.fetch_function_location!(module_data, {name, arity})
 
     %{
       doc_fallback: fn -> equiv_data(module_data.module, file, line, metadata) end,
@@ -164,7 +167,7 @@ defmodule ExDoc.Language.Erlang do
   def type_data(entry, module_data) do
     {{kind, name, arity}, anno, signature, _doc, metadata} = entry
 
-    case Source.get_type_from_module_data(module_data, name, arity) do
+    case Source.fetch_type!(module_data, name, arity) do
       %{} = map ->
         %{
           doc_fallback: fn ->

lib/ex_doc/language/source.ex

@@ -1,6 +1,19 @@
 defmodule ExDoc.Language.Source do
   @moduledoc false
 
+  def anno_line(line) when is_integer(line), do: abs(line)
+  def anno_line(anno), do: anno |> :erl_anno.line() |> abs()
+
+  def anno_file(anno) do
+    case :erl_anno.file(anno) do
+      :undefined ->
+        nil
+
+      file ->
+        String.Chars.to_string(file)
+    end
+  end
+
   @doc """
   Get abstract code and basedir for a module
 
@@ -18,7 +31,7 @@ defmodule ExDoc.Language.Source do
   end
 
   defp expand_records_in_types(abst_code) do
-    ## Find all records in ast and collect any fields with type declarations
+    # Find all records in ast and collect any fields with type declarations
     records =
       filtermap_ast(abst_code, nil, fn
         {:attribute, anno, :record, {name, fields}} ->
@@ -40,7 +53,7 @@ defmodule ExDoc.Language.Source do
       end)
       |> Map.new()
 
-    ## Expand records in all specs, callbacks, types and opaques
+    # Expand records in all specs, callbacks, types and opaques
     filtermap_ast(abst_code, nil, fn
       {:attribute, anno, kind, {mfa, ast}} when kind in [:spec, :callback] ->
         ast = Enum.map(ast, &expand_records(&1, records))
@@ -62,16 +75,16 @@ defmodule ExDoc.Language.Source do
     {:ann_type, anno, [name, expand_records(type, records)]}
   end
 
-  ## When we encounter a record, we fetch the type definitions in the record and
-  ## merge then with the type. If there are duplicates we take the one in the type
-  ## declaration
+  # When we encounter a record, we fetch the type definitions in the record and
+  # merge then with the type. If there are duplicates we take the one in the type
+  # declaration
   defp expand_records({:type, anno, :record, [{:atom, _, record} = name | args]}, records) do
     args =
       (args ++ Map.get(records, record, []))
       |> Enum.uniq_by(fn {:type, _, :field_type, [{:atom, _, name} | _]} -> name end)
 
-    ## We delete the record from the map so that recursive
-    ## record definitions are not expanded.
+    # We delete the record from the map so that recursive
+    # record definitions are not expanded.
     records = Map.delete(records, record)
 
     {:type, anno, :record, expand_records([name | args], records)}
@@ -90,32 +103,31 @@ defmodule ExDoc.Language.Source do
   end
 
   @doc """
-  Get the basedir of a module
+  Fetches the basedir of a module.
 
   The basedir is the cwd of the Elixir/Erlang compiler when compiling the module.
   All `-file` attributes in the module is relative to this directory.
   """
-  def get_basedir(abst_code, module) do
-    ## We look for the first -file attribute to see what the source file that
-    ## was compiled is called. Both Erlang and Elixir places one at the top.
+  def fetch_basedir!(abst_code, module) do
+    # We look for the first -file attribute to see what the source file that
+    # was compiled is called. Both Erlang and Elixir places one at the top.
     filename =
       Enum.find_value(abst_code, fn
         {:attribute, _anno, :file, {filename, _line}} ->
           filename
 
         _ ->
           nil
-      end)
+      end) || raise "could not find base directory for #{inspect(module)}"
 
-    ## The first -file attribute will be either relative or absolute
-    ## depending on whether the compiler was called with an absolute
-    ## or relative path.
+    # The first -file attribute will be either relative or absolute
+    # depending on whether the compiler was called with an absolute
+    # or relative path.
     if Path.type(filename) == :relative do
-      ## If the compiler was called with a relative path, then any other
-      ## relative -file attribute will be relative to the same directory.
-      ## We use `module_info(:compile)[:source]` to get an absolute path
-      ## to the source file and calculate the basedir from that
-
+      # If the compiler was called with a relative path, then any other
+      # relative -file attribute will be relative to the same directory.
+      # We use `module_info(:compile)[:source]` to get an absolute path
+      # to the source file and calculate the basedir from that
       compile_source =
         cond do
           source = module.module_info(:compile)[:source] ->
@@ -143,27 +155,27 @@ defmodule ExDoc.Language.Source do
       |> Enum.drop(Path.split(filename) |> Enum.count() |> Kernel.*(-1))
       |> Path.join()
     else
-      ## If an absolute path was used, then any relative -file attribute
-      ## is relative to the directory of the source file
+      # If an absolute path was used, then any relative -file attribute
+      # is relative to the directory of the source file
       Path.dirname(filename)
     end
   end
 
-  def get_module_location(abst_code, source_basedir, module) do
+  def fetch_module_location!(abst_code, source_basedir, module) do
     find_ast(abst_code, source_basedir, fn
       {:attribute, anno, :module, ^module} ->
         {anno_file(anno), anno_line(anno)}
 
       _ ->
         nil
-    end)
+    end) || raise "could not find module definition for #{inspect(module)}"
   end
 
-  def get_function_location(module_data, {name, arity}) do
+  def fetch_function_location!(module_data, {name, arity}) do
     find_ast(module_data.private.abst_code, module_data.source_basedir, fn
       {:function, anno, ^name, ^arity, _} -> {anno_file(anno), anno_line(anno)}
       _ -> nil
-    end)
+    end) || raise "could not find function definition for #{name}/#{arity}"
   end
 
   # Returns a map of {name, arity} => spec.
@@ -178,7 +190,7 @@ defmodule ExDoc.Language.Source do
     |> Map.new()
   end
 
-  def get_type_from_module_data(module_data, name, arity) do
+  def fetch_type!(module_data, name, arity) do
     find_ast(module_data.private.abst_code, module_data.source_basedir, fn
       {:attribute, anno, type, {^name, _, args} = spec} = attr ->
         if type in [:opaque, :type] and length(args) == arity do
@@ -193,7 +205,7 @@ defmodule ExDoc.Language.Source do
 
       _ ->
         nil
-    end)
+    end) || raise "could not find type definition for #{name}/#{arity}"
   end
 
   def get_callbacks(abst_code, source_basedir) do
@@ -215,20 +227,16 @@ defmodule ExDoc.Language.Source do
 
   def get_optional_callbacks(_module, _type), do: []
 
-  def find_ast(ast, source_basedir, fun) do
-    filtermap_ast(ast, source_basedir, fun) |> hd()
+  defp find_ast(ast, source_basedir, fun) do
+    filtermap_ast(ast, source_basedir, fun) |> List.first()
   end
 
-  @doc """
-  Does a filtermap operation over the forms in an abstract syntax tree with
-  updated anno for each form pointing to the correct file.
-  """
   # The file which a form belongs to is decided by the previous :file
   # attribute in the AST. The :file can be either relative, or absolute
   # depending on how the file was included. So when traversing the AST
   # we need to keep track of the :file attributes and update the anno
   # with the correct file.
-  def filtermap_ast(ast, source_basedir, fun) do
+  defp filtermap_ast(ast, source_basedir, fun) do
     Enum.reduce(ast, {nil, []}, fn
       {:attribute, _anno, :file, {filename, _line}} = entry, {_file, acc} ->
         {if Path.type(filename) == :relative && source_basedir do
@@ -262,17 +270,4 @@ defmodule ExDoc.Language.Source do
     |> elem(1)
     |> Enum.reverse()
   end
-
-  def anno_line(line) when is_integer(line), do: abs(line)
-  def anno_line(anno), do: anno |> :erl_anno.line() |> abs()
-
-  def anno_file(anno) do
-    case :erl_anno.file(anno) do
-      :undefined ->
-        nil
-
-      file ->
-        String.Chars.to_string(file)
-    end
-  end
 end