Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ft: add an alternative analyzer to detect unused code #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 78 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,40 @@ end
Then you just need to run `mix compile` or `mix compile --force` as usual
and unused hints will be added to the end of the output.

### Cleaning your project

The tool keeps track of the calls traced during the compilation. The first time make sure that there is no compiled code:

```shell
mix clean
```

Doing so all the application code is recompiled and the calls are traced properly.

It is recommended to also perform a clean in the CI when the build does not start from a fresh project, for instance:

```shell
mix do clean, compile --all-warnings --warnings-as-errors
```

Please make sure you don't improperly override the clean task with an alias:

```elixir
def project do
[
# ⋯
aliases: [
# don't do this:
clean: "deps.unlock --unused",

# do this:
clean: ["clean", "deps.unlock --unused"],
],
# ⋯
]
end
```

### Warning

This isn't perfect solution and this will not find dynamic calls in form of:
Expand All @@ -62,25 +96,64 @@ So this mean that, for example, if you have custom `child_spec/1` definition
then `mix unused` can return such function as unused even when you are using
that indirectly in your supervisor.

This issue can be mitigated using the `Unreachable` check, explained below.

### Configuration

You can define used functions by adding `mfa` in `unused: [ignored: [⋯]]`
in your project configuration:
You can configure the tool using the `unused` options in the project configuration.
The following is the default configuration.

```elixir
def project do
[
# ⋯
unused: [
ignore: [
{MyApp.Foo, :child_spec, 1}
]
checks: [
# find public functions that could be private
MixUnused.Analyzers.Private,
# find unused public functions
MixUnused.Analyzers.Unused,
# find functions called only recursively
MixUnused.Analyzers.RecursiveOnly
],
ignore: [],
limit: nil,
paths: nil,
severity: :hint
],
# ⋯
]
end
```

It supports the following options:

- `checks`: list of analyzer modules to use.

In alternative to the default set, you can use the [MixUnused.Analyzers.Unreachable](`MixUnused.Analyzers.Unreachable`) check (see the specific [guide](guides/unreachable-analyzer.md)).

- `ignore`: list of ignored functions, example:

```elixir
[
{:_, ~r/^__.+__\??$/, :_},
{~r/^MyAppWeb\..*Controller/, :_, 2},
{MyApp.Test, :foo, 1..2}
]
```

See the [Mix.Tasks.Compile.Unused](`Mix.Tasks.Compile.Unused`) task for further details.

- `limit`: max number of results to report (available also as the command option `--limit`).

- `paths`: report only functions defined in such paths.

Useful to restrict the reported functions only to the functions defined in specific paths
(i.e. set `paths: ["lib"]` to ignore functions defined in the `tests` folder).

- `severity`: severity of the reported messages.
Allowed levels are `:hint`, `:information`, `:warning`, and `:error`.

## Copyright and License

Copyright © 2021 by Łukasz Niemier
Expand Down
60 changes: 60 additions & 0 deletions guides/unreachable-analyzer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
The [MixUnused.Analyzers.Unreachable](`MixUnused.Analyzers.Unreachable`) check analyses the call graph by visiting all the functions reachable from a well-known set of functions, that act as starting points.

All the non reachable functions are considered unused. By default only the functions that have no caller are reported to the user.

## Configuration

It has a specific configuration:

```elixir
[
checks: [
{MixUnused.Analyzers.Unreachable,
%{
usages: [
{Foo, :bar, 1},
# ...
]
}}
]
]
```

In the example above, `Foo.bar/1` is declared as "used", so the check will consume all the functions reachable from it.

In addition to the user declared functions, the analyzer uses a set of "discovery modules", defined with the option `usages_discovery`:

```elixir
[
checks: [
{MixUnused.Analyzers.Unreachable,
%{
usages_discovery: [
MyDiscovery,
# ...
]
}}
]
]
```

A discovery module implements the [MixUnused.Analyzers.Unreachable.Usages](`MixUnused.Analyzers.Unreachable.Usages`) behaviour and it's called during the analysis to try to discover functions that should be considered as used.

Some discovery modules are included by default: check them under the `MixUnused.Analyzers.Unreachable.Usages` namespace (i.e. [PhoenixDiscovery](`MixUnused.Analyzers.Unreachable.Usages.PhoenixDiscovery`)).

All these modules are not perfect and can report false positives, but in summary they help to identify dynamically used code automatically.

### Other options

* `report_transitively_unused`: if true the analyzer reports also the unused
functions that are only called by other unused functions (default to false).

For instance, if `a()` calls `b()` that calls `c()`, by default only `a()` is reported as unused. Additionally, if `a()` is explicitly ignored or is defined out of the configured `paths`, nothing is reported.

## Corner cases

* Behaviours implementers are not considered used by default.
* Functions generated by a macro are not reported, since they generally are out
of our control.
* Structs must be used in the code (i.e. `%MyStruct{}` somewhere) or declared
(i.e. `ignore: [{MyStruct, :__struct__, 0}]`).
14 changes: 14 additions & 0 deletions lib/mix/tasks/compile.unused.ex
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,9 @@ defmodule Mix.Tasks.Compile.Unused do

config.checks
|> MixUnused.Analyze.analyze(data, all_functions, config)
|> filter_files_in_paths(config.paths)
|> Enum.sort_by(&{&1.file, &1.position, &1.details.mfa})
|> limit_results(config.limit)
|> tap_all(&print_diagnostic/1)
|> case do
[] ->
Expand Down Expand Up @@ -181,6 +183,18 @@ defmodule Mix.Tasks.Compile.Unused do
defp normalise_cache(map) when is_map(map), do: {:v0, map}
defp normalise_cache(_), do: %{}

defp filter_files_in_paths(diags, nil), do: diags

defp filter_files_in_paths(diags, paths) do
Enum.filter(diags, fn %Diagnostic{file: file} ->
[root | _] = file |> Path.relative_to_cwd() |> Path.split()
root in paths
end)
end

defp limit_results(diags, nil), do: diags
defp limit_results(diags, limit), do: Enum.take(diags, limit)

defp print_diagnostic(%Diagnostic{details: %{mfa: {_, :__struct__, 1}}}),
do: nil

Expand Down
23 changes: 18 additions & 5 deletions lib/mix_unused/analyze.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,34 @@ defmodule MixUnused.Analyze do

alias Mix.Task.Compiler.Diagnostic

alias MixUnused.Config
alias MixUnused.Exports
alias MixUnused.Tracer

@type config :: map()
@type analyzer :: module() | {module(), config()}

@callback message() :: String.t()
@callback analyze(Tracer.data(), Exports.t()) :: Exports.t()
@callback analyze(Tracer.data(), Exports.t(), config()) :: Exports.t()

@spec analyze(module() | [module()], Tracer.data(), Exports.t(), map()) ::
Diagnostic.t()
@spec analyze(
analyzer() | [analyzer()],
Tracer.data(),
Exports.t(),
Config.t()
) ::
[Diagnostic.t()]
def analyze(analyzers, data, all_functions, config) when is_list(analyzers),
do: Enum.flat_map(analyzers, &analyze(&1, data, all_functions, config))

def analyze(analyzer, data, all_functions, config) when is_atom(analyzer) do
def analyze(analyzer, data, all_functions, config) when is_atom(analyzer),
do: analyze({analyzer, %{}}, data, all_functions, config)

def analyze({analyzer, analyzer_config}, data, all_functions, config) do
message = analyzer.message()

for {mfa, meta} = desc <- analyzer.analyze(data, all_functions) do
for {mfa, meta} = desc <-
analyzer.analyze(data, all_functions, analyzer_config) do
%Diagnostic{
compiler_name: "unused",
message: "#{signature(desc)} #{message}",
Expand Down
2 changes: 1 addition & 1 deletion lib/mix_unused/analyzers/private.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule MixUnused.Analyzers.Private do
def message, do: "should be private (is not used outside defining module)"

@impl true
def analyze(data, all_functions) do
def analyze(data, all_functions, _config) do
data = Map.new(data)

for {{_, f, _} = mfa, meta} = desc <- all_functions,
Expand Down
2 changes: 1 addition & 1 deletion lib/mix_unused/analyzers/recursive_only.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule MixUnused.Analyzers.RecursiveOnly do
def message, do: "is called only recursively"

@impl true
def analyze(data, all_functions) do
def analyze(data, all_functions, _config) do
non_rec_calls =
for {mod, calls} <- data,
{{m, f, a} = mfa, %{caller: {call_f, call_a}}} <- calls,
Expand Down
51 changes: 51 additions & 0 deletions lib/mix_unused/analyzers/unreachable.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
defmodule MixUnused.Analyzers.Unreachable do
@moduledoc """
Reports all the exported functions that are not reachable from a set of well-known used functions.
"""

alias MixUnused.Analyzers.Calls
alias MixUnused.Analyzers.Unreachable.Config
alias MixUnused.Analyzers.Unreachable.Usages
alias MixUnused.Meta

@behaviour MixUnused.Analyze

@impl true
def message, do: "is unreachable"

@impl true
def analyze(data, exports, config) do
config = Config.cast(config)
graph = Calls.calls_graph(data, exports)

usages =
Usages.usages(config, %Usages.Context{calls: graph, exports: exports})

reachables = graph |> Graph.reachable(usages) |> MapSet.new()
called_at_compile_time = Calls.called_at_compile_time(data, exports)

for {mfa, _meta} = call <- exports,
filter_transitive_call?(config, graph, mfa),
filter_generated_function?(call),
mfa not in usages,
mfa not in reachables,
mfa not in called_at_compile_time,
into: %{},
do: call
end

@spec filter_transitive_call?(Config.t(), Graph.t(), mfa()) :: boolean()
defp filter_transitive_call?(
%Config{report_transitively_unused: report_transitively_unused},
graph,
mfa
) do
report_transitively_unused or Graph.in_degree(graph, mfa) == 0
end

# Clause to detect an unused struct (it is generated)
defp filter_generated_function?({{_f, :__struct__, _a}, _meta}), do: true
# Clause to ignore all generated functions
defp filter_generated_function?({_mfa, %Meta{generated: true}}), do: false
defp filter_generated_function?(_), do: true
end
21 changes: 21 additions & 0 deletions lib/mix_unused/analyzers/unreachable/config.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
defmodule MixUnused.Analyzers.Unreachable.Config do
@moduledoc """
Configuration specific to the [Unreachable](`MixUnused.Analyzers.Unreachable`) analyzer.
"""
alias MixUnused.Filter

@type t :: %__MODULE__{
usages: [Filter.pattern()],
usages_discovery: [module()],
report_transitively_unused: boolean()
}

defstruct usages: [],
usages_discovery: [],
report_transitively_unused: false

@spec cast(Enum.t()) :: t()
def cast(map) do
struct!(__MODULE__, map)
end
end
Loading
Loading