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

json output: unused_deps may be non-empty even if all dependencies are used #78

Closed
nagisa opened this issue Oct 30, 2020 · 6 comments · Fixed by #79
Closed

json output: unused_deps may be non-empty even if all dependencies are used #78

nagisa opened this issue Oct 30, 2020 · 6 comments · Fixed by #79

Comments

@nagisa
Copy link

nagisa commented Oct 30, 2020

{"success":true,"unused_deps":{"some crate":{"manifest_path":"/some/location","normal":[],"development":[],"build":[]}},"note":null}

is what I see when I build a workspace with many crates. The unused_deps map gets populated with further crates if and when they contain unused crates.

I think it might be because the some_crate is the only crate that has cargo-udeps.ignore specified in it.

@est31
Copy link
Owner

est31 commented Oct 30, 2020

Might be one of the known false positives? The cli version shows a warning that this is possible.

@nagisa
Copy link
Author

nagisa commented Oct 30, 2020

Note that there are no reported unused dependencies, all of the normal, development and build arrays for that crate are empty. Furthermore json contains both success: true and udeps' exit code is 0.

@nagisa
Copy link
Author

nagisa commented Oct 30, 2020

All in all it is fairly low prio to fix, just something I noticed and found weird.

@qryxip
Copy link
Collaborator

qryxip commented Oct 31, 2020

I've found the cause. It checks if the dependency is ignored after inserting ([], [], []).

cargo-udeps/src/lib.rs

Lines 473 to 485 in 13ea330

if !used_dependencies.contains(&(id, dependency)) {
let outcome = outcome
.unused_deps
.entry(id)
.or_insert(OutcomeUnusedDeps::new(packages[&id].manifest_path())?)
.unused_deps_mut(*kind);
if ignore.map_or(false, |ignore| ignore.contains(*kind, dependency)) {
config.shell().info(format_args!("Ignoring `{}` ({:?})", dependency, kind))?;
} else {
outcome.insert(dependency);
}
}

I don't remember the reason why we pass ([], [], []).

(edit) I mean, why we considered ([], [], []) was possible before #59.

cargo-udeps/src/lib.rs

Lines 489 to 494 in 13ea330

outcome.success = outcome
.unused_deps
.values()
.all(|OutcomeUnusedDeps { normal, development, build, .. }| {
normal.is_empty() && development.is_empty() && build.is_empty()
});

@qryxip
Copy link
Collaborator

qryxip commented Oct 31, 2020

I've just noticed the --output json option is not documented at all. Maybe we should write some descriptions.

@qryxip
Copy link
Collaborator

qryxip commented Oct 31, 2020

Either way this issue is actually weird and should be fixed.

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

Successfully merging a pull request may close this issue.

3 participants