From 274b767e8d10b45bd577873ccd999abf710af79a Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Thu, 27 Feb 2025 17:07:46 +0100 Subject: [PATCH 1/8] link `Module.function/arity` to hexdocs in exception messages Idea by @Gazler This is similar to ExDoc autolinking, but only works for the simple Elixir `Module.function/arity` format. --- lib/plug/debugger.ex | 49 +++++++++++++++++++++++++++- lib/plug/templates/debugger.html.eex | 2 +- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/lib/plug/debugger.ex b/lib/plug/debugger.ex index 2bbe8654..609b8c19 100644 --- a/lib/plug/debugger.ex +++ b/lib/plug/debugger.ex @@ -233,7 +233,7 @@ defmodule Plug.Debugger do assigns = Keyword.merge(assigns, conn: conn, - message: message, + message: maybe_autolink(message), markdown: markdown, style: style, banner: banner, @@ -549,4 +549,51 @@ defmodule Plug.Debugger do defp maybe_merge_dark_styles(style, default_dark_style) do Map.put(style, :dark, default_dark_style) end + + defp maybe_autolink(message) do + splitted = Regex.split(~r/`([^`]+)`/, message, include_captures: true, trim: true) + + Enum.map(splitted, &maybe_format_function_reference/1) + |> IO.iodata_to_binary() + end + + defp maybe_format_function_reference("`" <> reference = text) do + reference = String.trim_trailing(reference, "`") + + with true <- function_reference?(reference), + {:ok, m, f, a} <- get_mfa(reference), + url when is_binary(url) <- get_doc(m, f, a, Application.get_application(m)) do + ~s[`#{h(reference)}`] + else + _ -> h(text) + end + end + + defp maybe_format_function_reference(text), do: text + + defp function_reference?(text) do + Regex.match?(~r/^[A-Z][A-Za-z0-9_.]+\.[a-z][A-Za-z0-9_!?]*\/\d+$/, text) + end + + def get_mfa(capture) do + with [function_path, arity] <- String.split(capture, "/"), + {arity, ""} <- Integer.parse(arity), + index when is_integer(index) <- rindex(function_path, "."), + module <- String.slice(function_path, 0, index) |> String.split(".") |> Module.concat(), + function <- String.slice(function_path, (index + 1)..-1) |> String.to_atom() do + {:ok, module, function, arity} + else + _ -> :error + end + end + + defp rindex(string, pattern) do + string + |> String.reverse() + |> :binary.match(pattern) + |> case do + {index, _} -> String.length(string) - index - String.length(pattern) + :nomatch -> nil + end + end end diff --git a/lib/plug/templates/debugger.html.eex b/lib/plug/templates/debugger.html.eex index 7f4b4d6a..fdc8f20a 100644 --- a/lib/plug/templates/debugger.html.eex +++ b/lib/plug/templates/debugger.html.eex @@ -884,7 +884,7 @@ at <%= h method(@conn) %> <%= h @conn.request_path %> - 
<%= h @message %>
 + 
<%= @message %>
 <%= for %{label: label, encoded_handler: encoded_handler} <- @actions do %> From 0a547f76e4a8a0b5832a3103f80cfed95bc6b868 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Thu, 27 Feb 2025 17:15:05 +0100 Subject: [PATCH 2/8] fix missing escape for second clause --- lib/plug/debugger.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plug/debugger.ex b/lib/plug/debugger.ex index 609b8c19..d5b6fb63 100644 --- a/lib/plug/debugger.ex +++ b/lib/plug/debugger.ex @@ -569,7 +569,7 @@ defmodule Plug.Debugger do end end - defp maybe_format_function_reference(text), do: text + defp maybe_format_function_reference(text), do: h(text) defp function_reference?(text) do Regex.match?(~r/^[A-Z][A-Za-z0-9_.]+\.[a-z][A-Za-z0-9_!?]*\/\d+$/, text) From 38a0e14108f4f1ff272a2dc9b31ede6e638ab7e5 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Thu, 27 Feb 2025 17:19:53 +0100 Subject: [PATCH 3/8] add test --- lib/plug/debugger.ex | 2 +- test/plug/debugger_test.exs | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/plug/debugger.ex b/lib/plug/debugger.ex index d5b6fb63..d730e06d 100644 --- a/lib/plug/debugger.ex +++ b/lib/plug/debugger.ex @@ -580,7 +580,7 @@ defmodule Plug.Debugger do {arity, ""} <- Integer.parse(arity), index when is_integer(index) <- rindex(function_path, "."), module <- String.slice(function_path, 0, index) |> String.split(".") |> Module.concat(), - function <- String.slice(function_path, (index + 1)..-1) |> String.to_atom() do + function <- String.slice(function_path, (index + 1)..-1//1) |> String.to_atom() do {:ok, module, function, arity} else _ -> :error diff --git a/test/plug/debugger_test.exs b/test/plug/debugger_test.exs index e77bdcf4..4c88a288 100644 --- a/test/plug/debugger_test.exs +++ b/test/plug/debugger_test.exs @@ -599,4 +599,14 @@ defmodule Plug.DebuggerTest do assert conn.resp_body =~ " end" end + + test "links to hexdocs" do + conn = + conn(:get, "/foo/bar") + |> put_req_header("accept", "text/html") + |> render([], fn -> raise "please use `Plug.Conn.send_resp/3` instead" end) + + assert conn.resp_body =~ + ~r(`Plug.Conn.send_resp/3`) + end end From de93a89c4e06cbfb2f01b782b547ba0e428104f9 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Thu, 27 Feb 2025 17:20:50 +0100 Subject: [PATCH 4/8] remove deprecated syntax, add test_ignore_filters --- mix.exs | 3 ++- test/plug/adapters/test/conn_test.exs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/mix.exs b/mix.exs index e6ee853a..90ce33fc 100644 --- a/mix.exs +++ b/mix.exs @@ -26,7 +26,8 @@ defmodule Plug.MixProject do groups_for_extras: groups_for_extras(), source_ref: "v#{@version}", source_url: "https://github.com/elixir-plug/plug" - ] + ], + test_ignore_filters: [&String.starts_with?(&1, "test/fixtures/")] ] end diff --git a/test/plug/adapters/test/conn_test.exs b/test/plug/adapters/test/conn_test.exs index 2977aa5c..84a80fe1 100644 --- a/test/plug/adapters/test/conn_test.exs +++ b/test/plug/adapters/test/conn_test.exs @@ -161,13 +161,13 @@ defmodule Plug.Adapters.Test.ConnTest do end test "use existing conn.remote_ip if exists" do - conn_with_remote_ip = %Plug.Conn{conn(:get, "/") | remote_ip: {151, 236, 219, 228}} + conn_with_remote_ip = %{conn(:get, "/") | remote_ip: {151, 236, 219, 228}} child_conn = Plug.Adapters.Test.Conn.conn(conn_with_remote_ip, :get, "/", foo: "bar") assert child_conn.remote_ip == {151, 236, 219, 228} end test "use existing conn.port if exists" do - conn_with_port = %Plug.Conn{conn(:get, "/") | port: 4200} + conn_with_port = %{conn(:get, "/") | port: 4200} child_conn = Plug.Adapters.Test.Conn.conn(conn_with_port, :get, "/", foo: "bar") assert child_conn.port == 4200 end From bc57618a5ab4fcc4e8aeb253c2120ba7e1d7f58d Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Thu, 27 Feb 2025 17:25:41 +0100 Subject: [PATCH 5/8] simplify get_mfa --- lib/plug/debugger.ex | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/plug/debugger.ex b/lib/plug/debugger.ex index d730e06d..8f905ef1 100644 --- a/lib/plug/debugger.ex +++ b/lib/plug/debugger.ex @@ -578,22 +578,13 @@ defmodule Plug.Debugger do def get_mfa(capture) do with [function_path, arity] <- String.split(capture, "/"), {arity, ""} <- Integer.parse(arity), - index when is_integer(index) <- rindex(function_path, "."), - module <- String.slice(function_path, 0, index) |> String.split(".") |> Module.concat(), - function <- String.slice(function_path, (index + 1)..-1//1) |> String.to_atom() do + parts = String.split(function_path, "."), + {function_str, parts} <- List.pop_at(parts, -1), + module = Module.concat(parts), + function = String.to_atom(function_str) do {:ok, module, function, arity} else _ -> :error end end - - defp rindex(string, pattern) do - string - |> String.reverse() - |> :binary.match(pattern) - |> case do - {index, _} -> String.length(string) - index - String.length(pattern) - :nomatch -> nil - end - end end From 384d386a6c87323447f2437f376d70298c6f6d5d Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Thu, 27 Feb 2025 18:24:03 +0100 Subject: [PATCH 6/8] only use one regex, use to_existing_atom --- lib/plug/debugger.ex | 13 +++++-------- test/plug/debugger_test.exs | 10 ++++++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/plug/debugger.ex b/lib/plug/debugger.ex index 8f905ef1..145638d1 100644 --- a/lib/plug/debugger.ex +++ b/lib/plug/debugger.ex @@ -551,7 +551,7 @@ defmodule Plug.Debugger do end defp maybe_autolink(message) do - splitted = Regex.split(~r/`([^`]+)`/, message, include_captures: true, trim: true) + splitted = Regex.split(~r/`[A-Z][A-Za-z0-9_.]+\.[a-z][A-Za-z0-9_!?]*\/\d+`/, message, include_captures: true, trim: true) Enum.map(splitted, &maybe_format_function_reference/1) |> IO.iodata_to_binary() @@ -560,8 +560,7 @@ defmodule Plug.Debugger do defp maybe_format_function_reference("`" <> reference = text) do reference = String.trim_trailing(reference, "`") - with true <- function_reference?(reference), - {:ok, m, f, a} <- get_mfa(reference), + with {:ok, m, f, a} <- get_mfa(reference), url when is_binary(url) <- get_doc(m, f, a, Application.get_application(m)) do ~s[`#{h(reference)}`] else @@ -571,20 +570,18 @@ defmodule Plug.Debugger do defp maybe_format_function_reference(text), do: h(text) - defp function_reference?(text) do - Regex.match?(~r/^[A-Z][A-Za-z0-9_.]+\.[a-z][A-Za-z0-9_!?]*\/\d+$/, text) - end - def get_mfa(capture) do with [function_path, arity] <- String.split(capture, "/"), {arity, ""} <- Integer.parse(arity), parts = String.split(function_path, "."), {function_str, parts} <- List.pop_at(parts, -1), module = Module.concat(parts), - function = String.to_atom(function_str) do + function = String.to_existing_atom(function_str) do {:ok, module, function, arity} else _ -> :error end + rescue + _ -> :error end end diff --git a/test/plug/debugger_test.exs b/test/plug/debugger_test.exs index 4c88a288..43dc6f9c 100644 --- a/test/plug/debugger_test.exs +++ b/test/plug/debugger_test.exs @@ -609,4 +609,14 @@ defmodule Plug.DebuggerTest do assert conn.resp_body =~ ~r(`Plug.Conn.send_resp/3`) end + + test "does not create new atoms" do + conn = + conn(:get, "/foo/bar") + |> put_req_header("accept", "text/html") + |> render([], fn -> raise "please use `NotExisting.not_existing_atom_for_test_does_not_create_new_atom/1` instead" end) + + assert conn.resp_body =~ ~r(`NotExisting.not_existing_atom_for_test_does_not_create_new_atom/1`) + assert_raise ArgumentError, fn -> String.to_existing_atom("not_existing_atom_for_test_does_not_create_new_atom") end + end end From b4b10e513df11caa58fc94d4d141ec7d4a5781b8 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Thu, 27 Feb 2025 18:28:03 +0100 Subject: [PATCH 7/8] mix format --- lib/plug/debugger.ex | 6 +++++- test/plug/debugger_test.exs | 12 +++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/plug/debugger.ex b/lib/plug/debugger.ex index 145638d1..a89f3bff 100644 --- a/lib/plug/debugger.ex +++ b/lib/plug/debugger.ex @@ -551,7 +551,11 @@ defmodule Plug.Debugger do end defp maybe_autolink(message) do - splitted = Regex.split(~r/`[A-Z][A-Za-z0-9_.]+\.[a-z][A-Za-z0-9_!?]*\/\d+`/, message, include_captures: true, trim: true) + splitted = + Regex.split(~r/`[A-Z][A-Za-z0-9_.]+\.[a-z][A-Za-z0-9_!?]*\/\d+`/, message, + include_captures: true, + trim: true + ) Enum.map(splitted, &maybe_format_function_reference/1) |> IO.iodata_to_binary() diff --git a/test/plug/debugger_test.exs b/test/plug/debugger_test.exs index 43dc6f9c..618a84d3 100644 --- a/test/plug/debugger_test.exs +++ b/test/plug/debugger_test.exs @@ -614,9 +614,15 @@ defmodule Plug.DebuggerTest do conn = conn(:get, "/foo/bar") |> put_req_header("accept", "text/html") - |> render([], fn -> raise "please use `NotExisting.not_existing_atom_for_test_does_not_create_new_atom/1` instead" end) + |> render([], fn -> + raise "please use `NotExisting.not_existing_atom_for_test_does_not_create_new_atom/1` instead" + end) + + assert conn.resp_body =~ + ~r(`NotExisting.not_existing_atom_for_test_does_not_create_new_atom/1`) - assert conn.resp_body =~ ~r(`NotExisting.not_existing_atom_for_test_does_not_create_new_atom/1`) - assert_raise ArgumentError, fn -> String.to_existing_atom("not_existing_atom_for_test_does_not_create_new_atom") end + assert_raise ArgumentError, fn -> + String.to_existing_atom("not_existing_atom_for_test_does_not_create_new_atom") + end end end From b67ff0a27e840e43da9a73e505fea2585dcbe031 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 27 Feb 2025 19:04:45 +0100 Subject: [PATCH 8/8] Update lib/plug/debugger.ex --- lib/plug/debugger.ex | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/plug/debugger.ex b/lib/plug/debugger.ex index a89f3bff..1974db6b 100644 --- a/lib/plug/debugger.ex +++ b/lib/plug/debugger.ex @@ -575,16 +575,13 @@ defmodule Plug.Debugger do defp maybe_format_function_reference(text), do: h(text) def get_mfa(capture) do - with [function_path, arity] <- String.split(capture, "/"), - {arity, ""} <- Integer.parse(arity), - parts = String.split(function_path, "."), - {function_str, parts} <- List.pop_at(parts, -1), - module = Module.concat(parts), - function = String.to_existing_atom(function_str) do - {:ok, module, function, arity} - else - _ -> :error - end + [function_path, arity] = String.split(capture, "/") + {arity, ""} = Integer.parse(arity) + parts = String.split(function_path, ".") + {function_str, parts} = List.pop_at(parts, -1) + module = Module.safe_concat(parts) + function = String.to_existing_atom(function_str) + {:ok, module, function, arity} rescue _ -> :error end