Warn on unused require (#14779)

Author: thiamsantos

lib/elixir/lib/inspect.ex

@@ -275,8 +275,6 @@ defprotocol Inspect do
 end
 
 defimpl Inspect, for: Atom do
-  require Macro
-
   def inspect(atom, opts) do
     color_doc(Macro.inspect_atom(:literal, atom), color_key(atom), opts)
   end

lib/elixir/lib/kernel/lexical_tracker.ex

@@ -57,6 +57,11 @@ defmodule Kernel.LexicalTracker do
     :gen_server.cast(pid, {:add_export, module})
   end
 
+  @doc false
+  def add_require(pid, module, meta) when is_atom(module) do
+    :gen_server.cast(pid, {:add_require, module, meta})
+  end
+
   @doc false
   def add_import(pid, module, fas, meta, warn) when is_atom(module) do
     :gen_server.cast(pid, {:add_import, module, fas, meta, warn})
@@ -119,12 +124,18 @@ defmodule Kernel.LexicalTracker do
     :gen_server.call(pid, :unused_aliases, @timeout)
   end
 
+  @doc false
+  def collect_unused_requires(pid) do
+    :gen_server.call(pid, :unused_requires, @timeout)
+  end
+
   # Callbacks
 
   def init(:ok) do
     state = %{
       aliases: %{},
       imports: %{},
+      requires: %{},
       references: %{},
       exports: %{},
       cache: %{},
@@ -150,6 +161,17 @@ defmodule Kernel.LexicalTracker do
     {:reply, Enum.sort(imports), state}
   end
 
+  def handle_call(:unused_requires, _from, state) do
+    unused_requires =
+      for {module, meta} <- state.requires,
+          Map.get(state.references, module) != :compile,
+          Keyword.get(meta[:opts], :warn, true) != false do
+        {module, meta}
+      end
+
+    {:reply, Enum.sort(unused_requires), state}
+  end
+
   def handle_call(:references, _from, state) do
     {compile, runtime} = partition(Map.to_list(state.references), [], [])
     {:reply, {compile, Map.keys(state.exports), runtime, state.compile_env}, state}
@@ -245,6 +267,10 @@ defmodule Kernel.LexicalTracker do
     {:noreply, put_in(state.imports[module][@warn_key], true)}
   end
 
+  def handle_cast({:add_require, module, meta}, state) do
+    {:noreply, put_in(state.requires[module], meta)}
+  end
+
   @doc false
   def handle_info(_msg, state) do
     {:noreply, state}

lib/elixir/lib/protocol.ex

@@ -1032,7 +1032,7 @@ defmodule Protocol do
     # expression (and therefore most likely a compile-time one).
     behaviour =
       if is_atom(protocol) do
-        quote(do: require(unquote(protocol)))
+        quote(do: require(unquote(protocol), warn: false))
       else
         quote(do: protocol)
       end

lib/elixir/src/elixir_import.erl

@@ -20,7 +20,7 @@ import(Meta, Ref, Opts, E, Warn, Trace, InfoCallback) ->
     {Functions, Macros, Added} ->
       Trace andalso elixir_env:trace({import, Meta, Ref, Opts}, E),
       EI = E#{functions := Functions, macros := Macros},
-      {ok, Added, elixir_aliases:require(Meta, Ref, Opts, EI, Trace)};
+      {ok, Added, elixir_aliases:require(Meta, Ref, [{warn, false} | Opts], EI, Trace)};
 
     {error, Reason} ->
       {error, Reason}

lib/elixir/src/elixir_lexical.erl

@@ -18,6 +18,7 @@ run(#{tracers := Tracers} = E, ExecutionCallback, AfterExecutionCallback) ->
         Res ->
           warn_unused_aliases(Pid, LexEnv),
           warn_unused_imports(Pid, LexEnv),
+          warn_unused_requires(Pid, LexEnv),
           AfterExecutionCallback(LexEnv),
           Res
       after
@@ -33,10 +34,12 @@ run(#{tracers := Tracers} = E, ExecutionCallback, AfterExecutionCallback) ->
 trace({alias_expansion, _Meta, Lookup, _Result}, #{lexical_tracker := Pid}) ->
   ?tracker:alias_dispatch(Pid, Lookup),
   ok;
-trace({require, Meta, Module, _Opts}, #{lexical_tracker := Pid}) ->
+trace({require, Meta, Module, Opts}, #{lexical_tracker := Pid, module := CurrentModule}) ->
   case lists:keyfind(from_macro, 1, Meta) of
     {from_macro, true} -> ?tracker:remote_dispatch(Pid, Module, compile);
-    _ -> ?tracker:add_export(Pid, Module)
+    _ -> 
+      ?tracker:add_export(Pid, Module),
+      ?tracker:add_require(Pid, Module, [{module, CurrentModule}, {opts, Opts} | Meta])
   end,
   ok;
 trace({struct_expansion, _Meta, Module, _Keys}, #{lexical_tracker := Pid}) ->
@@ -95,6 +98,11 @@ warn_unused_imports(Pid, E) ->
       {ModOrMFA, Meta} <- unused_imports_for_module(Module, Imports)],
   ok.
 
+warn_unused_requires(Pid, E) ->
+  [elixir_errors:file_warn(Meta, ?key(E, file), ?MODULE, {unused_require, Module})
+   || {Module, Meta} <- ?tracker:collect_unused_requires(Pid)],
+  ok.
+
 unused_imports_for_module(Module, Imports) ->
   case Imports of
     #{Module := Meta} -> [{Module, Meta}];
@@ -111,4 +119,6 @@ format_error({unused_alias, Module}) ->
 format_error({unused_import, {Module, Function, Arity}}) ->
   io_lib:format("unused import ~ts.~ts/~w", [elixir_aliases:inspect(Module), Function, Arity]);
 format_error({unused_import, Module}) ->
-  io_lib:format("unused import ~ts", [elixir_aliases:inspect(Module)]).
+  io_lib:format("unused import ~ts", [elixir_aliases:inspect(Module)]);
+format_error({unused_require, Module}) ->
+  io_lib:format("unused require ~ts", [elixir_aliases:inspect(Module)]).

lib/elixir/test/elixir/code_test.exs

@@ -364,7 +364,7 @@ defmodule CodeTest do
     end
 
     test "with defguard" do
-      require Integer
+      require Integer, warn: false
       env = Code.env_for_eval(__ENV__)
       quoted = quote do: Integer.is_even(1)
       {false, binding, env} = Code.eval_quoted_with_env(quoted, [], env, prune_binding: true)

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

@@ -91,6 +91,53 @@ defmodule Kernel.LexicalTrackerTest do
     assert D.collect_unused_imports(config[:pid]) == [{String, %{}}]
   end
 
+  test "unused requires", config do
+    D.add_require(config[:pid], String, module: TestModule, opts: [])
+    assert D.collect_unused_requires(config[:pid]) == [{String, [module: TestModule, opts: []]}]
+  end
+
+  test "used requires are not unused", config do
+    D.add_require(config[:pid], String, module: TestModule, opts: [])
+    D.remote_dispatch(config[:pid], String, :compile)
+    assert D.collect_unused_requires(config[:pid]) == []
+  end
+
+  test "requires with warn: false are not unused", config do
+    D.add_require(config[:pid], String, module: TestModule, opts: [warn: false])
+    assert D.collect_unused_requires(config[:pid]) == []
+  end
+
+  test "requires with warn: true are unused when not referenced", config do
+    D.add_require(config[:pid], String, module: TestModule, opts: [warn: true])
+
+    assert D.collect_unused_requires(config[:pid]) == [
+             {String, [module: TestModule, opts: [warn: true]]}
+           ]
+  end
+
+  test "multiple unused requires", config do
+    D.add_require(config[:pid], String, module: TestModule, opts: [])
+    D.add_require(config[:pid], List, module: TestModule, opts: [])
+
+    assert D.collect_unused_requires(config[:pid]) == [
+             {List, [module: TestModule, opts: []]},
+             {String, [module: TestModule, opts: []]}
+           ]
+  end
+
+  test "mixed used and unused requires", config do
+    D.add_require(config[:pid], String, module: TestModule, opts: [])
+    D.add_require(config[:pid], List, module: TestModule, opts: [])
+    D.remote_dispatch(config[:pid], String, :compile)
+    assert D.collect_unused_requires(config[:pid]) == [{List, [module: TestModule, opts: []]}]
+  end
+
+  test "function calls do not count as macro usage", config do
+    D.add_require(config[:pid], String, module: TestModule, opts: [])
+    D.remote_dispatch(config[:pid], String, :runtime)
+    assert D.collect_unused_requires(config[:pid]) == [{String, [module: TestModule, opts: []]}]
+  end
+
   test "imports with no warn are not unused", config do
     D.add_import(config[:pid], String, [], 1, false)
     assert D.collect_unused_imports(config[:pid]) == []

lib/elixir/test/elixir/kernel/warning_test.exs

@@ -2269,6 +2269,27 @@ defmodule Kernel.WarningTest do
     Enum.each(list, &purge/1)
   end
 
+  test "unused require" do
+    assert_warn_compile(
+      ["nofile:2:3", "unused require Application"],
+      """
+      defmodule Sample do
+        require Application 
+        def a, do: nil
+      end
+      """
+    )
+
+    assert_warn_compile(
+      ["nofile:1:1", "unused require Logger"],
+      """
+      require Logger 
+      """
+    )
+  after
+    purge(Sample)
+  end
+
   defp purge(module) when is_atom(module) do
     :code.purge(module)
     :code.delete(module)