Fix some crashes on Erlang docs

[erszcz]

Hey, this is a follow up to erlef/documentation-wg#4 (comment). This just fixes the crashes, but might not be a real solution.

[wojtekmach]

Hi @erszcz, sorry for delay. These errors happens on edoc app, right?

I rebased my branch with master and added a couple workarounds, perhaps they fixed the problem you were running into. I just tested this on OTP 23.0-rc3 and couldn't reproduce it, could you double-check please?

[erszcz]

Hi, @wojtekmach!

I've checked again after you rebased and I still had to apply the patches.

One of them fixes returning :no_tool when an Erlang module is processed - based on the presence of __info__/1 it decides to return :otp so that fragment/4 does not crash.

The other one bypasses a call to Code.format when the data to format is not an Elixir AST.

I'm still working on the chunk format, so it might be the case that I'm outputting something that causes ExDoc to fail while OTP modules at the same time are processed properly.

Let's keep this PR open to track necessary fixes and once I'm sure the format is OK, see which of them are still necessary - what do you think?

[erszcz]

Just to make sure, I've also run ExDoc on stdlib @ OTP 23 rc2:

• 48299cd is necessary
• 8449a69 is not necessary

[wojtekmach]

how are you generating the docs? I was using this test: https://github.com/elixir-lang/ex_doc/blob/wm-erlang/test/otp_docs_test.exs on 23.0-rc2 with a couple different apps and there were no failures.

[erszcz]

In this particular case it doesn't matter if the chunks are generated by OTP make docs or by EDoc, though I'm dogfooding EDoc.

Please have a look at erszcz@2745a06 for extra debugging info. With these extra printouts:

$ mix test test/otp_docs_test.exs
"catch all"
"catch all"
.

Finished in 0.4 seconds
1 test, 0 failures

Randomized with seed 987349
$

But:

$ ERL_LIBS=/Users/erszcz/work/erszcz/edoc/_build/default/lib/ mix test test/otp_docs_test.exs
"erlang module, but not an OTP module"


  1) test otp (OTPDocsTest)
     test/otp_docs_test.exs:4
     ** (FunctionClauseError) no function clause matching in ExDoc.Autolink.fragment/4

     The following arguments were given to ExDoc.Autolink.fragment/4:
...

We can't rely only on :code.lib_dir() and Erlang module path in the definition of tool/1, because Erlang modules may very well be found outside of the Erlang/OTP installation directory.

[wojtekmach]

On 5 May 2020, at 15:26, Radek Szymczyszyn ***@***.***> wrote: We can't rely only on :code.lib_dir() and Erlang module path in the definition of tool/1, because Erlang modules may very well be found outside of the Erlang/OTP installation directory.

That makes sense, thanks!

[erszcz]

I've found the reason why 8449a69 is necessary. The Rebar3 plugin implements a provider behaviour, which defines a do callback. At the same time do is an Elixir keyword requiring a corresponding end. It seems that Code.format_string!/2 can't handle that :| Anyway, it seems we should branch at this point in the autolinker anyway, as the Elixir formatter won't format Erlang code appropriately (for example, it lowcases variable names) - it kind of works, but not accurately.

Here's the raw error with some extra debug info:

typespec: {'::',[{line,51}],
                [{do,[{line,51}],
                     [{{'.',[{line,51}],[rebar_state,t]},[{line,51}],[]}]},
                 {'|',[{line,51}],
                      [{'{}',[{line,51}],
                             [ok,
                              {{'.',[{line,51}],[rebar_state,t]},
                               [{line,51}],
                               []}]},
                       {'{}',[{line,51}],[error,{string,[{line,51}],[]}]}]}]}
options: [{app,edoc},
          {current_module,rebar3_edoc_chunks},
          {ext,<<".html">>},
          {skip_undefined_reference_warnings_on,[]},
          {module_id,<<"rebar3_edoc_chunks">>},
          {file,<<"src/rebar3_edoc_chunks.erl">>},
          {line,0},
          {language,'Elixir.ExDoc.Language.Erlang'},
          {id,<<"rebar3_edoc_chunks.do/1">>},
          {line,0}]
** (TokenMissingError) nofile:1: missing terminator: end (for "do" starting at line 1)
    (elixir 1.10.2) lib/code.ex:645: Code.format_string!/2
    (ex_doc 0.22.0-dev) lib/ex_doc/autolink.ex:281: ExDoc.Autolink.typespec/2
    (elixir 1.10.2) lib/enum.ex:1396: Enum."-map/2-lists^map/1-0-"/2
    (ex_doc 0.22.0-dev) lib/ex_doc/formatter/html.ex:86: anonymous fn/5 in ExDoc.Formatter.HTML.render_all/4
    (elixir 1.10.2) lib/enum.ex:2111: Enum."-reduce/3-lists^foldl/2-0-"/3
    (ex_doc 0.22.0-dev) lib/ex_doc/formatter/html.ex:83: anonymous fn/4 in ExDoc.Formatter.HTML.render_all/4
    (elixir 1.10.2) lib/enum.ex:1396: Enum."-map/2-lists^map/1-0-"/2

[erszcz]

I've played with the idea of sidestepping Code.format_string! completely for non-Elixir code. I think it would require not processing the spec/type attributes at all in ExDoc.Retriever, but keeping them as is. Later, in ExDoc.Autolink we can match on language to tell how to proceed: either process according to the previously deferred steps in case of Elixir, or use erl_pp / erl_prettypr for Erlang. The raw and definitely unmergeable (they break ExDoc for Elixir) changes are available at erszcz@c6fc2ff.

Another idea would be to introduce FunctionNode, TypeNode etc as protocols and then ElixirFunctionNode, ErlangFunctionNode, etc language specific implementations, but I'm totally too green with regard to ExDoc internal design to decide on that.

[wojtekmach]

what we're planning to do is introduce a :language field on ModuleNode struct (maybe just there or perhaps on all *Node structs). The value will be an atom that is an implementation of ExDoc.Language behaviour, something like this:

defmodule ExDoc.Language.Elixir do
  @behaviour ExDoc.Language

  @impl true
  def format_typespec(ast) do
    ast |> Macro.to_string() |> Code.format_string!()
  end
end

defmodule ExDoc.Language.Erlang do
  @behaviour ExDoc.Language

  @impl true
  def format_typespec(ast) do
    :erl_pp.format(ast)
  end
end

[erszcz]

This is superseded by #1291. I'm closing this one.