From 13b9b5f3ac046e6429ff10a0808531120ec557fd Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sat, 29 Mar 2025 02:31:11 -0400 Subject: [PATCH 1/3] make `CargoFmtStrategy` an argument to cargo-fmt test helper function --- src/cargo-fmt/test/targets.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/cargo-fmt/test/targets.rs b/src/cargo-fmt/test/targets.rs index 34accb2136a..2ff5730300d 100644 --- a/src/cargo-fmt/test/targets.rs +++ b/src/cargo-fmt/test/targets.rs @@ -14,11 +14,12 @@ mod all_targets { source_root: &str, exp_targets: &[ExpTarget], exp_num_targets: usize, + strategy: CargoFmtStrategy, ) { let root_path = Path::new("tests/cargo-fmt/source").join(source_root); let get_path = |exp: &str| PathBuf::from(&root_path).join(exp).canonicalize().unwrap(); let manifest_path = Path::new(&root_path).join(manifest_suffix); - let targets = get_targets(&CargoFmtStrategy::All, Some(manifest_path.as_path())) + let targets = get_targets(&strategy, Some(manifest_path.as_path())) .expect("Targets should have been loaded"); assert_eq!(targets.len(), exp_num_targets); @@ -58,6 +59,7 @@ mod all_targets { "divergent-crate-dir-names", &exp_targets, 3, + CargoFmtStrategy::All, ); } @@ -113,6 +115,7 @@ mod all_targets { "workspaces/path-dep-above", &exp_targets, 6, + CargoFmtStrategy::All, ); } From e8113682081709b09c0b9827e207b382c83802c9 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sat, 29 Mar 2025 02:40:30 -0400 Subject: [PATCH 2/3] Add a cargo-fmt test case for issue 6517 The test case loads package targets from the root of a workspace. --- src/cargo-fmt/test/targets.rs | 28 +++++++++++++++++++ tests/cargo-fmt/source/issues_6517/Cargo.toml | 2 ++ .../source/issues_6517/inner_bin/Cargo.toml | 6 ++++ .../source/issues_6517/inner_bin/src/main.rs | 3 ++ .../source/issues_6517/inner_lib/Cargo.toml | 6 ++++ .../source/issues_6517/inner_lib/src/lib.rs | 14 ++++++++++ 6 files changed, 59 insertions(+) create mode 100644 tests/cargo-fmt/source/issues_6517/Cargo.toml create mode 100644 tests/cargo-fmt/source/issues_6517/inner_bin/Cargo.toml create mode 100644 tests/cargo-fmt/source/issues_6517/inner_bin/src/main.rs create mode 100644 tests/cargo-fmt/source/issues_6517/inner_lib/Cargo.toml create mode 100644 tests/cargo-fmt/source/issues_6517/inner_lib/src/lib.rs diff --git a/src/cargo-fmt/test/targets.rs b/src/cargo-fmt/test/targets.rs index 2ff5730300d..ad2842c91a2 100644 --- a/src/cargo-fmt/test/targets.rs +++ b/src/cargo-fmt/test/targets.rs @@ -134,4 +134,32 @@ mod all_targets { assert_correct_targets_loaded("ws/a/d/f/Cargo.toml"); } } + + mod cargo_fmt_strategy_root { + use super::*; + + #[test] + fn assert_correct_targets_loaded_from_root() { + let exp_targets = &[ + ExpTarget { + path: "inner_bin/src/main.rs", + edition: Edition::E2021, + kind: "bin", + }, + ExpTarget { + path: "inner_lib/src/lib.rs", + edition: Edition::E2021, + kind: "lib", + }, + ]; + + super::assert_correct_targets_loaded( + "Cargo.toml", + "issues_6517", + exp_targets, + 2, + CargoFmtStrategy::Root, + ); + } + } } diff --git a/tests/cargo-fmt/source/issues_6517/Cargo.toml b/tests/cargo-fmt/source/issues_6517/Cargo.toml new file mode 100644 index 00000000000..7667843cf75 --- /dev/null +++ b/tests/cargo-fmt/source/issues_6517/Cargo.toml @@ -0,0 +1,2 @@ +[workspace] +members = ["inner_bin", "inner_lib"] diff --git a/tests/cargo-fmt/source/issues_6517/inner_bin/Cargo.toml b/tests/cargo-fmt/source/issues_6517/inner_bin/Cargo.toml new file mode 100644 index 00000000000..bb6a80cd0ce --- /dev/null +++ b/tests/cargo-fmt/source/issues_6517/inner_bin/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "inner_bin" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/tests/cargo-fmt/source/issues_6517/inner_bin/src/main.rs b/tests/cargo-fmt/source/issues_6517/inner_bin/src/main.rs new file mode 100644 index 00000000000..e7a11a969c0 --- /dev/null +++ b/tests/cargo-fmt/source/issues_6517/inner_bin/src/main.rs @@ -0,0 +1,3 @@ +fn main() { + println!("Hello, world!"); +} diff --git a/tests/cargo-fmt/source/issues_6517/inner_lib/Cargo.toml b/tests/cargo-fmt/source/issues_6517/inner_lib/Cargo.toml new file mode 100644 index 00000000000..c9ddf1793b8 --- /dev/null +++ b/tests/cargo-fmt/source/issues_6517/inner_lib/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "inner_lib" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/tests/cargo-fmt/source/issues_6517/inner_lib/src/lib.rs b/tests/cargo-fmt/source/issues_6517/inner_lib/src/lib.rs new file mode 100644 index 00000000000..b93cf3ffd9c --- /dev/null +++ b/tests/cargo-fmt/source/issues_6517/inner_lib/src/lib.rs @@ -0,0 +1,14 @@ +pub fn add(left: u64, right: u64) -> u64 { + left + right +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_works() { + let result = add(2, 2); + assert_eq!(result, 4); + } +} From 7b7ceb1a5d3edc89986c470b8d8779e2675b1642 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sat, 29 Mar 2025 02:43:46 -0400 Subject: [PATCH 3/3] correctly compare directory paths in `get_targets_root_only` Previously, when running `cargo fmt --manifest-path Cargo.toml` from a cargo workspace's root directory `cargo fmt` would error with a message that read `Failed to find targets`. The issue stemmed from an incorrect comparison between the path to the workspace's root **directory** and the path to the specified `Cargo.toml` **file**. This lead `cargo fmt` to incorrectly determine that the command wasn't being run from the workspace's root. --- src/cargo-fmt/main.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/cargo-fmt/main.rs b/src/cargo-fmt/main.rs index eedb43defb9..4aef1a9ae45 100644 --- a/src/cargo-fmt/main.rs +++ b/src/cargo-fmt/main.rs @@ -364,11 +364,16 @@ fn get_targets_root_only( targets: &mut BTreeSet, ) -> Result<(), io::Error> { let metadata = get_cargo_metadata(manifest_path)?; + // `workspace_root_path` is the path to the workspace's root directory let workspace_root_path = PathBuf::from(&metadata.workspace_root).canonicalize()?; let (in_workspace_root, current_dir_manifest) = if let Some(target_manifest) = manifest_path { + // `target_manifest` is the canonicalized path to a `Cargo.toml` file + let target_manifest = target_manifest.canonicalize()?; ( - workspace_root_path == target_manifest, - target_manifest.canonicalize()?, + target_manifest + .parent() + .is_some_and(|manifest| workspace_root_path == manifest), + target_manifest, ) } else { let current_dir = env::current_dir()?.canonicalize()?;