Improve error message when escaping default values with custom rules in structs, closes #14817

Author: josevalim

lib/elixir/lib/kernel/utils.ex

@@ -126,10 +126,11 @@ defmodule Kernel.Utils do
         key == :__struct__ and raise(ArgumentError, "cannot set :__struct__ in struct definition")
 
         try do
-          :elixir_quote.escape(val, :escape, false)
+          :elixir_quote.escape(val, {:struct, module}, false)
         rescue
           e in [ArgumentError] ->
-            raise ArgumentError, "invalid value for struct field #{key}, " <> Exception.message(e)
+            raise ArgumentError,
+                  "invalid default value for struct field #{key}, " <> Exception.message(e)
         else
           _ -> {key, val}
         end
@@ -171,7 +172,7 @@ defmodule Kernel.Utils do
 
     :lists.foreach(foreach, enforce_keys)
     struct = :maps.from_list([__struct__: module] ++ fields)
-    escaped_struct = :elixir_quote.escape(struct, :escape, false)
+    escaped_struct = :elixir_quote.escape(struct, {:struct, module}, false)
 
     body =
       case bootstrapped? do

lib/elixir/lib/regex.ex

@@ -1017,9 +1017,6 @@ defmodule Regex do
 
     pattern_ast =
       cond do
-        is_nil(regex.re_pattern) ->
-          nil
-
         # TODO: Remove this when we require Erlang/OTP 28+
         # Before OTP 28.0, patterns did not contain any refs and could be safely be escaped
         :erlang.system_info(:otp_release) < [?2, ?8] ->

lib/elixir/src/elixir_bootstrap.erl

@@ -23,7 +23,7 @@
 'MACRO-defmacrop'(Caller, Call, Expr) -> define(Caller, defmacrop, Call, Expr).
 
 'MACRO-defmodule'({Line, _S, _E} = _Caller, Alias, [{do, Block}]) ->
-  Escaped = elixir_quote:escape(Block, none, false),
+  Escaped = elixir_quote:escape(Block, escape, false),
   Args = [[{line, Line}], Alias, Escaped, [], false, env()],
   {{'.', [], [elixir_module, compile]}, [], Args}.
 
@@ -46,7 +46,7 @@ define({Line, _S, #{module := Module} = E}, Kind, Call, Expr) ->
   Store =
     case UC or UE of
       true ->
-        elixir_quote:escape({Call, Expr}, none, true);
+        elixir_quote:escape({Call, Expr}, escape, true);
 
       false ->
         Key = erlang:unique_integer(),

lib/elixir/src/elixir_map.erl

@@ -37,9 +37,8 @@ expand_struct(Meta, Left, {'%{}', MapMeta, MapArgs}, S, #{context := Context} =
           Struct = load_struct(Meta, ELeft, Assocs, EE),
           Keys = ['__struct__'] ++ AssocKeys,
           WithoutKeys = lists:sort(maps:to_list(maps:without(Keys, Struct))),
-          StructAssocs = elixir_quote:escape(WithoutKeys, none, false),
-          {EStructAssocs, SA, EA} = elixir_expand:expand(StructAssocs, SE, EE),
-          {{'%', Meta, [ELeft, {'%{}', MapMeta, EStructAssocs ++ Assocs}]}, SA, EA};
+          StructAssocs = elixir_quote:escape(WithoutKeys, escape, false),
+          {{'%', Meta, [ELeft, {'%{}', MapMeta, StructAssocs ++ Assocs}]}, SE, EE};
 
         {'%{}', MapMeta, Assocs} ->
           _ = load_struct_info(Meta, ELeft, Assocs, EE),

lib/elixir/src/elixir_quote.erl

@@ -18,7 +18,7 @@
   line=false,
   file=nil,
   context=nil,
-  op=escape, % escape | escape_and_prune | quote
+  op=escape, % escape | escape_and_prune | {struct, Module} | quote
   aliases_hygiene=nil,
   imports_hygiene=nil,
   unquote=true,
@@ -140,15 +140,24 @@ do_tuple_linify(Fun, Meta, Left, Right, Var) ->
 %% Escapes the given expression. It is similar to quote, but
 %% lines are kept and hygiene mechanisms are disabled.
 escape(Expr, Op, Unquote) ->
-  Q = #elixir_quote{
-    line=true,
-    file=nil,
-    op=Op,
-    unquote=Unquote
-  },
-  case Unquote of
-    true -> do_quote(Expr, Q);
-    false -> do_escape(Expr, Q)
+  try
+    Q = #elixir_quote{
+      line=true,
+      file=nil,
+      op=Op,
+      unquote=Unquote
+    },
+    case Unquote of
+      true -> do_quote(Expr, Q);
+      false -> do_escape(Expr, Q)
+    end
+  catch
+    Kind:Reason:Stacktrace ->
+      Pruned = lists:dropwhile(fun
+        ({?MODULE, _, _, _}) -> true;
+        (_) -> false
+      end, Stacktrace),
+      erlang:raise(Kind, Reason, Pruned)
   end.
 
 do_escape({Left, Meta, Right}, #elixir_quote{op=escape_and_prune} = Q) when is_list(Meta) ->
@@ -176,13 +185,24 @@ do_escape(Map, Q) when is_map(Map) ->
   maybe
     #{'__struct__' := Module} ?= Map,
     true ?= is_atom(Module),
+    % We never escape ourselves (it can only happen during Elixir bootstrapping)
+    true ?= (Q#elixir_quote.op /= {struct, Module}),
     {module, Module} ?= code:ensure_loaded(Module),
     true ?= erlang:function_exported(Module, '__escape__', 1),
-    Expr = Module:'__escape__'(Map),
-    case shallow_valid_ast(Expr) of
-      true -> Expr;
-      false -> argument_error(
-        <<('Elixir.Kernel':inspect(Module))/binary, ".__escape__/1 returned invalid AST: ", ('Elixir.Kernel':inspect(Expr))/binary>>)
+    case Q#elixir_quote.op of
+      {struct, _Module} ->
+        argument_error(<<('Elixir.Kernel':inspect(Module))/binary,
+          " defines custom escaping rules which are not supported in struct defaults",
+          (bad_escape_hint())/binary>>);
+
+      _ ->
+        Expr = Module:'__escape__'(Map),
+        case shallow_valid_ast(Expr) of
+          true -> Expr;
+          false -> argument_error(
+            <<('Elixir.Kernel':inspect(Module))/binary, ".__escape__/1 returned invalid AST: ", ('Elixir.Kernel':inspect(Expr))/binary>>
+          )
+        end
     end
   else
     _ ->
@@ -234,8 +254,8 @@ escape_map_key_value(K, V, Map, Q) ->
   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>>);
+                        ('Elixir.Kernel':inspect(MaybeRef, []))/binary, ") and therefore it cannot be escaped",
+                        (bad_escape_hint())/binary>>);
     true ->
       {do_escape(K, Q), do_escape(V, Q)}
   end.
@@ -248,11 +268,11 @@ find_tuple_ref(Tuple, Index) ->
   end.
 
 bad_escape(Arg) ->
-  argument_error(<<"cannot escape ", ('Elixir.Kernel':inspect(Arg, []))/binary, ". ",
+  argument_error(<<"cannot escape ", ('Elixir.Kernel':inspect(Arg, []))/binary,
                    (bad_escape_hint())/binary>>).
 
 bad_escape_hint() ->
-  <<"The supported values are: lists, tuples, maps, atoms, numbers, bitstrings, ",
+  <<". The supported values are: lists, tuples, maps, atoms, numbers, bitstrings, ",
     "PIDs and remote functions in the format &Mod.fun/arity">>.
 
 %% Quote entry points

lib/elixir/test/elixir/kernel/errors_test.exs

@@ -435,7 +435,7 @@ defmodule Kernel.ErrorsTest do
   end
 
   test "invalid struct field value" do
-    msg = ~r"invalid value for struct field baz, cannot escape "
+    msg = ~r"invalid default value for struct field baz, cannot escape "
 
     assert_raise ArgumentError, msg, fn ->
       defmodule InvalidStructFieldValue do

lib/elixir/test/elixir/macro_test.exs

@@ -177,14 +177,14 @@ defmodule MacroTest do
     end
 
     @tag :re_import
-    test "escape works within structs fields" do
-      defmodule Test do
-        # Structs have their own escape logic, which now needs to be expanded
-        defstruct my_regex: ~r/^hi$/
-        def init, do: %__MODULE__{}
-      end
-
-      assert %Regex{} = Test.init().my_regex
+    test "escape raises within structs fields" do
+      assert_raise ArgumentError,
+                   ~r"Regex defines custom escaping rules which are not supported in struct defaults",
+                   fn ->
+                     defmodule Test do
+                       defstruct my_regex: ~r/^hi$/
+                     end
+                   end
     end
 
     defmodule EscapedStruct do