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

feat: remove features when removing dependencies #113

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
26 changes: 22 additions & 4 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ on:
tags:
- "*"
pull_request:
branches: [ main ]
branches: [main]

jobs:
ci:
name: CI
env:
RUST_BACKTRACE: 1
runs-on: ubuntu-latest
Expand All @@ -35,7 +36,26 @@ jobs:
- run: cargo clippy --all --all-features -- -D warnings

- run: cargo test --workspace --verbose

autofix:
name: Autofix Tests
runs-on: ubuntu-latest
strategy:
fail-fast: false
steps:
- uses: actions/checkout@v3
- uses: dtolnay/rust-toolchain@master
with:
toolchain: stable
components: clippy, rustfmt
- run: cargo build --release
- run: cargo install --path .
- run: |
for dir in cli-tests/actual/*; do
if [ -d "$dir" ]; then
cd "$dir" && cargo machete --fix || true
fi
done
- run: cd cli-tests && diff -r expected actual
# Code from here: https://docs.github.com/en/actions/publishing-packages/publishing-docker-images
# TODO: Add a step to publish the image to the Docker Hub
# TODO: Build multiple images for different architectures, see this: https://docs.docker.com/build/ci/github-actions/multi-platform/
Expand Down Expand Up @@ -80,8 +100,6 @@ jobs:
push: true
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}


# Code mutably borrowed from https://github.com/EmbarkStudios/cargo-deny/, thanks Embark!
release:
name: Release
Expand Down
14 changes: 14 additions & 0 deletions cli-tests/actual/autofix-with-feature/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "just-unused"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
log = "0.4.14"

[features]
default = ["std"]
std = ["log/std", "other"]
other = []
3 changes: 3 additions & 0 deletions cli-tests/actual/autofix-with-feature/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
16 changes: 16 additions & 0 deletions cli-tests/actual/feature-named-like-dep-2/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
name = "just-unused"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
log = "0.4.14"

[features]
default = ["std"]
std = ["log/std", "other"]
dontlog = []
log = []
other = ["log", "dontlog"]
3 changes: 3 additions & 0 deletions cli-tests/actual/feature-named-like-dep-2/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
14 changes: 14 additions & 0 deletions cli-tests/actual/feature-named-like-dep/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "just-unused"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
log = "0.4.14"

[features]
default = ["std"]
std = ["log/std", "log"]
log = []
3 changes: 3 additions & 0 deletions cli-tests/actual/feature-named-like-dep/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
13 changes: 13 additions & 0 deletions cli-tests/expected/autofix-with-feature/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[package]
name = "just-unused"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

[features]
default = ["std"]
std = [ "other"]
other = []
3 changes: 3 additions & 0 deletions cli-tests/expected/autofix-with-feature/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
16 changes: 16 additions & 0 deletions cli-tests/expected/feature-named-like-dep-2/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
name = "just-unused"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
log = "0.4.14"
Copy link
Owner

Choose a reason for hiding this comment

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

I think log should be removed in this expected test result, since it's unused in the src directory?


[features]
default = ["std"]
std = ["log/std", "other"]
Copy link
Owner

Choose a reason for hiding this comment

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

and log/std here too

dontlog = []
log = []
other = ["log", "dontlog"]
3 changes: 3 additions & 0 deletions cli-tests/expected/feature-named-like-dep-2/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
14 changes: 14 additions & 0 deletions cli-tests/expected/feature-named-like-dep/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "just-unused"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
log = "0.4.14"
Copy link
Owner

Choose a reason for hiding this comment

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

and same in this file


[features]
default = ["std"]
std = ["log/std", "log"]
log = []
3 changes: 3 additions & 0 deletions cli-tests/expected/feature-named-like-dep/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
23 changes: 23 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,29 @@ fn remove_dependencies(manifest: &str, dependencies_list: &[String]) -> anyhow::
.with_context(|| format!("Dependency {k} not found"))?;
}

let features = manifest
.iter_mut()
.find_map(|(k, v)| (v.is_table_like() && k == "features").then_some(Some(v)))
.flatten()
.and_then(|v| v.as_table_mut());

if let Some(features) = features {
for (_, deps) in features.iter_mut() {
if let Some(deps) = deps.as_array_mut() {
deps.retain(|dep| {
if let Some(dep) = dep.as_str() {
!dependencies_list.iter().any(|d| {
let prefix = d.to_string() + "/";
dep.contains(&prefix)
})
Comment on lines +283 to +286
Copy link
Owner

Choose a reason for hiding this comment

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

So, scanning https://doc.rust-lang.org/cargo/reference/features.html, I think the whole set of rules is the following:

  • you can remove the dependency name, if there's no other feature named like it. e.g. if you depend on log, and in the [features] array there's only myfeat = ["log"], then it can be removed automatically. But it can't be removed if such a feature exists with the same name, because in that case it's referring to that feature with the same name (as far as I can tell).
  • there's another way to precise, starting from Rust 1.60, a dependency name over a feature name: dep:{dependency_name}, so it should be handled here too.
  • we can remove anything that starts with {dependency_name}/, as you're doing here 👍 .
  • we can remove anything that starts with {dependency_name}?, for optional dependencies

It would be super sweet to address those four cases completely, or someone could do this as a followup. Do you have a preference?

} else {
true
}
});
}
}
}

let serialized = manifest.to_string();
Ok(serialized)
}
Expand Down
Loading