Deprecate function/1 and function/3

José Valim · José Valim · commit 6a68ca5eeff5 · 2013-07-31T12:58:07.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -36,6 +36,8 @@
   * [OptionParser] Ensure `--no-flag` sets `:flag` to false when `:flag` is a registered boolean switch
 
 * deprecations
+  * [Kernel] `function(Mod.fun/arity)` and `function(fun/arity)` are deprecated in favor of `&Mod.fun/arity` and `&fun/arity`
+  * [Kernel] `function/3` is deprecated in favor of `Module.function/3`
   * [Kernel] `Kernel.ParallelCompiler` now receives a set of callbacks instead of a single one
   * [Mix] `:test_coverage` option now expect keywords arguments and the `--cover` flag is now treated as a boolean
 
diff --git a/lib/eex/lib/eex/tokenizer.ex b/lib/eex/lib/eex/tokenizer.ex
@@ -128,11 +128,11 @@ defmodule EEx.Tokenizer do
   end
 
   defp fn_index(tokens) do
-    Enum.find_index(tokens, function do
+    Enum.find_index tokens, fn
       { :fn_paren, _ } -> true
       { :fn, _ }       -> true
       _                -> false
-    end)
+    end
   end
 
   defp end_index(tokens) do
diff --git a/lib/elixir/lib/kernel.ex b/lib/elixir/lib/kernel.ex
@@ -2296,101 +2296,6 @@ defmodule Kernel do
     { other, acc }
   end
 
-  @doc """
-  Construct an anonymous function based on the given expression
-  or retrieve an existing one.
-
-  ## Function composition
-
-      iex> sum = function do
-      ...>   (x, y) -> x + y
-      ...> end
-      ...> sum.(1, 2)
-      3
-
-  Notice that a function needs to be invoked using the dot between
-  the function and the arguments.
-
-  Multiple clauses can be specified as in `case`, `receive` and
-  similar macros:
-
-      iex> sum = function do
-      ...>   x, y when y > 0 -> x + y
-      ...>   x, y -> x - y
-      ...> end
-      ...> sum.(1, 2)
-      3
-
-  All clauses must expect the same number of arguments.
-
-  ## Function retrieval
-
-  The `function` macro can also be used to retrieve local, imported
-  and remote functions.
-
-      # Retrieve local/import
-      iex> f = function(is_atom/1)
-      iex> f.(:foo)
-      true
-
-      # Retrieve remote
-      iex> f = function(Kernel.is_atom/1)
-      iex> f.(:foo)
-      true
-
-  In case a function needs to be dynamically retrieved based on its
-  module, name or arity, use `function/3` instead.
-
-  ## Shortcut syntax
-
-  In order to reduce verbosity, functions in Elixir can be written
-  using a shortcut syntax via `fn`:
-
-      Enum.map [1, 2, 3], fn x ->
-        x * 2
-      end
-
-  Not only is the example shorter, it solves ambiguity issues. Since
-  `do/end` always matches the furthest call, if we used the `function`
-  macro as below:
-
-      Enum.map [1, 2, 3], function do
-        x -> x * 2
-      end
-
-  It would be parsed as:
-
-      Enum.map([1, 2, 3], function) do
-        x -> x * 2
-      end
-
-  The stab shortcut syntax has the proper precedence:
-
-      Enum.map [1, 2, 3], fn
-        x -> x * 2
-      end
-
-  Which is handled as:
-
-      Enum.map([1, 2, 3], fn
-        x -> x * 2
-      end)
-
-  """
-  defmacro function(args)
-
-  @doc """
-  Retrieves a function with given name and arity from a module.
-
-  ## Examples
-
-      iex> f = function(Kernel, :is_atom, 1)
-      ...> f.(:foo)
-      true
-
-  """
-  defmacro function(module, function, arity)
-
   @doc """
   Matches the given expression against the match clauses.
 
diff --git a/lib/elixir/lib/module.ex b/lib/elixir/lib/module.ex
@@ -463,6 +463,19 @@ defmodule Module do
     :elixir_aliases.safe_concat([left, right])
   end
 
+  @doc """
+  Gets an anonymous function from the given module, function
+  and arity. The module and function are not verified to exist.
+
+      iex> fun = Module.function(Kernel, :is_atom, 1)
+      iex> fun.(:hello)
+      true
+
+  """
+  def function(mod, fun, arity) do
+    :erlang.make_fun(mod, fun, arity)
+  end
+
   @doc """
   Attaches documentation to a given function. It expects
   the module the function belongs to, the line (a non negative
diff --git a/lib/elixir/lib/stream.ex b/lib/elixir/lib/stream.ex
@@ -293,7 +293,7 @@ defmodule Stream do
 
   ## Examples
 
-      iex> Stream.repeatedly(function(:random.uniform/0)) |> Enum.take(3)
+      iex> Stream.repeatedly(&:random.uniform/0) |> Enum.take(3)
       [0.4435846174457203, 0.7230402056221108, 0.94581636451987]
 
   """
@@ -307,7 +307,6 @@ defmodule Stream do
     do_repeatedly(generator_fun, fun.(generator_fun.(), acc), fun)
   end
 
-
   @doc """
   Lazily takes the next `n` items from the enumerable and stops
   enumeration.
diff --git a/lib/elixir/src/elixir_fn.erl b/lib/elixir/src/elixir_fn.erl
@@ -1,8 +1,26 @@
 -module(elixir_fn).
 -export([fn/3, capture/3]).
 -import(elixir_scope, [umergec/2]).
+-import(elixir_errors, [syntax_error/3, syntax_error/4, compile_error/4]).
 -include("elixir.hrl").
 
+fn(Meta, Clauses, S) ->
+  Transformer = fun({ ArgsWithGuards, CMeta, Expr }, Acc) ->
+    { Args, Guards } = elixir_clauses:extract_splat_guards(ArgsWithGuards),
+    elixir_clauses:assigns_block(?line(CMeta), fun elixir_translator:translate/2, Args, [Expr], Guards, umergec(S, Acc))
+  end,
+
+  { TClauses, NS } = lists:mapfoldl(Transformer, S, Clauses),
+  Arities = [length(Args) || { clause, _Line, Args, _Guards, _Exprs } <- TClauses],
+
+  case length(lists:usort(Arities)) of
+    1 ->
+      { { 'fun', ?line(Meta), { clauses, TClauses } }, umergec(S, NS) };
+    _ ->
+      syntax_error(Meta, S#elixir_scope.file,
+                   "cannot mix clauses with different arities in function definition")
+  end.
+
 capture(Meta, { '/', _, [{ { '.', _, [M, F] }, _ , [] }, A] }, S) when is_atom(F), is_integer(A) ->
   { [MF, FF, AF], SF } = elixir_translator:translate_args([M, F, A], S),
   { { 'fun', ?line(Meta), { function, MF, FF, AF } }, SF };
@@ -19,24 +37,11 @@ capture(Meta, { '/', _, [{ F, _, C }, A] }, S) when is_atom(F), is_integer(A), i
     end,
 
   case elixir_dispatch:import_function(WrappedMeta, F, A, S) of
-    false -> elixir_errors:compile_error(WrappedMeta, S#elixir_scope.file,
-                                         "expected ~ts/~B to be a function, but it is a macro", [F, A]);
+    false -> compile_error(WrappedMeta, S#elixir_scope.file,
+                           "expected ~ts/~B to be a function, but it is a macro", [F, A]);
     Else  -> Else
-  end.
-
-fn(Meta, Clauses, S) ->
-  Transformer = fun({ ArgsWithGuards, CMeta, Expr }, Acc) ->
-    { Args, Guards } = elixir_clauses:extract_splat_guards(ArgsWithGuards),
-    elixir_clauses:assigns_block(?line(CMeta), fun elixir_translator:translate/2, Args, [Expr], Guards, umergec(S, Acc))
-  end,
+  end;
 
-  { TClauses, NS } = lists:mapfoldl(Transformer, S, Clauses),
-  Arities = [length(Args) || { clause, _Line, Args, _Guards, _Exprs } <- TClauses],
-
-  case length(lists:usort(Arities)) of
-    1 ->
-      { { 'fun', ?line(Meta), { clauses, TClauses } }, umergec(S, NS) };
-    _ ->
-      elixir_errors:syntax_error(Meta, S#elixir_scope.file,
-                                 "cannot mix clauses with different arities in function definition")
-  end.
+capture(Meta, Arg, S) ->
+  syntax_error(Meta, S#elixir_scope.file,
+               "invalid args for &: ~ts", ['Elixir.Macro':to_string(Arg)]).
diff --git a/lib/elixir/src/elixir_macros.erl b/lib/elixir/src/elixir_macros.erl
@@ -49,17 +49,22 @@ translate({ in, Meta, [Left, Right] }, #elixir_scope{extra_guards=Extra} = S) ->
   { TVar, TS#elixir_scope{extra_guards=[TExpr|Extra]} };
 
 %% Functions
-%% Once this function is removed, the related checks
-%% from quote needs to be removed too.
+%% Once this function is removed, the related checks from quote needs to be removed too.
+%% We also need to remove it from the Kernel in erlang list.
 
 translate({ function, Meta, [[{do,{ '->',_,Pairs}}]] }, S) ->
+  elixir_errors:deprecation(Meta, S#elixir_scope.file, "function do ... end is deprecated, please use fn ... end instead"),
   assert_no_match_or_guard_scope(Meta, 'function', S),
   elixir_fn:fn(Meta, Pairs, S);
 
 translate({ function, _, [{ '/', _, [{{ '.', Meta, [M, F] }, _ , []}, A]}] }, S) when is_atom(F), is_integer(A) ->
-  translate({ function, Meta, [M, F, A] }, S);
+  elixir_errors:deprecation(Meta, S#elixir_scope.file, "function(Mod.fun/a) is deprecated, please use &Mod.fun/a instead"),
+  assert_no_match_or_guard_scope(Meta, 'function', S),
+  { [A0,B0,C0], SA } = translate_args([M, F, A], S),
+  { { 'fun', ?line(Meta), { function, A0, B0, C0 } }, SA };
 
 translate({ function, MetaFA, [{ '/', _, [{F, Meta, C}, A]}] }, S) when is_atom(F), is_integer(A), is_atom(C) ->
+  elixir_errors:deprecation(Meta, S#elixir_scope.file, "function(fun/a) is deprecated, please use &fun/a instead"),
   assert_no_match_or_guard_scope(Meta, 'function', S),
 
   WrappedMeta =
@@ -84,6 +89,7 @@ translate({ function, Meta, [Arg] }, S) ->
                ['Elixir.Macro':to_string(Arg)]);
 
 translate({ function, Meta, [_,_,_] = Args }, S) when is_list(Args) ->
+  elixir_errors:deprecation(Meta, S#elixir_scope.file, "function/3 is deprecated, please use Module.function/3 instead"),
   assert_no_match_or_guard_scope(Meta, 'function', S),
   { [A,B,C], SA } = translate_args(Args, S),
   { { 'fun', ?line(Meta), { function, A, B, C } }, SA };
diff --git a/lib/elixir/src/elixir_partials.erl b/lib/elixir/src/elixir_partials.erl
@@ -41,7 +41,7 @@ convert([{'|', Meta, [_, _] = Args}|T], S, allow_tail, CallAcc, DefAcc) ->
   { NewArgs, NewDef, NewS } = convert(Args, S, allow_tail, [], DefAcc),
   convert(T, NewS, allow_tail, [{ '|', Meta, NewArgs}|CallAcc], NewDef);
 
-convert([{'&', Meta, [Pos]}|T], S, Opt, CallAcc, DefAcc) ->
+convert([{'&', Meta, [Pos]}|T], S, Opt, CallAcc, DefAcc) when is_integer(Pos) ->
   case lists:keyfind(Pos, 1, DefAcc) of
     false ->
       { Var, SC } = elixir_scope:build_ex_var(?line(Meta), S),
diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl
@@ -152,7 +152,7 @@ do_quote({ { '.', Meta, [Left, unquote] }, _, [Expr] }, #elixir_quote{unquote=tr
 
 do_quote({ '&', Meta, [{ '/', _, [{F, _, C}, A]}] = Args },
     #elixir_quote{imports_hygiene=true} = Q, S) when is_atom(F), is_integer(A), is_atom(C) ->
-  do_quote_fa(function, Meta, Args, F, A, Q, S);
+  do_quote_fa('&', Meta, Args, F, A, Q, S);
 
 do_quote({ function, Meta, [{ '/', _, [{F, _, C}, A]}] = Args },
     #elixir_quote{imports_hygiene=true} = Q, S) when is_atom(F), is_integer(A), is_atom(C) ->
diff --git a/lib/elixir/test/elixir/inspect_test.exs b/lib/elixir/test/elixir/inspect_test.exs
@@ -224,12 +224,12 @@ defmodule Inspect.OthersTest do
   end
 
   test :external_elixir_funs do
-    bin = inspect(function(Enum.map/2))
+    bin = inspect(&Enum.map/2)
     assert bin == "&Enum.map/2"
   end
 
   test :external_erlang_funs do
-    bin = inspect(function(:lists.map/2))
+    bin = inspect(&:lists.map/2)
     assert bin == "&:lists.map/2"
   end
 
diff --git a/lib/elixir/test/elixir/kernel/quote_test.exs b/lib/elixir/test/elixir/kernel/quote_test.exs
@@ -304,13 +304,7 @@ defmodule Kernel.QuoteTest.ImportsHygieneTest do
 
   defmacrop get_bin_size_with_function do
     quote do
-      function(size/1).("hello")
-    end
-  end
-
-  defmacrop get_bin_size_with_kernel_function do
-    quote do
-      Kernel.function(size/1).("hello")
+      (&size/1).("hello")
     end
   end
 
@@ -319,7 +313,6 @@ defmodule Kernel.QuoteTest.ImportsHygieneTest do
     assert get_bin_size == 5
     assert get_bin_size_with_partial == 5
     assert get_bin_size_with_function == 5
-    assert get_bin_size_with_kernel_function == 5
   end
 
   defmacrop get_dict_size do
@@ -343,7 +336,6 @@ defmodule Kernel.QuoteTest.ImportsHygieneTest do
     assert get_bin_size == 5
     assert get_bin_size_with_partial == 5
     assert get_bin_size_with_function == 5
-    assert get_bin_size_with_kernel_function == 5
   end
 
   defmacrop with_size do
diff --git a/lib/elixir/test/elixir/kernel/record_rewriter_test.exs b/lib/elixir/test/elixir/kernel/record_rewriter_test.exs
@@ -146,11 +146,6 @@ defmodule Kernel.RecordRewriterTest do
     assert optimize_clause(clause) == { clause, [], nil }
   end
 
-  test "inside function retrieval" do
-    clause = clause(fn -> function(x = Macro.Env[], y, z) end)
-    assert optimize_clause(clause) == { clause, [x: Macro.Env], nil }
-  end
-
   test "inside anonymous function" do
     clause = clause(fn -> fn (x = Macro.Env[]) -> x end end)
     assert optimize_clause(clause) == { clause, [], nil }
diff --git a/lib/elixir/test/elixir/kernel_test.exs b/lib/elixir/test/elixir/kernel_test.exs
@@ -270,47 +270,6 @@ defmodule KernelTest do
     end
   end
 
-  defmodule Function do
-    use ExUnit.Case, async: true
-
-    test :remote_syntax_function do
-      assert function(:erlang, :atom_to_list, 1).(:a) == 'a'
-      assert function(:erlang.atom_to_list/1) ==
-             function(:erlang, :atom_to_list, 1)
-      assert function(Enum.map/2) == function(Enum, :map, 2)
-    end
-
-    test :local_syntax_function do
-      assert function(atl/1).(:a) == 'a'
-    end
-
-    test :retrieve_imported_function do
-      assert function(atom_to_list/1).(:a) == 'a'
-    end
-
-    test :retrieve_dynamic_function do
-      a = :erlang
-      b = :atom_to_list
-      c = 1
-
-      assert function(a, b, c).(:a) == 'a'
-    end
-
-    test :function do
-      f = function do
-        x, y when y > 0 -> x + y
-        x, y -> x - y
-      end
-
-      assert f.(1, 2)  == 3
-      assert f.(1, -2) == 3
-    end
-
-    defp atl(arg) do
-      :erlang.atom_to_list arg
-    end
-  end
-
   defmodule DefDelegate do
     use ExUnit.Case, async: true
 
diff --git a/lib/elixir/test/elixir/module_test.exs b/lib/elixir/test/elixir/module_test.exs
@@ -227,4 +227,9 @@ defmodule ModuleTest do
       assert Module.definitions_in(__MODULE__, :defp) == []
     end
   end
+
+  test :function do
+    assert Module.function(:erlang, :atom_to_list, 1).(:hello) == 'hello'
+    assert is_function Module.function(This, :also_works, 0)
+  end
 end
diff --git a/lib/elixir/test/elixir/stream_test.exs b/lib/elixir/test/elixir/stream_test.exs
@@ -83,7 +83,7 @@ defmodule StreamTest do
   test :repeatedly do 
     stream = Stream.repeatedly(fn -> 1 end)
     assert Enum.take(stream, 5) == [1,1,1,1,1]
-    stream = Stream.repeatedly(function(:random.uniform/0))
+    stream = Stream.repeatedly(&:random.uniform/0)
     [r1,r2] = Enum.take(stream, 2)
     assert r1 != r2
   end
diff --git a/lib/ex_unit/lib/ex_unit/doc_test.ex b/lib/ex_unit/lib/ex_unit/doc_test.ex
diff --git a/lib/mix/lib/mix/tasks/do.ex b/lib/mix/lib/mix/tasks/do.ex
