search

AdRoll / rebar3_hank

The Erlang Dead Code Cleaner
MIT License
68 stars 9 forks source link

New Rule: Unused configuration options #72

Closed elbrujohalcon closed 3 years ago

elbrujohalcon commented 3 years ago

Name

Unused Configuration Options

Brief Description

This rule should detect environment parameters (the ones set in .app.src and .config files) that are not consumed through application:get_env(…) anywhere.

Reasoning

These are usually leftovers from a previous time when they were used as feature flags or when they configured things in the app that are no longer configurable.

Refactoring Proposal

Just remove the environment params.

Implementation Notes

This is not a simple rule to implement, for various reasons:

  1. Hank will need to start parsing .config and .app.src files and ktn_dodger doesn't parse them yet. It might be possible to use file:consult/1, tho.
  2. Hank will need to be careful in case someone uses application:get_env(…) indirectly. For instance, with a variable (e.g. application:get_env(my_app, Param, Default)) or even through erlang:apply(application, get_env, […]). Maybe it would be better not to look for application:get_env(…) at all and just verify if the atoms corresponding to the properties are present in the code (i.e. regardless of what they're used for).
pbrudnick commented 3 years ago

More deeply:

elbrujohalcon commented 3 years ago

More deeply:

  • application:get_all_env/0, application:get_all_env/1: an option param could be used from the list of param/values it returns.

  • application:set_env/1, application:set_env/2: could we remove an option which will be set? (same for unset_env)

Exactly. But in all those scenarios the atom corresponding to the config key will still be used in the code. That's why we should only mark as unused the atoms that are keys in config files but not present anywhere in the code.

pbrudnick commented 3 years ago

This rule will ignore:

It should consider the rebar.config.script and <app-name>.app.src.script files.