diff --git a/lib/mix/tasks/compile.unused.ex b/lib/mix/tasks/compile.unused.ex index 43a7fee..7dcb209 100644 --- a/lib/mix/tasks/compile.unused.ex +++ b/lib/mix/tasks/compile.unused.ex @@ -18,7 +18,7 @@ defmodule Mix.Tasks.Compile.Unused do ## Configuration - You can define used functions by adding `mfa` in `unused: [ignored: [⋯]]` + You can define used functions by adding pattern in `unused: [ignored: [⋯]]` in your project configuration: def project do @@ -33,6 +33,39 @@ defmodule Mix.Tasks.Compile.Unused do ] end + ### Patterns + + `unused` patterns are similar to the match specs from Erlang, but extends + their API to be much more flexible. Simplest possible patter is to match + exactly one function, which mean that we use 3-ary tuple with module, + function name, and arity as respective elements, ex.: + + [{Foo, :bar, 1}] + + This will match function `Foo.bar/1`, however often we want to use more + broad patterns, in such case there are few tricks we can use. First is + to use `:_` which will mean "wildcard" aka any value will match, ex.: + + [{:_, :child_spec, 1}] + + Will ignore all functions `child_spec/1` in your application (you probably + should add it, as `unused` is not able to notice that this function is used + even if it is used in any supervisor, as it will be dynamic call). + + In additiona to wildcard matches, which isn't often what we really want, we + can use regular expressions for module and function name or range for arity: + + [ + {:_, ~r/^__.+__\??$/, :_}, + {~r/^MyAppWeb\..*Controller/, :_, 2}, + {MyApp.Test, :foo, 1..2} + ] + + To make the ignore specification list less verbose there is also option to + omit last `:_`, i.e.: `{Foo, :bar, :_}` is the same as `{Foo, :bar}`, if you + want to ignore whole module, then you can just use `Foo` (it also works for + regular expressions). + ## Options - `severity` - severity of the reported messages, defaults to `hint`. @@ -49,6 +82,7 @@ defmodule Mix.Tasks.Compile.Unused do alias MixUnused.Tracer alias MixUnused.Filter + alias MixUnused.Exports @impl true def run(argv) do @@ -106,8 +140,7 @@ defmodule Mix.Tasks.Compile.Unused do compiler_name: "unused", message: "#{inspect(m)}.#{f}/#{a} is unused", severity: severity, - # TODO: Find a way to extract position of the function - position: nil, + position: meta.line, file: meta.file } |> print_diagnostic() @@ -121,7 +154,7 @@ defmodule Mix.Tasks.Compile.Unused do :ok = Application.load(app) Application.spec(app, :modules) - |> Enum.flat_map(&Filter.exports/1) + |> Enum.flat_map(&Exports.fetch/1) |> Map.new() end @@ -139,7 +172,17 @@ defmodule Mix.Tasks.Compile.Unused do defp severity("error"), do: :error defp print_diagnostic(diag) do - Mix.shell().info([level(diag.severity), diag.message]) + file = Path.relative_to_cwd(diag.file) + + Mix.shell().info([ + level(diag.severity), + diag.message, + "\n ", + file, + ?:, + Integer.to_string(diag.position), + "\n" + ]) diag end diff --git a/lib/mix_unused/exports.ex b/lib/mix_unused/exports.ex new file mode 100644 index 0000000..a270b81 --- /dev/null +++ b/lib/mix_unused/exports.ex @@ -0,0 +1,38 @@ +defmodule MixUnused.Exports do + @moduledoc false + + @type t() :: %{mfa() => metadata()} | [{mfa(), metadata()}] + @type metadata() :: %{ + file: String.t() + } + + @types ~w[function macro]a + + @spec fetch(module()) :: [{mfa(), metadata()}] + def fetch(module) do + # Check exported functions without loading modules as this could cause + # unexpected behaviours in case of `on_load` callbacks + with path when is_list(path) <- :code.which(module), + {:ok, {^module, data}} <- :beam_lib.chunks(path, [:attributes, :compile_info]), + {:docs_v1, _anno, _lang, _format, _mod_doc, _meta, docs} <- + Code.fetch_docs(to_string(path)) do + callbacks = data[:attributes] |> Keyword.get(:behaviour, []) |> callbacks() + source = Keyword.get(data[:compile_info], :source, "nofile") |> to_string() + + for {{type, name, arity}, anno, _sig, _doc, meta} when type in @types <- docs, + not Map.get(meta, :export, false), + {name, arity} not in callbacks do + line = :erl_anno.line(anno) + {{module, name, arity}, %{file: source, line: line}} + end + else + _ -> [] + end + end + + defp callbacks(behaviours) do + # We need to load behaviours as there is no other way to get list of + # callbacks than to call `behaviour_info/1` + Enum.flat_map(behaviours, & &1.behaviour_info(:callbacks)) + end +end diff --git a/lib/mix_unused/filter.ex b/lib/mix_unused/filter.ex index ace57e7..0338641 100644 --- a/lib/mix_unused/filter.ex +++ b/lib/mix_unused/filter.ex @@ -3,9 +3,7 @@ defmodule MixUnused.Filter do import Kernel, except: [match?: 2] - @type metadata() :: %{ - file: String.t() - } + alias MixUnused.Exports @type module_pattern() :: module() | Regex.t() | :_ @type function_pattern() :: atom() | Regex.t() | :_ @@ -84,13 +82,14 @@ defmodule MixUnused.Filter do [{{Foo, :bar, 1}, %{}}] ``` """ - @spec reject_matching(exports :: %{mfa() => metadata()}, patterns :: [pattern()]) :: - [{mfa(), metadata()}] + @spec reject_matching(exports :: Exports.t(), patterns :: [pattern()]) :: + Exports.t() def reject_matching(exports, patterns) do filters = Enum.map(patterns, fn {_m, _f, _a} = entry -> entry {m, f} -> {m, f, :_} + {m} -> {m, :_, :_} m -> {m, :_, :_} end) @@ -116,40 +115,4 @@ defmodule MixUnused.Filter do defp arity_match?(value, value), do: true defp arity_match?(_.._ = range, value), do: value in range defp arity_match?(_, _), do: false - - # These are functions generated by Elixir and Erlang, this list probably - # should not grow. - @built_ins [ - __info__: 1, - __struct__: 0, - __struct__: 1, - __impl__: 1, - module_info: 0, - module_info: 1, - behaviour_info: 1 - ] - - @spec exports(module()) :: [{mfa(), metadata()}] - def exports(module) do - # Check exported functions without loading modules as this could cause - # unexpected behaviours in case of `on_load` callbacks - with path when is_list(path) <- :code.which(module), - {:ok, {^module, data}} <- :beam_lib.chunks(path, [:exports, :attributes, :compile_info]) do - callbacks = data[:attributes] |> Keyword.get(:behaviour, []) |> callbacks() - source = Keyword.get(data[:compile_info], :source, "nofile") |> to_string() - - for {name, arity} = func <- data[:exports], - func not in @built_ins, - func not in callbacks, - do: {{module, name, arity}, %{file: source}} - else - _ -> [] - end - end - - defp callbacks(behaviours) do - # We need to load behaviours as there is no other way to get list of - # callbacks than to call `behaviour_info/1` - Enum.flat_map(behaviours, & &1.behaviour_info(:callbacks)) - end end diff --git a/lib/mix_unused/tracer.ex b/lib/mix_unused/tracer.ex index aa1649d..bf3e015 100644 --- a/lib/mix_unused/tracer.ex +++ b/lib/mix_unused/tracer.ex @@ -19,14 +19,27 @@ defmodule MixUnused.Tracer do GenServer.start_link(__MODULE__, [], name: __MODULE__) end + @remote ~w[ + imported_function + remote_function + imported_macro + remote_macro + ]a + def trace({action, _meta, module, name, arity}, env) - when action in ~w[imported_function remote_function]a do + when action in @remote do add_call(module, name, arity, env) :ok end - def trace({:local_function, _meta, name, arity}, env) do + @local ~w[ + local_function + local_macro + ]a + + def trace({action, _meta, name, arity}, env) + when action in @local do add_call(env.module, name, arity, env) :ok diff --git a/test/mix_unused/tracer_test.exs b/test/mix_unused/tracer_test.exs index 2694d86..c6cfb14 100644 --- a/test/mix_unused/tracer_test.exs +++ b/test/mix_unused/tracer_test.exs @@ -61,4 +61,37 @@ defmodule MixUnused.TracerTest do test "contains information about called imported function" do assert {String, :first, 1} in @subject.get_calls() end + + @code (quote do + defmacro foo(), do: :ok + + def test do + foo() + end + end) + test "contains information about called local macros", ctx do + assert {ctx.module_name, :foo, 0} in @subject.get_calls() + end + + @code (quote do + require Logger + + def test do + Logger.info("foo") + end + end) + test "contains information about called remote macros" do + assert {Logger, :info, 1} in @subject.get_calls() + end + + @code (quote do + import Logger + + def test do + info("foo") + end + end) + test "contains information about called imported macros" do + assert {Logger, :info, 1} in @subject.get_calls() + end end diff --git a/test/mix_unused_test.exs b/test/mix_unused_test.exs index b1f9c6a..0601eef 100644 --- a/test/mix_unused_test.exs +++ b/test/mix_unused_test.exs @@ -36,8 +36,8 @@ defmodule MixUnusedTest do in_fixture("unclean", fn -> assert {{:ok, diagnostics}, output} = run(:unclean, "compile") - assert has_diagnostics_for?(diagnostics, Foo, :foo, 0) assert output =~ "Foo.foo/0 is unused" + assert has_diagnostics_for?(diagnostics, Foo, :foo, 0) end) end end