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

Fixes #104: correctly parse id repr for Rust 1.78+ #106

Merged
merged 4 commits into from
Mar 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
matrix:
rust:
- stable
#- nightly # re-enable once #104 has been fixed
- nightly

steps:
- uses: actions/checkout@v3
Expand Down
57 changes: 50 additions & 7 deletions src/search_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ fn get_full_manifest(
.and_then(|metadata| metadata.cargo_machete.as_ref())
.map(|machete| &machete.ignored)
{
workspace_ignored = ignored.clone();
workspace_ignored.clone_from(ignored);
}

ws_manifest_and_path = Some((workspace_manifest, workspace_cargo_path));
Expand Down Expand Up @@ -352,12 +352,55 @@ pub(crate) fn find_unused(
.nodes
.iter()
.find(|node| {
// e.g. "aa 0.1.0 (path+file:///tmp/aa)"
node.id
.repr
.split(' ')
.next() // e.g. "aa"
.map_or(false, |node_package_name| node_package_name == package_name)
// Note: this is dependent on rustc, so it's a bit fragile, and may break with
// nightly versions of the compiler (see cargo-machete issue #104).

// The node id can have multiple representations:
// - on rust 1.77 and before, something like "aa 0.1.0 (path+file:///tmp/aa)".
// - on rust 1.78+, something like:
// - "path+file:///home/ben/cargo-machete/integration-tests/aa#0.1.0"
// - "path+file:///home/ben/cargo-machete/integration-tests/directory#[email protected]"
//
// See https://doc.rust-lang.org/cargo/reference/pkgid-spec.html for the full
// spec. If this breaks in the future (>= March 2024), consider using
// `cargo-util-schemas`.
let repr = &node.id.repr;

let package_found = if repr.contains(' ') {
// Rust < 1.78, take everything up to the space.
node.id.repr.split(' ').next() // e.g. "aa"
} else {
// Rust >= 1.78. Oh boy.
let mut temp = None;

let mut slash_split = node.id.repr.split('/').peekable();

while let Some(subset) = slash_split.next() {
// Continue until the last part of the path.
if slash_split.peek().is_some() {
continue;
}

// If there's no next slash separator, we've reached the end, and subset is
// one of:
// - aa#0.1.0
// - directory#[email protected]
if subset.contains('@') {
let mut hash_split = subset.split('#');
// Skip everything before the hash.
hash_split.next();
// Now in the rest, take everything up to the @.
temp = hash_split.next().and_then(|end| end.split('@').next());
} else {
// Otherwise, take everything up to the #.
temp = subset.split('#').next();
}
}
Comment on lines +373 to +398
Copy link

Choose a reason for hiding this comment

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

Or you could use cargo_util_schemas::core::PackageIdSpec::parse and then extract the path from the URL. Then if we extend the PackageIdSpec syntax, its just a dependency update away.

btw the spec for Specs is at https://doc.rust-lang.org/cargo/reference/pkgid-spec.html


temp
};

package_found.map_or(false, |node_package_name| node_package_name == package_name)
})
.expect("the current package must be in the dependency graph")
.deps
Expand Down
Loading