From 1ba6a1f343170be908c788844bb016fcf483915c Mon Sep 17 00:00:00 2001 From: Arno Dirlam Date: Sat, 26 Oct 2024 17:06:51 +0200 Subject: [PATCH] Support private defdp functions with proper 'unused' warnings --- .formatter.exs | 2 + lib/dx/defd.ex | 39 ++++++++++++ lib/dx/defd/ast.ex | 10 +++ lib/dx/defd/compiler.ex | 78 +++++++++++++++++++---- test/dx/defd_test.exs | 136 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 252 insertions(+), 13 deletions(-) diff --git a/.formatter.exs b/.formatter.exs index 5bf626fc..7b7458ef 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -1,6 +1,8 @@ locals_without_parens = [ defd: 1, defd: 2, + defdp: 1, + defdp: 2, field_group: 1, import_rules: 1, infer: 1, diff --git a/lib/dx/defd.ex b/lib/dx/defd.ex index 00f98a52..16608f68 100644 --- a/lib/dx/defd.ex +++ b/lib/dx/defd.ex @@ -1,4 +1,5 @@ defmodule Dx.Defd do + alias Dx.Defd.Ast alias Dx.Defd.Util alias Dx.Evaluation, as: Eval @@ -12,6 +13,7 @@ defmodule Dx.Defd do unquote(defd_call) end) end + |> mark_use(call) end defmacro load!(call, opts \\ []) do @@ -22,6 +24,7 @@ defmodule Dx.Defd do unquote(defd_call) end) end + |> mark_use(call) end defmacro get(call, opts \\ []) do @@ -32,6 +35,7 @@ defmodule Dx.Defd do unquote(defd_call) end + |> mark_use(call) end defmacro get!(call, opts \\ []) do @@ -39,6 +43,7 @@ defmodule Dx.Defd do Dx.Defd.get(unquote(call), unquote(opts)) |> Dx.Result.unwrap!() end + |> mark_use(call) end defp call_to_defd({:|>, _meta, _pipeline} = ast, env) do @@ -61,6 +66,32 @@ defmodule Dx.Defd do {defd_name, meta, args} end + defp mark_use(ast, call) do + case call_to_use(call, __ENV__) do + {name, arity} -> + Ast.block([ + Ast.local_fun_ref(name, arity), + ast + ]) + + _ -> + ast + end + end + + defp call_to_use({:|>, _meta, _pipeline} = ast, env) do + ast + |> Macro.expand_once(env) + |> call_to_use(env) + end + + defp call_to_use(call, _env) do + case Macro.decompose_call(call) do + {name, args} -> {name, length(args)} + _ -> nil + end + end + defmacro defd(call) do define_defd(:def, call, __CALLER__) end @@ -69,6 +100,14 @@ defmodule Dx.Defd do define_defd(:def, call, block, __CALLER__) end + defmacro defdp(call) do + define_defd(:defp, call, __CALLER__) + end + + defmacro defdp(call, do: block) do + define_defd(:defp, call, block, __CALLER__) + end + @doc """ Used to wrap calls to non-Dx defined functions. It doesn't run any code, but makes these calls explicit and mutes Dx compiler warnings. diff --git a/lib/dx/defd/ast.ex b/lib/dx/defd/ast.ex index 5dcc39e4..4e69d545 100644 --- a/lib/dx/defd/ast.ex +++ b/lib/dx/defd/ast.ex @@ -6,6 +6,16 @@ defmodule Dx.Defd.Ast do import Dx.Defd.Ast.Guards + def local_fun_ref({name, arity}), do: local_fun_ref(name, arity) + + def local_fun_ref(name, meta \\ [], arity, meta2 \\ []) do + {:&, meta, [{:/, [], [{name, meta2, nil}, arity]}]} + end + + def block(lines \\ [], meta \\ []) do + {:__block__, meta, lines} + end + def is_function( {:ok, {:%, _, diff --git a/lib/dx/defd/compiler.ex b/lib/dx/defd/compiler.ex index e0c04544..afb1c930 100644 --- a/lib/dx/defd/compiler.ex +++ b/lib/dx/defd/compiler.ex @@ -21,6 +21,16 @@ defmodule Dx.Defd.Compiler do @doc false def __compile__(%Macro.Env{module: module, file: file, line: line}, exports, eval_var) do defds = compile_prepare_arities(exports) + all_arities = all_arities(exports) + + generated_private_functions = + Enum.flat_map(defds, fn {name, arity} -> + [ + {Util.defd_name(name), arity + 1}, + {Util.final_args_name(name), arity + 1}, + {Util.scope_name(name), arity} + ] + end) state = %{ module: module, @@ -28,6 +38,9 @@ defmodule Dx.Defd.Compiler do line: line, function: nil, defds: defds, + all_arities: all_arities, + generated_private_functions: generated_private_functions, + used_defdps: [], args: MapSet.new(), var_index: 1, scope_args: [], @@ -39,9 +52,33 @@ defmodule Dx.Defd.Compiler do rewrite_underscore?: false } - quoted = Enum.flat_map(exports, &compile_each_defd(&1, state)) + {quoted, state} = + Enum.flat_map_reduce(exports, state, fn def, state -> + {definitions, new_state} = compile_each_defd(def, state) + + state = %{state | used_defdps: new_state.used_defdps} + + {definitions, state} + end) + + quoted_used_private_functions = + case state.generated_private_functions ++ Enum.uniq(state.used_defdps) do + [] -> + [] + + defs -> + ast = Enum.map(defs, &Ast.local_fun_ref/1) + + [ + quote do + def unquote(:"__dx:suppress_unused_warnings__")() do + unquote(ast) + end + end + ] + end - {:__block__, [], quoted} + Ast.block(quoted_used_private_functions ++ quoted) end defp compile_prepare_arities(definitions) do @@ -51,6 +88,12 @@ defmodule Dx.Defd.Compiler do do: {name, arity} end + defp all_arities(definitions) do + Map.new(definitions, fn {{_name, arity} = def, %{defaults: defaults}} -> + {def, (arity - map_size(defaults))..arity} + end) + end + defp compile_each_defd({{name, arity} = def, def_meta}, state) do %{defaults: defaults, opts: opts} = def_meta debug_flags = List.wrap(opts[:debug]) @@ -75,6 +118,14 @@ defmodule Dx.Defd.Compiler do other end + all_args_with_defaults = + Enum.with_index(all_args, fn arg, i -> + case defaults do + %{^i => {meta, default}} -> {:\\, meta, [arg, default]} + %{} -> arg + end + end) + scope_args = Enum.with_index(scope_args, fn arg, i -> case defaults do @@ -105,10 +156,11 @@ defmodule Dx.Defd.Compiler do entrypoints = case Keyword.get(opts, :def, :warn) do :warn -> - Module.delete_definition(state.module, def) + for arity <- state.all_arities[def], + do: Module.delete_definition(state.module, {name, arity}) quote line: state.line do - Kernel.unquote(kind)(unquote(name)(unquote_splicing(all_args))) do + unquote(kind)(unquote(name)(unquote_splicing(all_args_with_defaults))) do IO.warn(""" Use Dx.Defd.load as entrypoint. """) @@ -120,20 +172,17 @@ defmodule Dx.Defd.Compiler do |> List.wrap() :no_warn -> - Module.delete_definition(state.module, def) + for arity <- state.all_arities[def], + do: Module.delete_definition(state.module, {name, arity}) quote line: state.line do - Kernel.unquote(kind)(unquote(name)(unquote_splicing(all_args))) do + unquote(kind)(unquote(name)(unquote_splicing(all_args_with_defaults))) do Dx.Defd.load!(unquote(name)(unquote_splicing(all_args))) end end |> strip_definition_context() |> List.wrap() - false -> - Module.delete_definition(state.module, def) - [] - :original -> [] @@ -157,9 +206,9 @@ defmodule Dx.Defd.Compiler do end end - if Enum.any?([:compiled_scope, :all], &(&1 in debug_flags)), do: Ast.p(scope) + definitions = entrypoints ++ [defd, final_args, scope] - entrypoints ++ [defd, final_args, scope] + {definitions, state} end defp append_arg({:when, meta, [args | guards]}, arg), @@ -194,7 +243,6 @@ defmodule Dx.Defd.Compiler do defp get_and_normalize_defd_and_scope({name, arity} = def, state) do {:v1, kind, meta, clauses} = Module.get_definition(state.module, def) - # |> IO.inspect(label: "ORIG #{name}/#{arity}\n") state = %{state | function: def, line: meta[:line] || state.line, rewrite_underscore?: true} @@ -477,6 +525,8 @@ defmodule Dx.Defd.Compiler do final_args_name = Util.final_args_name(fun_name) scope_name = Util.scope_name(fun_name) + state = Map.update!(state, :used_defdps, &[{fun_name, arity} | &1]) + {:ok, {:%, [line: line], [ @@ -590,6 +640,8 @@ defmodule Dx.Defd.Compiler do {fun_name, arity} in state.defds -> defd_name = Util.defd_name(fun_name) + state = Map.update!(state, :used_defdps, &[{fun_name, arity} | &1]) + normalize_call_args(args, state, fn args -> {defd_name, meta, args ++ [state.eval_var]} end) diff --git a/test/dx/defd_test.exs b/test/dx/defd_test.exs index c0a03615..c4aef21c 100644 --- a/test/dx/defd_test.exs +++ b/test/dx/defd_test.exs @@ -216,6 +216,142 @@ defmodule Dx.DefdTest do end end + describe "defdp" do + test "warns when unused" do + assert_stderr("function add/2 is unused", fn -> + defmodule UnusedTest do + import Dx.Defd + + defdp add(a, b), do: a + b + end + end) + end + + test "warns when default args are unused" do + assert_stderr("default values for the optional arguments in add/2 are never used", fn -> + defmodule UnusedDefaultArgsTest do + import Dx.Defd + + defd main() do + add(1, 2) + end + + defdp add(a, b \\ 1), do: a + b + end + end) + end + + test "does not warn when default args are used" do + refute_stderr("never used", fn -> + defmodule NoWarnUnusedDefaultArgsTest do + import Dx.Defd + + defd main() do + add(1) + end + + defdp add(a, b \\ 1), do: a + b + end + end) + end + + test "does not warn that clause always matches for default args" do + refute_stderr("always matches", fn -> + defmodule NoWarnAlwaysMatchingDefaultArgsTest do + import Dx.Defd + + defd main() do + add(1) + end + + defdp add(a, b \\ 1), do: a + b + end + end) + end + + test "does not warn when used by defd" do + refute_stderr("is unused", fn -> + defmodule UsedByDefdTest do + import Dx.Defd + + defd main() do + add(1, 2) + end + + defdp add(a, b), do: a + b + end + + assert load(UsedByDefdTest.main()) == {:ok, 3} + end) + end + + test "does not warn when used by non-defd" do + refute_stderr("is unused", fn -> + defmodule UsedByNonDefdTest do + import Dx.Defd + + def main() do + load(add(1, 2)) + end + + defdp add(a, b), do: a + b + end + + assert UsedByNonDefdTest.main() == {:ok, 3} + end) + end + + test "does not warn when def: :original and used by non-defd" do + refute_stderr("is unused", fn -> + defmodule DefOriginalUsedByNonDefdTest do + import Dx.Defd + + def main() do + load(add(1, 2)) + end + + @dx def: :original + defdp add(a, b), do: a + b + end + + assert DefOriginalUsedByNonDefdTest.main() == {:ok, 3} + end) + end + + test "does not warn when def: :no_warn and used by non-defd" do + refute_stderr("is unused", fn -> + defmodule DefNoWarnUsedByNonDefdTest do + import Dx.Defd + + def main() do + load(add(1, 2)) + end + + @dx def: :no_warn + defdp add(a, b), do: a + b + end + + assert DefNoWarnUsedByNonDefdTest.main() == {:ok, 3} + end) + end + + test "does not warn when referenced by defd" do + refute_stderr("is unused", fn -> + defmodule ReferencedByDefdTest do + import Dx.Defd + + defd main() do + Enum.map([1, 2, 3], &add_one/1) + end + + defdp add_one(a), do: a + 1 + end + + assert load!(ReferencedByDefdTest.main()) == [2, 3, 4] + end) + end + end + describe "data loading" do setup context do user_attrs = context[:user] || %{}