erlef / build-and-packaging-wg

Build and Packaging Working Group
19 stars 5 forks source link

beam chunk doc content changed from hidden to empty map #53

Closed starbelly closed 2 years ago

starbelly commented 2 years ago

Background

The doc chunks in OTP in regards to private <-> hidden modules and/or funs changed slightly in OTP 25 (seemingly).

Specifically, where ex_doc is expecting doc content to be possibly be hidden, is now an empty map. The fix is simple trivial, but it's not clear whether this change was intentional or not in Erlang/OTP. The last related erlang commit is https://github.com/erlang/otp/commit/03d9930f6d23e890ebf3ce9b375620fe46a447fa

It may be that ex_doc is out of date in regards to the changes.

Chunk difference

24.x

{docs_v1,[{file,"rebar3_hex_client.erl"},{location,2}],
         erlang,<<"application/erlang+html">>,hidden,#{},
         [{{function,create_user,4},
           [{file,"rebar3_hex_client.erl"},{location,29}],
           [<<"create_user(HexConfig,">>,<<"Username,">>,
            <<"Password,">>,<<"Email)">>],
           none,#{}},
           ... 

25.x

 docs_v1,[{file,"rebar3_hex_client.erl"},{location,2}],
         erlang,<<"application/erlang+html">>,#{},#{},
         [{{function,create_user,4},
           [{file,"rebar3_hex_client.erl"},{location,29}],
           [<<"create_user(HexConfig,">>,<<"Username,">>,
            <<"Password,">>,<<"Email)">>],
           #{},#{}},
           ...

Objective(s)

starbelly commented 2 years ago

The fix in ex_doc if the change is intentional in ex_doc would merely be :

diff --git a/lib/ex_doc/language/erlang.ex b/lib/ex_doc/language/erlang.ex
index 883d1431..402a73fb 100644
--- a/lib/ex_doc/language/erlang.ex
+++ b/lib/ex_doc/language/erlang.ex
@@ -40,7 +40,7 @@ defmodule ExDoc.Language.Erlang do

     # TODO: Edoc on Erlang/OTP24.1+ includes private functions in
     # the chunk, so we manually yank them out for now.
-    if kind == :function and doc_content != :hidden and
+    if kind == :function and (doc_content != :hidden or doc_content != %{}) and
          function_exported?(module_data.module, name, arity) do
       function_data(name, arity, doc_content, module_data)
     else
josevalim commented 2 years ago

IIRC, an empty map means "I was not documented". hidden means I was explicitly asked to not be documented. ExDoc generally not documented as public, and only hidden as false.

erszcz commented 2 years ago

Hey, @starbelly!

it's not clear whether this change was intentional or not in Erlang/OTP. The last related erlang commit is https://github.com/erlang/otp/commit/03d9930f6d23e890ebf3ce9b375620fe46a447fa

Please check the PR the linked to commit was part of - there it's explained why this change was introduced. There's also a link to the original ExDoc issue where the problem of modules with docs not being exported to HTML was reported.

starbelly commented 2 years ago

I see. I think the issue is limited to ex_doc and this issue was hastily open by yours truly. I can not replicate the problem edoc, but I can replicate it outside of rebar3_ex_doc by just using ex_doc escript. I'll look for further into ex_doc for the issue and a fix and/or open up an issue on ex_doc.

josevalim commented 2 years ago

Right, so it may not be an ex_doc issue. If you don't want something in the docs, then you should tag it as hidden. :)

starbelly commented 2 years ago

@josevalim The amount of derp on this one was strong. The problem is in rebar3_ex_doc and a misunderstanding of edoc opts. We had [{hidden, true}, {private, true}], which should have always included the hidden and private modules docs. The the problem was fixed, and now the incorrectness has reared its head.

Thank you Jose and Radek for breaking me out of my error loop 😁