Add compile-time dependencies on require

Projects like Plug use require to establish compile
time dependencies inside a Plug. The fact require
only added a compile-time dependency in v1.13.0 was
therefore a regression, addressed by this commit.

Author: josevalim

lib/elixir/lib/kernel/lexical_tracker.ex

@@ -30,8 +30,8 @@ defmodule Kernel.LexicalTracker do
   end
 
   @doc false
-  def add_require(pid, module) when is_atom(module) do
-    :gen_server.cast(pid, {:add_require, module})
+  def add_export(pid, module) when is_atom(module) do
+    :gen_server.cast(pid, {:add_export, module})
   end
 
   @doc false
@@ -168,15 +168,16 @@ defmodule Kernel.LexicalTracker do
     {:noreply, update_in(state.compile_env, &:ordsets.add_element({app, path, return}, &1))}
   end
 
-  def handle_cast({:add_require, module}, state) do
+  def handle_cast({:add_export, module}, state) do
     {:noreply, put_in(state.exports[module], true)}
   end
 
   def handle_cast({:add_import, module, fas, line, warn}, state) do
-    to_remove = for {{:import, {^module, _, _}} = key, _} <- state.directives, do: key
+    %{directives: directives, exports: exports} = state
+    to_remove = for {{:import, {^module, _, _}} = key, _} <- directives, do: key
 
     directives =
-      state.directives
+      directives
       |> Map.drop(to_remove)
       |> add_directive(module, line, warn, :import)
 
@@ -185,7 +186,7 @@ defmodule Kernel.LexicalTracker do
         add_directive(directives, {module, function, arity}, line, warn, :import)
       end)
 
-    {:noreply, %{state | directives: directives}}
+    {:noreply, %{state | directives: directives, exports: Map.put(exports, module, true)}}
   end
 
   def handle_cast({:add_alias, module, line, warn}, state) do
@@ -221,9 +222,9 @@ defmodule Kernel.LexicalTracker do
     do: Map.put(references, module, :compile)
 
   defp add_reference(references, module, :runtime) when is_atom(module) do
-    case Map.fetch(references, module) do
-      {:ok, _} -> references
-      :error -> Map.put(references, module, :runtime)
+    case references do
+      %{^module => _} -> references
+      _ -> Map.put(references, module, :runtime)
     end
   end
 

lib/elixir/lib/kernel/special_forms.ex

@@ -554,7 +554,9 @@ defmodule Kernel.SpecialForms do
   defmacro alias(module, opts), do: error!([module, opts])
 
   @doc """
-  Requires a module in order to use its macros.
+  Requires a module as a compile-time dependency.
+
+  Requiring a module is necessary in order to use its macros.
 
   ## Examples
 
@@ -576,7 +578,6 @@ defmodule Kernel.SpecialForms do
 
   `require/2` also accepts `:as` as an option so it automatically sets
   up an alias. Please check `alias/2` for more information.
-
   """
   defmacro require(module, opts), do: error!([module, opts])
 

lib/elixir/lib/module.ex

@@ -1811,7 +1811,8 @@ defmodule Module do
           acc
 
         true ->
-          :elixir_env.trace({:require, [], behaviour, []}, env)
+          event = {:remote_function, [], behaviour, :behaviour_info, 1}
+          :elixir_env.trace(event, %{env | function: {:__info__, 1}})
           optional_callbacks = behaviour_info(behaviour, :optional_callbacks)
           callbacks = behaviour_info(behaviour, :callbacks)
           Enum.reduce(callbacks, acc, &add_callback(&1, behaviour, env, optional_callbacks, &2))

lib/elixir/src/elixir_expand.erl

@@ -85,6 +85,7 @@ expand({require, Meta, [Ref, Opts]}, S, E) ->
   if
     is_atom(ERef) ->
       elixir_aliases:ensure_loaded(Meta, ERef, ET),
+      elixir_env:trace({require, Meta, ERef, EOpts}, ET),
       {ERef, ST, expand_require(Meta, ERef, EOpts, ET)};
     true ->
       form_error(Meta, E, ?MODULE, {expected_compile_time_module, require, Ref})
@@ -931,7 +932,6 @@ no_alias_expansion(Other) ->
   Other.
 
 expand_require(Meta, Ref, Opts, E) ->
-  elixir_env:trace({require, Meta, Ref, Opts}, E),
   RE = E#{requires := ordsets:add_element(Ref, ?key(E, requires))},
   expand_alias(Meta, false, Ref, Opts, RE).
 

lib/elixir/src/elixir_lexical.erl

@@ -47,10 +47,10 @@ trace({alias_expansion, _Meta, Lookup, _Result}, #{lexical_tracker := Pid}) ->
   ?tracker:alias_dispatch(Pid, Lookup),
   ok;
 trace({require, _Meta, Module, _Opts}, #{lexical_tracker := Pid}) ->
-  ?tracker:add_require(Pid, Module),
+  ?tracker:remote_dispatch(Pid, Module, compile),
   ok;
 trace({struct_expansion, _Meta, Module, _Keys}, #{lexical_tracker := Pid}) ->
-  ?tracker:add_require(Pid, Module),
+  ?tracker:add_export(Pid, Module),
   ok;
 trace({alias_reference, _Meta, Module}, #{lexical_tracker := Pid} = E) ->
   ?tracker:remote_dispatch(Pid, Module, mode(E)),

lib/elixir/test/elixir/kernel/lexical_tracker_test.exs

@@ -21,8 +21,8 @@ defmodule Kernel.LexicalTrackerTest do
     assert D.references(config[:pid]) == {[String], [], [], []}
   end
 
-  test "can add requires", config do
-    D.add_require(config[:pid], URI)
+  test "can add module requires and exports", config do
+    D.add_export(config[:pid], URI)
     assert D.references(config[:pid]) == {[], [URI], [], []}
 
     D.remote_dispatch(config[:pid], URI, :runtime)
@@ -33,7 +33,6 @@ defmodule Kernel.LexicalTrackerTest do
   end
 
   test "can add module imports", config do
-    D.add_require(config[:pid], String)
     D.add_import(config[:pid], String, [], 1, true)
 
     D.import_dispatch(config[:pid], String, {:upcase, 1}, :runtime)
@@ -44,7 +43,7 @@ defmodule Kernel.LexicalTrackerTest do
   end
 
   test "can add module with {function, arity} imports", config do
-    D.add_require(config[:pid], String)
+    D.add_export(config[:pid], String)
     D.add_import(config[:pid], String, [upcase: 1], 1, true)
 
     D.import_dispatch(config[:pid], String, {:upcase, 1}, :compile)
@@ -181,18 +180,18 @@ defmodule Kernel.LexicalTrackerTest do
       {{compile, _exports, runtime, _}, _binding} =
         Code.eval_string("""
         defmodule Kernel.LexicalTrackerTest.Defdelegate do
-          defdelegate a, to: A
+          defdelegate parse(arg), to: URI
 
-          opts = [to: B]
-          defdelegate b, opts
+          opts = [to: Range]
+          defdelegate disjoint?(left, right), opts
 
           Kernel.LexicalTracker.references(__ENV__.lexical_tracker)
         end |> elem(3)
         """)
 
-      refute A in compile
-      assert B in compile
-      assert A in runtime
+      refute URI in compile
+      assert Range in compile
+      assert URI in runtime
     end
 
     test "imports adds an export dependency" do
@@ -209,6 +208,20 @@ defmodule Kernel.LexicalTrackerTest do
       refute String in runtime
     end
 
+    test "requires adds a compile dependency" do
+      {{compile, exports, runtime, _}, _binding} =
+        Code.eval_string("""
+        defmodule Kernel.LexicalTrackerTest.Requires do
+          require String
+          Kernel.LexicalTracker.references(__ENV__.lexical_tracker)
+        end |> elem(3)
+        """)
+
+      assert String in compile
+      refute String in exports
+      refute String in runtime
+    end
+
     test "structs are exports or compile time" do
       {{compile, exports, runtime, _}, _binding} =
         Code.eval_string("""

lib/mix/lib/mix/tasks/xref.ex

@@ -510,13 +510,16 @@ defmodule Mix.Tasks.Xref do
 
   @doc false
   def trace({:require, meta, module, _opts}, env),
-    do: add_trace(:export, :require, module, module, meta, env)
+    do: add_trace(:compile, :require, module, module, meta, env)
+
+  def trace({:import, meta, module, _opts}, env),
+    do: add_trace(:export, :import, module, module, meta, env)
 
   def trace({:struct_expansion, meta, module, _keys}, env),
     do: add_trace(:export, :struct, module, module, meta, env)
 
   def trace({:alias_reference, meta, module}, env) when env.module != module,
-    do: add_trace(mode(env), :alias, module, module, meta, env)
+    do: add_trace(mode(env), :module, module, module, meta, env)
 
   def trace({:remote_function, meta, module, function, arity}, env),
     do: add_trace(mode(env), :call, module, {module, function, arity}, meta, env)
@@ -551,14 +554,14 @@ defmodule Mix.Tasks.Xref do
     # We don't want to show aliases if there is an entry of the same type
     non_aliases =
       for {_file, _line, module_or_mfa, mode, type} <- entries,
-          type != :alias,
+          type != :module,
           into: %{},
           do: {{trace_module(module_or_mfa), mode}, []}
 
     shell = Mix.shell()
 
     for {file, line, module_or_mfa, mode, type} <- entries,
-        type != :alias or not Map.has_key?(non_aliases, {module_or_mfa, mode}) do
+        type != :module or not Map.has_key?(non_aliases, {module_or_mfa, mode}) do
       shell.info([
         Exception.format_file_line(Path.relative_to_cwd(file), line),
         ?\s,

lib/mix/test/mix/tasks/xref_test.exs

@@ -253,7 +253,7 @@ defmodule Mix.Tasks.XrefTest do
       output = """
       Compiling 2 files (.ex)
       Generated sample app
-      lib/b.ex:2: require A (export)
+      lib/b.ex:2: import A (export)
       lib/b.ex:3: call A.macro/0 (compile)
       lib/b.ex:4: import A.macro/0 (compile)
       lib/b.ex:5: call A.fun/0 (compile)
@@ -287,6 +287,7 @@ defmodule Mix.Tasks.XrefTest do
       output = """
       Compiling 2 files (.ex)
       Generated sample app
+      lib/b.ex:2: require A (compile)
       lib/b.ex:3: call A.macro/0 (compile)
       """
 
@@ -313,10 +314,11 @@ defmodule Mix.Tasks.XrefTest do
       output = """
       Compiling 2 files (.ex)
       Generated sample app
+      lib/b.ex:3: require A (compile)
       lib/b.ex:3: call A.macro/0 (compile)
       """
 
-      message = "Too many traces (found: 1, permitted: 0)"
+      message = "Too many traces (found: 2, permitted: 0)"
 
       assert_raise Mix.Error, message, fn ->
         assert_trace(~w[--label compile --fail-above 0], "lib/b.ex", files, output)
@@ -759,10 +761,7 @@ defmodule Mix.Tasks.XrefTest do
         defmodule B do
           # Let's also test that we track literal atom behaviours
           @behaviour :"Elixir.A.Behaviour"
-
-          def foo do
-            A.foo()
-          end
+          def foo, do: :foo
         end
         """)
 
@@ -772,7 +771,7 @@ defmodule Mix.Tasks.XrefTest do
                digraph "xref graph" {
                  "lib/a.ex"
                  "lib/a.ex" -> "lib/b.ex" [label="(compile)"]
-                 "lib/b.ex" -> "lib/a.ex" [label="(export)"]
+                 "lib/b.ex" -> "lib/a.ex"
                  "lib/b.ex"
                }
                """

lib/mix/test/mix/umbrella_test.exs

@@ -391,7 +391,7 @@ defmodule Mix.UmbrellaTest do
         assert_received {:mix_shell, :info, ["Compiled lib/bar.ex"]}
 
         # But exports dependencies are not recompiled
-        File.write!("lib/bar.ex", "defmodule Bar, do: (require Foo)")
+        File.write!("lib/bar.ex", "defmodule Bar, do: (import Foo, warn: false)")
 
         assert Mix.Tasks.Compile.Elixir.run(["--verbose"]) == {:ok, []}
         assert_received {:mix_shell, :info, ["Compiled lib/bar.ex"]}