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

Refactor autolinking typespecs #1815

Open
wojtekmach opened this issue Nov 13, 2023 · 1 comment
Open

Refactor autolinking typespecs #1815

wojtekmach opened this issue Nov 13, 2023 · 1 comment
Assignees

Comments

@wojtekmach
Copy link
Member

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, elixir-lang/elixir#13101, so we might have to wait a couple releases before we do this.

WDYT?

@wojtekmach wojtekmach self-assigned this Nov 13, 2023
@wojtekmach
Copy link
Member Author

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

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

No branches or pull requests

1 participant