Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't link to remote types in Erlang projects #1005

Closed
garthk opened this issue May 16, 2019 · 6 comments
Closed

Can't link to remote types in Erlang projects #1005

garthk opened this issue May 16, 2019 · 6 comments

Comments

@garthk
Copy link

garthk commented May 16, 2019

Start a new project:

mix new demo && cd demo

Replace mix.exs with:

defmodule Demo.MixProject do
  use Mix.Project

  def project do
    [
      app: :demo,
      version: "0.1.0",
      elixir: "~> 1.8",
      start_permanent: Mix.env() == :prod,
      deps: [
        {:dialyxir, "~> 0.5.1"},
        {:ex_doc, "~> 0.20.2"},
        {:opencensus, "~> 0.9.2"}
      ]
    ]
  end

  def application, do: [extra_applications: [:logger]]
end

Replace lib/demo.ex with:

defmodule Demo do
  @moduledoc """
  Demonstrates a problem with links to remote types.
  """

  @doc """
  Do nothing with a `t::opencensus.span/0`.
  Or, is it a `t:opencensus.span/0`?
  """
  @spec hello(:opencensus.span() | nil) :: :opencensus.span() | nil
  def hello(span), do: span
end

Build the documentation and open it:

mix do deps.get, deps.compile, docs
open doc/Demo.html

Observe:

  • There doesn't appear to be a way to link from the @doc string to the remote type.

  • The description of hello appears to correctly describe the types of the argument and return value:

    hello(span)
    hello(:opencensus.span() | nil) :: :opencensus.span() | nil

  • When you follow the link from :opencensus.span(), you get a broken link, not a working link

Double-check with Dialyzer:

mix dialyzer

If you see this, you're going to have long enough to grab a coffee, eat dinner, or perhaps even cook dinner:

Adding 903 modules to dialyxir_erlang-21.3.6_elixir-1.8.1_deps-dev.plt

I've got something else to get to, so I'll click Submit and report back later with how it went. From what happened in my main project, however, I expect a complaint about the missing export in opencensus (census-instrumentation/opencensus-erlang#152) but not the reference to :opencensus.span().

@josevalim
Copy link
Member

josevalim commented May 16, 2019 via email

@garthk
Copy link
Author

garthk commented May 16, 2019

I might be good for a PR once I get Opencensus.Absinthe and Opencensus.Honeycomb settled.

Simplest fix might be to suppress the type link from the function if the package atom starts with a lower case letter At least then we wouldn’t generate bad links. We’d also want to document the limitation. Would a warning on @doc backtick refs like t:lowercasepackage.type/0 be appropriate? Then devs would know to remove the t: and not waste time trying to make it work.

If we’re up for per-dependency type link resolution options: is there an obvious problem with defaulting to rebar3 hex docs style for lower case package atoms? We might also want to make it easy to switch to Erlang.mk style.

Would checking in deps for mix.exs or rebar.config be out of the question? Does Erlang.mk have a file that makes it obvious?

@josevalim
Copy link
Member

Would checking in deps for mix.exs or rebar.config be out of the question?

I actually like this idea. the Mix integration already looks for dependencies in there. We can possibly see if the manager is :mix and not include the dependencies otherwise.

@wojtekmach
Copy link
Member

We already have some support for cross linking to Erlang functions and types, e.g. this already works:

@type t :: :file.posix

and it builds the correct EDoc-style url. I'm not sure why linking to OTP works but to modules in dependencies doesn't, seem like a bug that should be relatively easy to fix.

However, this indeed never worked:

@doc """
`t::file.posix/0`
"""

but I think we could support this.

To sum up, I think we could assume that "Erlang" modules are documented using EDoc and link to their types and callbacks. (We already link to functions.)

@chulkilee
Copy link
Contributor

Just hit this bug - as function link for Edoc works great, it would great if we can make type link work!

erlang library functions:

if doc = module_docs(:erlang, module, lib_dirs) do
case kind do
:module ->
"[#{text}](#{doc}#{module}.html)"
:function ->
"[#{text}](#{doc}#{module}.html##{function}-#{arity})"
end
else

erlang core type:

defp type_remote_url(@erlang_docs = source, module, name, _args) do

@wojtekmach
Copy link
Member

I think we can solve it in #1132. When opencensus will start producing doc chunks we may assume it's docs are hosted on HexDocs.pm (maybe even using ExDoc, or we may need to make it configurable.) Until it's producing chunks we won't be auto linking to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants