cargo-rail: fixing the commands and cleaning up for uniformity

Author: loadingalias

src/commands/affected.rs

@@ -8,6 +8,7 @@
 use crate::change_detection::classify::{ChangeKind, classify_file};
 use crate::config::ChangeDetectionConfig;
 use crate::error::{RailError, RailResult};
+use crate::git::detect_default_base_ref;
 use crate::graph::AffectedAnalysis;
 use crate::workspace::WorkspaceContext;
 use glob::Pattern;
@@ -48,22 +49,28 @@ impl OutputFormat {
 /// Run the affected command
 pub fn run_affected(
   ctx: &WorkspaceContext,
-  since: String,
+  since: Option<String>,
   from: Option<String>,
   to: Option<String>,
   format: String,
   all: bool,
-  output_file: Option<PathBuf>,
+  output: Option<PathBuf>,
 ) -> RailResult<()> {
   let output_format = OutputFormat::from_str(&format)?;
 
   // If --all flag is set, show all workspace crates regardless of changes
   if all {
-    return display_all_crates(ctx, output_format, output_file.as_ref());
+    return display_all_crates(ctx, output_format, output.as_ref());
   }
 
+  // Auto-detect default branch if --since not specified
+  let since_ref = match since {
+    Some(s) => s,
+    None => detect_default_base_ref(ctx.git.git())?,
+  };
+
   // Get changed files from git
-  let changed_files = get_changed_files(ctx, &since, from.as_deref(), to.as_deref())?;
+  let changed_files = get_changed_files(ctx, &since_ref, from.as_deref(), to.as_deref())?;
 
   // Get change-detection config (from rail.toml or defaults)
   let cd_config = ctx
@@ -79,7 +86,7 @@ pub fn run_affected(
   let analysis = crate::graph::analyze(&ctx.graph, &changed_files)?;
 
   // Output results
-  display_results(&analysis, &classification, output_format, output_file.as_ref())?;
+  display_results(&analysis, &classification, output_format, output.as_ref())?;
 
   Ok(())
 }

src/commands/clean.rs

@@ -5,9 +5,7 @@ use crate::workspace::WorkspaceContext;
 use std::fs;
 
 /// Run the clean command
-pub fn run_clean(ctx: &WorkspaceContext, cache: bool, backups: bool, reports: bool) -> RailResult<()> {
-  let mut cleaned_any = false;
-
+pub fn run_clean(ctx: &WorkspaceContext, cache: bool, backups: bool, reports: bool, check: bool) -> RailResult<()> {
   // If no flags provided, clean everything (cache, reports, and ALL backups)
   let clean_all = !cache && !backups && !reports;
 
@@ -17,6 +15,78 @@ pub fn run_clean(ctx: &WorkspaceContext, cache: bool, backups: bool, reports: bo
   let prune_backups = backups && !clean_all;
   let delete_all_backups = clean_all;
 
+  // Dry-run mode: show what would be cleaned
+  if check {
+    println!("🔍 DRY-RUN MODE - Showing what would be cleaned:\n");
+    let mut would_clean = false;
+
+    if clean_cache {
+      let cache_path = ctx
+        .workspace_root
+        .join("target")
+        .join("cargo-rail")
+        .join("metadata.json");
+      let old_cache = ctx.workspace_root.join("target").join("rail");
+
+      if cache_path.exists() {
+        println!("  📄 {}", cache_path.display());
+        would_clean = true;
+      }
+      if old_cache.exists() {
+        println!("  📁 {} (legacy)", old_cache.display());
+        would_clean = true;
+      }
+    }
+
+    if clean_reports {
+      let report_dir = ctx.workspace_root.join("target").join("cargo-rail");
+      if report_dir.exists() {
+        for entry in fs::read_dir(&report_dir).ok().into_iter().flatten().flatten() {
+          let path = entry.path();
+          if path.is_file() && path.extension().is_some_and(|ext| ext == "md") {
+            println!("  📄 {}", path.display());
+            would_clean = true;
+          }
+        }
+      }
+    }
+
+    if prune_backups || delete_all_backups {
+      let backup_manager = BackupManager::new(&ctx.workspace_root);
+      if backup_manager.has_backups() {
+        let backup_list = backup_manager.list_backups()?;
+        if delete_all_backups {
+          for backup in &backup_list {
+            println!("  📦 backup: {}", backup.id);
+            would_clean = true;
+          }
+        } else {
+          let max_backups = ctx
+            .config
+            .as_ref()
+            .map(|c| c.unify.max_backups)
+            .unwrap_or_else(|| UnifyConfig::default().max_backups);
+          if backup_list.len() > max_backups {
+            for backup in backup_list.iter().skip(max_backups) {
+              println!("  📦 backup (prune): {}", backup.id);
+              would_clean = true;
+            }
+          }
+        }
+      }
+    }
+
+    if would_clean {
+      println!("\n✋ To execute cleanup, run: cargo rail clean");
+    } else {
+      println!("Nothing to clean");
+    }
+    return Ok(());
+  }
+
+  // Execute cleanup
+  let mut cleaned_any = false;
+
   if clean_cache {
     clean_metadata_cache(ctx)?;
     cleaned_any = true;

src/commands/common.rs

@@ -6,6 +6,51 @@ use crate::split::SplitConfig;
 use crate::sync::SyncConfig;
 use crate::workspace::WorkspaceContext;
 use std::path::PathBuf;
+use std::str::FromStr;
+
+/// Standard output format for most commands
+///
+/// Use this for commands that just need text/json output.
+/// For commands with more formats (like `affected`), define a specialized enum.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
+pub enum OutputFormat {
+  /// Human-readable text output (default)
+  #[default]
+  Text,
+  /// Machine-readable JSON output
+  Json,
+}
+
+impl OutputFormat {
+  /// Check if this format is JSON
+  pub fn is_json(&self) -> bool {
+    matches!(self, Self::Json)
+  }
+}
+
+impl FromStr for OutputFormat {
+  type Err = RailError;
+
+  fn from_str(s: &str) -> Result<Self, Self::Err> {
+    match s.to_lowercase().as_str() {
+      "text" => Ok(Self::Text),
+      "json" => Ok(Self::Json),
+      _ => Err(RailError::message(format!(
+        "Unknown format '{}'. Valid formats: text, json",
+        s
+      ))),
+    }
+  }
+}
+
+impl std::fmt::Display for OutputFormat {
+  fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+    match self {
+      Self::Text => write!(f, "text"),
+      Self::Json => write!(f, "json"),
+    }
+  }
+}
 
 /// Builder for split/sync configurations
 ///

src/commands/mod.rs

@@ -38,6 +38,7 @@ pub mod unify;
 
 pub use affected::run_affected;
 pub use clean::run_clean;
+pub use common::OutputFormat;
 pub use init::{run_init, run_init_standalone};
 pub use release::{run_release_check, run_release_init, run_release_plan, run_release_publish};
 pub use split::{run_split, run_split_init};

src/commands/release.rs

@@ -1,5 +1,6 @@
 //! `cargo rail release` - Release automation commands
 
+use crate::commands::common::OutputFormat;
 use crate::error::{RailError, RailResult};
 use crate::release::planner::ReleasePlanner;
 use crate::release::publisher::ReleasePublisher;
@@ -17,8 +18,11 @@ pub fn run_release_plan(
   bump: String,
   skip_publish: bool,
   skip_tag: bool,
-  json: bool,
+  format: String,
 ) -> RailResult<()> {
+  let output_format: OutputFormat = format.parse()?;
+  let json = output_format.is_json();
+
   if !json {
     eprintln!("🔍 Planning release...\n");
   }
@@ -151,8 +155,18 @@ pub fn run_release_publish(
 /// Run release check command (CI validation)
 ///
 /// Validates that crates are ready for release without making changes.
-pub fn run_release_check(ctx: &WorkspaceContext, crate_names: Option<Vec<String>>, all: bool) -> RailResult<()> {
-  println!("🔍 Checking release readiness...\n");
+pub fn run_release_check(
+  ctx: &WorkspaceContext,
+  crate_names: Option<Vec<String>>,
+  all: bool,
+  format: String,
+) -> RailResult<()> {
+  let output_format: OutputFormat = format.parse()?;
+  let json = output_format.is_json();
+
+  if !json {
+    println!("🔍 Checking release readiness...\n");
+  }
 
   // Get release config
   let config = ctx.config.as_ref().map(|c| &c.release);
@@ -179,13 +193,30 @@ pub fn run_release_check(ctx: &WorkspaceContext, crate_names: Option<Vec<String>
   let validator = ReleaseValidator::new(ctx);
   validator.validate(&target_crates, release_config.require_clean)?;
 
-  // Check publishability
+  // Check publishability and collect results
+  let mut results = Vec::new();
   for crate_name in &target_crates {
     validator.validate_publishable(crate_name)?;
-    println!("✅ {} is ready for release", crate_name);
+    results.push(crate_name.clone());
+    if !json {
+      println!("✅ {} is ready for release", crate_name);
+    }
+  }
+
+  if json {
+    let output = serde_json::json!({
+      "status": "passed",
+      "crates": results,
+      "count": results.len()
+    });
+    println!(
+      "{}",
+      serde_json::to_string_pretty(&output).map_err(|e| RailError::message(format!("JSON error: {}", e)))?
+    );
+  } else {
+    println!("\n✅ All release checks passed!");
   }
 
-  println!("\n✅ All release checks passed!");
   Ok(())
 }
 

src/commands/split.rs

@@ -1,6 +1,6 @@
 use std::io::IsTerminal;
 
-use crate::commands::common::SplitSyncConfigBuilder;
+use crate::commands::common::{OutputFormat, SplitSyncConfigBuilder};
 use crate::config::RailConfig;
 use crate::error::RailResult;
 use crate::split::SplitEngine;
@@ -18,8 +18,11 @@ pub fn run_split(
   all: bool,
   remote: Option<String>,
   dry_run: bool,
-  json: bool,
+  format: String,
 ) -> RailResult<()> {
+  let output_format: OutputFormat = format.parse()?;
+  let json = output_format.is_json();
+
   if !json {
     eprintln!("📦 Loaded configuration");
   }

src/commands/sync.rs

@@ -1,7 +1,7 @@
 use std::io::IsTerminal;
 use std::str::FromStr;
 
-use crate::commands::common::SplitSyncConfigBuilder;
+use crate::commands::common::{OutputFormat, SplitSyncConfigBuilder};
 use crate::error::RailResult;
 use crate::sync::{ConflictStrategy, SyncDirection, SyncEngine};
 use crate::utils;
@@ -23,8 +23,11 @@ pub fn run_sync(
   strategy_str: String,
   _no_protected_branches: bool,
   dry_run: bool,
-  json: bool,
+  format: String,
 ) -> RailResult<()> {
+  let output_format: OutputFormat = format.parse()?;
+  let json = output_format.is_json();
+
   // Parse conflict strategy
   let conflict_strategy = ConflictStrategy::from_str(&strategy_str)?;
 

src/commands/unify.rs

@@ -6,7 +6,8 @@
 //! 3. Clean unification with intersection-based features
 
 use crate::cargo::{ManifestWriter, UnifyAnalyzer, UnifyReport};
-use crate::error::RailResult;
+use crate::commands::common::OutputFormat;
+use crate::error::{RailError, RailResult};
 use crate::workspace::WorkspaceContext;
 
 /// Run dependency unification analysis (dry-run)
@@ -17,8 +18,14 @@ pub fn run_unify_analyze(
   pin_transitives_flag: bool,
   include_renamed_flag: bool,
   show_diff: bool,
+  format: String,
 ) -> RailResult<()> {
-  eprintln!("🔍 Analyzing workspace dependencies...\n");
+  let output_format: OutputFormat = format.parse()?;
+  let json = output_format.is_json();
+
+  if !json {
+    eprintln!("🔍 Analyzing workspace dependencies...\n");
+  }
 
   // Create analyzer
   let mut analyzer = UnifyAnalyzer::new(ctx)?;
@@ -40,6 +47,31 @@ pub fn run_unify_analyze(
   // Run analysis
   let plan = analyzer.analyze()?;
 
+  // JSON output mode
+  if json {
+    let output = serde_json::json!({
+      "workspace_deps": plan.workspace_deps.iter().map(|d| serde_json::json!({
+        "name": d.name,
+        "version": d.version_req,
+        "features": d.features,
+      })).collect::<Vec<_>>(),
+      "member_edits_count": plan.member_edits.values().map(|v| v.len()).sum::<usize>(),
+      "members_affected": plan.member_edits.len(),
+      "transitive_pins": plan.transitive_pins.len(),
+      "has_blocking_issues": plan.has_blocking_issues(),
+      "issues": plan.issues.iter().map(|i| serde_json::json!({
+        "dep_name": i.dep_name,
+        "severity": format!("{:?}", i.severity),
+        "message": i.message,
+      })).collect::<Vec<_>>(),
+    });
+    println!(
+      "{}",
+      serde_json::to_string_pretty(&output).map_err(|e| RailError::message(format!("JSON error: {}", e)))?
+    );
+    return Ok(());
+  }
+
   // Display summary
   println!("{}", plan.summary());