diff --git a/lib/elixir/lib/macro.ex b/lib/elixir/lib/macro.ex index f86e9571cae..6a1923eaf03 100644 --- a/lib/elixir/lib/macro.ex +++ b/lib/elixir/lib/macro.ex @@ -814,7 +814,8 @@ defmodule Macro do iex> value = {:a, :b, :c} iex> quote do: unquote(value) - {:a, :b, :c} + ** (ArgumentError) tried to unquote invalid AST: {:a, :b, :c} + Did you forget to escape term using Macro.escape/1? `escape/2` is used to escape *values* (either directly passed or variable bound), while `quote/2` produces syntax trees for diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 8e2fe1a3110..46ee8910c44 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -1,6 +1,6 @@ -module(elixir_quote). -export([escape/3, linify/3, linify_with_context_counter/3, build/7, quote/2, has_unquotes/1, fun_to_quoted/1]). --export([dot/5, tail_list/3, list/2, validate_runtime/2]). %% Quote callbacks +-export([dot/5, tail_list/3, list/2, validate_runtime/2, shallow_validate_ast/1]). %% Quote callbacks -include("elixir.hrl"). -define(defs(Kind), Kind == def; Kind == defp; Kind == defmacro; Kind == defmacrop; Kind == '@'). @@ -15,7 +15,8 @@ aliases_hygiene=nil, imports_hygiene=nil, unquote=true, - generated=false + generated=false, + shallow_validate=false }). %% fun_to_quoted @@ -215,7 +216,8 @@ build(Meta, Line, File, Context, Unquote, Generated, E) -> file=VFile, unquote=Unquote, context=VContext, - generated=Generated + generated=Generated, + shallow_validate=true }, {Q, VContext, Acc3}. @@ -254,6 +256,27 @@ is_valid(context, Context) -> is_atom(Context) andalso (Context /= nil); is_valid(generated, Generated) -> is_boolean(Generated); 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, + "\nDid you forget to escape term using Macro.escape/1?">>) + end. + +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, " "wrap it in parens if you want it to work with one-liners">>); @@ -283,8 +306,12 @@ do_quote({quote, Meta, [Opts, Arg]}, Q) when is_list(Meta) -> {'{}', [], [quote, meta(NewMeta, Q), [TOpts, TArg]]}; -do_quote({unquote, Meta, [Expr]}, #elixir_quote{unquote=true}) when is_list(Meta) -> - Expr; +% +do_quote({unquote, Meta, [Expr]}, #elixir_quote{unquote=true, shallow_validate=Validate}) when is_list(Meta) -> + case Validate of + true -> {{'.', Meta, [?MODULE, shallow_validate_ast]}, Meta, [Expr]}; + false -> Expr + end; %% Aliases diff --git a/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs b/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs index 8b5877f799a..30730bafa9f 100644 --- a/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs +++ b/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs @@ -405,12 +405,16 @@ defmodule Code.Normalizer.QuotedASTTest do end test "range" do - assert quoted_to_string(quote(do: unquote(-1..+2))) == "-1..2" + assert quoted_to_string(quote(do: -1..+2)) == "-1..+2" assert quoted_to_string(quote(do: Foo.integer()..3)) == "Foo.integer()..3" - assert quoted_to_string(quote(do: unquote(-1..+2//-3))) == "-1..2//-3" + assert quoted_to_string(quote(do: -1..+2//-3)) == "-1..+2//-3" assert quoted_to_string(quote(do: Foo.integer()..3//Bar.bat())) == "Foo.integer()..3//Bar.bat()" + + # invalid AST + assert quoted_to_string(-1..+2) == "-1..2" + assert quoted_to_string(-1..+2//-3) == "-1..2//-3" end test "when" do diff --git a/lib/elixir/test/elixir/kernel/expansion_test.exs b/lib/elixir/test/elixir/kernel/expansion_test.exs index c762a5ca5d5..8a95d3d6c39 100644 --- a/lib/elixir/test/elixir/kernel/expansion_test.exs +++ b/lib/elixir/test/elixir/kernel/expansion_test.exs @@ -2871,11 +2871,11 @@ defmodule Kernel.ExpansionTest do test "handles invalid expressions" do assert_compile_error(~r"invalid quoted expression: {1, 2, 3}", fn -> - expand_env(quote(do: unquote({1, 2, 3})), __ENV__) + expand_env({1, 2, 3}, __ENV__) end) assert_compile_error(~r"invalid quoted expression: #Function\<", fn -> - expand(quote(do: unquote({:sample, fn -> nil end}))) + expand({:sample, fn -> nil end}) end) assert_compile_error(~r"invalid pattern in match", fn -> diff --git a/lib/elixir/test/elixir/kernel/quote_test.exs b/lib/elixir/test/elixir/kernel/quote_test.exs index fe06a61f021..5d720c5a5ce 100644 --- a/lib/elixir/test/elixir/kernel/quote_test.exs +++ b/lib/elixir/test/elixir/kernel/quote_test.exs @@ -789,4 +789,29 @@ defmodule Kernel.QuoteTest.HasUnquoteTest do refute :elixir_quote.has_unquotes(ast) end + + test "unquote with invalid AST (shallow check)" do + 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 + + test "unquote with invalid AST is not checked deeply" do + assert quote do: unquote(foo: [1 | 2]) == [foo: [1 | 2]] + assert quote do: unquote(foo: [bar: %{}]) == [foo: [bar: %{}]] + end end diff --git a/lib/elixir/test/elixir/macro_test.exs b/lib/elixir/test/elixir/macro_test.exs index 452dbef6cf4..227127855f8 100644 --- a/lib/elixir/test/elixir/macro_test.exs +++ b/lib/elixir/test/elixir/macro_test.exs @@ -839,7 +839,7 @@ defmodule MacroTest do end test "converts invalid AST with inspect" do - assert Macro.to_string(quote do: unquote(1..3)) == "1..3" + assert Macro.to_string(1..3) == "1..3" end end @@ -1172,12 +1172,16 @@ defmodule MacroTest do end test "range" do - assert macro_to_string(quote(do: unquote(-1..+2))) == "-1..2" + assert macro_to_string(quote(do: -1..+2)) == "-1..+2" assert macro_to_string(quote(do: Foo.integer()..3)) == "Foo.integer()..3" - assert macro_to_string(quote(do: unquote(-1..+2//-3))) == "-1..2//-3" + assert macro_to_string(quote(do: -1..+2//-3)) == "-1..+2//-3" assert macro_to_string(quote(do: Foo.integer()..3//Bar.bat())) == "Foo.integer()..3//Bar.bat()" + + # invalid AST + assert macro_to_string(-1..+2) == "-1..2" + assert macro_to_string(-1..+2//-3) == "-1..2//-3" end test "when" do diff --git a/lib/mix/test/mix/tasks/xref_test.exs b/lib/mix/test/mix/tasks/xref_test.exs index 5d9a7509491..1d231e14be5 100644 --- a/lib/mix/test/mix/tasks/xref_test.exs +++ b/lib/mix/test/mix/tasks/xref_test.exs @@ -63,7 +63,13 @@ defmodule Mix.Tasks.XrefTest do } output = [ - %{callee: {A, :b, 1}, caller_module: B, file: "lib/b.ex", line: 3} + %{callee: {A, :b, 1}, caller_module: B, file: "lib/b.ex", line: 3}, + %{ + callee: {:elixir_quote, :shallow_validate_ast, 1}, + caller_module: A, + file: "lib/a.ex", + line: 4 + } ] assert_all_calls(files, output)