Skip to content

Commit 29d9095

Browse files
authored
fix: handle Cargo.lock and Cargo.toml changes in ripples command (#28)
When Cargo.lock or Cargo.toml files at a workspace root are changed, all workspace members are now correctly marked as directly affected. This ensures that dependency version changes (via Cargo.lock) and workspace configuration changes (via Cargo.toml) properly propagate to all affected crates. Added comprehensive test coverage for various scenarios including nested workspaces and standalone crates.
1 parent 520167a commit 29d9095

File tree

1 file changed

+297
-11
lines changed

1 file changed

+297
-11
lines changed

src/commands/affected.rs

Lines changed: 297 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Ripples command implementation
22
33
use std::collections::{HashMap, HashSet};
4-
use std::path::PathBuf;
4+
use std::path::{Path, PathBuf};
55

66
use miette::{Result, WrapErr};
77
use petgraph::graph::{DiGraph, NodeIndex};
@@ -177,6 +177,42 @@ impl AffectedAnalysis {
177177
})
178178
}
179179

180+
/// Handle workspace-level Cargo files (Cargo.toml or Cargo.lock)
181+
fn handle_workspace_cargo_file(
182+
&self,
183+
abs_file: &Path,
184+
cwd: &Path,
185+
directly_affected_crates: &mut HashSet<String>,
186+
directly_affected_crate_paths: &mut HashSet<(String, PathBuf)>,
187+
) -> bool {
188+
// Check if this file is at a workspace root
189+
for ws_path in self.workspaces.keys() {
190+
let abs_ws_path = if ws_path.is_absolute() {
191+
ws_path.clone()
192+
} else {
193+
cwd.join(ws_path)
194+
};
195+
let abs_ws_path = abs_ws_path.canonicalize().unwrap_or(abs_ws_path);
196+
197+
// Check if the Cargo file is directly in the workspace root
198+
if let Some(parent) = abs_file.parent()
199+
&& parent == abs_ws_path
200+
{
201+
// This is a workspace-level Cargo file
202+
// Mark all crates in this workspace as directly affected
203+
for ((crate_name, crate_path), crate_ws_path) in &self.crate_path_to_workspace {
204+
if crate_ws_path == ws_path {
205+
directly_affected_crates.insert(crate_name.clone());
206+
directly_affected_crate_paths
207+
.insert((crate_name.clone(), crate_path.clone()));
208+
}
209+
}
210+
return true;
211+
}
212+
}
213+
false
214+
}
215+
180216
/// Analyze which crates and workspaces are affected by the given files
181217
pub fn analyze_affected_files(&self, files: &[String]) -> AffectedResult {
182218
let mut directly_affected_crates = HashSet::new();
@@ -199,6 +235,22 @@ impl AffectedAnalysis {
199235
// Try to canonicalize to resolve symlinks (e.g., /private/var -> /var on macOS)
200236
let abs_file = abs_file.canonicalize().unwrap_or(abs_file);
201237

238+
// Check if this is a Cargo.lock or Cargo.toml file
239+
let filename = abs_file.file_name().and_then(|f| f.to_str());
240+
let is_cargo_file = matches!(filename, Some("Cargo.lock") | Some("Cargo.toml"));
241+
242+
// Handle workspace-level Cargo files
243+
if is_cargo_file
244+
&& self.handle_workspace_cargo_file(
245+
&abs_file,
246+
&cwd,
247+
&mut directly_affected_crates,
248+
&mut directly_affected_crate_paths,
249+
)
250+
{
251+
continue;
252+
}
253+
202254
// Try to find by checking if file is under any crate directory
203255
// When multiple crates match, prefer the one with the longest matching path
204256
let mut best_match: Option<(String, PathBuf, usize)> = None;
@@ -570,6 +622,24 @@ name = "crate-b"
570622
)
571623
.unwrap();
572624

625+
// Add Cargo.lock file to the workspace root
626+
fs::write(
627+
root.join("my-workspace/Cargo.lock"),
628+
r#"# This file is automatically @generated by Cargo.
629+
# It is not intended for manual editing.
630+
version = 3
631+
632+
[[package]]
633+
name = "crate-a"
634+
version = "0.1.0"
635+
636+
[[package]]
637+
name = "crate-b"
638+
version = "0.1.0"
639+
"#,
640+
)
641+
.unwrap();
642+
573643
temp
574644
}
575645

@@ -1096,12 +1166,12 @@ version = "0.1.0"
10961166
let files = vec![format!("{}/my-workspace/Cargo.toml", temp.path().display())];
10971167
let result = analysis.analyze_affected_files(&files);
10981168

1099-
// Workspace Cargo.toml should not map to any specific crate
1100-
assert_eq!(result.unmatched_files.len(), 1);
1101-
assert_eq!(
1102-
result.unmatched_files[0],
1103-
format!("{}/my-workspace/Cargo.toml", temp.path().display())
1104-
);
1169+
// Workspace Cargo.toml should affect all workspace members
1170+
assert_eq!(result.directly_affected_crates.len(), 2);
1171+
assert!(result.directly_affected_crates.contains("crate-a"));
1172+
assert!(result.directly_affected_crates.contains("crate-b"));
1173+
assert!(result.directly_affected_workspaces.contains("my-workspace"));
1174+
assert!(result.unmatched_files.is_empty());
11051175
}
11061176

11071177
#[test]
@@ -1123,10 +1193,226 @@ version = "0.1.0"
11231193
.contains("standalone-test-crate")
11241194
);
11251195

1126-
// Workspace Cargo.lock should not map to any specific crate
1127-
assert!(result.unmatched_files.contains(&format!(
1128-
"{}/real-workspace/Cargo.lock",
1196+
// Workspace Cargo.lock should affect all workspace members
1197+
assert!(result.directly_affected_crates.contains("crate-a"));
1198+
assert!(result.directly_affected_crates.contains("crate-b"));
1199+
1200+
// No unmatched files
1201+
assert!(result.unmatched_files.is_empty());
1202+
}
1203+
1204+
#[test]
1205+
fn test_workspace_cargo_lock_affects_all_members() {
1206+
let temp = create_simple_test_workspace();
1207+
let analysis = build_test_analysis(temp.path());
1208+
1209+
// Test that changing Cargo.lock at workspace root affects all members
1210+
let files = vec![format!("{}/my-workspace/Cargo.lock", temp.path().display())];
1211+
let result = analysis.analyze_affected_files(&files);
1212+
1213+
// All workspace members should be directly affected
1214+
assert!(result.directly_affected_crates.contains("crate-a"));
1215+
assert!(result.directly_affected_crates.contains("crate-b"));
1216+
assert_eq!(result.directly_affected_crates.len(), 2);
1217+
1218+
// The workspace should be affected
1219+
assert!(result.directly_affected_workspaces.contains("my-workspace"));
1220+
1221+
// No unmatched files
1222+
assert!(result.unmatched_files.is_empty());
1223+
}
1224+
1225+
#[test]
1226+
fn test_workspace_cargo_toml_affects_all_members() {
1227+
let temp = create_simple_test_workspace();
1228+
let analysis = build_test_analysis(temp.path());
1229+
1230+
// Test that changing workspace Cargo.toml affects all members
1231+
let files = vec![format!("{}/my-workspace/Cargo.toml", temp.path().display())];
1232+
let result = analysis.analyze_affected_files(&files);
1233+
1234+
// All workspace members should be directly affected
1235+
assert!(result.directly_affected_crates.contains("crate-a"));
1236+
assert!(result.directly_affected_crates.contains("crate-b"));
1237+
assert_eq!(result.directly_affected_crates.len(), 2);
1238+
1239+
// The workspace should be affected
1240+
assert!(result.directly_affected_workspaces.contains("my-workspace"));
1241+
1242+
// No unmatched files
1243+
assert!(result.unmatched_files.is_empty());
1244+
}
1245+
1246+
#[test]
1247+
fn test_standalone_cargo_lock_affects_only_that_crate() {
1248+
let temp = create_mixed_workspace_and_standalone();
1249+
let analysis = build_test_analysis(temp.path());
1250+
1251+
// Test that changing a standalone crate's Cargo.lock only affects that crate
1252+
let files = vec![format!(
1253+
"{}/standalone-test-crate/Cargo.lock",
1254+
temp.path().display()
1255+
)];
1256+
let result = analysis.analyze_affected_files(&files);
1257+
1258+
// Only the standalone crate should be affected
1259+
assert!(
1260+
result
1261+
.directly_affected_crates
1262+
.contains("standalone-test-crate")
1263+
);
1264+
assert_eq!(result.directly_affected_crates.len(), 1);
1265+
1266+
// No workspace members should be affected
1267+
assert!(!result.directly_affected_crates.contains("crate-a"));
1268+
assert!(!result.directly_affected_crates.contains("crate-b"));
1269+
1270+
// No unmatched files
1271+
assert!(result.unmatched_files.is_empty());
1272+
}
1273+
1274+
#[test]
1275+
fn test_crate_cargo_toml_affects_dependents() {
1276+
let temp = create_simple_test_workspace();
1277+
let analysis = build_test_analysis(temp.path());
1278+
1279+
// Test that changing a crate's Cargo.toml affects its dependents
1280+
let files = vec![format!(
1281+
"{}/my-workspace/crate-b/Cargo.toml",
11291282
temp.path().display()
1130-
)));
1283+
)];
1284+
let result = analysis.analyze_affected_files(&files);
1285+
1286+
// crate-b should be directly affected
1287+
assert!(result.directly_affected_crates.contains("crate-b"));
1288+
assert_eq!(result.directly_affected_crates.len(), 1);
1289+
1290+
// crate-a depends on crate-b, so it should be affected through reverse
1291+
// dependencies
1292+
assert!(result.all_affected_crates.contains("crate-a"));
1293+
assert!(result.all_affected_crates.contains("crate-b"));
1294+
assert_eq!(result.all_affected_crates.len(), 2);
1295+
}
1296+
1297+
#[test]
1298+
fn test_multiple_cargo_files_affected() {
1299+
let temp = create_simple_test_workspace();
1300+
let analysis = build_test_analysis(temp.path());
1301+
1302+
// Test multiple Cargo files changed at once
1303+
let files = vec![
1304+
format!("{}/my-workspace/Cargo.toml", temp.path().display()),
1305+
format!("{}/my-workspace/Cargo.lock", temp.path().display()),
1306+
format!("{}/my-workspace/crate-a/Cargo.toml", temp.path().display()),
1307+
];
1308+
let result = analysis.analyze_affected_files(&files);
1309+
1310+
// All crates should be directly affected
1311+
assert!(result.directly_affected_crates.contains("crate-a"));
1312+
assert!(result.directly_affected_crates.contains("crate-b"));
1313+
assert_eq!(result.directly_affected_crates.len(), 2);
1314+
1315+
// No unmatched files
1316+
assert!(result.unmatched_files.is_empty());
1317+
}
1318+
1319+
#[test]
1320+
fn test_nested_workspace_cargo_lock() {
1321+
let temp = TempDir::new().unwrap();
1322+
let root = temp.path();
1323+
1324+
// Create a nested workspace structure
1325+
fs::create_dir_all(root.join("outer-workspace")).unwrap();
1326+
fs::write(
1327+
root.join("outer-workspace/Cargo.toml"),
1328+
r#"
1329+
[workspace]
1330+
members = ["inner-workspace", "outer-crate"]
1331+
"#,
1332+
)
1333+
.unwrap();
1334+
1335+
// Create inner workspace
1336+
fs::create_dir_all(root.join("outer-workspace/inner-workspace")).unwrap();
1337+
fs::write(
1338+
root.join("outer-workspace/inner-workspace/Cargo.toml"),
1339+
r#"
1340+
[workspace]
1341+
members = ["inner-crate-a", "inner-crate-b"]
1342+
"#,
1343+
)
1344+
.unwrap();
1345+
1346+
// Create crates
1347+
fs::create_dir_all(root.join("outer-workspace/inner-workspace/inner-crate-a/src")).unwrap();
1348+
fs::write(
1349+
root.join("outer-workspace/inner-workspace/inner-crate-a/Cargo.toml"),
1350+
r#"
1351+
[package]
1352+
name = "inner-crate-a"
1353+
"#,
1354+
)
1355+
.unwrap();
1356+
fs::write(
1357+
root.join("outer-workspace/inner-workspace/inner-crate-a/src/lib.rs"),
1358+
"pub fn func() {}",
1359+
)
1360+
.unwrap();
1361+
1362+
fs::create_dir_all(root.join("outer-workspace/inner-workspace/inner-crate-b/src")).unwrap();
1363+
fs::write(
1364+
root.join("outer-workspace/inner-workspace/inner-crate-b/Cargo.toml"),
1365+
r#"
1366+
[package]
1367+
name = "inner-crate-b"
1368+
"#,
1369+
)
1370+
.unwrap();
1371+
fs::write(
1372+
root.join("outer-workspace/inner-workspace/inner-crate-b/src/lib.rs"),
1373+
"pub fn func() {}",
1374+
)
1375+
.unwrap();
1376+
1377+
fs::create_dir_all(root.join("outer-workspace/outer-crate/src")).unwrap();
1378+
fs::write(
1379+
root.join("outer-workspace/outer-crate/Cargo.toml"),
1380+
r#"
1381+
[package]
1382+
name = "outer-crate"
1383+
"#,
1384+
)
1385+
.unwrap();
1386+
fs::write(
1387+
root.join("outer-workspace/outer-crate/src/lib.rs"),
1388+
"pub fn func() {}",
1389+
)
1390+
.unwrap();
1391+
1392+
// Add Cargo.lock files
1393+
fs::write(
1394+
root.join("outer-workspace/Cargo.lock"),
1395+
"# Outer workspace lock file",
1396+
)
1397+
.unwrap();
1398+
fs::write(
1399+
root.join("outer-workspace/inner-workspace/Cargo.lock"),
1400+
"# Inner workspace lock file",
1401+
)
1402+
.unwrap();
1403+
1404+
let analysis = build_test_analysis(root);
1405+
1406+
// Test that inner workspace Cargo.lock only affects inner crates
1407+
let files = vec![format!(
1408+
"{}/outer-workspace/inner-workspace/Cargo.lock",
1409+
root.display()
1410+
)];
1411+
let result = analysis.analyze_affected_files(&files);
1412+
1413+
assert!(result.directly_affected_crates.contains("inner-crate-a"));
1414+
assert!(result.directly_affected_crates.contains("inner-crate-b"));
1415+
assert!(!result.directly_affected_crates.contains("outer-crate"));
1416+
assert_eq!(result.directly_affected_crates.len(), 2);
11311417
}
11321418
}

0 commit comments

Comments
 (0)