Do not check deeply, improve message, handle improper lists

Author: sabiwara

lib/elixir/src/elixir_quote.erl

@@ -259,15 +259,23 @@ is_valid(unquote, Unquote) -> is_boolean(Unquote).
 shallow_validate_ast(Expr) ->
   case shallow_valid_ast(Expr) of
     true -> Expr;
-    false -> argument_error(<<"tried to unquote invalid AST: ", ('Elixir.Kernel':inspect(Expr))/binary>>)
+    false -> argument_error(
+      <<"tried to unquote invalid AST: ", ('Elixir.Kernel':inspect(Expr))/binary,
+        "\nDid you forget to escape term using Macro.escape/1?">>)
   end.
 
-shallow_valid_ast(Expr) when is_list(Expr) -> lists:all(fun shallow_valid_ast/1, Expr);
-shallow_valid_ast(Expr) when is_atom(Expr); is_binary(Expr); is_number(Expr); is_pid(Expr) -> true;
-shallow_valid_ast({Left, Right}) -> shallow_valid_ast(Left) andalso shallow_valid_ast(Right);
-shallow_valid_ast({Atom, Meta, Args}) when is_atom(Atom), is_list(Meta), is_atom(Args) orelse is_list(Args) -> true;
-shallow_valid_ast({Call, Meta, Args}) when is_list(Meta), is_list(Args) -> shallow_valid_ast(Call);
-shallow_valid_ast(_Term) -> false.
+shallow_valid_ast(Expr) when is_list(Expr) -> valid_ast_list(Expr);
+shallow_valid_ast(Expr) -> valid_ast_elem(Expr).
+
+valid_ast_list([]) -> true;
+valid_ast_list([Head | Tail]) -> valid_ast_elem(Head) andalso valid_ast_list(Tail);
+valid_ast_list(_Improper) -> false.
+
+valid_ast_elem(Expr) when is_list(Expr); is_atom(Expr); is_binary(Expr); is_number(Expr); is_pid(Expr) -> true;
+valid_ast_elem({Left, Right}) -> valid_ast_elem(Left) andalso valid_ast_elem(Right);
+valid_ast_elem({Atom, Meta, Args}) when is_atom(Atom), is_list(Meta), is_atom(Args) orelse is_list(Args) -> true;
+valid_ast_elem({Call, Meta, Args}) when is_list(Meta), is_list(Args) -> shallow_valid_ast(Call);
+valid_ast_elem(_Term) -> false.
 
 quote({unquote_splicing, _, [_]}, #elixir_quote{unquote=true}) ->
   argument_error(<<"unquote_splicing only works inside arguments and block contexts, "

lib/elixir/test/elixir/kernel/quote_test.exs

@@ -791,24 +791,26 @@ defmodule Kernel.QuoteTest.HasUnquoteTest do
   end
 
   test "unquote with invalid AST (shallow check)" do
-    assert_raise ArgumentError, "tried to unquote invalid AST: %{unescaped: :map}", fn ->
-      quote do: unquote(%{unescaped: :map})
-    end
-
-    assert_raise ArgumentError, "tried to unquote invalid AST: {:bad_meta, nil, []}", fn ->
-      quote do: unquote({:bad_meta, nil, []})
-    end
-
-    assert_raise ArgumentError, "tried to unquote invalid AST: {:bad_arg, nil, 1}", fn ->
-      quote do: unquote({:bad_arg, nil, 1})
-    end
-
-    assert_raise ArgumentError, "tried to unquote invalid AST: {:bad_tuple}", fn ->
-      quote do: unquote({:bad_tuple})
+    for term <- [
+          %{unescaped: :map},
+          1..10,
+          {:bad_meta, nil, []},
+          {:bad_arg, nil, 1},
+          {:bad_tuple},
+          make_ref(),
+          [:improper | :list],
+          [nested: {}]
+        ] do
+      message = """
+      tried to unquote invalid AST: #{inspect(term)}
+      Did you forget to escape term using Macro.escape/1?\
+      """
+
+      assert_raise ArgumentError, message, fn -> quote do: unquote(term) end
     end
+  end
 
-    assert_raise ArgumentError, ~r/tried to unquote invalid AST: #Reference</, fn ->
-      quote do: unquote(make_ref())
-    end
+  test "unquote with invalid AST is not checked deeply" do
+    assert quote do: unquote(foo: [bar: %{}]) == [foo: [bar: %{}]]
   end
 end