Deprecate complex module expressions in &mod.fun/arity captures (#14095)

Author: sabiwara

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,31 +60,39 @@ 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});
 
 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
+      case ELeft of
+        _ 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
         {Name, _, Context} when is_atom(Name), is_atom(Context) ->
           {remote, ELeft, Right, length(Args)};
         _ when is_atom(ELeft) ->
@@ -94,24 +102,25 @@ 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);
+    %% 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}]}]},
@@ -166,12 +175,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
@@ -186,6 +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}) ->
+  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) ->

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"

lib/elixir/test/elixir/kernel/warning_test.exs

@@ -2194,6 +2194,25 @@ defmodule Kernel.WarningTest do
     )
   end
 
+  test "deprecate &module.fun/arity captures with complex expressions as modules" do
+    assert_warn_eval(
+      [
+        "nofile:2:",
+        """
+        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.
+        """
+      ],
+      """
+      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 ->