From a28a339d916a146f1d74fbf783e6fe27589ee6a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Niemier?= <lukasz@niemier.pl>
Date: Mon, 27 Sep 2021 14:44:55 +0200
Subject: [PATCH 1/3] ft: add support for macros

---
 lib/mix/tasks/compile.unused.ex | 12 +++++++++---
 lib/mix_unused/filter.ex        | 26 ++++++++++----------------
 lib/mix_unused/tracer.ex        | 17 +++++++++++++++--
 test/mix_unused/tracer_test.exs | 33 +++++++++++++++++++++++++++++++++
 test/mix_unused_test.exs        |  2 +-
 5 files changed, 68 insertions(+), 22 deletions(-)

diff --git a/lib/mix/tasks/compile.unused.ex b/lib/mix/tasks/compile.unused.ex
index 43a7fee..048a313 100644
--- a/lib/mix/tasks/compile.unused.ex
+++ b/lib/mix/tasks/compile.unused.ex
@@ -106,8 +106,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()
@@ -139,7 +138,14 @@ defmodule Mix.Tasks.Compile.Unused do
   defp severity("error"), do: :error
 
   defp print_diagnostic(diag) do
-    Mix.shell().info([level(diag.severity), diag.message])
+    Mix.shell().info([
+      level(diag.severity),
+      diag.message,
+      "\n    ",
+      diag.file,
+      ?:,
+      Integer.to_string(diag.position)
+    ])
 
     diag
   end
diff --git a/lib/mix_unused/filter.ex b/lib/mix_unused/filter.ex
index ace57e7..194a304 100644
--- a/lib/mix_unused/filter.ex
+++ b/lib/mix_unused/filter.ex
@@ -117,31 +117,25 @@ defmodule MixUnused.Filter do
   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
-  ]
+  @types ~w[function macro]a
 
   @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
+         {: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 {name, arity} = func <- data[:exports],
-          func not in @built_ins,
-          func not in callbacks,
-          do: {{module, name, arity}, %{file: source}}
+      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
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

From 44264a2b30e91b038194fd7fa2d43f2ba0740947 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Niemier?= <lukasz@niemier.pl>
Date: Mon, 27 Sep 2021 14:57:16 +0200
Subject: [PATCH 2/3] fix: print path to file as a relative path

---
 lib/mix/tasks/compile.unused.ex | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/mix/tasks/compile.unused.ex b/lib/mix/tasks/compile.unused.ex
index 048a313..d8c781c 100644
--- a/lib/mix/tasks/compile.unused.ex
+++ b/lib/mix/tasks/compile.unused.ex
@@ -142,9 +142,10 @@ defmodule Mix.Tasks.Compile.Unused do
       level(diag.severity),
       diag.message,
       "\n    ",
-      diag.file,
+      Path.relative_to_cwd(diag.file),
       ?:,
-      Integer.to_string(diag.position)
+      Integer.to_string(diag.position),
+      "\n"
     ])
 
     diag

From a16b2a48fb458341e4e722c18f4aae588b0bc274 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Niemier?= <lukasz@niemier.pl>
Date: Mon, 27 Sep 2021 15:56:30 +0200
Subject: [PATCH 3/3] docs: improve documentation of `compile.unused`

---
 lib/mix/tasks/compile.unused.ex | 42 ++++++++++++++++++++++++++++++---
 lib/mix_unused/exports.ex       | 38 +++++++++++++++++++++++++++++
 lib/mix_unused/filter.ex        | 39 ++++--------------------------
 3 files changed, 81 insertions(+), 38 deletions(-)
 create mode 100644 lib/mix_unused/exports.ex

diff --git a/lib/mix/tasks/compile.unused.ex b/lib/mix/tasks/compile.unused.ex
index d8c781c..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
@@ -120,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
 
@@ -138,11 +172,13 @@ defmodule Mix.Tasks.Compile.Unused do
   defp severity("error"), do: :error
 
   defp print_diagnostic(diag) do
+    file = Path.relative_to_cwd(diag.file)
+
     Mix.shell().info([
       level(diag.severity),
       diag.message,
       "\n    ",
-      Path.relative_to_cwd(diag.file),
+      file,
       ?:,
       Integer.to_string(diag.position),
       "\n"
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 194a304..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,34 +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
-
-  @types ~w[function macro]a
-
-  @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, [: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