ex_doc icon indicating copy to clipboard operation
ex_doc copied to clipboard

Refactor autolinking typespecs

Open wojtekmach opened this issue 2 years ago • 2 comments

As a reminder, we want to replace, say, atom() with <a href="...">atom</a>(). The challenging part was we wanted to also format the type specs (using Code.format_string, erl_pp) and preserve that text "layout", spaces and new lines.

I think I have finally figured out a good way to do this. We are already working on type specs AST since it gives us name/arity of each thing we may want to link. I just realised that we can use the column information from the AST to precisely know where to make the transformation. We just need to do the transformations from the end of the string.

Here's a proof of concept: https://gist.github.com/wojtekmach/bc0cf1f1405ddd41962e7ddcfee12904.

What's nice about this is that the text transformation piece is totally agnostic between Erlang, Elixir type specs and upcoming types (as long as they have AST with line/column). The actual logic is about 40 LOC. :)

This requires a minor change in Elixir, https://github.com/elixir-lang/elixir/pull/13101, so we might have to wait a couple releases before we do this.

WDYT?

wojtekmach avatar Nov 13 '23 16:11 wojtekmach

There's a bug with warnings around callbacks:

@spec foo() :: String.bad()
def foo(), do: :ok
    warning: documentation references type "String.bad()" but it is undefined or private
    │
  3 │   def foo(), do: :ok
    │   ~~~~~~~~~~~~~~~~~~
    │
    └─ (myapp 1.0.0) lib/myapp.ex:3: MyApp.foo/0

the warning uses the wrong line. It's tricky to solve it with the existing design but in the design described in this proposal where we walk though the spec AST it should be easy.

Here's an integration test that reproduces this:

defmodule ExDoc.IntegrationTest do
  use ExUnit.Case, async: true

  @moduletag :tmp_dir

  test "warnings", %{tmp_dir: dir} do
    File.write!("#{dir}/mix.exs", """
    defmodule MyApp.MixProject do
      use Mix.Project

      def project do
        [
          app: :myapp,
          version: "1.0.0",
          deps: [
            {:ex_doc, path: "#{__DIR__}/../.."}
          ]
        ]
      end
    end
    """)

    File.mkdir!("#{dir}/_build")
    File.cp_r!("#{__DIR__}/../../_build/dev", "#{dir}/_build/dev")

    File.mkdir!("#{dir}/lib")

    File.write!("#{dir}/lib/myapp.ex", ~S'''
    defmodule MyApp do
      @spec foo() :: String.bad()
      def foo(), do: :ok
    end
    ''')

    Mix.Project.in_project(:myapp, dir, fn _mod ->
      File.cp!("#{__DIR__}/../../mix.lock", "#{dir}/mix.lock")

      path =
        Path.wildcard("#{__DIR__}/../../_build/dev/lib/**/ebin")
        |> Enum.join(":")

      options =
        [
          env: %{
            "MIX_DEPS_PATH" => "#{__DIR__}/../../deps",
            "MIX_PATH" => path
          },
          stderr_to_stdout: true
        ]

      {_, 0} = System.cmd("mix", ["compile"], options)
      {_, 0} = System.cmd("mix", ["docs", "-f", "html"], [into: IO.stream()] ++ options)
    end)
  end
end

wojtekmach avatar Jan 25 '24 11:01 wojtekmach

In the long term, we probably want to have the logic for annotating typespecs in Erlang, as today we rely on undocumented Elixir APIs. Elixir should have its own separate logic for the typesystem.

josevalim avatar Dec 16 '24 17:12 josevalim