cargo-rail: pre v1 commit - it's CLOSE

Author: loadingalias

src/cargo/manifest.rs

@@ -267,6 +267,7 @@ impl CargoTransform {
     &self,
     workspace_toml_path: &std::path::Path,
     unified_deps: &[crate::cargo::unify::UnifiedDep],
+    add_comments: bool,
   ) -> RailResult<()> {
     use toml_edit::{Array, InlineTable, table};
 
@@ -322,6 +323,19 @@ impl CargoTransform {
 
       // Insert the dependency
       workspace_deps.insert(&unified.name, toml_edit::Item::Value(dep_table.into()));
+
+      // Add comments if enabled and any exist
+      if add_comments
+        && !unified.comments.is_empty()
+        && let Some(item) = workspace_deps.get_mut(&unified.name)
+      {
+        let comment_str = unified.comments.join(", ");
+        item
+          .as_value_mut()
+          .unwrap()
+          .decor_mut()
+          .set_suffix(format!(" # {}", comment_str));
+      }
     }
 
     // Write back to file
@@ -614,10 +628,11 @@ members = ["crate-a", "crate-b"]
       dep_kinds: HashSet::new(),
       fragmentation_count: 2,
       path: None,
+      comments: Vec::new(),
     }];
 
     // Write workspace dependencies
-    let result = transformer.write_workspace_dependencies(temp_file.path(), &unified_deps);
+    let result = transformer.write_workspace_dependencies(temp_file.path(), &unified_deps, true);
     assert!(result.is_ok(), "Should successfully write workspace dependencies");
 
     // Read back and verify
@@ -669,10 +684,11 @@ members = ["crate-a"]
       dep_kinds: HashSet::new(),
       fragmentation_count: 1,
       path: None,
+      comments: Vec::new(),
     }];
 
     transformer
-      .write_workspace_dependencies(temp_file.path(), &unified_deps)
+      .write_workspace_dependencies(temp_file.path(), &unified_deps, true)
       .unwrap();
 
     let content = std::fs::read_to_string(temp_file.path()).unwrap();
@@ -706,10 +722,11 @@ members = ["crate-a"]
       dep_kinds: HashSet::new(),
       fragmentation_count: 1,
       path: None,
+      comments: Vec::new(),
     }];
 
     transformer
-      .write_workspace_dependencies(temp_file.path(), &unified_deps)
+      .write_workspace_dependencies(temp_file.path(), &unified_deps, true)
       .unwrap();
 
     let content = std::fs::read_to_string(temp_file.path()).unwrap();

src/cargo/mod.rs

@@ -20,5 +20,5 @@ pub mod validate;
 
 pub use manifest::{CargoTransform, TransformContext};
 pub use metadata::WorkspaceMetadata;
-pub use unify::{UnifiedDep, UnifyConfig, UnifyStrategy, WorkspaceUnifier};
+pub use unify::{IssueSeverity, UnifiedDep, UnifyConfig, UnifyReport, UnifyStrategy, WorkspaceUnifier};
 pub use validate::validate_targets;

src/cargo/unify/config.rs

@@ -21,8 +21,14 @@ pub struct UnifyConfig {
   /// Dependencies to force-include in unification
   pub include: HashSet<String>,
 
-  /// Only unify normal dependencies (exclude dev/build)
-  pub normal_only: bool,
+  /// Auto-resolve version conflicts by picking the highest version (default: true)
+  pub auto_resolve_version_conflicts: bool,
+
+  /// Add conflict comments to workspace.dependencies (default: true)
+  pub add_conflict_comments: bool,
+
+  /// Generate a detailed unification report (default: true)
+  pub generate_report: bool,
 }
 
 /// Strategy for selecting which dependencies to unify
@@ -39,7 +45,9 @@ impl Default for UnifyConfig {
       allow_renamed: false,
       exclude: HashSet::new(),
       include: HashSet::new(),
-      normal_only: false,
+      auto_resolve_version_conflicts: true,
+      add_conflict_comments: true,
+      generate_report: true,
     }
   }
 }

src/cargo/unify/issue_detection.rs

@@ -1,7 +1,7 @@
 //! Issue detection for dependency unification
 
 use super::path_handling::are_all_identical_workspace_paths;
-use super::types::{DependencyInstance, IssueType, UnificationIssue};
+use super::types::{DependencyInstance, IssueSeverity, IssueType, UnificationIssue};
 use crate::cargo::WorkspaceMetadata;
 use crate::error::RailResult;
 use std::collections::HashMap;
@@ -50,9 +50,10 @@ pub fn detect_multi_version_conflicts(metadata: &WorkspaceMetadata) -> RailResul
     if versions.len() > 1 {
       issues.push(UnificationIssue {
         dep_name: pkg_name.clone(),
-        issue_type: IssueType::MultipleVersions {
+        issue_type: IssueType::MultipleResolvedVersions {
           versions: versions.clone(),
         },
+        severity: IssueSeverity::Soft, // Can be resolved via workspace.dependencies
         affected_members: vec![],
         suggestion: format!(
           "Multiple versions of '{}' found: {}. Standardize to a single version across the workspace.",
@@ -86,6 +87,7 @@ pub fn detect_issues(
         original: dep_name.to_string(),
         renames,
       },
+      severity: IssueSeverity::Hard, // Renames block unification
       affected_members: instances.iter().map(|i| i.member.clone()).collect(),
       suggestion: "Standardize dependency name across all workspace members".to_string(),
     });
@@ -97,12 +99,13 @@ pub fn detect_issues(
   if versions.len() > 1 {
     return Some(UnificationIssue {
       dep_name: dep_name.to_string(),
-      issue_type: IssueType::VersionConflict {
+      issue_type: IssueType::IncompatibleVersionRequirements {
         requirements: instances
           .iter()
           .map(|i| (i.member.clone(), i.version_req.to_string()))
           .collect(),
       },
+      severity: IssueSeverity::Soft, // Can be resolved via workspace.dependencies
       affected_members: instances.iter().map(|i| i.member.clone()).collect(),
       suggestion: "Standardize version requirement across all workspace members".to_string(),
     });
@@ -124,6 +127,7 @@ pub fn detect_issues(
     return Some(UnificationIssue {
       dep_name: dep_name.to_string(),
       issue_type: IssueType::PathDependency { paths },
+      severity: IssueSeverity::Hard, // Incompatible paths block unification
       affected_members: instances.iter().map(|i| i.member.clone()).collect(),
       suggestion:
         "Convert external path dependencies to version dependencies, or ensure all workspace members use the same path"
@@ -139,6 +143,7 @@ pub fn detect_issues(
       issue_type: IssueType::AllTargetSpecific {
         targets: targets.clone(),
       },
+      severity: IssueSeverity::Info, // Just informational, not a blocker
       affected_members: instances.iter().map(|i| i.member.clone()).collect(),
       suggestion: "Target-specific dependencies cannot be unified at workspace level".to_string(),
     });
@@ -163,10 +168,138 @@ pub fn detect_issues(
         members_with_default: with_default.clone(),
         members_without_default: without_default.clone(),
       },
+      severity: IssueSeverity::Soft, // Can be resolved via workspace.dependencies
       affected_members: instances.iter().map(|i| i.member.clone()).collect(),
       suggestion: "Standardize default-features setting. Consider setting default-features = false and explicitly enabling needed features".to_string(),
     });
   }
 
   None
 }
+
+#[cfg(test)]
+mod tests {
+  use super::*;
+  use cargo_metadata::{DependencyKind, camino::Utf8PathBuf};
+  use semver::VersionReq;
+
+  fn create_test_metadata() -> WorkspaceMetadata {
+    let current_dir = std::env::current_dir().unwrap();
+    WorkspaceMetadata::load(&current_dir).unwrap()
+  }
+
+  fn create_instance(member: &str, version: &str) -> DependencyInstance {
+    DependencyInstance {
+      member: member.to_string(),
+      name: "test-dep".to_string(),
+      version_req: VersionReq::parse(version).unwrap(),
+      features: vec![],
+      default_features: true,
+      optional: false,
+      kind: DependencyKind::Normal,
+      target: None,
+      rename: None,
+      path: None,
+    }
+  }
+
+  #[test]
+  fn test_detect_renamed_dependency_hard() {
+    let metadata = create_test_metadata();
+    let mut instance1 = create_instance("member1", "1.0.0");
+    instance1.rename = Some("renamed-dep".to_string());
+
+    let instance2 = create_instance("member2", "1.0.0");
+
+    let instances = vec![instance1, instance2];
+
+    let issue = detect_issues("test-dep", &instances, false, &metadata).unwrap();
+
+    assert_eq!(issue.severity, IssueSeverity::Hard);
+    if let IssueType::Renamed { .. } = issue.issue_type {
+      // OK
+    } else {
+      panic!("Expected Renamed issue type");
+    }
+  }
+
+  #[test]
+  fn test_detect_version_conflict_soft() {
+    let metadata = create_test_metadata();
+    let instance1 = create_instance("member1", "1.0.0");
+    let instance2 = create_instance("member2", "2.0.0");
+
+    let instances = vec![instance1, instance2];
+
+    let issue = detect_issues("test-dep", &instances, false, &metadata).unwrap();
+
+    assert_eq!(issue.severity, IssueSeverity::Soft);
+    if let IssueType::IncompatibleVersionRequirements { .. } = issue.issue_type {
+      // OK
+    } else {
+      panic!("Expected IncompatibleVersionRequirements issue type");
+    }
+  }
+
+  #[test]
+  fn test_detect_inconsistent_default_features_soft() {
+    let metadata = create_test_metadata();
+    let mut instance1 = create_instance("member1", "1.0.0");
+    instance1.default_features = true;
+
+    let mut instance2 = create_instance("member2", "1.0.0");
+    instance2.default_features = false;
+
+    let instances = vec![instance1, instance2];
+
+    let issue = detect_issues("test-dep", &instances, false, &metadata).unwrap();
+
+    assert_eq!(issue.severity, IssueSeverity::Soft);
+    if let IssueType::InconsistentDefaultFeatures { .. } = issue.issue_type {
+      // OK
+    } else {
+      panic!("Expected InconsistentDefaultFeatures issue type");
+    }
+  }
+
+  #[test]
+  fn test_detect_path_dependency_hard() {
+    let metadata = create_test_metadata();
+    let mut instance1 = create_instance("member1", "1.0.0");
+    instance1.path = Some(Utf8PathBuf::from("/some/external/path"));
+
+    let instance2 = create_instance("member2", "1.0.0");
+
+    let instances = vec![instance1, instance2];
+
+    let issue = detect_issues("test-dep", &instances, false, &metadata).unwrap();
+
+    assert_eq!(issue.severity, IssueSeverity::Hard);
+    if let IssueType::PathDependency { .. } = issue.issue_type {
+      // OK
+    } else {
+      panic!("Expected PathDependency issue type");
+    }
+  }
+
+  #[test]
+  fn test_detect_target_specific_info() {
+    let metadata = create_test_metadata();
+    let mut instance1 = create_instance("member1", "1.0.0");
+    instance1.target = Some("cfg(unix)".to_string());
+
+    let mut instance2 = create_instance("member2", "1.0.0");
+    instance2.target = Some("cfg(windows)".to_string());
+
+    let instances = vec![instance1, instance2];
+
+    let issue = detect_issues("test-dep", &instances, false, &metadata).unwrap();
+
+    assert_eq!(issue.severity, IssueSeverity::Info);
+    if let IssueType::AllTargetSpecific { .. } = issue.issue_type {
+      // OK
+    } else {
+      panic!("Expected AllTargetSpecific issue type");
+    }
+  }
+}

src/cargo/unify/mod.rs

@@ -40,6 +40,7 @@ mod config;
 mod issue_detection;
 mod path_handling;
 mod plan;
+mod report;
 mod resolution;
 mod strategy;
 mod transitive;
@@ -49,7 +50,8 @@ mod version_merge;
 
 // Re-export public types
 pub use config::{UnifyConfig, UnifyStrategy};
-pub use types::{MemberEdit, UnificationPlan, UnificationStats, UnifiedDep};
+pub use report::UnifyReport;
+pub use types::{IssueSeverity, MemberEdit, UnificationPlan, UnificationStats, UnifiedDep};
 
 /// Workspace dependency unifier
 ///
@@ -124,16 +126,67 @@ impl<'a> WorkspaceUnifier<'a> {
       }
 
       // Detect issues (will naturally pass if versions were successfully merged)
+      // Detect issues (will naturally pass if versions were successfully merged)
+      let mut resolution_comment = None;
       if let Some(issue) =
         issue_detection::detect_issues(&dep_name, &instances, self.config.allow_renamed, self.metadata)
       {
-        issues.push(issue);
-        stats.issue_count += 1;
-        continue;
+        let mut resolved = false;
+
+        // Attempt auto-resolution if enabled
+        if self.config.auto_resolve_version_conflicts {
+          match &issue.issue_type {
+            types::IssueType::IncompatibleVersionRequirements { .. } => {
+              // Auto-resolve: Pick the "highest" version requirement
+              // Heuristic: Sort by string representation and pick the last one
+              // This is a simple heuristic but effective for standard caret requirements
+              let mut versions: Vec<_> = instances.iter().map(|i| i.version_req.clone()).collect();
+              versions.sort_by_key(|a| a.to_string());
+
+              if let Some(highest) = versions.last() {
+                let highest_clone = highest.clone();
+                // Apply to all instances
+                for instance in &mut instances {
+                  instance.version_req = highest_clone.clone();
+                }
+                resolution_comment = Some(format!("Auto-resolved version conflict to {}", highest));
+                resolved = true;
+              }
+            }
+            types::IssueType::InconsistentDefaultFeatures { .. } => {
+              // Auto-resolve: Enable default features (already done by unify_instances logic)
+              // We just need to suppress the issue
+              resolution_comment = Some("Auto-resolved inconsistent default-features (enabled)".to_string());
+              resolved = true;
+            }
+            _ => {}
+          }
+        }
+
+        if !resolved {
+          // If auto-resolution is disabled, version conflicts should block the apply
+          let mut issue = issue;
+          if !self.config.auto_resolve_version_conflicts
+            && matches!(
+              issue.issue_type,
+              types::IssueType::IncompatibleVersionRequirements { .. }
+            )
+          {
+            issue.severity = types::IssueSeverity::Hard;
+          }
+          issues.push(issue);
+          stats.issue_count += 1;
+          continue;
+        }
       }
 
       // Compute unified dependency
-      let unified = unifier::unify_instances(&dep_name, &instances, self.metadata)?;
+      let mut unified = unifier::unify_instances(&dep_name, &instances, self.metadata)?;
+
+      // Add resolution comment if any
+      if let Some(comment) = resolution_comment {
+        unified.comments.push(comment);
+      }
 
       // Track fragmentation savings
       if unified.fragmentation_count > 1 {