Merge pull request #937 from alexrp/master

Only error on ambiguous imported function calls, not on the imports themselves.

Author: José Valim

lib/elixir/lib/macro.ex

@@ -563,7 +563,7 @@ defmodule Macro do
         end
 
         expand = :elixir_dispatch.expand_import(line, { atom, length(args) }, args,
-          env.module, env.function, env.requires, extra ++ env.macros, env)
+          env.module, env.function, env.requires, extra ++ env.macros, env.functions, env)
         case expand do
           { :ok, _, expanded } -> expanded
           { :error, _ }     -> original

lib/elixir/src/elixir_dispatch.erl

@@ -5,7 +5,7 @@
 -export([default_macros/0, default_functions/0, default_requires/0,
   dispatch_require/6, dispatch_import/5,
   require_function/5, import_function/4,
-  expand_import/8, expand_require/8, find_import/4,
+  expand_import/9, expand_require/8, find_import/4,
   format_error/1, in_erlang_functions/0, in_erlang_macros/0]).
 -include("elixir.hrl").
 -compile({parse_transform, elixir_transform}).
@@ -20,11 +20,11 @@ default_macros() ->
 default_requires() ->
   [ ?BUILTIN, 'Elixir.Kernel.Typespec' ].
 
-find_import(_Meta, Name, Arity, S) ->
+find_import(Meta, Name, Arity, S) ->
   Tuple = { Name, Arity },
-  case find_dispatch(Tuple, S#elixir_scope.functions) of
+  case find_dispatch(Meta, S#elixir_scope.file, Tuple, S#elixir_scope.functions, S#elixir_scope.macros) of
     false ->
-      case find_dispatch(Tuple, S#elixir_scope.macros) of
+      case find_dispatch(Meta, S#elixir_scope.file, Tuple, S#elixir_scope.macros, S#elixir_scope.functions) of
         false -> false;
         Receiver -> Receiver
       end;
@@ -35,9 +35,9 @@ find_import(_Meta, Name, Arity, S) ->
 
 import_function(Meta, Name, Arity, S) ->
   Tuple = { Name, Arity },
-  case find_dispatch(Tuple, S#elixir_scope.functions) of
+  case find_dispatch(Meta, S#elixir_scope.file, Tuple, S#elixir_scope.functions, S#elixir_scope.macros) of
     false ->
-      case find_dispatch(Tuple, S#elixir_scope.macros) of
+      case find_dispatch(Meta, S#elixir_scope.file, Tuple, S#elixir_scope.macros, S#elixir_scope.functions) of
         false -> { { 'fun', ?line(Meta), { function, Name, Arity } }, S };
         _ -> false
       end;
@@ -61,10 +61,10 @@ dispatch_import(Meta, Name, Args, S, Callback) ->
   Arity  = length(Args),
   Tuple  = { Name, Arity },
 
-  case find_dispatch(Tuple, S#elixir_scope.functions) of
+  case find_dispatch(Meta, S#elixir_scope.file, Tuple, S#elixir_scope.functions, S#elixir_scope.macros) of
     false ->
       case expand_import(Meta, Tuple, Args, Module, S#elixir_scope.function,
-          S#elixir_scope.requires, S#elixir_scope.macros, S) of
+          S#elixir_scope.requires, S#elixir_scope.macros, S#elixir_scope.functions, S) of
         { error, noexpansion } ->
           Callback();
         { error, internal } ->
@@ -104,8 +104,8 @@ dispatch_require(Meta, Receiver, Name, Args, S, Callback) ->
 
 %% Macros expansion
 
-expand_import(Meta, { Name, Arity } = Tuple, Args, Module, Function, Requires, Macros, SEnv) ->
-  case find_dispatch(Tuple, Macros) of
+expand_import(Meta, { Name, Arity } = Tuple, Args, Module, Function, Requires, Macros, Functions, SEnv) ->
+  case find_dispatch(Meta, elixir_scope:filename(SEnv), Tuple, Macros, Functions) of
     false ->
       Fun = (Function /= Tuple) andalso
         elixir_def_local:macro_for(Tuple, true, Module),
@@ -199,13 +199,25 @@ merge_aliases(A1, A2) ->
 skip_requires(#elixir_scope{check_requires=false}) -> true;
 skip_requires(_) -> false.
 
-find_dispatch(Tuple, [{ Name, Values }|T]) ->
-  case is_element(Tuple, Values) of
-    true  -> Name;
-    false -> find_dispatch(Tuple, T)
-  end;
+find_dispatch(Meta, File, {Name, Arity} = Tuple, List1, List2) ->
+  Matcher = fun(List) -> lists:filter(fun({_, Vals}) -> is_element(Tuple, Vals) end, List) end,
+  Matches1 = Matcher(List1),
+
+  case length(Matches1) of
+    0 -> false;
+    1 ->
+      Matches2 = Matcher(List2),
 
-find_dispatch(_Tuple, []) -> false.
+      case length(Matches2) of
+        0 -> element(1, hd(Matches1));
+        _ ->
+          Err = {ambiguous_call, {element(1, hd(Matches1)), element(1, hd(Matches2)), Name, Arity}},
+          elixir_errors:form_error(Meta, File, ?MODULE, Err)
+      end;
+    _ ->
+      Err = {ambiguous_call, {element(1, hd(Matches1)), element(1, hd(tl(Matches1))), Name, Arity}},
+      elixir_errors:form_error(Meta, File, ?MODULE, Err)
+  end.
 
 munge_stacktrace(Info, [{ _, _, [S|_], _ }|_], S) ->
   [Info];
@@ -224,7 +236,11 @@ munge_stacktrace(_, [], _) ->
 format_error({ unrequired_module,{Receiver, Name, Arity, Required }}) ->
   String = string:join([elixir_errors:inspect(R) || R <- Required], ", "),
   io_lib:format("tried to invoke macro ~ts.~ts/~B but module was not required. Required: ~ts",
-    [elixir_errors:inspect(Receiver), Name, Arity, String]).
+    [elixir_errors:inspect(Receiver), Name, Arity, String]);
+
+format_error({ ambiguous_call,{Mod1, Mod2, Name, Arity }}) ->
+  io_lib:format("function ~ts/~B imported from both ~ts and ~ts; call is ambiguous",
+    [Name, Arity, elixir_errors:inspect(Mod1), elixir_errors:inspect(Mod2)]).
 
 %% INTROSPECTION
 
@@ -398,4 +414,4 @@ in_erlang_macros() ->
     {'receive',1},
     {'try',1},
     {'xor',2}
-  ].
+  ].

lib/elixir/src/elixir_import.erl

@@ -100,8 +100,6 @@ calculate(Meta, Key, Opts, Old, AvailableFun, S) ->
   case Final of
     [] -> All;
     _  ->
-      ensure_no_conflicts(Meta, File, Final, keydelete(Key, S#elixir_scope.macros)),
-      ensure_no_conflicts(Meta, File, Final, keydelete(Key, S#elixir_scope.functions)),
       ensure_no_in_erlang_macro_conflict(Meta, File, Key, Final, internal_conflict),
       [{ Key, Final }|All]
   end.
@@ -207,22 +205,6 @@ ensure_no_in_erlang_macro_conflict(Meta, File, Key, [{Name,Arity}|T], Reason) ->
 
 ensure_no_in_erlang_macro_conflict(_Meta, _File, _Key, [], _) -> ok.
 
-%% Find conlicts in the given list of functions with the set of imports.
-%% Used internally to ensure a newly imported fun or macro does not
-%% conflict with an already imported set.
-
-ensure_no_conflicts(Meta, File, Functions, [{Key,Value}|T]) ->
-  Filtered = lists:filter(fun(X) -> lists:member(X, Functions) end, Value),
-  case Filtered of
-    [{Name,Arity}|_] ->
-      Tuple = { already_imported, { Key, Name, Arity } },
-      elixir_errors:form_error(Meta, File, ?MODULE, Tuple);
-    [] ->
-      ensure_no_conflicts(Meta, File, Functions, T)
-  end;
-
-ensure_no_conflicts(_Meta, _File, _Functions, _S) -> ok.
-
 %% ERROR HANDLING
 
 format_error({already_imported,{Receiver, Name, Arity}}) ->

lib/elixir/test/elixir/kernel/errors_test.exs

@@ -108,9 +108,9 @@ defmodule Kernel.ErrorsTest do
       format_rescue 'defmodule Foo do def Bar do :baz end\nend\n'
   end
 
-  test :erlang_function_conflict do
-    assert "nofile:1: function exit/1 already imported from Kernel" ==
-      format_rescue 'defmodule Foo do import Kernel.ErrorsTest.UnproperMacro, only: [exit: 1]\nend'
+  test :function_import_conflict do
+    assert "nofile:2: function exit/1 imported from both erlang and Kernel; call is ambiguous" ==
+      format_rescue 'defmodule Foo do import :erlang\n def foo, do: exit(:test)\nend'
   end
 
   test :import_invalid_macro do
@@ -308,4 +308,4 @@ defmodule Kernel.ErrorsTest do
 
     result || raise(ExUnit.AssertionError, message: "Expected function given to format_rescue to fail")
   end
-end
+end

lib/elixir/test/elixir/kernel/import_test.exs

@@ -109,4 +109,16 @@ defmodule Kernel.ImportMacrosTest do
     assert bor(0, 1) == 1
     assert bnot(0) == -1
   end
-end
+end
+
+defmodule Kernel.AmbiguousImportTest do
+  use ExUnit.Case, async: true
+
+  test :import_ambiguous do
+    # Simply make sure that we can indeed import functions with
+    # the same name and arity from different modules without the
+    # import itself causing any errors.
+    import List
+    import String
+  end
+end