From afd3faaf6bdd69a23c8e616d108ecdaad6560caa Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Fri, 19 Sep 2025 20:32:37 -0300 Subject: [PATCH 1/3] Add require warning --- lib/elixir/lib/inspect.ex | 2 - lib/elixir/lib/kernel/lexical_tracker.ex | 26 +++ lib/elixir/lib/protocol.ex | 2 +- lib/elixir/src/elixir_import.erl | 2 +- lib/elixir/src/elixir_lexical.erl | 20 +- lib/elixir/test/elixir/code_test.exs | 2 +- .../elixir/kernel/lexical_tracker_test.exs | 47 +++++ .../test/elixir/kernel/warning_test.exs | 174 ++++++++++++++++++ 8 files changed, 267 insertions(+), 8 deletions(-) diff --git a/lib/elixir/lib/inspect.ex b/lib/elixir/lib/inspect.ex index ff1c86d728f..4362ae2c5ae 100644 --- a/lib/elixir/lib/inspect.ex +++ b/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 diff --git a/lib/elixir/lib/kernel/lexical_tracker.ex b/lib/elixir/lib/kernel/lexical_tracker.ex index ad73863e6d3..676b0bdccae 100644 --- a/lib/elixir/lib/kernel/lexical_tracker.ex +++ b/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} diff --git a/lib/elixir/lib/protocol.ex b/lib/elixir/lib/protocol.ex index ebedd282f36..65301871399 100644 --- a/lib/elixir/lib/protocol.ex +++ b/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 diff --git a/lib/elixir/src/elixir_import.erl b/lib/elixir/src/elixir_import.erl index b44e446cebc..1d9781a8d7a 100644 --- a/lib/elixir/src/elixir_import.erl +++ b/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([{from_import, true} | Meta], Ref, Opts, EI, Trace)}; {error, Reason} -> {error, Reason} diff --git a/lib/elixir/src/elixir_lexical.erl b/lib/elixir/src/elixir_lexical.erl index 68d3fbdef74..5878d6b8aba 100644 --- a/lib/elixir/src/elixir_lexical.erl +++ b/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,16 @@ 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), + % Only track explicit requires, not implicit ones from imports + case lists:keyfind(from_import, 1, Meta) of + {from_import, true} -> ok; + _ -> ?tracker:add_require(Pid, Module, [{module, CurrentModule}, {opts, Opts} | Meta]) + end end, ok; trace({struct_expansion, _Meta, Module, _Keys}, #{lexical_tracker := Pid}) -> @@ -95,6 +102,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 +123,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)]). diff --git a/lib/elixir/test/elixir/code_test.exs b/lib/elixir/test/elixir/code_test.exs index 23bc49f0acb..2a71e824dd3 100644 --- a/lib/elixir/test/elixir/code_test.exs +++ b/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) diff --git a/lib/elixir/test/elixir/kernel/lexical_tracker_test.exs b/lib/elixir/test/elixir/kernel/lexical_tracker_test.exs index b278ea02384..bac56067f69 100644 --- a/lib/elixir/test/elixir/kernel/lexical_tracker_test.exs +++ b/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]) == [] diff --git a/lib/elixir/test/elixir/kernel/warning_test.exs b/lib/elixir/test/elixir/kernel/warning_test.exs index bc6de7c49dd..bae71ca92f5 100644 --- a/lib/elixir/test/elixir/kernel/warning_test.exs +++ b/lib/elixir/test/elixir/kernel/warning_test.exs @@ -2269,6 +2269,180 @@ defmodule Kernel.WarningTest do Enum.each(list, &purge/1) end + # Test modules with macros for require testing + defmodule RequireTestMacroModule do + defmacro test_macro(value) do + quote do: unquote(value) * 2 + end + + defmacro another_macro(value) do + quote do: unquote(value) + 1 + end + + def test_function(value) do + value * 2 + end + end + + defmodule RequireTestAnotherMacroModule do + defmacro some_macro(value) do + quote do: unquote(value) - 1 + end + end + + test "unused require" do + assert_warn_compile( + ["nofile:2:3", "unused require Kernel.WarningTest.RequireTestMacroModule"], + """ + defmodule Sample do + require Kernel.WarningTest.RequireTestMacroModule + def a, do: nil + end + """ + ) + + assert_warn_compile( + ["nofile:1:1", "unused require Kernel.WarningTest.RequireTestMacroModule"], + """ + require Kernel.WarningTest.RequireTestMacroModule + """ + ) + after + purge(Sample) + end + + test "unused require with alias" do + assert_warn_compile( + ["nofile:2:3", "unused require Kernel.WarningTest.RequireTestMacroModule"], + """ + defmodule Sample do + require Kernel.WarningTest.RequireTestMacroModule, as: TestMacro + def a, do: nil + end + """ + ) + after + purge(Sample) + end + + test "used require does not warn" do + assert capture_err(fn -> + defmodule KernelTest.UsedRequire do + require Kernel.WarningTest.RequireTestMacroModule + def fun, do: Kernel.WarningTest.RequireTestMacroModule.test_macro(5) + end + end) == "" + after + purge(KernelTest.UsedRequire) + end + + test "function calls do not prevent require warnings" do + assert_warn_compile( + ["nofile:2:3", "unused require Kernel.WarningTest.RequireTestMacroModule"], + """ + defmodule Sample do + require Kernel.WarningTest.RequireTestMacroModule + def fun, do: Kernel.WarningTest.RequireTestMacroModule.test_function(5) + end + """ + ) + after + purge(Sample) + end + + test "used require with alias does not warn" do + assert capture_err(fn -> + defmodule KernelTest.UsedRequireWithAlias do + require Kernel.WarningTest.RequireTestMacroModule, as: TestMacro + def fun, do: TestMacro.test_macro(5) + end + end) == "" + after + purge(KernelTest.UsedRequireWithAlias) + end + + test "unused require with warn: false" do + assert capture_err(fn -> + defmodule KernelTest.UnusedRequireNoWarn do + require Kernel.WarningTest.RequireTestMacroModule, warn: false + def fun, do: nil + end + end) == "" + after + purge(KernelTest.UnusedRequireNoWarn) + end + + test "unused require with warn: true (explicit)" do + assert_warn_compile( + ["nofile:2:3", "unused require Kernel.WarningTest.RequireTestMacroModule"], + """ + defmodule Sample do + require Kernel.WarningTest.RequireTestMacroModule, warn: true + def a, do: nil + end + """ + ) + after + purge(Sample) + end + + test "unused require with warn: false and alias" do + assert capture_err(fn -> + defmodule KernelTest.UnusedRequireNoWarnAlias do + require Kernel.WarningTest.RequireTestMacroModule, as: TestMacro, warn: false + def fun, do: nil + end + end) == "" + after + purge(KernelTest.UnusedRequireNoWarnAlias) + end + + test "multiple unused requires" do + assert_warn_compile( + [ + "nofile:2:3", + "unused require Kernel.WarningTest.RequireTestMacroModule", + "nofile:3:3", + "unused require Kernel.WarningTest.RequireTestAnotherMacroModule" + ], + """ + defmodule Sample do + require Kernel.WarningTest.RequireTestMacroModule + require Kernel.WarningTest.RequireTestAnotherMacroModule + def a, do: nil + end + """ + ) + after + purge(Sample) + end + + test "mixed warn options" do + assert_warn_compile( + ["nofile:3:3", "unused require Kernel.WarningTest.RequireTestAnotherMacroModule"], + """ + defmodule Sample do + require Kernel.WarningTest.RequireTestMacroModule, warn: false + require Kernel.WarningTest.RequireTestAnotherMacroModule, warn: true + def a, do: nil + end + """ + ) + after + purge(Sample) + end + + test "warn: false preserves functionality" do + assert capture_err(fn -> + defmodule KernelTest.WarnFalsePreservesFunctionality do + require Kernel.WarningTest.RequireTestMacroModule, warn: false + def fun, do: Kernel.WarningTest.RequireTestMacroModule.test_macro(5) + end + end) == "" + after + purge(KernelTest.WarnFalsePreservesFunctionality) + end + defp purge(module) when is_atom(module) do :code.purge(module) :code.delete(module) From 32ac776cc76df71353b899f5a2a2644cd5a17baa Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Mon, 22 Sep 2025 20:44:58 -0300 Subject: [PATCH 2/3] Remove from_import and use warn: false --- lib/elixir/src/elixir_import.erl | 2 +- lib/elixir/src/elixir_lexical.erl | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/elixir/src/elixir_import.erl b/lib/elixir/src/elixir_import.erl index 1d9781a8d7a..ea7e07833cc 100644 --- a/lib/elixir/src/elixir_import.erl +++ b/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([{from_import, true} | Meta], Ref, Opts, EI, Trace)}; + {ok, Added, elixir_aliases:require(Meta, Ref, [{warn, false} | Opts], EI, Trace)}; {error, Reason} -> {error, Reason} diff --git a/lib/elixir/src/elixir_lexical.erl b/lib/elixir/src/elixir_lexical.erl index 5878d6b8aba..28d0316e11d 100644 --- a/lib/elixir/src/elixir_lexical.erl +++ b/lib/elixir/src/elixir_lexical.erl @@ -39,11 +39,7 @@ trace({require, Meta, Module, Opts}, #{lexical_tracker := Pid, module := Current {from_macro, true} -> ?tracker:remote_dispatch(Pid, Module, compile); _ -> ?tracker:add_export(Pid, Module), - % Only track explicit requires, not implicit ones from imports - case lists:keyfind(from_import, 1, Meta) of - {from_import, true} -> ok; - _ -> ?tracker:add_require(Pid, Module, [{module, CurrentModule}, {opts, Opts} | Meta]) - end + ?tracker:add_require(Pid, Module, [{module, CurrentModule}, {opts, Opts} | Meta]) end, ok; trace({struct_expansion, _Meta, Module, _Keys}, #{lexical_tracker := Pid}) -> From 4f2772423b42942c2af44c85ed1c1583551cd9ac Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Mon, 22 Sep 2025 20:45:20 -0300 Subject: [PATCH 3/3] Remove unnecessary tests --- .../test/elixir/kernel/warning_test.exs | 161 +----------------- 1 file changed, 4 insertions(+), 157 deletions(-) diff --git a/lib/elixir/test/elixir/kernel/warning_test.exs b/lib/elixir/test/elixir/kernel/warning_test.exs index bae71ca92f5..c33a681689a 100644 --- a/lib/elixir/test/elixir/kernel/warning_test.exs +++ b/lib/elixir/test/elixir/kernel/warning_test.exs @@ -2269,180 +2269,27 @@ defmodule Kernel.WarningTest do Enum.each(list, &purge/1) end - # Test modules with macros for require testing - defmodule RequireTestMacroModule do - defmacro test_macro(value) do - quote do: unquote(value) * 2 - end - - defmacro another_macro(value) do - quote do: unquote(value) + 1 - end - - def test_function(value) do - value * 2 - end - end - - defmodule RequireTestAnotherMacroModule do - defmacro some_macro(value) do - quote do: unquote(value) - 1 - end - end - test "unused require" do assert_warn_compile( - ["nofile:2:3", "unused require Kernel.WarningTest.RequireTestMacroModule"], + ["nofile:2:3", "unused require Application"], """ defmodule Sample do - require Kernel.WarningTest.RequireTestMacroModule + require Application def a, do: nil end """ ) assert_warn_compile( - ["nofile:1:1", "unused require Kernel.WarningTest.RequireTestMacroModule"], + ["nofile:1:1", "unused require Logger"], """ - require Kernel.WarningTest.RequireTestMacroModule + require Logger """ ) after purge(Sample) end - test "unused require with alias" do - assert_warn_compile( - ["nofile:2:3", "unused require Kernel.WarningTest.RequireTestMacroModule"], - """ - defmodule Sample do - require Kernel.WarningTest.RequireTestMacroModule, as: TestMacro - def a, do: nil - end - """ - ) - after - purge(Sample) - end - - test "used require does not warn" do - assert capture_err(fn -> - defmodule KernelTest.UsedRequire do - require Kernel.WarningTest.RequireTestMacroModule - def fun, do: Kernel.WarningTest.RequireTestMacroModule.test_macro(5) - end - end) == "" - after - purge(KernelTest.UsedRequire) - end - - test "function calls do not prevent require warnings" do - assert_warn_compile( - ["nofile:2:3", "unused require Kernel.WarningTest.RequireTestMacroModule"], - """ - defmodule Sample do - require Kernel.WarningTest.RequireTestMacroModule - def fun, do: Kernel.WarningTest.RequireTestMacroModule.test_function(5) - end - """ - ) - after - purge(Sample) - end - - test "used require with alias does not warn" do - assert capture_err(fn -> - defmodule KernelTest.UsedRequireWithAlias do - require Kernel.WarningTest.RequireTestMacroModule, as: TestMacro - def fun, do: TestMacro.test_macro(5) - end - end) == "" - after - purge(KernelTest.UsedRequireWithAlias) - end - - test "unused require with warn: false" do - assert capture_err(fn -> - defmodule KernelTest.UnusedRequireNoWarn do - require Kernel.WarningTest.RequireTestMacroModule, warn: false - def fun, do: nil - end - end) == "" - after - purge(KernelTest.UnusedRequireNoWarn) - end - - test "unused require with warn: true (explicit)" do - assert_warn_compile( - ["nofile:2:3", "unused require Kernel.WarningTest.RequireTestMacroModule"], - """ - defmodule Sample do - require Kernel.WarningTest.RequireTestMacroModule, warn: true - def a, do: nil - end - """ - ) - after - purge(Sample) - end - - test "unused require with warn: false and alias" do - assert capture_err(fn -> - defmodule KernelTest.UnusedRequireNoWarnAlias do - require Kernel.WarningTest.RequireTestMacroModule, as: TestMacro, warn: false - def fun, do: nil - end - end) == "" - after - purge(KernelTest.UnusedRequireNoWarnAlias) - end - - test "multiple unused requires" do - assert_warn_compile( - [ - "nofile:2:3", - "unused require Kernel.WarningTest.RequireTestMacroModule", - "nofile:3:3", - "unused require Kernel.WarningTest.RequireTestAnotherMacroModule" - ], - """ - defmodule Sample do - require Kernel.WarningTest.RequireTestMacroModule - require Kernel.WarningTest.RequireTestAnotherMacroModule - def a, do: nil - end - """ - ) - after - purge(Sample) - end - - test "mixed warn options" do - assert_warn_compile( - ["nofile:3:3", "unused require Kernel.WarningTest.RequireTestAnotherMacroModule"], - """ - defmodule Sample do - require Kernel.WarningTest.RequireTestMacroModule, warn: false - require Kernel.WarningTest.RequireTestAnotherMacroModule, warn: true - def a, do: nil - end - """ - ) - after - purge(Sample) - end - - test "warn: false preserves functionality" do - assert capture_err(fn -> - defmodule KernelTest.WarnFalsePreservesFunctionality do - require Kernel.WarningTest.RequireTestMacroModule, warn: false - def fun, do: Kernel.WarningTest.RequireTestMacroModule.test_macro(5) - end - end) == "" - after - purge(KernelTest.WarnFalsePreservesFunctionality) - end - defp purge(module) when is_atom(module) do :code.purge(module) :code.delete(module)