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 14 commits into
base: main
Choose a base branch
from
34 changes: 32 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ on:
tags:
- "*"
pull_request:
branches: [ main ]
branches: [main]

jobs:
ci:
env:
RUST_BACKTRACE: 1
RUST_BACKTRACE: 1
runs-on: ubuntu-latest
strategy:
matrix:
Expand All @@ -36,6 +36,36 @@ jobs:

- 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: ls
# The below works, but we need it to instead run with a different structure
# There is now a cli-tests directory, with actual and expected subdirectories
# In each of those is a number of tests we dont know hte names of in advance
# We want to dynamically run the below for each of those, and well as any added in actual
# And then diff the whole directory
# - run: cd integration-tests/autofix-with-feature/actual && cargo machete --fix || true
# - run: cd integration-tests/autofix-with-feature && diff -r expected actual
# Get all directoriies in cli-tests/actual, and for each run cargo-machete --fix,
- 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 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!");
}
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!");
}
20 changes: 20 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,26 @@ 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| dep.contains(d))
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 contains is a bit too vague here: a dependency flagged for removal called log would cause removal of a feature enabling something (a feature or another dependency) called flog.

Can you handle that better, and add a test that checks that too, please?

} else {
true
}
});
}
}
}

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