Skip application boundary warnings for protocols

Otherwise we can get false positives when compiling
a protocol and there is an implementation defined
in another application that the protocol definition
does not depend on.

This commit also rewrites @protocol to @__protocol__
to prepare for future tracking changes.

5 files changed, 61 insertions, 41 deletions

diff --git a/lib/elixir/lib/protocol.ex b/lib/elixir/lib/protocol.ex
index b39f61922..9e764a551 100644
--- a/lib/elixir/lib/protocol.ex
+++ b/lib/elixir/lib/protocol.ex
@@ -191,15 +191,17 @@ defmodule Protocol do
       iex> Enumerable.impl_for(42)
       nil
 
-  In addition, every protocol implementation module contains the `__impl__/1` function. The
-  function takes one of the following atoms:
+  In addition, every protocol implementation module contains the `__impl__/1`
+  function. The function takes one of the following atoms:
 
-      * `:for` - returns the module responsible for the data structure of the protocol implementation
+      * `:for` - returns the module responsible for the data structure of the
+        protocol implementation
 
-      * `:protocol` - returns the protocol module for which this implementation is provided
+      * `:protocol` - returns the protocol module for which this implementation
+      is provided
 
-  For example, the module implementing the `Enumerable` protocol for lists is `Enumerable.List`.
-  Therefore, we can invoke `__impl__/1` on this module:
+  For example, the module implementing the `Enumerable` protocol for lists is
+  `Enumerable.List`. Therefore, we can invoke `__impl__/1` on this module:
 
       iex(1)> Enumerable.List.__impl__(:for)
       List
@@ -209,22 +211,17 @@ defmodule Protocol do
 
   ## Consolidation
 
-  In order to cope with code loading in development, protocols in
-  Elixir provide a slow implementation of protocol dispatching specific
-  to development.
-
-  In order to speed up dispatching in production environments, where
-  all implementations are known up-front, Elixir provides a feature
-  called *protocol consolidation*. Consolidation directly links protocols
-  to their implementations in a way that invoking a function from a
+  In order to speed up protocol dispatching, whenever all protocol implementations
+  are known up-front, typically after all Elixir code in a project is compiled,
+  Elixir provides a feature called *protocol consolidation*. Consolidation directly
+  links protocols to their implementations in a way that invoking a function from a
   consolidated protocol is equivalent to invoking two remote functions.
 
-  Protocol consolidation is applied by default to all Mix projects during
-  compilation. This may be an issue during test. For instance, if you want
-  to implement a protocol during test, the implementation will have no
-  effect, as the protocol has already been consolidated. One possible
-  solution is to include compilation directories that are specific to your
-  test environment in your mix.exs:
+  Protocol consolidation is applied by default to all Mix projects during compilation.
+  This may be an issue during test. For instance, if you want to implement a protocol
+  during test, the implementation will have no effect, as the protocol has already been
+  consolidated. One possible solution is to include compilation directories that are
+  specific to your test environment in your mix.exs:
 
       def project do
         ...
@@ -439,7 +436,7 @@ defmodule Protocol do
   @spec extract_protocols([charlist | String.t()]) :: [atom]
   def extract_protocols(paths) do
     extract_matching_by_attribute(paths, 'Elixir.', fn module, attributes ->
-      case attributes[:protocol] do
+      case attributes[:__protocol__] do
         [fallback_to_any: _] -> module
         _ -> nil
       end
@@ -470,7 +467,7 @@ defmodule Protocol do
     prefix = Atom.to_charlist(protocol) ++ '.'
 
     extract_matching_by_attribute(paths, prefix, fn _mod, attributes ->
-      case attributes[:protocol_impl] do
+      case attributes[:__impl__] do
         [protocol: ^protocol, for: for] -> for
         _ -> nil
       end
@@ -561,7 +558,7 @@ defmodule Protocol do
         chunks = :lists.filter(fn {_name, value} -> value != :missing_chunk end, chunks)
         chunks = :lists.map(fn {name, value} -> {List.to_string(name), value} end, chunks)
 
-        case attributes[:protocol] do
+        case attributes[:__protocol__] do
           [fallback_to_any: any] ->
             {:ok, {any, definitions}, specs, {info, chunks}}
 
@@ -795,8 +792,8 @@ defmodule Protocol do
 
       # Store information as an attribute so it
       # can be read without loading the module.
-      Module.register_attribute(__MODULE__, :protocol, persist: true)
-      @protocol [fallback_to_any: !!@fallback_to_any]
+      Module.register_attribute(__MODULE__, :__protocol__, persist: true)
+      @__protocol__ [fallback_to_any: !!@fallback_to_any]
 
       @doc false
       @spec __protocol__(:module) :: __MODULE__
@@ -855,8 +852,8 @@ defmodule Protocol do
 
         unquote(block)
 
-        Module.register_attribute(__MODULE__, :protocol_impl, persist: true)
-        @protocol_impl [protocol: @protocol, for: @for]
+        Module.register_attribute(__MODULE__, :__impl__, persist: true)
+        @__impl__ [protocol: @protocol, for: @for]
 
         unquote(impl)
       end
@@ -897,8 +894,8 @@ defmodule Protocol do
       else
         quoted =
           quote do
-            Module.register_attribute(__MODULE__, :protocol_impl, persist: true)
-            @protocol_impl [protocol: unquote(protocol), for: unquote(for)]
+            Module.register_attribute(__MODULE__, :__impl__, persist: true)
+            @__impl__ [protocol: unquote(protocol), for: unquote(for)]
 
             @doc false
             @spec __impl__(:target) :: unquote(impl)
diff --git a/lib/elixir/test/elixir/protocol_test.exs b/lib/elixir/test/elixir/protocol_test.exs
index c8cf71d2c..7c736388d 100644
--- a/lib/elixir/test/elixir/protocol_test.exs
+++ b/lib/elixir/test/elixir/protocol_test.exs
@@ -155,13 +155,13 @@ defmodule ProtocolTest do
     assert Sample.__protocol__(:functions) == [ok: 1]
     refute Sample.__protocol__(:consolidated?)
     assert Sample.__protocol__(:impls) == :not_consolidated
-    assert Sample.__info__(:attributes)[:protocol] == [fallback_to_any: false]
+    assert Sample.__info__(:attributes)[:__protocol__] == [fallback_to_any: false]
 
     assert WithAny.__protocol__(:module) == WithAny
     assert WithAny.__protocol__(:functions) == [ok: 1]
     refute WithAny.__protocol__(:consolidated?)
     assert WithAny.__protocol__(:impls) == :not_consolidated
-    assert WithAny.__info__(:attributes)[:protocol] == [fallback_to_any: true]
+    assert WithAny.__info__(:attributes)[:__protocol__] == [fallback_to_any: true]
   end
 
   test "defimpl" do
@@ -169,7 +169,7 @@ defmodule ProtocolTest do
     assert module.__impl__(:for) == ImplStruct
     assert module.__impl__(:target) == module
     assert module.__impl__(:protocol) == Sample
-    assert module.__info__(:attributes)[:protocol_impl] == [protocol: Sample, for: ImplStruct]
+    assert module.__info__(:attributes)[:__impl__] == [protocol: Sample, for: ImplStruct]
   end
 
   test "defimpl with implicit derive" do
@@ -177,7 +177,7 @@ defmodule ProtocolTest do
     assert module.__impl__(:for) == ImplStruct
     assert module.__impl__(:target) == WithAny.Any
     assert module.__impl__(:protocol) == WithAny
-    assert module.__info__(:attributes)[:protocol_impl] == [protocol: WithAny, for: ImplStruct]
+    assert module.__info__(:attributes)[:__impl__] == [protocol: WithAny, for: ImplStruct]
   end
 
   test "defimpl with explicit derive" do
@@ -185,7 +185,7 @@ defmodule ProtocolTest do
     assert module.__impl__(:for) == ImplStruct
     assert module.__impl__(:target) == module
     assert module.__impl__(:protocol) == Derivable
-    assert module.__info__(:attributes)[:protocol_impl] == [protocol: Derivable, for: ImplStruct]
+    assert module.__info__(:attributes)[:__impl__] == [protocol: Derivable, for: ImplStruct]
   end
 
   test "defimpl with multiple for" do
diff --git a/lib/mix/lib/mix/compilers/application_tracer.ex b/lib/mix/lib/mix/compilers/application_tracer.ex
index 78ea144d2..1e0daa9df 100644
--- a/lib/mix/lib/mix/compilers/application_tracer.ex
+++ b/lib/mix/lib/mix/compilers/application_tracer.ex
@@ -25,6 +25,14 @@ defmodule Mix.Compilers.ApplicationTracer do
     :ok
   end
 
+  # Skip protocol implementations too as the goal of protocols
+  # is to invert the dependency graph. Perhaps it would be best
+  # to skip tracing altogether if env.module is a protocol but
+  # currently there is no cheap way to track this information.
+  def trace({_, _, _, :__impl__, _}, _env) do
+    :ok
+  end
+
   def trace({type, meta, module, function, arity}, env)
       when type in [:remote_function, :remote_macro, :imported_function, :imported_macro] do
     # Unknown modules need to be looked up and filtered later
@@ -52,10 +60,10 @@ defmodule Mix.Compilers.ApplicationTracer do
     warnings =
       :ets.foldl(
         fn {module, function, arity, env_module, env_function, env_file, env_line}, acc ->
-          # If the module is either preloaded, it is always available.
-          # If the module is non existing, the compiler will warn, so we don't.
+          # If the module is preloaded, it is always available, so we skip it.
+          # If the module is non existing, the compiler will warn, so we skip it.
           # If the module belongs to this application (from another compiler), we skip it.
-          # If the module is excluded, we skip it too.
+          # If the module is excluded, we skip it.
           with path when is_list(path) <- :code.which(module),
                {:ok, module_app} <- app(path),
                true <- module_app != app,
diff --git a/lib/mix/lib/mix/compilers/elixir.ex b/lib/mix/lib/mix/compilers/elixir.ex
index b22def0b4..b9b7b83ac 100644
--- a/lib/mix/lib/mix/compilers/elixir.ex
+++ b/lib/mix/lib/mix/compilers/elixir.ex
@@ -401,13 +401,13 @@ defmodule Mix.Compilers.Elixir do
   end
 
   defp detect_kind(module) do
-    protocol_metadata = Module.get_attribute(module, :protocol_impl)
+    protocol_metadata = Module.get_attribute(module, :__impl__)
 
     cond do
       is_list(protocol_metadata) and protocol_metadata[:protocol] ->
         {:impl, protocol_metadata[:protocol]}
 
-      is_list(Module.get_attribute(module, :protocol)) ->
+      is_list(Module.get_attribute(module, :__protocol__)) ->
         :protocol
 
       true ->
diff --git a/lib/mix/test/mix/tasks/compile.elixir_test.exs b/lib/mix/test/mix/tasks/compile.elixir_test.exs
index 5b2b4afb2..1a55bfb97 100644
--- a/lib/mix/test/mix/tasks/compile.elixir_test.exs
+++ b/lib/mix/test/mix/tasks/compile.elixir_test.exs
@@ -74,8 +74,8 @@ defmodule Mix.Tasks.Compile.ElixirTest do
     in_fixture("no_mixfile", fn ->
       File.write!("lib/a.ex", """
       defmodule A do
-      require Logger
-      def info, do: Logger.info("hello")
+        require Logger
+        def info, do: Logger.info("hello")
       end
       """)
 
@@ -94,6 +94,21 @@ defmodule Mix.Tasks.Compile.ElixirTest do
     end)
   end
 
+  test "does not warn when __info__ is used but not depended on" do
+    in_fixture("no_mixfile", fn ->
+      File.write!("lib/a.ex", """
+      defmodule A do
+        require Logger
+        def info, do: Logger.__impl__("hello")
+      end
+      """)
+
+      assert capture_io(:stderr, fn ->
+               Mix.Task.run("compile", [])
+             end) == ""
+    end)
+  end
+
   test "recompiles module-application manifest if manifest is outdated" do
     in_fixture("no_mixfile", fn ->
       Mix.Tasks.Compile.Elixir.run(["--force"])