From 29bda5c9f44606a8bfeda4eedef712c56aa55c7b Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 12 Feb 2018 19:56:42 +0100 Subject: [PATCH 01/10] Fix formatting typespec that happens to match placeholder regex --- lib/ex_doc/formatter/html/autolink.ex | 8 ++++---- test/ex_doc/formatter/html/autolink_test.exs | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/ex_doc/formatter/html/autolink.ex b/lib/ex_doc/formatter/html/autolink.ex index 9cc04a51f..e63bf594d 100644 --- a/lib/ex_doc/formatter/html/autolink.ex +++ b/lib/ex_doc/formatter/html/autolink.ex @@ -290,15 +290,15 @@ defmodule ExDoc.Formatter.HTML.Autolink do count = map_size(placeholders) + 1 type_size = form |> Macro.to_string() |> byte_size() int_size = count |> Integer.to_string() |> byte_size() - parens_size = 2 - pad = String.duplicate("p", max(type_size - int_size - parens_size, 1)) - placeholder = :"#{pad}#{count}" + extra_size = 4 # _, (, ), _ + pad = String.duplicate("p", max(type_size - int_size - extra_size, 1)) + placeholder = :"_#{pad}#{count}_" form = put_elem(form, 0, placeholder) {form, Map.put(placeholders, Atom.to_string(placeholder), string)} end defp replace_placeholders(string, placeholders) do - Regex.replace(~r"p+\d+", string, &Map.fetch!(placeholders, &1)) + Regex.replace(~r"_p+\d+_", string, &Map.fetch!(placeholders, &1)) end defp format_ast(ast) do diff --git a/test/ex_doc/formatter/html/autolink_test.exs b/test/ex_doc/formatter/html/autolink_test.exs index c97c830ad..cedd08c53 100644 --- a/test/ex_doc/formatter/html/autolink_test.exs +++ b/test/ex_doc/formatter/html/autolink_test.exs @@ -318,6 +318,11 @@ defmodule ExDoc.Formatter.HTML.AutolinkTest do ~s[t() :: %{foo: bar(), really_long_name_that_will_trigger_multiple_line_breaks: String.t()}] end + test "autolink types that look like formatter placeholders" do + assert Autolink.typespec(quote(do: p1() :: foo()), [foo: 0], []) == + ~s[p1() :: foo()] + end + test "autolink Elixir types in typespecs" do assert Autolink.typespec(quote(do: String.t), [], []) == ~s[String.t()] From f4656b4c090cca17c2ead5d65eaaa7690d08a0bb Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 12 Feb 2018 20:46:00 +0100 Subject: [PATCH 02/10] Move some functions around --- lib/ex_doc/formatter/html/autolink.ex | 56 +++++++++++++-------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/ex_doc/formatter/html/autolink.ex b/lib/ex_doc/formatter/html/autolink.ex index e63bf594d..d8ff44bd4 100644 --- a/lib/ex_doc/formatter/html/autolink.ex +++ b/lib/ex_doc/formatter/html/autolink.ex @@ -254,7 +254,7 @@ defmodule ExDoc.Formatter.HTML.Autolink do alias = expand_alias(alias) if source = get_source(alias, aliases, lib_dirs) do - url = remote_url(source, alias, name, args) + url = type_remote_url(source, alias, name, args) string = format_typespec_form(form, url) put_placeholder(form, string, placeholders) else @@ -270,12 +270,12 @@ defmodule ExDoc.Formatter.HTML.Autolink do |> replace_placeholders(placeholders) end - defp remote_url(@erlang_docs = source, module, name, _args) do + defp type_remote_url(@erlang_docs = source, module, name, _args) do module = enc_h("#{module}") name = enc_h("#{name}") "#{source}#{module}.html#type-#{name}" end - defp remote_url(source, alias, name, args) do + defp type_remote_url(source, alias, name, args) do name = enc_h("#{name}") "#{source}#{enc_h(inspect alias)}.html#t:#{name}/#{length(args)}" end @@ -313,31 +313,6 @@ defmodule ExDoc.Formatter.HTML.Autolink do end end - # TODO: remove when we require Elixir v1.6+ - defp formatter_available? do - function_exported?(Code, :format_string!, 2) - end - - defp split_string_to_link(string) do - case :binary.split(string, "(") do - [head, tail] -> {head, "(" <> tail} - [head] -> {head, ""} - end - end - - defp expand_alias({:__aliases__, _, [h|t]}) when is_atom(h), do: Module.concat([h|t]) - defp expand_alias(atom) when is_atom(atom), do: atom - defp expand_alias(_), do: nil - - defp get_source(alias, aliases, lib_dirs) do - cond do - is_nil(alias) -> nil - alias in aliases -> "" - doc = lib_dirs_to_doc(alias, lib_dirs) -> doc - true -> nil - end - end - @doc """ Create links to locally defined functions, specified in `locals` as a list of `fun/arity` strings. @@ -584,4 +559,29 @@ defmodule ExDoc.Formatter.HTML.Autolink do lib_dirs end end + + defp split_string_to_link(string) do + case :binary.split(string, "(") do + [head, tail] -> {head, "(" <> tail} + [head] -> {head, ""} + end + end + + defp expand_alias({:__aliases__, _, [h|t]}) when is_atom(h), do: Module.concat([h|t]) + defp expand_alias(atom) when is_atom(atom), do: atom + defp expand_alias(_), do: nil + + defp get_source(alias, aliases, lib_dirs) do + cond do + is_nil(alias) -> nil + alias in aliases -> "" + doc = lib_dirs_to_doc(alias, lib_dirs) -> doc + true -> nil + end + end + + # TODO: remove when we require Elixir v1.6+ + defp formatter_available? do + function_exported?(Code, :format_string!, 2) + end end From 9f7b24f3f3a0bf9f4b9e09043b0e23d8beaaa9fe Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 12 Feb 2018 20:47:55 +0100 Subject: [PATCH 03/10] DRY up formatting typespec --- lib/ex_doc/formatter/html/autolink.ex | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/ex_doc/formatter/html/autolink.ex b/lib/ex_doc/formatter/html/autolink.ex index d8ff44bd4..7752ad526 100644 --- a/lib/ex_doc/formatter/html/autolink.ex +++ b/lib/ex_doc/formatter/html/autolink.ex @@ -232,19 +232,16 @@ defmodule ExDoc.Formatter.HTML.Autolink do cond do {name, arity} in @basic_types -> url = elixir_source <> @basic_types_page - string = format_typespec_form(form, url) - put_placeholder(form, string, placeholders) + put_placeholder(form, url, placeholders) {name, arity} in @built_in_types -> url = elixir_source <> @built_in_types_page - string = format_typespec_form(form, url) - put_placeholder(form, string, placeholders) + put_placeholder(form, url, placeholders) {name, arity} in typespecs -> n = enc_h("#{name}") url = "#t:#{n}/#{arity}" - string = format_typespec_form(form, url) - put_placeholder(form, string, placeholders) + put_placeholder(form, url, placeholders) true -> {form, placeholders} @@ -255,8 +252,7 @@ defmodule ExDoc.Formatter.HTML.Autolink do if source = get_source(alias, aliases, lib_dirs) do url = type_remote_url(source, alias, name, args) - string = format_typespec_form(form, url) - put_placeholder(form, string, placeholders) + put_placeholder(form, url, placeholders) else {form, placeholders} end @@ -286,7 +282,8 @@ defmodule ExDoc.Formatter.HTML.Autolink do ~s[#{h(string_to_link)}] end - defp put_placeholder(form, string, placeholders) do + defp put_placeholder(form, url, placeholders) do + string = format_typespec_form(form, url) count = map_size(placeholders) + 1 type_size = form |> Macro.to_string() |> byte_size() int_size = count |> Integer.to_string() |> byte_size() From 5560a1e7a5bedcc906b0ad34d8d766388e7471aa Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 12 Feb 2018 20:56:12 +0100 Subject: [PATCH 04/10] Improve function name --- lib/ex_doc/formatter/html/autolink.ex | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/ex_doc/formatter/html/autolink.ex b/lib/ex_doc/formatter/html/autolink.ex index 7752ad526..28802174e 100644 --- a/lib/ex_doc/formatter/html/autolink.ex +++ b/lib/ex_doc/formatter/html/autolink.ex @@ -276,22 +276,22 @@ defmodule ExDoc.Formatter.HTML.Autolink do "#{source}#{enc_h(inspect alias)}.html#t:#{name}/#{length(args)}" end - defp format_typespec_form(form, url) do - string = Macro.to_string(form) + defp typespec_string_to_link(string, url) do {string_to_link, _string_with_parens} = split_string_to_link(string) ~s[#{h(string_to_link)}] end defp put_placeholder(form, url, placeholders) do - string = format_typespec_form(form, url) + string = Macro.to_string(form) + link = typespec_string_to_link(string, url) count = map_size(placeholders) + 1 - type_size = form |> Macro.to_string() |> byte_size() + type_size = byte_size(string) int_size = count |> Integer.to_string() |> byte_size() extra_size = 4 # _, (, ), _ pad = String.duplicate("p", max(type_size - int_size - extra_size, 1)) placeholder = :"_#{pad}#{count}_" form = put_elem(form, 0, placeholder) - {form, Map.put(placeholders, Atom.to_string(placeholder), string)} + {form, Map.put(placeholders, Atom.to_string(placeholder), link)} end defp replace_placeholders(string, placeholders) do From bad6a1decac85de46d3cafcdda33e75c1babc1fe Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 12 Feb 2018 20:56:41 +0100 Subject: [PATCH 05/10] Better counting --- lib/ex_doc/formatter/html/autolink.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ex_doc/formatter/html/autolink.ex b/lib/ex_doc/formatter/html/autolink.ex index 28802174e..442bdc8fd 100644 --- a/lib/ex_doc/formatter/html/autolink.ex +++ b/lib/ex_doc/formatter/html/autolink.ex @@ -285,8 +285,8 @@ defmodule ExDoc.Formatter.HTML.Autolink do string = Macro.to_string(form) link = typespec_string_to_link(string, url) count = map_size(placeholders) + 1 - type_size = byte_size(string) - int_size = count |> Integer.to_string() |> byte_size() + type_size = String.length(string) + int_size = count |> Integer.digits() |> length() extra_size = 4 # _, (, ), _ pad = String.duplicate("p", max(type_size - int_size - extra_size, 1)) placeholder = :"_#{pad}#{count}_" From 03b7749a353752f006649237bc08df835d4e7777 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 12 Feb 2018 20:57:59 +0100 Subject: [PATCH 06/10] Extract placeholder/2 --- lib/ex_doc/formatter/html/autolink.ex | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/ex_doc/formatter/html/autolink.ex b/lib/ex_doc/formatter/html/autolink.ex index 442bdc8fd..415726d2b 100644 --- a/lib/ex_doc/formatter/html/autolink.ex +++ b/lib/ex_doc/formatter/html/autolink.ex @@ -285,13 +285,17 @@ defmodule ExDoc.Formatter.HTML.Autolink do string = Macro.to_string(form) link = typespec_string_to_link(string, url) count = map_size(placeholders) + 1 + placeholder = placeholder(string, count) + form = put_elem(form, 0, placeholder) + {form, Map.put(placeholders, Atom.to_string(placeholder), link)} + end + + defp placeholder(string, count) do type_size = String.length(string) int_size = count |> Integer.digits() |> length() extra_size = 4 # _, (, ), _ pad = String.duplicate("p", max(type_size - int_size - extra_size, 1)) - placeholder = :"_#{pad}#{count}_" - form = put_elem(form, 0, placeholder) - {form, Map.put(placeholders, Atom.to_string(placeholder), link)} + :"_#{pad}#{count}_" end defp replace_placeholders(string, placeholders) do From 7eb557836119350c0ae75d6280640fca05393e6d Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 12 Feb 2018 21:04:05 +0100 Subject: [PATCH 07/10] Expose format_and_extract_typespec_placeholders/4 --- lib/ex_doc/formatter/html/autolink.ex | 12 ++++++++---- test/ex_doc/formatter/html/autolink_test.exs | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lib/ex_doc/formatter/html/autolink.ex b/lib/ex_doc/formatter/html/autolink.ex index 415726d2b..34d34475d 100644 --- a/lib/ex_doc/formatter/html/autolink.ex +++ b/lib/ex_doc/formatter/html/autolink.ex @@ -214,10 +214,16 @@ defmodule ExDoc.Formatter.HTML.Autolink do end defp format_typespec(ast, typespecs, aliases, lib_dirs) do + {formatted, placeholders} = format_and_extract_typespec_placeholders(ast, typespecs, aliases, lib_dirs) + replace_placeholders(formatted, placeholders) + end + + @doc false + def format_and_extract_typespec_placeholders(ast, typespecs, aliases, lib_dirs) do ref = make_ref() elixir_source = get_source(Kernel, aliases, lib_dirs) - {ast, placeholders} = + {formatted_ast, placeholders} = Macro.prewalk(ast, %{}, fn {:::, _, [{name, meta, args}, right]}, placeholders when is_atom(name) and is_list(args) -> {{:::, [], [{{ref, name}, meta, args}, right]}, placeholders} @@ -261,9 +267,7 @@ defmodule ExDoc.Formatter.HTML.Autolink do {form, placeholders} end) - ast - |> format_ast() - |> replace_placeholders(placeholders) + {format_ast(formatted_ast), placeholders} end defp type_remote_url(@erlang_docs = source, module, name, _args) do diff --git a/test/ex_doc/formatter/html/autolink_test.exs b/test/ex_doc/formatter/html/autolink_test.exs index cedd08c53..8b7ac66ab 100644 --- a/test/ex_doc/formatter/html/autolink_test.exs +++ b/test/ex_doc/formatter/html/autolink_test.exs @@ -323,6 +323,15 @@ defmodule ExDoc.Formatter.HTML.AutolinkTest do ~s[p1() :: foo()] end + @tag :formatter + test "placeholders" do + assert_typespec_placeholders( + "foobar()", + "_ppp1_()", + [foobar: 0] + ) + end + test "autolink Elixir types in typespecs" do assert Autolink.typespec(quote(do: String.t), [], []) == ~s[String.t()] @@ -392,4 +401,10 @@ defmodule ExDoc.Formatter.HTML.AutolinkTest do ~s[list(] <> ~s[function())] end + + defp assert_typespec_placeholders(original, expected, typespecs) do + ast = Code.string_to_quoted!(original) + {actual, _} = Autolink.format_and_extract_typespec_placeholders(ast, typespecs, [], []) + assert actual == expected, "Original: #{original}\nExpected: #{expected}\nActual: #{actual}" + end end From ad4ce9c4fbaba71a073d2702ee7e80cd5d7b9d5d Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 12 Feb 2018 21:37:27 +0100 Subject: [PATCH 08/10] Fix placeholder generation for nested types --- lib/ex_doc/formatter/html/autolink.ex | 7 +++-- test/ex_doc/formatter/html/autolink_test.exs | 30 ++++++++++++++++++-- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/lib/ex_doc/formatter/html/autolink.ex b/lib/ex_doc/formatter/html/autolink.ex index 34d34475d..608713028 100644 --- a/lib/ex_doc/formatter/html/autolink.ex +++ b/lib/ex_doc/formatter/html/autolink.ex @@ -295,10 +295,11 @@ defmodule ExDoc.Formatter.HTML.Autolink do end defp placeholder(string, count) do - type_size = String.length(string) + [name | _] = String.split(string, "(", trim: true) + name_size = String.length(name) int_size = count |> Integer.digits() |> length() - extra_size = 4 # _, (, ), _ - pad = String.duplicate("p", max(type_size - int_size - extra_size, 1)) + underscores_size = 2 + pad = String.duplicate("p", max(name_size - int_size - underscores_size, 1)) :"_#{pad}#{count}_" end diff --git a/test/ex_doc/formatter/html/autolink_test.exs b/test/ex_doc/formatter/html/autolink_test.exs index 8b7ac66ab..73f5921cf 100644 --- a/test/ex_doc/formatter/html/autolink_test.exs +++ b/test/ex_doc/formatter/html/autolink_test.exs @@ -325,11 +325,37 @@ defmodule ExDoc.Formatter.HTML.AutolinkTest do @tag :formatter test "placeholders" do + assert_typespec_placeholders( + "t()", + "_p1_()", + [t: 0] + ) + assert_typespec_placeholders( "foobar()", "_ppp1_()", [foobar: 0] ) + + assert_typespec_placeholders( + "Mod.foobar()", + "_ppppppp1_()", + [], + [Mod] + ) + + assert_typespec_placeholders( + "foobar(barbaz())", + "_ppp1_(_ppp2_())", + [foobar: 1, barbaz: 0] + ) + + assert_typespec_placeholders( + "Mod.foobar(Mod.barbaz())", + "_ppppppp1_(_ppppppp2_())", + [], + [Mod] + ) end test "autolink Elixir types in typespecs" do @@ -402,9 +428,9 @@ defmodule ExDoc.Formatter.HTML.AutolinkTest do ~s[function())] end - defp assert_typespec_placeholders(original, expected, typespecs) do + defp assert_typespec_placeholders(original, expected, typespecs, aliases \\ []) do ast = Code.string_to_quoted!(original) - {actual, _} = Autolink.format_and_extract_typespec_placeholders(ast, typespecs, [], []) + {actual, _} = Autolink.format_and_extract_typespec_placeholders(ast, typespecs, aliases, []) assert actual == expected, "Original: #{original}\nExpected: #{expected}\nActual: #{actual}" end end From 4efc43c7c3aac7feac5503dad15c7689876d7479 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 12 Feb 2018 21:45:21 +0100 Subject: [PATCH 09/10] Reuse placeholders --- lib/ex_doc/formatter/html/autolink.ex | 16 ++++++++++++---- test/ex_doc/formatter/html/autolink_test.exs | 6 ++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/ex_doc/formatter/html/autolink.ex b/lib/ex_doc/formatter/html/autolink.ex index 608713028..fae83bb12 100644 --- a/lib/ex_doc/formatter/html/autolink.ex +++ b/lib/ex_doc/formatter/html/autolink.ex @@ -288,10 +288,18 @@ defmodule ExDoc.Formatter.HTML.Autolink do defp put_placeholder(form, url, placeholders) do string = Macro.to_string(form) link = typespec_string_to_link(string, url) - count = map_size(placeholders) + 1 - placeholder = placeholder(string, count) - form = put_elem(form, 0, placeholder) - {form, Map.put(placeholders, Atom.to_string(placeholder), link)} + + case Enum.find(placeholders, fn {_key, value} -> value == link end) do + {placeholder, _} -> + form = put_elem(form, 0, String.to_atom(placeholder)) + {form, placeholders} + + nil -> + count = map_size(placeholders) + 1 + placeholder = placeholder(string, count) + form = put_elem(form, 0, placeholder) + {form, Map.put(placeholders, Atom.to_string(placeholder), link)} + end end defp placeholder(string, count) do diff --git a/test/ex_doc/formatter/html/autolink_test.exs b/test/ex_doc/formatter/html/autolink_test.exs index 73f5921cf..a48ee3cc3 100644 --- a/test/ex_doc/formatter/html/autolink_test.exs +++ b/test/ex_doc/formatter/html/autolink_test.exs @@ -356,6 +356,12 @@ defmodule ExDoc.Formatter.HTML.AutolinkTest do [], [Mod] ) + + assert_typespec_placeholders( + "foobar(foobar(barbaz()))", + "_ppp1_(_ppp1_(_ppp2_()))", + [foobar: 1, barbaz: 0] + ) end test "autolink Elixir types in typespecs" do From edbe69986583aaf305d8a5e3b7c08b72b270c0cf Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Mon, 12 Feb 2018 21:46:27 +0100 Subject: [PATCH 10/10] Keep placeholders as atoms --- lib/ex_doc/formatter/html/autolink.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ex_doc/formatter/html/autolink.ex b/lib/ex_doc/formatter/html/autolink.ex index fae83bb12..8790eb3a8 100644 --- a/lib/ex_doc/formatter/html/autolink.ex +++ b/lib/ex_doc/formatter/html/autolink.ex @@ -291,14 +291,14 @@ defmodule ExDoc.Formatter.HTML.Autolink do case Enum.find(placeholders, fn {_key, value} -> value == link end) do {placeholder, _} -> - form = put_elem(form, 0, String.to_atom(placeholder)) + form = put_elem(form, 0, placeholder) {form, placeholders} nil -> count = map_size(placeholders) + 1 placeholder = placeholder(string, count) form = put_elem(form, 0, placeholder) - {form, Map.put(placeholders, Atom.to_string(placeholder), link)} + {form, Map.put(placeholders, placeholder, link)} end end @@ -312,7 +312,7 @@ defmodule ExDoc.Formatter.HTML.Autolink do end defp replace_placeholders(string, placeholders) do - Regex.replace(~r"_p+\d+_", string, &Map.fetch!(placeholders, &1)) + Regex.replace(~r"_p+\d+_", string, &Map.fetch!(placeholders, String.to_atom(&1))) end defp format_ast(ast) do