Only raise on local macro conflicts if it was previously defined

Author: José Valim

lib/elixir/src/elixir_def_local.erl

@@ -5,11 +5,11 @@
 
 macro_for(_Tuple, _All, #elixir_scope{module=nil}) -> false;
 
-macro_for(Tuple, All, #elixir_scope{module=Module,function=Function}) ->
+macro_for(Tuple, All, #elixir_scope{module=Module}) ->
   try elixir_def:lookup_definition(Module, Tuple) of
-    { { Tuple, Kind, Line, _, _, _, _ }, Clauses } when Kind == defmacro; All, Kind == defmacrop ->
-      elixir_tracker:record_local(Tuple, Module, Function),
-      get_function(Line, Module, Clauses);
+    { { Tuple, Kind, Line, _, _, _, _ }, Clauses } when Kind == defmacro, Clauses /= [];
+                                                        All, Kind == defmacrop, Clauses /= [] ->
+      fun() -> get_function(Line, Module, Clauses) end;
     _ ->
       false
   catch
@@ -19,7 +19,7 @@ macro_for(Tuple, All, #elixir_scope{module=Module,function=Function}) ->
 function_for(Module, Name, Arity) ->
   Tuple = { Name, Arity },
   case elixir_def:lookup_definition(Module, Tuple) of
-    { { Tuple, _, Line, _, _, _, _ }, Clauses } ->
+    { { Tuple, _, Line, _, _, _, _ }, Clauses } when Clauses /= [] ->
       %% There is no need to record such calls
       %% since they come from funtions that were
       %% already analyzed at compilation time.

lib/elixir/src/elixir_dispatch.erl

@@ -45,7 +45,7 @@ import_function(Meta, Name, Arity, S) ->
       false;
     { import, Receiver } ->
       require_function(Meta, Receiver, Name, Arity, S);
-    nomatch ->
+    false ->
       case elixir_import:special_form(Name, Arity) of
         true  -> false;
         false ->
@@ -123,6 +123,28 @@ expand_import(Meta, Tuple, Args, Module, Extra, S) ->
   do_expand_import(Meta, Tuple, Args, Module, S, Result).
 
 do_expand_import(Meta, { Name, Arity } = Tuple, Args, Module, S, Result) ->
+  Function = S#elixir_scope.function,
+
+  Fun = (Function /= nil) andalso (Function /= Tuple) andalso
+        elixir_def_local:macro_for(Tuple, true, S),
+
+  case Fun of
+    false ->
+      do_expand_import_no_local(Meta, Tuple, Args, Module, S, Result);
+    _ ->
+      case Result of
+        { Kind, Receiver } when Kind == import; Receiver == Module ->
+          do_expand_import_no_local(Meta, Tuple, Args, Module, S, Result);
+        { _, Receiver } ->
+          Error = { macro_conflict, { Receiver, Name, Arity } },
+          elixir_errors:form_error(Meta, S#elixir_scope.file, ?MODULE, Error);
+        false ->
+          elixir_tracker:record_local(Tuple, Module, S#elixir_scope.function),
+          { ok, Module, expand_macro_fun(Meta, Fun(), Module, Name, Args, Module, S) }
+      end
+  end.
+
+do_expand_import_no_local(Meta, { Name, Arity } = Tuple, Args, Module, S, Result) ->
   case Result of
     { macro, ?builtin } ->
       case is_element(Tuple, in_erlang_macros()) of
@@ -137,13 +159,7 @@ do_expand_import(Meta, { Name, Arity } = Tuple, Args, Module, S, Result) ->
     { import, Receiver } ->
       expand_require(Meta, Receiver, Tuple, Args, Module, S);
     _ ->
-      Fun = (S#elixir_scope.function /= Tuple) andalso
-        elixir_def_local:macro_for(Tuple, true, S),
-      case Fun of
-        false -> { error, noexpansion };
-        _ ->
-          { ok, Module, expand_macro_fun(Meta, Fun, Module, Name, Args, Module, S) }
-      end
+      { error, noexpansion }
   end.
 
 expand_require(Meta, ?builtin, { Name, Arity } = Tuple, Args, Module, S) ->
@@ -160,19 +176,22 @@ expand_require(Meta, ?builtin, { Name, Arity } = Tuple, Args, Module, S) ->
   end;
 
 expand_require(Meta, Receiver, { Name, Arity } = Tuple, Args, Module, S) ->
-  Fun = (Module == Receiver) andalso (S#elixir_scope.function /= Tuple) andalso
-    elixir_def_local:macro_for(Tuple, false, S),
+  Function = S#elixir_scope.function,
+
+  Fun = (Module == Receiver) andalso (Function /= nil) andalso
+        (Function /= Tuple) andalso elixir_def_local:macro_for(Tuple, false, S),
 
   case Fun of
     false ->
       case is_element(Tuple, get_optional_macros(Receiver)) of
         true  ->
-          elixir_tracker:record_remote(Tuple, Receiver, S#elixir_scope.module, S#elixir_scope.function),
+          elixir_tracker:record_remote(Tuple, Receiver, Module, Function),
           { ok, Receiver, expand_macro_named(Meta, Receiver, Name, Arity, Args, Module, S) };
         false -> { error, noexpansion }
       end;
     _ ->
-      { ok, Receiver, expand_macro_fun(Meta, Fun, Receiver, Name, Args, Module, S) }
+      elixir_tracker:record_local(Tuple, Module, Function),
+      { ok, Receiver, expand_macro_fun(Meta, Fun(), Receiver, Name, Args, Module, S) }
   end.
 
 %% Expansion helpers
@@ -250,7 +269,7 @@ find_dispatch(Meta, Tuple, Extra, S) ->
       case { FunMatch, MacMatch } of
         { [], [Receiver] } -> { macro, Receiver };
         { [Receiver], [] } -> { function, Receiver };
-        { [], [] } -> nomatch;
+        { [], [] } -> false;
         _ ->
           { Name, Arity } = Tuple,
           [First, Second|_] = FunMatch ++ MacMatch,
@@ -322,6 +341,10 @@ format_error({ unrequired_module, { Receiver, Name, Arity, 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]);
 
+format_error({ macro_conflict, { Receiver, Name, Arity } }) ->
+  io_lib:format("call to local macro ~ts/~B conflicts with imported ~ts.~ts/~B",
+    [Name, Arity, elixir_errors:inspect(Receiver), Name, Arity]);
+
 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)]).

lib/elixir/src/elixir_module.erl

@@ -78,9 +78,10 @@ compile(Line, Module, Block, Vars, #elixir_scope{context_modules=FileModules} =
         [elixir_tracker:record_local(Tuple, Module) || Tuple <- OnLoad]
     end,
 
-    elixir_tracker:warn_unused_local(File, Module, Private),
-    elixir_tracker:ensure_no_import_conflict(Line, File, Module, All),
+    AllFunctions = Def ++ [T || { T, defp, _, _, _ } <- Private],
+    elixir_tracker:ensure_no_function_conflict(Line, File, Module, AllFunctions),
     elixir_tracker:ensure_all_imports_used(Line, File, Module),
+    elixir_tracker:warn_unused_local(File, Module, Private),
     warn_unused_docs(Line, File, Module, All),
 
     Final = [
@@ -293,7 +294,6 @@ warn_unused_docs(_Line, File, Module, All) ->
       end
   end, ok, docs_table(Module)).
 
-
 % EXTRA FUNCTIONS
 
 add_info_function(Line, File, Module, Export, Def, Defmacro, C) ->

lib/elixir/src/elixir_tracker.erl

@@ -7,7 +7,7 @@
   record_import/4, record_remote/4,
   record_warn/4, record_definition/3,
   record_defaults/4, record_alias/2,
-  ensure_no_import_conflict/4, ensure_all_imports_used/3,
+  ensure_no_function_conflict/4, ensure_all_imports_used/3,
   warn_unused_local/3, format_error/1
 ]).
 -include("elixir.hrl").
@@ -98,10 +98,10 @@ if_tracker(Module, Callback) ->
 
 %% ERROR HANDLING
 
-ensure_no_import_conflict(Meta, File, Module, AllDefined) ->
+ensure_no_function_conflict(Meta, File, Module, AllDefined) ->
   if_tracker(Module, fun(Pid) ->
     [ begin
-        elixir_errors:form_error(Meta, File, ?MODULE, { import_conflict, Error })
+        elixir_errors:form_error(Meta, File, ?MODULE, { function_conflict, Error })
       end || Error <- ?tracker:collect_imports_conflicts(Pid, AllDefined) ]
   end),
   ok.
@@ -127,7 +127,7 @@ warn_unused_local(File, Module, Private) ->
       end || Error <- Unused ]
   end).
 
-format_error({import_conflict,{Receivers, Name, Arity}}) ->
+format_error({function_conflict,{Receivers, Name, Arity}}) ->
   io_lib:format("imported ~ts.~ts/~B conflicts with local function",
     [elixir_errors:inspect(hd(Receivers)), Name, Arity]);
 

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

@@ -230,18 +230,21 @@ defmodule Kernel.ErrorsTest do
       '''
       defmodule ErrorsTest do
         1 && 2
-        def _ && _, do: OMG
+        def _ && _, do: :error
       end
       '''
   end
 
   test :macro_local_conflict do
     assert_compile_fail CompileError,
-      "nofile:1: imported Kernel.&&/2 conflicts with local function",
+      "nofile:6: call to local macro &&/2 conflicts with imported Kernel.&&/2",
       '''
       defmodule ErrorsTest do
-        1 && 2
-        defmacro _ && _, do: OMG
+        def hello, do: 1 || 2
+        defmacro _ || _, do: :ok
+
+        defmacro _ && _, do: :error
+        def world, do: 1 && 2
       end
       '''
   end