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

Conversation

enerdgumen
Copy link

@enerdgumen enerdgumen commented Nov 11, 2022

Hello @hauleth, we at Prima.it are using mix_unused in our Elixir services.
This is a simplified PR that excludes all the discovery modules included in the previous one.

We found some issues with the current implementation, which marked as unused lots of code, depending on the application's design.
We explored a different approach which could make the results more reliable when used properly.

Although the PR is a bit fat, most of the changes are additive and the rest is of course backward compatible.

Summary

  • add the MixUnused.Analyzers.Unreachable analyzer, alternative to the Unused one;
  • add paths configuration option to report only functions defined in such paths (i.e. to hide code made only for tests purposes);
  • add limit configuration option to report only a limited number of functions;
  • make the checks options overridable, so that the user can enable the "unreachable" analyzer;
  • fix usage detection of public functions called with default arguments;
  • detect some generated functions and omit them from report;
  • log debug information if the MIX_UNUSED_DEBUG env var is set to true.

The "unreachable" analyzer

It just analyses the calls graph finding all the functions that are reachable from a specific set of well-known used functions. This makes the analysis simple and deterministic.

The analyzer requires the user to specify the set of usages, or "usages-discovery" (modules that try to discover dynamically-called functions - for instance by reading the application config or by analyze the source code).

Unreachable has some differences from Unused:

  • Unreachable does not assume that all behaviour implementers are used by default.
  • Unreachable does not report generated functions.
  • Unreachable, by default, does not report unused functions called transitively (to make the report cleaner).

Please look at the README.md and guides/unreachable-analyzer.md for further details.

@enerdgumen
Copy link
Author

No feedback received, so long and thanks for all the fish!

@enerdgumen enerdgumen closed this Feb 9, 2023
@cpiemontese
Copy link

Hey @hauleth! 👋

I've taken over maintaining our (Prima.it) fork and I was wondering if this PR could still be useful to merge
I know it's been sometime but hopefully it's still valuable 😄

@hauleth
Copy link
Owner

hauleth commented Jun 19, 2024

I will take a look today and will see.

@cpiemontese
Copy link

Hello! Any news?

@hauleth
Copy link
Owner

hauleth commented Jul 10, 2024

I will look at it today. I was highly occupied with my $WORK and I had little time to look at this in the meantime.

Copy link
Owner

@hauleth hauleth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to view it as a 2 separate PRs - one that adds option for more configuration options, and separate with the unreachable analyser.

When I was creating mix_unused the idea behind MixUnused.Analyze was that it would support externally implemented analysers. So this Unreachable analyser could be library like mix_unused_unreachable. I would need to think whether this particular analyser is useful outside of projects like Phoenix to make it part of the core.

Don't understand me wrong, it looks useful, it is just that additional code will push maintenance burden to me, and as you may have noticed, I often have other stuff that I need to maintain and I would want to avoid having such issues stalled again.

@cpiemontese
Copy link

I completely understand and thanks for looking into this 😄

I will open separate PRs and once we actually have the one on the unreachable analyzer we can see if it's actually useful to integrate it in the core or may extract it in a library, does that make sense?

@hauleth
Copy link
Owner

hauleth commented Jul 10, 2024

Yes, that would be awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants