bnjbvr / cargo-machete

Remove unused Rust dependencies with this one weird trick!
MIT License
784 stars 28 forks source link

Detect dependencies required for features #58

Closed nanocryk closed 1 year ago

nanocryk commented 1 year ago

It is necessary to have a crate as dependency if a feature of the package enables a feature of the dependency, even if it is not used in the code.

Exemple: https://github.com/PureStake/moonbeam/blob/master/node/Cargo.toml#L36 (moonbeam-service is flagged as unused).

Would be great to look at features and don't flag dependencies as unused if they are used in features.

If that sounds like a good idea I can probably work on a PR for it :)

bnjbvr commented 1 year ago

Hi! thanks for opening an issue about this. cargo-machete does not detect which sets of features are enabled in the current build, so conceptually it's as if all the features were all enabled all the time. Based on that, we could make heuristics, but now that means that we can have false positives if some features become obsolete / are unused or not enabled all the time, so the mechanism you're suggesting would basically become another way to express... an ignore list. I've myself ran into that in the past, and decided to use ignore to not add too much conceptual complexity into cargo-machete itself. Would it make sense to use the existing ignore mechanism instead, for this kind of use case?

nanocryk commented 1 year ago

It is more at a cargo level. Even if a feature is not enabled, cargo will complain if a feature lists crate/feature of an unknown crate (that should be listen as optional). By being listed in there, it is not unused.

I don't think it would add much complexity. Iterate over all features and what they enable, detect crate(?)/feature, and don't report them as unused. I can make a PR for it and we'll see if it adds too much complexity.

nanocryk commented 1 year ago

Hmm by looking for exemple crates online it seems that what we have is in fact a rare pattern. I guess using ignore is fine in that case.

bnjbvr commented 1 year ago

Thanks for looking into it! We can take a look back at this if it turns out to be(come) common.