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

Add more configuration options #48

Open
wants to merge 2 commits 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
26 changes: 19 additions & 7 deletions .github/workflows/elixir.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,38 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
otp_version: ['24', '25', '26']
elixir_version: ['1.14', '1.15', '1.16']
version: [
{ elixir: '1.14', otp: '23' },
{ elixir: '1.14', otp: '24' },
{ elixir: '1.14', otp: '25' },
{ elixir: '1.15', otp: '24' },
{ elixir: '1.15', otp: '25' },
{ elixir: '1.15', otp: '26' },
{ elixir: '1.16', otp: '24' },
{ elixir: '1.16', otp: '25' },
{ elixir: '1.16', otp: '26' },
{ elixir: '1.17', otp: '25' },
{ elixir: '1.17', otp: '26' },
{ elixir: '1.17', otp: '27' }
]
steps:
- uses: actions/checkout@v3
- name: Set up Elixir
uses: erlef/setup-beam@v1
with:
otp-version: ${{ matrix.otp_version }}
elixir-version: ${{ matrix.elixir_version }}
otp-version: ${{ matrix.version.otp }}
elixir-version: ${{ matrix.version.elixir }}
- name: Restore dependencies cache
uses: actions/cache@v3
with:
path: deps
key: ${{ runner.os }}-otp-${{ matrix.otp_version }}-mix-${{ hashFiles('**/mix.lock') }}
restore-keys: ${{ runner.os }}-otp-${{ matrix.otp_version }}-mix-
key: ${{ runner.os }}-otp-${{ matrix.version.otp }}-mix-${{ hashFiles('**/mix.lock') }}
restore-keys: ${{ runner.os }}-otp-${{ matrix.version.otp }}-mix-
- name: Install dependencies
run: mix do deps.get, deps.compile
- name: Run tests
run: mix test --cover
- uses: codecov/codecov-action@v3
with:
files: ./coverage.xml
flags: elixir-${{ matrix.elixir_version }},otp-${{ matrix.otp_version }}
flags: elixir-${{ matrix.version.elixir }},otp-${{ matrix.version.otp }}
79 changes: 74 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 @@ -64,23 +98,58 @@ that indirectly in your supervisor.

### 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.

- `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
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
2 changes: 1 addition & 1 deletion lib/mix_unused/analyzers/unused.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule MixUnused.Analyzers.Unused do
def message, do: "is unused"

@impl true
def analyze(data, possibly_uncalled) do
def analyze(data, possibly_uncalled, _config) do
graph = Graph.new(type: :directed)

uncalled_funcs = MapSet.new(possibly_uncalled, fn {mfa, _} -> mfa end)
Expand Down
16 changes: 16 additions & 0 deletions lib/mix_unused/config.ex
Original file line number Diff line number Diff line change
@@ -1,20 +1,33 @@
defmodule MixUnused.Config do
@moduledoc false

@type t :: %__MODULE__{
checks: [MixUnused.Analyze.analyzer()],
ignore: [mfa()],
limit: integer() | nil,
paths: [String.t()] | nil,
severity: :hint | :information | :warning | :error,
warnings_as_errors: boolean()
}

defstruct checks: [
MixUnused.Analyzers.Private,
MixUnused.Analyzers.Unused,
MixUnused.Analyzers.RecursiveOnly
],
ignore: [],
limit: nil,
paths: nil,
severity: :hint,
warnings_as_errors: false

@options [
limit: :integer,
severity: :string,
warnings_as_errors: :boolean
]

@spec build([binary], nil | maybe_improper_list | map) :: MixUnused.Config.t()
def build(argv, config) do
{opts, _rest, _other} = OptionParser.parse(argv, strict: @options)

Expand All @@ -27,12 +40,15 @@ defmodule MixUnused.Config do
config
|> maybe_set(:checks, mix_config[:checks])
|> maybe_set(:ignore, mix_config[:ignore])
|> maybe_set(:limit, mix_config[:limit])
|> maybe_set(:paths, mix_config[:paths])
|> maybe_set(:severity, mix_config[:severity])
|> maybe_set(:warnings_as_errors, mix_config[:warnings_as_errors])
end

defp extract_opts(%__MODULE__{} = config, opts) do
config
|> maybe_set(:limit, opts[:limit])
|> maybe_set(:severity, opts[:severity], &severity/1)
|> maybe_set(:warnings_as_errors, opts[:warnings_as_errors])
end
Expand Down
11 changes: 7 additions & 4 deletions lib/mix_unused/exports.ex
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
defmodule MixUnused.Exports do
@moduledoc false
@moduledoc """
Detects the functions exported by the application.
In Elixir slang, an "exported" function is called "public" function.
"""

alias MixUnused.Meta

@type t() :: %{mfa() => Meta.t()} | [{mfa(), Meta.t()}]
@type t() :: %{mfa() => Meta.t()}

@types ~w[function macro]a

Expand All @@ -17,8 +20,8 @@ defmodule MixUnused.Exports do
|> Map.new()
end

@spec fetch(module()) :: t()
def fetch(module) do
@spec fetch(module()) :: [{mfa(), Meta.t()}]
defp 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),
Expand Down
42 changes: 27 additions & 15 deletions lib/mix_unused/filter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,35 @@ defmodule MixUnused.Filter do
@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 when is_atom(m) -> {m, :_, :_}
%Regex{} = m -> {m, :_, :_}
cb when is_function(cb) -> cb
end)

Enum.reject(exports, fn {func, meta} ->
Enum.any?(filters, &mfa_match?(&1, func, meta))
Map.reject(exports, matcher(patterns))
end

@spec filter_matching(exports :: Exports.t(), patterns :: [pattern()]) ::
Exports.t()
def filter_matching(exports, patterns) do
Map.filter(exports, matcher(patterns))
end

@spec matcher(patterns :: [pattern()]) :: ({mfa(), Meta.t()} -> boolean())
defp matcher(patterns) do
filters = normalize_filter_patterns(patterns)

fn {mfa, meta} -> Enum.any?(filters, &mfa_match?(&1, mfa, meta)) end
end

@spec normalize_filter_patterns(patterns :: [pattern()]) :: [pattern()]
defp normalize_filter_patterns(patterns) do
Enum.map(patterns, fn
{_m, _f, _a} = entry -> entry
{m, f} -> {m, f, :_}
{m} -> {m, :_, :_}
m when is_atom(m) -> {m, :_, :_}
%Regex{} = m -> {m, :_, :_}
cb when is_function(cb) -> cb
end)
|> Map.new()
end

@spec mfa_match?(mfa(), pattern(), Meta.t()) :: boolean()
@spec mfa_match?(pattern(), mfa(), Meta.t()) :: boolean()
defp mfa_match?({pmod, pname, parity}, {fmod, fname, farity}, _meta) do
match?(pmod, fmod) and match?(pname, fname) and arity_match?(parity, farity)
end
Expand All @@ -58,6 +70,6 @@ defmodule MixUnused.Filter do

defp arity_match?(:_, _value), do: true
defp arity_match?(value, value), do: true
defp arity_match?(_.._ = range, value), do: value in range
defp arity_match?(_.._//_ = range, value), do: value in range
defp arity_match?(_, _), do: false
end
Loading
Loading