jelly-beam / rebar3_ex_doc

rebar3 plugin for generating docs with ex_doc
Apache License 2.0
43 stars 13 forks source link

internal inconsistency with `uri_string:uri_string()` #66

Closed maennchen closed 1 year ago

maennchen commented 1 year ago
$ rebar3 ex_doc                                                                                                                             
===> Verifying dependencies...
===> Analyzing applications...
===> Compiling oidcc
===> Running edoc for oidcc
oidcc_userinfo.erl: warning: 'tt' is not allowed - skipping tag, extracting content
oidcc_userinfo.erl: warning: 'tt' is not allowed - skipping tag, extracting content
oidcc_authorization.erl: warning: 'tt' is not allowed - skipping tag, extracting content
oidcc_authorization.erl: warning: 'tt' is not allowed - skipping tag, extracting content
oidcc_provider_configuration.erl: warning: 'tt' is not allowed - skipping tag, extracting content
oidcc_provider_configuration.erl: warning: 'tt' is not allowed - skipping tag, extracting content
oidcc_token_introspection.erl: warning: 'tt' is not allowed - skipping tag, extracting content
oidcc_token_introspection.erl: warning: 'tt' is not allowed - skipping tag, extracting content
oidcc_token.erl: warning: 'tt' is not allowed - skipping tag, extracting content
oidcc_token.erl: warning: 'tt' is not allowed - skipping tag, extracting content
===> Running ex_doc for oidcc
===> warning: internal inconsistency, please submit bug: "uri_string:uri_string" != "'.'"
  src/oidcc_provider_configuration.erl:28: t:oidcc_provider_configuration.t/0

warning: internal inconsistency, please submit bug: "uri_string:uri_string" != "'.'"
  src/oidcc_provider_configuration.erl:28: t:oidcc_provider_configuration.t/0

warning: internal inconsistency, please submit bug: "oidcc_token:refresh" != "'.'"
  src/oidcc_token.erl:63: t:oidcc_token.t/0

warning: internal inconsistency, please submit bug: "uri_string:uri_string" != "'.'"
  src/oidcc_provider_configuration.erl:28: t:oidcc_provider_configuration.t/0

warning: internal inconsistency, please submit bug: "uri_string:uri_string" != "'.'"
  src/oidcc_provider_configuration.erl:28: t:oidcc_provider_configuration.t/0

warning: internal inconsistency, please submit bug: "oidcc_token:refresh" != "'.'"
  src/oidcc_token.erl:63: t:oidcc_token.t/0

Reproduction Repo: https://github.com/Erlang-Openid/oidcc/tree/ec4bd934b251874f09fbfb909bc575da8aa1368a/

Can also be seen in GitHub Actions: https://github.com/Erlang-Openid/oidcc/actions/runs/6113646364/job/16593557595?pr=220

paulo-ferraz-oliveira commented 1 year ago

Hm, your

-type t() ::
    #oidcc_provider_configuration{

seems to be redefining for the record. I've not used this syntax before, but if you reduce it to

-type t() :: #oidcc_provider_configuration{}

other issues surface

===> warning: internal inconsistency, please submit bug: "oidcc_token:refresh" != "'.'"
  src/oidcc_token.erl:63: t:oidcc_token.t/0

warning: internal inconsistency, please submit bug: "oidcc_token:refresh" != "'.'"
  src/oidcc_token.erl:63: t:oidcc_token.t/0
paulo-ferraz-oliveira commented 1 year ago

In the latter, reducing it to

-type t() ::
    #oidcc_token{
        id :: oidcc_token:id(),
        access :: oidcc_token:access(),
        refresh :: oidcc_token:refresh(),
        scope :: oidcc_scope:scopes()
    }.

seems to do the trick. Seems EDoc doesn't like either compound types (|) or something similar (though the code compiles just fine). I'd try to "fix" the code (if it was not your intention to override the types) or create a bug report upstream (next to ~erlang/otp~elixir-lang/ex_doc) since this doesn't seem rebar3_ex_doc-specific.

paulo-ferraz-oliveira commented 1 year ago

I see the error comes from ex_doc/lib/ex_doc/language/erlang.ex.

paulo-ferraz-oliveira commented 1 year ago

There might be something missing/wrong around ex_doc's autolink_spec/4 function.

I get (partial AST)

{field_type,
  [{line,66}],
  [refresh,
   {'|',
    [{line,66}],
    [{{'.',[{line,66}],[oidcc_token,refresh]},[{line,66}],[]},
     none]}
  ]}

and maybe it's not being handled 🤷

@starbelly, any idea about this? Pinging you also since you touched this file recently, so it's very likely you know much more than I do 😄

maennchen commented 1 year ago

@paulo-ferraz-oliveira

Hm, your

-type t() ::
    #oidcc_provider_configuration{

seems to be redefining for the record. [...]

The reason I'm redefining the type is that I want to display the contents in the generated docs. If I do not redefine it, the generated docs just show #oidcc_provider_configuration{}.

If there's a better way to show the full record types in the docs, I'd like to do that instead of repeating the definition.

paulo-ferraz-oliveira commented 1 year ago

I think there's an issue in generating the docs, which is why you're not seeing it. Let's wait a while for @starbelly and then we'll figure out how to move forward. 👍

starbelly commented 1 year ago

To add to everything stated above you get warning about the element tt because of the way links are done in your source (i.e., [https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.3]).

We are currently limited to the following tags per what shell docs support :

[a,p,'div',br,h1,h2,h3,h4,h5,h6,i,b,em,strong,pre,code,ul, ol,li,dl,dt,dd]

Thus if we want teletype element support, it has to be added to OTP.

starbelly commented 1 year ago

Note : I'll look into the other issue later today or tomorrow, it looks 😋 . Definitely a bug in ex_doc itself around what gets put in the process dictionary during parse and evaluation. I haven't dug into that code in anger yet for this particular problem, but I can tell you if we break the types down to what it actually is iolist() | binary(), that issue is gone. None the less, it's a bug and it should be fixed and I do believe I'll have fun doing so 😄

paulo-ferraz-oliveira commented 1 year ago

@maennchen, it seems <tt> is no longer supported in more modern HTML versions, so probably erlang/otp won't even accept a pull request related to that, and it's best to move forward 😄

maennchen commented 1 year ago

@starbelly / @paulo-ferraz-oliveira Adding square brackets areound a URL seems to be the way EDoc does external links:

https://www.erlang.org/doc/apps/edoc/chapter.html#external-linkshttps://www.erlang.org/doc/apps/edoc/chapter.html#external-links

I care about the link and not really about the tt.

Is there a way besides writing HTML by hand to create an external link?

paulo-ferraz-oliveira commented 1 year ago

Using [...] seems to be the official EDoc way to do it. It's just strange that <tt> is deprecated but still considered (though it's probably due to the fact EDoc doesn't get much love these days) 😄

If it's just a warning it should be Ok to just disregard it, for now, while it works...

starbelly commented 1 year ago

@paulo-ferraz-oliveira @maennchen Could you test the latest changes I pushed to a PR? https://github.com/elixir-lang/ex_doc/pull/1763

If you're not sure how to test :

  1. Go to the project you want to test
  2. mkdir _checkouts
  3. cd _checkouts
  4. git clone my ex_doc fork and checkout the branch in question
  5. mix deps.compile && mix escript.build
  6. cd .. (to your project)
  7. rebar3 ex_doc 8 Inspect the generated docs.
maennchen commented 1 year ago

@starbelly Works perfectly, thanks :)

I have found a new issue in the meantime which I though is connected. It seems to be something else though. I will report a new issue for that.

maennchen commented 1 year ago

Mentioned issue reported as https://github.com/elixir-lang/ex_doc/issues/1782

paulo-ferraz-oliveira commented 1 year ago

@starbelly, I didn't test it, but if @maennchen is happy, I'm happy 😄

paulo-ferraz-oliveira commented 1 year ago

@starbelly, shall we close it or wait until a new ex_doc is cut? It'll tie in nicely with #65.

starbelly commented 1 year ago

@paulo-ferraz-oliveira let's wait until the next release of ex_doc and thus the next release of rebar3_ex_doc

starbelly commented 1 year ago

0.2.21 was released to github with 0.30.7 of ex_doc. Closing this, feel free to re-open if the issue was not fixed.