From 1177c509af84ad85d908954a39ba2afe39c59da0 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Sat, 7 Jun 2025 10:36:36 +0900 Subject: [PATCH 1/3] More precise error message when escaping a regex with a ref --- lib/elixir/src/elixir_quote.erl | 19 +++++++++++++++---- lib/elixir/test/elixir/macro_test.exs | 2 +- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index d4f0815fdc..7269abcbe8 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -165,12 +165,12 @@ do_escape(BitString, _) when is_bitstring(BitString) -> do_escape(Map, Q) when is_map(Map) -> TT = - [if - is_reference(V) -> + [case extract_value_ref(V) of + Ref when is_reference(Ref) -> argument_error(<<('Elixir.Kernel':inspect(Map, []))/binary, " contains a reference (", - ('Elixir.Kernel':inspect(V, []))/binary, ") and therefore it cannot be escaped ", + ('Elixir.Kernel':inspect(Ref, []))/binary, ") and therefore it cannot be escaped ", "(it must be defined within a function instead). ", (bad_escape_hint())/binary>>); - true -> + _ -> {do_quote(K, Q), do_quote(V, Q)} end || {K, V} <- lists:sort(maps:to_list(Map))], {'%{}', [], TT}; @@ -206,6 +206,17 @@ do_escape(Fun, _) when is_function(Fun) -> do_escape(Other, _) -> bad_escape(Other). +extract_value_ref(Ref) when is_reference(Ref) -> Ref; +extract_value_ref(Tuple) when is_tuple(Tuple) -> extract_value_ref_from_tuple(Tuple, 1); +extract_value_ref(_) -> nil. + +extract_value_ref_from_tuple(Tuple, Index) when Index > tuple_size(Tuple) -> nil; +extract_value_ref_from_tuple(Tuple, Index) -> + case element(Index, Tuple) of + Ref when is_reference(Ref) -> Ref; + _ -> extract_value_ref_from_tuple(Tuple, Index + 1) + end. + bad_escape(Arg) -> argument_error(<<"cannot escape ", ('Elixir.Kernel':inspect(Arg, []))/binary, ". ", (bad_escape_hint())/binary>>). diff --git a/lib/elixir/test/elixir/macro_test.exs b/lib/elixir/test/elixir/macro_test.exs index 6c1503c74e..e53111362c 100644 --- a/lib/elixir/test/elixir/macro_test.exs +++ b/lib/elixir/test/elixir/macro_test.exs @@ -143,7 +143,7 @@ defmodule MacroTest do test "inspects container when a reference cannot be escaped" do assert_raise ArgumentError, ~r"~r/foo/ contains a reference", fn -> - Macro.escape(%{~r/foo/ | re_pattern: make_ref()}) + Macro.escape(%{~r/foo/ | re_pattern: {:re_pattern, 0, 0, 0, make_ref()}}) end end end From 30ee6fda46432ebcc4f7cca0b0adc35e06c268ee Mon Sep 17 00:00:00 2001 From: sabiwara Date: Sat, 7 Jun 2025 13:42:48 +0900 Subject: [PATCH 2/3] Refactor as do_quote_map_value --- lib/elixir/src/elixir_quote.erl | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 7269abcbe8..100a896e40 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -165,13 +165,13 @@ do_escape(BitString, _) when is_bitstring(BitString) -> do_escape(Map, Q) when is_map(Map) -> TT = - [case extract_value_ref(V) of - Ref when is_reference(Ref) -> + [case do_quote_map_value(V, Q) of + {ok, QV} -> + {do_quote(K, Q), QV}; + {error, Ref} -> argument_error(<<('Elixir.Kernel':inspect(Map, []))/binary, " contains a reference (", ('Elixir.Kernel':inspect(Ref, []))/binary, ") and therefore it cannot be escaped ", - "(it must be defined within a function instead). ", (bad_escape_hint())/binary>>); - _ -> - {do_quote(K, Q), do_quote(V, Q)} + "(it must be defined within a function instead). ", (bad_escape_hint())/binary>>) end || {K, V} <- lists:sort(maps:to_list(Map))], {'%{}', [], TT}; @@ -206,15 +206,16 @@ do_escape(Fun, _) when is_function(Fun) -> do_escape(Other, _) -> bad_escape(Other). -extract_value_ref(Ref) when is_reference(Ref) -> Ref; -extract_value_ref(Tuple) when is_tuple(Tuple) -> extract_value_ref_from_tuple(Tuple, 1); -extract_value_ref(_) -> nil. +do_quote_map_value(Ref, _Q) when is_reference(Ref) -> {error, Ref}; +do_quote_map_value(Tuple, Q) when is_tuple(Tuple) -> do_quote_map_value_tuple(Tuple, Q, 1); +do_quote_map_value(Value, Q) -> {ok, do_quote(Value, Q)}. -extract_value_ref_from_tuple(Tuple, Index) when Index > tuple_size(Tuple) -> nil; -extract_value_ref_from_tuple(Tuple, Index) -> +do_quote_map_value_tuple(Tuple, Q, Index) when Index > tuple_size(Tuple) -> + {ok, do_quote(Tuple, Q)}; +do_quote_map_value_tuple(Tuple, Q, Index) -> case element(Index, Tuple) of - Ref when is_reference(Ref) -> Ref; - _ -> extract_value_ref_from_tuple(Tuple, Index + 1) + Ref when is_reference(Ref) -> {error, Ref}; + _ -> do_quote_map_value_tuple(Tuple, Q, Index + 1) end. bad_escape(Arg) -> From 26f35841be9fe408ef463520cf9ec6980c801b31 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Sat, 7 Jun 2025 13:59:33 +0900 Subject: [PATCH 3/3] Yet another refactoring proposal --- lib/elixir/src/elixir_quote.erl | 36 +++++++++++++++++---------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 100a896e40..3f872cd794 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -164,15 +164,7 @@ do_escape(BitString, _) when is_bitstring(BitString) -> end; do_escape(Map, Q) when is_map(Map) -> - TT = - [case do_quote_map_value(V, Q) of - {ok, QV} -> - {do_quote(K, Q), QV}; - {error, Ref} -> - argument_error(<<('Elixir.Kernel':inspect(Map, []))/binary, " contains a reference (", - ('Elixir.Kernel':inspect(Ref, []))/binary, ") and therefore it cannot be escaped ", - "(it must be defined within a function instead). ", (bad_escape_hint())/binary>>) - end || {K, V} <- lists:sort(maps:to_list(Map))], + TT = [escape_map_key_value(K, V, Map, Q) || {K, V} <- lists:sort(maps:to_list(Map))], {'%{}', [], TT}; do_escape([], _) -> @@ -206,16 +198,26 @@ do_escape(Fun, _) when is_function(Fun) -> do_escape(Other, _) -> bad_escape(Other). -do_quote_map_value(Ref, _Q) when is_reference(Ref) -> {error, Ref}; -do_quote_map_value(Tuple, Q) when is_tuple(Tuple) -> do_quote_map_value_tuple(Tuple, Q, 1); -do_quote_map_value(Value, Q) -> {ok, do_quote(Value, Q)}. +escape_map_key_value(K, V, Map, Q) -> + MaybeRef = if + is_reference(V) -> V; + is_tuple(V) -> find_tuple_ref(V, 1); + true -> nil + end, + if + is_reference(MaybeRef) -> + argument_error(<<('Elixir.Kernel':inspect(Map, []))/binary, " contains a reference (", + ('Elixir.Kernel':inspect(MaybeRef, []))/binary, ") and therefore it cannot be escaped ", + "(it must be defined within a function instead). ", (bad_escape_hint())/binary>>); + true -> + {do_quote(K, Q), do_quote(V, Q)} + end. -do_quote_map_value_tuple(Tuple, Q, Index) when Index > tuple_size(Tuple) -> - {ok, do_quote(Tuple, Q)}; -do_quote_map_value_tuple(Tuple, Q, Index) -> +find_tuple_ref(Tuple, Index) when Index > tuple_size(Tuple) -> nil; +find_tuple_ref(Tuple, Index) -> case element(Index, Tuple) of - Ref when is_reference(Ref) -> {error, Ref}; - _ -> do_quote_map_value_tuple(Tuple, Q, Index + 1) + Ref when is_reference(Ref) -> Ref; + _ -> find_tuple_ref(Tuple, Index + 1) end. bad_escape(Arg) ->