-
Notifications
You must be signed in to change notification settings - Fork 16
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
False positives when detecting unused imports (RabbitMQ codebase) #30
Comments
@mkuratczyk Thanks for reporting! While we work on a fix, you can prepend the problematic line with something like:
Which will silent the linter (so you can run it from CLI periodically or in CI) @alanz |
|
ELP generally assumes header files are self-contained - this is violated in the 2nd case you describe since The "proper" fix from the view point of ELP and self-contained headers would be to move |
yeah, that makes sense. we can solve the second case on our side |
Describe the bug
I tried to use
elp
to remove unused imports across RabbitMQ codebase.However, there are two imports that I need to restore afterwards, for RabbitMQ to start.
To Reproduce**
The imports that are incorrectly removed are here:
https://github.com/rabbitmq/rabbitmq-server/blob/main/deps/rabbit/src/rabbit_amqp_util.erl#L9
Here the problem I think is related to the fact that
amqp10_framing.hrl
is a generated file.rabbit_amqp_util.erl
includesrabbit_amqp.hrl
which in turn includesamqp10_common/include/amqp10_framing.hrl
but this file doesn't exist if you start from a fresh repo. If you runmake start-cluster
first and then run the sameelp
command,rabbit_amqp_uti.erl
is not modified byelp
.https://github.com/rabbitmq/rabbitmq-server/blob/main/deps/rabbitmq_peer_discovery_consul/src/rabbitmq_peer_discovery_consul_health_check_helper.erl#L18
In this case, the need for the removed include file is "hidden" by a macro -
?CONFIG_MAPPING
expands to code that uses thepeer_discovery_config_entry_meta
record.Context
elp version
):elp 1.1.0+build-2024-05-17
Thank you!
The text was updated successfully, but these errors were encountered: