diff --git a/src/commands/deps.rs b/src/commands/deps.rs index d088740..e40cbbc 100644 --- a/src/commands/deps.rs +++ b/src/commands/deps.rs @@ -26,6 +26,7 @@ pub struct WorkspaceDepsJsonReport { #[derive(Debug, Serialize, Deserialize)] pub struct WorkspaceDepsEntry { pub workspace: String, + pub path: String, pub dependencies: Vec, pub reverse: bool, pub transitive: bool, @@ -121,6 +122,14 @@ impl WorkspaceDependencyAnalysis { .find(|ws| ws.name() == workspace_name) } + /// Get workspace path by name + pub fn get_workspace_path(&self, workspace_name: &str) -> Option<&PathBuf> { + self.workspaces + .iter() + .find(|(_, ws)| ws.name() == workspace_name) + .map(|(path, _)| path) + } + /// Get direct dependencies of a workspace pub fn get_direct_dependencies(&mut self, workspace: &str) -> &HashSet { if !self.direct_deps_cache.contains_key(workspace) { @@ -226,6 +235,11 @@ impl WorkspaceDepsReportGenerator { for workspace in workspaces { writeln!(output, "\nšŸ“¦ Workspace: {workspace}")?; + // Add workspace path if available + if let Some(workspace_path) = analysis.get_workspace_path(&workspace) { + writeln!(output, " šŸ“ Path: {}", workspace_path.display())?; + } + if self.reverse { writeln!(output, " ā¬†ļø Reverse dependencies (who depends on this):")?; let reverse_deps = analysis.get_reverse_dependencies(&workspace); @@ -300,8 +314,14 @@ impl WorkspaceDepsReportGenerator { .map(|info| info.is_standalone()) .unwrap_or(false); + let workspace_path = analysis + .get_workspace_path(&workspace) + .map(|p| p.display().to_string()) + .unwrap_or_else(|| "(unknown)".to_string()); + workspace_data.push(WorkspaceDepsEntry { - workspace, + workspace: workspace.clone(), + path: workspace_path, dependencies: deps, reverse: self.reverse, transitive: self.transitive, @@ -583,6 +603,7 @@ mod tests { let report = generator.generate_human_report(&mut analysis).unwrap(); assert!(report.contains("workspace-a")); + assert!(report.contains("Path: /test/workspace-a")); assert!(report.contains("Direct dependencies")); assert!(report.contains("workspace-b")); } @@ -598,5 +619,10 @@ mod tests { let json: serde_json::Value = serde_json::from_str(&report).unwrap(); assert!(json["workspace_dependencies"].is_array()); + + // Verify path field exists in the JSON output + let workspace_deps = json["workspace_dependencies"].as_array().unwrap(); + assert!(!workspace_deps.is_empty()); + assert!(workspace_deps[0]["path"].is_string()); } } diff --git a/src/executors/analyze.rs b/src/executors/analyze.rs index 8bdaf07..fd4836d 100644 --- a/src/executors/analyze.rs +++ b/src/executors/analyze.rs @@ -137,7 +137,7 @@ impl CommandExecutor for AnalyzeExecutor { }; match report_result { - Ok(report) => print!("{report}"), + Ok(report) => println!("{report}"), Err(e) => { return Err(e).wrap_err("Failed to generate report for crate analysis"); } diff --git a/src/executors/deps.rs b/src/executors/deps.rs index 362ebe1..94f558e 100644 --- a/src/executors/deps.rs +++ b/src/executors/deps.rs @@ -82,7 +82,7 @@ impl CommandExecutor for DepsExecutor { }; match report_result { - Ok(report) => print!("{report}"), + Ok(report) => println!("{report}"), Err(e) => { return Err(e) .into_diagnostic() diff --git a/src/toml_parser.rs b/src/toml_parser.rs index a0d40f7..f5e5ad2 100644 --- a/src/toml_parser.rs +++ b/src/toml_parser.rs @@ -101,6 +101,15 @@ impl CargoToml { .unwrap_or_default() } + /// Returns the list of workspace exclude patterns from the Cargo.toml + pub fn get_workspace_excludes(&self) -> Vec { + self.workspace + .as_ref() + .and_then(|ws| ws.exclude.as_ref()) + .cloned() + .unwrap_or_default() + } + pub fn get_workspace_dependencies(&self) -> HashMap { let mut deps = HashMap::new(); diff --git a/src/workspace_discovery.rs b/src/workspace_discovery.rs index a9818b7..e67b6e2 100644 --- a/src/workspace_discovery.rs +++ b/src/workspace_discovery.rs @@ -12,6 +12,15 @@ pub struct WorkspaceDiscovery { discovered_roots: HashSet, /// Warnings collected during discovery that didn't prevent processing warnings: Vec, + /// Track discovered workspaces for member checking + discovered_workspaces: Vec, +} + +#[derive(Debug, Clone)] +struct DiscoveredWorkspace { + path: PathBuf, + member_patterns: Vec, + exclude_patterns: Vec, } impl WorkspaceDiscovery { @@ -19,6 +28,7 @@ impl WorkspaceDiscovery { Self { discovered_roots: HashSet::new(), warnings: Vec::new(), + discovered_workspaces: Vec::new(), } } @@ -27,6 +37,60 @@ impl WorkspaceDiscovery { &self.warnings } + /// Check if a path is a member of any discovered workspace + fn is_path_workspace_member(&self, crate_path: &Path) -> bool { + for workspace in &self.discovered_workspaces { + if self.is_member_of_workspace(crate_path, workspace) { + return true; + } + } + false + } + + /// Check if a path is a member of a specific workspace + fn is_member_of_workspace(&self, crate_path: &Path, workspace: &DiscoveredWorkspace) -> bool { + // First check if the crate is within the workspace directory + if !crate_path.starts_with(&workspace.path) { + return false; + } + + // Get the relative path from workspace root + let Ok(relative_path) = crate_path.strip_prefix(&workspace.path) else { + return false; + }; + let relative_str = relative_path.to_string_lossy(); + + // Check exclude patterns first + for exclude_pattern in &workspace.exclude_patterns { + if self.matches_pattern(&relative_str, exclude_pattern) { + return false; + } + } + + // Check member patterns + for member_pattern in &workspace.member_patterns { + if self.matches_pattern(&relative_str, member_pattern) { + return true; + } + } + + false + } + + /// Check if a path matches a glob pattern + fn matches_pattern(&self, relative_path: &str, pattern: &str) -> bool { + // Try to use glob::Pattern::new for all patterns, not just those with '*' + if let Ok(pattern_matcher) = glob::Pattern::new(pattern) { + // Match against the relative path + return pattern_matcher.matches(relative_path); + } + + // If glob pattern parsing fails, fall back to direct path comparison + let pattern_path = Path::new(pattern); + Path::new(relative_path) == pattern_path + || Path::new(relative_path).starts_with(pattern_path) + } + /// Discover all workspace roots and standalone crates in the given paths /// /// Returns discovered workspace roots. Any non-fatal errors (like invalid @@ -128,6 +192,8 @@ impl WorkspaceDiscovery { .to_string(), ) .members(Vec::new()) // Will be populated later + .member_patterns(cargo_toml.get_workspace_members()) + .exclude_patterns(cargo_toml.get_workspace_excludes()) .workspace_dependencies(cargo_toml.get_workspace_dependencies()) .with_is_standalone(false) .build() @@ -155,6 +221,8 @@ impl WorkspaceDiscovery { .path(dir) .name(package.name.clone()) .members(vec![member]) + .member_patterns(vec![]) // Standalone crates have no member patterns + .exclude_patterns(vec![]) // Standalone crates have no exclude patterns .workspace_dependencies(Default::default()) .with_is_standalone(true) .build() @@ -195,9 +263,23 @@ impl WorkspaceDiscovery { // Separate roots and warnings let mut new_roots = Vec::new(); + let mut potential_standalone_crates = Vec::new(); + for (root, warnings) in results { if let Some(r) = root { - new_roots.push(r); + if r.is_standalone { + // Don't add standalone crates yet, we need to verify they're not workspace + // members + potential_standalone_crates.push(r); + } else { + // This is a workspace root, track it + self.discovered_workspaces.push(DiscoveredWorkspace { + path: r.path.clone(), + member_patterns: r.member_patterns().to_vec(), + exclude_patterns: r.exclude_patterns().to_vec(), + }); + new_roots.push(r); + } } self.warnings.extend(warnings); } @@ -229,6 +311,22 @@ impl WorkspaceDiscovery { roots.push(root); } + // Now check potential standalone crates to see if they're actually workspace + // members + for crate_root in potential_standalone_crates { + if !self.is_path_workspace_member(&crate_root.path) { + // This is truly a standalone crate + roots.push(crate_root); + } else { + // This is actually a workspace member, skip it + self.warnings.push(format!( + "Skipping '{}' at {} - it's a workspace member with an incorrect Cargo.lock", + crate_root.name, + crate_root.path.display() + )); + } + } + // Also check for workspace roots without Cargo.lock (less common but possible) self.find_additional_workspaces(path, roots, progress)?; @@ -269,6 +367,16 @@ impl WorkspaceDiscovery { match CargoToml::parse_file(cargo_toml_path) { Ok(cargo_toml) if cargo_toml.is_workspace_root() => { self.discovered_roots.insert(dir.to_path_buf()); + let member_patterns = cargo_toml.get_workspace_members(); + let exclude_patterns = cargo_toml.get_workspace_excludes(); + + // Track this workspace for member checking + self.discovered_workspaces.push(DiscoveredWorkspace { + path: dir.to_path_buf(), + member_patterns: member_patterns.to_vec(), + exclude_patterns: exclude_patterns.to_vec(), + }); + match self.expand_workspace_members(dir, &cargo_toml) { Ok(members) => { roots.push(WorkspaceRoot { @@ -279,6 +387,8 @@ impl WorkspaceDiscovery { .to_string_lossy() .to_string(), members, + member_patterns, + exclude_patterns, workspace_dependencies: cargo_toml.get_workspace_dependencies(), is_standalone: false, }); @@ -428,6 +538,8 @@ pub struct WorkspaceRoot { path: PathBuf, name: String, members: Vec, + member_patterns: Vec, + exclude_patterns: Vec, workspace_dependencies: std::collections::HashMap, is_standalone: bool, } @@ -462,6 +574,16 @@ impl WorkspaceRoot { pub fn is_standalone(&self) -> bool { self.is_standalone } + + /// Gets the member patterns + pub fn member_patterns(&self) -> &[String] { + &self.member_patterns + } + + /// Gets the exclude patterns + pub fn exclude_patterns(&self) -> &[String] { + &self.exclude_patterns + } } /// Builder for WorkspaceRoot @@ -470,6 +592,8 @@ pub struct WorkspaceRootBuilder { path: Option, name: Option, members: Vec, + member_patterns: Vec, + exclude_patterns: Vec, workspace_dependencies: std::collections::HashMap, is_standalone: bool, } @@ -508,6 +632,18 @@ impl WorkspaceRootBuilder { self } + /// Sets the member patterns + pub fn member_patterns(mut self, patterns: Vec) -> Self { + self.member_patterns = patterns; + self + } + + /// Sets the exclude patterns + pub fn exclude_patterns(mut self, patterns: Vec) -> Self { + self.exclude_patterns = patterns; + self + } + /// Builds the WorkspaceRoot pub fn build(self) -> Result { let path = self.path.ok_or("path is required")?; @@ -517,6 +653,8 @@ impl WorkspaceRootBuilder { path, name, members: self.members, + member_patterns: self.member_patterns, + exclude_patterns: self.exclude_patterns, workspace_dependencies: self.workspace_dependencies, is_standalone: self.is_standalone, }) @@ -693,4 +831,107 @@ name = "standalone-crate" assert_eq!(workspace.members.len(), 2); assert!(workspace.workspace_dependencies.contains_key("shared")); } + + #[test] + fn test_workspace_member_with_incorrect_cargo_lock() { + let temp = TempDir::new().unwrap(); + let root = temp.path(); + + // Create workspace root + fs::create_dir_all(root.join("workspace")).unwrap(); + fs::write( + root.join("workspace/Cargo.toml"), + r#" +[workspace] +members = ["crate-a"] +"#, + ) + .unwrap(); + fs::write(root.join("workspace/Cargo.lock"), "# workspace lock file").unwrap(); + + // Create member crate with its own Cargo.lock (incorrect) + fs::create_dir_all(root.join("workspace/crate-a")).unwrap(); + fs::write( + root.join("workspace/crate-a/Cargo.toml"), + r#" +[package] +name = "crate-a" +"#, + ) + .unwrap(); + fs::write( + root.join("workspace/crate-a/Cargo.lock"), + "# incorrect lock file", + ) + .unwrap(); + + let mut discovery = WorkspaceDiscovery::new(); + let roots = discovery.discover_all(&[root.to_path_buf()], None).unwrap(); + + // Should only find one workspace, not a standalone crate + assert_eq!(roots.len(), 1); + assert!(!roots[0].is_standalone); + assert_eq!(roots[0].name, "workspace"); + + // Check that we got a warning about the incorrect Cargo.lock + let warnings = discovery.warnings(); + assert!(warnings.iter().any(|w| w.contains("incorrect Cargo.lock"))); + } + + #[test] + fn test_workspace_with_glob_members() { + let temp = TempDir::new().unwrap(); + let root = temp.path(); + + // Create workspace root with glob pattern + fs::create_dir_all(root).unwrap(); + fs::write( + root.join("Cargo.toml"), + r#" +[workspace] +members = ["crates/*"] +exclude = ["crates/ignored"] +"#, + ) + .unwrap(); + fs::write(root.join("Cargo.lock"), "# workspace lock file").unwrap(); + + // Create member crates + fs::create_dir_all(root.join("crates/foo")).unwrap(); + fs::write( + root.join("crates/foo/Cargo.toml"), + r#" +[package] +name = "foo" +"#, + ) + .unwrap(); + // Add incorrect Cargo.lock + fs::write(root.join("crates/foo/Cargo.lock"), "# incorrect lock file").unwrap(); + + // Create excluded crate (should be standalone) + fs::create_dir_all(root.join("crates/ignored")).unwrap(); + fs::write( + root.join("crates/ignored/Cargo.toml"), + r#" +[package] +name = "ignored" +"#, + ) + .unwrap(); + fs::write(root.join("crates/ignored/Cargo.lock"), "# lock file").unwrap(); + + let mut discovery = WorkspaceDiscovery::new(); + let roots = discovery.discover_all(&[root.to_path_buf()], None).unwrap(); + + // Should find workspace and the excluded standalone crate + assert_eq!(roots.len(), 2); + + let workspace = roots.iter().find(|r| !r.is_standalone).unwrap(); + assert_eq!(workspace.member_patterns(), &["crates/*"]); + assert_eq!(workspace.exclude_patterns(), &["crates/ignored"]); + + let standalone = roots.iter().find(|r| r.is_standalone).unwrap(); + assert_eq!(standalone.name, "ignored"); + } }