From 5f5ef9c919213e1fdae6a2d1efee1e48f31851fb Mon Sep 17 00:00:00 2001 From: sabiwara Date: Sat, 21 Dec 2024 09:38:28 +0900 Subject: [PATCH 1/4] Refactor: distinguish between sequential & arity --- lib/elixir/src/elixir_fn.erl | 50 ++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/elixir/src/elixir_fn.erl b/lib/elixir/src/elixir_fn.erl index fafdbd8207a..477c1fbfd6c 100644 --- a/lib/elixir/src/elixir_fn.erl +++ b/lib/elixir/src/elixir_fn.erl @@ -41,17 +41,17 @@ fn_arity(Args) -> length(Args). capture(Meta, {'/', _, [{{'.', _, [M, F]} = Dot, RequireMeta, []}, A]}, S, E) when is_atom(F), is_integer(A) -> Args = args_from_arity(Meta, A, E), handle_capture_possible_warning(Meta, RequireMeta, M, F, A, E), - capture_require({Dot, RequireMeta, Args}, S, E, true); + capture_require({Dot, RequireMeta, Args}, S, E, arity); capture(Meta, {'/', _, [{F, ImportMeta, C}, A]}, S, E) when is_atom(F), is_integer(A), is_atom(C) -> Args = args_from_arity(Meta, A, E), - capture_import({F, ImportMeta, Args}, S, E, true); + capture_import({F, ImportMeta, Args}, S, E, arity); capture(_Meta, {{'.', _, [_, Fun]}, _, Args} = Expr, S, E) when is_atom(Fun), is_list(Args) -> - capture_require(Expr, S, E, is_sequential_and_not_empty(Args)); + capture_require(Expr, S, E, check_sequential_and_not_empty(Args)); capture(Meta, {{'.', _, [_]}, _, Args} = Expr, S, E) when is_list(Args) -> - capture_expr(Meta, Expr, S, E, false); + capture_expr(Meta, Expr, S, E, non_sequential); capture(Meta, {'__block__', _, [Expr]}, S, E) -> capture(Meta, Expr, S, E); @@ -60,13 +60,13 @@ capture(Meta, {'__block__', _, _} = Expr, _S, E) -> file_error(Meta, E, ?MODULE, {block_expr_in_capture, Expr}); capture(_Meta, {Atom, _, Args} = Expr, S, E) when is_atom(Atom), is_list(Args) -> - capture_import(Expr, S, E, is_sequential_and_not_empty(Args)); + capture_import(Expr, S, E, check_sequential_and_not_empty(Args)); capture(Meta, {Left, Right}, S, E) -> capture(Meta, {'{}', Meta, [Left, Right]}, S, E); capture(Meta, List, S, E) when is_list(List) -> - capture_expr(Meta, List, S, E, is_sequential_and_not_empty(List)); + capture_expr(Meta, List, S, E, check_sequential_and_not_empty(List)); capture(Meta, Integer, _S, E) when is_integer(Integer) -> file_error(Meta, E, ?MODULE, {capture_arg_outside_of_capture, Integer}); @@ -74,17 +74,17 @@ capture(Meta, Integer, _S, E) when is_integer(Integer) -> capture(Meta, Arg, _S, E) -> invalid_capture(Meta, Arg, E). -capture_import({Atom, ImportMeta, Args} = Expr, S, E, Sequential) -> - Res = Sequential andalso +capture_import({Atom, ImportMeta, Args} = Expr, S, E, ArgsType) -> + Res = ArgsType /= non_sequential andalso elixir_dispatch:import_function(ImportMeta, Atom, length(Args), E), - handle_capture(Res, ImportMeta, ImportMeta, Expr, S, E, Sequential). + handle_capture(Res, ImportMeta, ImportMeta, Expr, S, E, ArgsType). -capture_require({{'.', DotMeta, [Left, Right]}, RequireMeta, Args}, S, E, Sequential) -> +capture_require({{'.', DotMeta, [Left, Right]}, RequireMeta, Args}, S, E, ArgsType) -> case escape(Left, E, []) of {EscLeft, []} -> {ELeft, SE, EE} = elixir_expand:expand(EscLeft, S, E), - Res = Sequential andalso case ELeft of + Res = ArgsType /= non_sequential andalso case ELeft of {Name, _, Context} when is_atom(Name), is_atom(Context) -> {remote, ELeft, Right, length(Args)}; _ when is_atom(ELeft) -> @@ -94,23 +94,23 @@ capture_require({{'.', DotMeta, [Left, Right]}, RequireMeta, Args}, S, E, Sequen end, Dot = {{'.', DotMeta, [ELeft, Right]}, RequireMeta, Args}, - handle_capture(Res, RequireMeta, DotMeta, Dot, SE, EE, Sequential); + handle_capture(Res, RequireMeta, DotMeta, Dot, SE, EE, ArgsType); {EscLeft, Escaped} -> Dot = {{'.', DotMeta, [EscLeft, Right]}, RequireMeta, Args}, - capture_expr(RequireMeta, Dot, S, E, Escaped, Sequential) + capture_expr(RequireMeta, Dot, S, E, Escaped, ArgsType) end. -handle_capture(false, Meta, _DotMeta, Expr, S, E, Sequential) -> - capture_expr(Meta, Expr, S, E, Sequential); -handle_capture(LocalOrRemote, Meta, DotMeta, _Expr, S, E, _Sequential) -> +handle_capture(false, Meta, _DotMeta, Expr, S, E, ArgsType) -> + capture_expr(Meta, Expr, S, E, ArgsType); +handle_capture(LocalOrRemote, Meta, DotMeta, _Expr, S, E, _ArgsType) -> {LocalOrRemote, Meta, DotMeta, S, E}. -capture_expr(Meta, Expr, S, E, Sequential) -> - capture_expr(Meta, Expr, S, E, [], Sequential). -capture_expr(Meta, Expr, S, E, Escaped, Sequential) -> +capture_expr(Meta, Expr, S, E, ArgsType) -> + capture_expr(Meta, Expr, S, E, [], ArgsType). +capture_expr(Meta, Expr, S, E, Escaped, ArgsType) -> case escape(Expr, E, Escaped) of - {_, []} when not Sequential -> + {_, []} when ArgsType == non_sequential -> invalid_capture(Meta, Expr, E); {{{'.', _, [_, _]} = Dot, _, Args}, []} -> Meta2 = lists:keydelete(no_parens, 1, Meta), @@ -166,12 +166,12 @@ args_from_arity(_Meta, A, _E) when is_integer(A), A >= 0, A =< 255 -> args_from_arity(Meta, A, E) -> file_error(Meta, E, ?MODULE, {invalid_arity_for_capture, A}). -is_sequential_and_not_empty([]) -> false; -is_sequential_and_not_empty(List) -> is_sequential(List, 1). +check_sequential_and_not_empty([]) -> non_sequential; +check_sequential_and_not_empty(List) -> check_sequential(List, 1). -is_sequential([{'&', _, [Int]} | T], Int) -> is_sequential(T, Int + 1); -is_sequential([], _Int) -> true; -is_sequential(_, _Int) -> false. +check_sequential([{'&', _, [Int]} | T], Int) -> check_sequential(T, Int + 1); +check_sequential([], _Int) -> sequential; +check_sequential(_, _Int) -> non_sequential. handle_capture_possible_warning(Meta, DotMeta, Mod, Fun, Arity, E) -> case (Arity =:= 0) andalso (lists:keyfind(no_parens, 1, DotMeta) /= {no_parens, true}) of From d4c626c1fa202f82a6feb71fb502821cb1e347ed Mon Sep 17 00:00:00 2001 From: sabiwara Date: Sat, 21 Dec 2024 10:03:09 +0900 Subject: [PATCH 2/4] Deprecate complex module expressions in &mod.fun/arity captures --- lib/elixir/src/elixir_fn.erl | 20 ++++++++++++++++ .../test/elixir/kernel/warning_test.exs | 23 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/lib/elixir/src/elixir_fn.erl b/lib/elixir/src/elixir_fn.erl index 477c1fbfd6c..96ca588cf79 100644 --- a/lib/elixir/src/elixir_fn.erl +++ b/lib/elixir/src/elixir_fn.erl @@ -84,6 +84,18 @@ capture_require({{'.', DotMeta, [Left, Right]}, RequireMeta, Args}, S, E, ArgsTy {EscLeft, []} -> {ELeft, SE, EE} = elixir_expand:expand(EscLeft, S, E), + case ELeft of + _ when ArgsType /= arity -> + ok; + Atom when is_atom(Atom) -> + ok; + {Var, _, Ctx} when is_atom(Var), is_atom(Ctx) -> + ok; + _ -> + elixir_errors:file_warn(RequireMeta, E, ?MODULE, + {complex_module_capture, Left, Right, length(Args)}) + end, + Res = ArgsType /= non_sequential andalso case ELeft of {Name, _, Context} when is_atom(Name), is_atom(Context) -> {remote, ELeft, Right, length(Args)}; @@ -112,6 +124,7 @@ capture_expr(Meta, Expr, S, E, Escaped, ArgsType) -> case escape(Expr, E, Escaped) of {_, []} when ArgsType == non_sequential -> invalid_capture(Meta, Expr, E); + % TODO remove this clause once we raise on complex module captures like &get_mod().fun/0 {{{'.', _, [_, _]} = Dot, _, Args}, []} -> Meta2 = lists:keydelete(no_parens, 1, Meta), Fn = {fn, Meta2, [{'->', Meta2, [[], {Dot, Meta2, Args}]}]}, @@ -186,6 +199,13 @@ format_error({parens_remote_capture, Mod, Fun}) -> io_lib:format("extra parentheses on a remote function capture &~ts.~ts()/0 have been " "deprecated. Please remove the parentheses: &~ts.~ts/0", ['Elixir.Macro':to_string(Mod), Fun, 'Elixir.Macro':to_string(Mod), Fun]); +format_error({complex_module_capture, Mod, Fun, Arity}) -> + io_lib:format("using complex expressions for modules in &module.function/arity capture syntax has been deprecated:\n" + " &~ts.~ts/~B\n\n" + "You can either:\n" + " * use the fn syntax\n" + " * assign the module to a variable if it can be evaluated outside of the capture", + ['Elixir.Macro':to_string(Mod), Fun, Arity]); format_error(clauses_with_different_arities) -> "cannot mix clauses with different arities in anonymous functions"; format_error(defaults_in_args) -> diff --git a/lib/elixir/test/elixir/kernel/warning_test.exs b/lib/elixir/test/elixir/kernel/warning_test.exs index 7c53acf98ae..516665ebfbb 100644 --- a/lib/elixir/test/elixir/kernel/warning_test.exs +++ b/lib/elixir/test/elixir/kernel/warning_test.exs @@ -2194,6 +2194,29 @@ defmodule Kernel.WarningTest do ) end + test "deprecate &module.fun/arity captures with complex expressions as modules" do + assert_warn_eval( + [ + "nofile:2:", + """ + using complex expressions for modules in &module.function/arity capture syntax has been deprecated: + &hd(modules).module_info/0 + + You can either: + * use the fn syntax + * assign the module to a variable if it can be evaluated outside of the capture + """ + ], + """ + defmodule Sample do + def foo(modules), do: &hd(modules).module_info/0 + end + """ + ) + after + purge(Sample) + end + defp assert_compile_error(messages, string) do captured = capture_err(fn -> From 8617fe693f91186bf7f6b994a715ca861d855917 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Sat, 21 Dec 2024 21:14:32 +0900 Subject: [PATCH 3/4] Apply feedback from review --- lib/elixir/src/elixir_fn.erl | 27 +++++++------------ .../test/elixir/kernel/warning_test.exs | 8 ++---- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/lib/elixir/src/elixir_fn.erl b/lib/elixir/src/elixir_fn.erl index 96ca588cf79..60cca99d848 100644 --- a/lib/elixir/src/elixir_fn.erl +++ b/lib/elixir/src/elixir_fn.erl @@ -85,15 +85,11 @@ capture_require({{'.', DotMeta, [Left, Right]}, RequireMeta, Args}, S, E, ArgsTy {ELeft, SE, EE} = elixir_expand:expand(EscLeft, S, E), case ELeft of - _ when ArgsType /= arity -> - ok; - Atom when is_atom(Atom) -> - ok; - {Var, _, Ctx} when is_atom(Var), is_atom(Ctx) -> - ok; - _ -> - elixir_errors:file_warn(RequireMeta, E, ?MODULE, - {complex_module_capture, Left, Right, length(Args)}) + _ when ArgsType /= arity -> ok; + Atom when is_atom(Atom) -> ok; + {Var, _, Ctx} when is_atom(Var), is_atom(Ctx) -> ok; + %% TODO: Raise on Elixir v2.0 + _ -> elixir_errors:file_warn(RequireMeta, E, ?MODULE, {complex_module_capture, Left}) end, Res = ArgsType /= non_sequential andalso case ELeft of @@ -124,7 +120,7 @@ capture_expr(Meta, Expr, S, E, Escaped, ArgsType) -> case escape(Expr, E, Escaped) of {_, []} when ArgsType == non_sequential -> invalid_capture(Meta, Expr, E); - % TODO remove this clause once we raise on complex module captures like &get_mod().fun/0 + %% TODO: Remove this clause once we raise on complex module captures like &get_mod().fun/0 {{{'.', _, [_, _]} = Dot, _, Args}, []} -> Meta2 = lists:keydelete(no_parens, 1, Meta), Fn = {fn, Meta2, [{'->', Meta2, [[], {Dot, Meta2, Args}]}]}, @@ -199,13 +195,10 @@ format_error({parens_remote_capture, Mod, Fun}) -> io_lib:format("extra parentheses on a remote function capture &~ts.~ts()/0 have been " "deprecated. Please remove the parentheses: &~ts.~ts/0", ['Elixir.Macro':to_string(Mod), Fun, 'Elixir.Macro':to_string(Mod), Fun]); -format_error({complex_module_capture, Mod, Fun, Arity}) -> - io_lib:format("using complex expressions for modules in &module.function/arity capture syntax has been deprecated:\n" - " &~ts.~ts/~B\n\n" - "You can either:\n" - " * use the fn syntax\n" - " * assign the module to a variable if it can be evaluated outside of the capture", - ['Elixir.Macro':to_string(Mod), Fun, Arity]); +format_error({complex_module_capture, Mod}) -> + io_lib:format("expected the module in &module.fun/arity to expand to a variable or an atom, got: ~ts\n" + "You can either compute the module name outside of & or convert it to a regular anonymous function.", + ['Elixir.Macro':to_string(Mod)]); format_error(clauses_with_different_arities) -> "cannot mix clauses with different arities in anonymous functions"; format_error(defaults_in_args) -> diff --git a/lib/elixir/test/elixir/kernel/warning_test.exs b/lib/elixir/test/elixir/kernel/warning_test.exs index 516665ebfbb..9fcae4ae3aa 100644 --- a/lib/elixir/test/elixir/kernel/warning_test.exs +++ b/lib/elixir/test/elixir/kernel/warning_test.exs @@ -2199,12 +2199,8 @@ defmodule Kernel.WarningTest do [ "nofile:2:", """ - using complex expressions for modules in &module.function/arity capture syntax has been deprecated: - &hd(modules).module_info/0 - - You can either: - * use the fn syntax - * assign the module to a variable if it can be evaluated outside of the capture + expected the module in &module.fun/arity to expand to a variable or an atom, got: hd(modules) + You can either compute the module name outside of & or convert it to a regular anonymous function. """ ], """ From 34484a940ed7d0d8951dcc76c3eba367f079ee91 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Sat, 21 Dec 2024 21:15:32 +0900 Subject: [PATCH 4/4] Remove test for deprecated syntax --- lib/elixir/test/elixir/kernel/fn_test.exs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/elixir/test/elixir/kernel/fn_test.exs b/lib/elixir/test/elixir/kernel/fn_test.exs index 0d0e8471994..1304d753577 100644 --- a/lib/elixir/test/elixir/kernel/fn_test.exs +++ b/lib/elixir/test/elixir/kernel/fn_test.exs @@ -95,12 +95,6 @@ defmodule Kernel.FnTest do assert (&mod.flatten/1) == (&List.flatten/1) end - test "capture with module from local call" do - assert (&math_mod().pi/0).() == :math.pi() - end - - defp math_mod, do: :math - test "local partial application" do assert (&atb(&1, :utf8)).(:a) == "a" assert (&atb(List.to_atom(&1), :utf8)).(~c"a") == "a"