refactor: make GenerateDiagram pure — return RenderOutput instead of writing files

crates/application/src/use_cases/generate_diagram.rs

@@ -1,122 +1,68 @@
-use std::path::PathBuf;
-
 use archlens_domain::{
-    BoundaryRule, DomainError, NormalizedGraph, RenderOutput, RenderedFile, check_boundary_rules,
+    BoundaryRule, DomainError, NormalizedGraph, RenderOutput, RenderedFile, RuleViolation,
-    ports::DiagramRenderer,
+    check_boundary_rules, ports::DiagramRenderer,
 };
 
-/// Result of running the generate use case — exposed violations and any output
-/// that should be written to disk.
 pub struct GenerateDiagramResult {
-    pub violations: Vec<String>,
+    pub violations: Vec<RuleViolation>,
     pub output: RenderOutput,
 }
 
-/// Orchestrates diagram generation: renders the graph (split or single),
-/// checks boundary rules, and returns the output for the caller to write.
 pub struct GenerateDiagram {
     pub graph: NormalizedGraph,
     pub renderer: Box<dyn DiagramRenderer>,
     pub allow_rules: Vec<BoundaryRule>,
     pub deny_rules: Vec<BoundaryRule>,
     pub split_by_module: bool,
-    pub format_ext: String,
-    pub output_dir: Option<PathBuf>,
 }
 
 impl GenerateDiagram {
-    pub fn execute(self) -> Result<(), DomainError> {
+    pub fn execute(self) -> Result<GenerateDiagramResult, DomainError> {
-        // Boundary rule checking
         let violations = if !self.allow_rules.is_empty() || !self.deny_rules.is_empty() {
             check_boundary_rules(self.graph.as_graph(), &self.allow_rules, &self.deny_rules)
         } else {
             Vec::new()
         };
 
-        // Render and write
+        let output = if self.split_by_module {
-        if self.split_by_module {
+            render_split(&self.graph, &*self.renderer)?
-            write_split(
-                &self.graph,
-                &*self.renderer,
-                &self.output_dir,
-                &self.format_ext,
-            )?;
         } else {
-            let rendered = self.renderer.render(self.graph.as_graph())?;
+            self.renderer.render(self.graph.as_graph())?
-            write_to_output(rendered, &self.output_dir)?;
+        };
-        }
 
-        // Report violations (after writing so the diagram is still produced)
+        Ok(GenerateDiagramResult { violations, output })
-        for v in &violations {
-            eprintln!("RULE VIOLATION: {}", v.message());
-        }
-
-        Ok(())
-    }
-
-    pub fn check_violations_only(&self) -> Vec<String> {
-        if self.allow_rules.is_empty() && self.deny_rules.is_empty() {
-            return Vec::new();
-        }
-        check_boundary_rules(self.graph.as_graph(), &self.allow_rules, &self.deny_rules)
-            .into_iter()
-            .map(|v| v.message())
-            .collect()
     }
 }
 
-pub fn write_split(
+fn render_split(
     graph: &NormalizedGraph,
     renderer: &dyn DiagramRenderer,
-    output_dir: &Option<PathBuf>,
+) -> Result<RenderOutput, DomainError> {
-    ext: &str,
+    let mut files = Vec::new();
-) -> Result<(), DomainError> {
-    let dir = output_dir.clone().unwrap_or_else(|| PathBuf::from("."));
 
     let overview = renderer.render(graph.as_graph())?;
-    let overview_file = RenderedFile::new(
+    let ext = overview
-        &format!("overview.{ext}"),
+        .files()
-        overview.files().first().map(|f| f.content()).unwrap_or(""),
+        .first()
-    )?;
+        .and_then(|f| std::path::Path::new(f.name()).extension())
-    write_file_to_dir(&dir, overview_file)?;
+        .and_then(|e| e.to_str())
+        .unwrap_or("txt");
+
+    if let Some(f) = overview.files().first() {
+        files.push(RenderedFile::new(&format!("overview.{ext}"), f.content())?);
+    }
 
     for module in graph.modules() {
         let subgraph = graph.subgraph_by_module(&module);
         let cross_deps = graph.cross_module_deps_for(&module);
         let module_output = renderer.render_for_module(&subgraph, &module, &cross_deps)?;
-        let module_file = RenderedFile::new(
+        if let Some(f) = module_output.files().first() {
-            &format!("{}.{ext}", module.as_str().to_lowercase()),
+            files.push(RenderedFile::new(
-            module_output
+                &format!("{}.{ext}", module.as_str().to_lowercase()),
-                .files()
+                f.content(),
-                .first()
+            )?);
-                .map(|f| f.content())
-                .unwrap_or(""),
-        )?;
-        write_file_to_dir(&dir, module_file)?;
-    }
-
-    Ok(())
-}
-
-fn write_file_to_dir(dir: &PathBuf, file: RenderedFile) -> Result<(), DomainError> {
-    let path = dir.join(file.name());
-    std::fs::create_dir_all(dir).map_err(|e| DomainError::IoError(e.to_string()))?;
-    std::fs::write(&path, file.content()).map_err(|e| DomainError::IoError(e.to_string()))?;
-    Ok(())
-}
-
-fn write_to_output(rendered: RenderOutput, output: &Option<PathBuf>) -> Result<(), DomainError> {
-    let content = rendered.files().first().map(|f| f.content()).unwrap_or("");
-    match output {
-        Some(path) => {
-            if let Some(parent) = path.parent() {
-                std::fs::create_dir_all(parent).map_err(|e| DomainError::IoError(e.to_string()))?;
-            }
-            std::fs::write(path, content).map_err(|e| DomainError::IoError(e.to_string()))
-        }
-        None => {
-            print!("{content}");
-            Ok(())
         }
     }
+
+    Ok(RenderOutput::new(files))
 }

crates/application/tests/generate_diagram_tests.rs

@@ -0,0 +1,113 @@
+mod fakes;
+
+use std::path::Path;
+
+use archlens_application::queries::AnalyzeCodebase;
+use archlens_application::use_cases::generate_diagram::GenerateDiagram;
+use archlens_domain::{
+    AnalysisConfig, AnalysisResult, BoundaryRule, CodeElement, CodeElementKind, CodeGraph,
+    FilePath, Language, ModuleName, NormalizedGraph, Relationship, RelationshipKind, SourceFile,
+};
+
+use fakes::{FakeDiagramRenderer, FakeFileDiscovery, FakeSourceAnalyzer};
+
+fn empty_graph() -> NormalizedGraph {
+    NormalizedGraph::from_project(CodeGraph::new())
+}
+
+fn graph_with_one_module() -> NormalizedGraph {
+    let files = vec![SourceFile::new(
+        FilePath::new("/p/src/orders/order.rs").unwrap(),
+        Language::Rust,
+    )];
+    let discovery = FakeFileDiscovery::new(files);
+    let analyzer = FakeSourceAnalyzer::new().with_result(
+        "/p/src/orders/order.rs",
+        AnalysisResult::new(
+            vec![CodeElement::new(
+                "Order",
+                CodeElementKind::Struct,
+                FilePath::new("/p/src/orders/order.rs").unwrap(),
+                1,
+            )
+            .unwrap()],
+            vec![],
+            vec![],
+        ),
+    );
+    AnalyzeCodebase::new(discovery, analyzer)
+        .execute(Path::new("/p"), &AnalysisConfig::default())
+        .unwrap()
+        .graph()
+        .clone()
+}
+
+fn graph_with_violation() -> NormalizedGraph {
+    let mut cg = CodeGraph::new();
+    let a = CodeElement::new("A", CodeElementKind::Struct, FilePath::new("a.rs").unwrap(), 1)
+        .unwrap()
+        .with_module(ModuleName::new("Alpha").unwrap());
+    let b = CodeElement::new("B", CodeElementKind::Struct, FilePath::new("b.rs").unwrap(), 1)
+        .unwrap()
+        .with_module(ModuleName::new("Beta").unwrap());
+    cg.add_element(a);
+    cg.add_element(b);
+    cg.add_relationship(Relationship::new("A", "B", RelationshipKind::Composition).unwrap());
+    NormalizedGraph::from_project(cg)
+}
+
+#[test]
+fn execute_returns_render_output_without_writing_files() {
+    let use_case = GenerateDiagram {
+        graph: empty_graph(),
+        renderer: Box::new(FakeDiagramRenderer::new()),
+        allow_rules: vec![],
+        deny_rules: vec![],
+        split_by_module: false,
+    };
+    let result = use_case.execute().unwrap();
+    assert!(!result.output.files().is_empty());
+}
+
+#[test]
+fn execute_returns_empty_violations_when_no_rules_set() {
+    let use_case = GenerateDiagram {
+        graph: empty_graph(),
+        renderer: Box::new(FakeDiagramRenderer::new()),
+        allow_rules: vec![],
+        deny_rules: vec![],
+        split_by_module: false,
+    };
+    let result = use_case.execute().unwrap();
+    assert!(result.violations.is_empty());
+}
+
+#[test]
+fn execute_returns_violations_as_rule_violation_type() {
+    let deny = vec![BoundaryRule::parse("Alpha --> Beta").unwrap()];
+    let use_case = GenerateDiagram {
+        graph: graph_with_violation(),
+        renderer: Box::new(FakeDiagramRenderer::new()),
+        allow_rules: vec![],
+        deny_rules: deny,
+        split_by_module: false,
+    };
+    let result = use_case.execute().unwrap();
+    assert_eq!(result.violations.len(), 1);
+    assert!(result.violations[0].message().contains("Alpha"));
+    assert!(result.violations[0].message().contains("Beta"));
+}
+
+#[test]
+fn split_by_module_returns_multiple_rendered_files() {
+    let use_case = GenerateDiagram {
+        graph: graph_with_one_module(),
+        renderer: Box::new(FakeDiagramRenderer::new()),
+        allow_rules: vec![],
+        deny_rules: vec![],
+        split_by_module: true,
+    };
+    let result = use_case.execute().unwrap();
+    // overview + at least one module file
+    assert!(result.output.files().len() >= 2);
+}

crates/presentation/src/lib.rs

@@ -6,13 +6,13 @@ use archlens_application::queries::AnalyzeCodebase;
 use archlens_application::use_cases::{
     check_freshness::CheckFreshness,
     diff_diagram::DiffDiagram,
-    generate_diagram::{GenerateDiagram, write_split},
+    generate_diagram::GenerateDiagram,
 };
 use archlens_ascii::AsciiRenderer;
 use archlens_cargo_workspace::CargoWorkspaceAnalyzer;
 use archlens_d2::D2Renderer;
 use archlens_domain::{
-    BoundaryRule, DiagramLevel, NormalizedGraph,
+    BoundaryRule, DiagramLevel, NormalizedGraph, RenderOutput,
     ports::{ConfigLoader, ProjectAnalyzer},
 };
 use archlens_html::HtmlRenderer;
@@ -70,7 +70,6 @@ pub fn run(args: Cli) -> Result<()> {
         .iter()
         .filter_map(|s| BoundaryRule::parse(s))
         .collect();
-    let output_dir = args.output.as_ref().map(std::path::PathBuf::from);
 
     let use_case = GenerateDiagram {
         graph,
@@ -78,22 +77,44 @@ pub fn run(args: Cli) -> Result<()> {
         allow_rules: allow,
         deny_rules: deny,
         split_by_module: args.split_by_module,
-        format_ext: format_extension(&args.format).to_string(),
-        output_dir,
     };
 
-    let violations = use_case.check_violations_only();
+    let result = use_case.execute()?;
-    if args.strict && !violations.is_empty() {
+    if args.strict && !result.violations.is_empty() {
         bail!(
             "{} boundary rule violation(s) in strict mode",
-            violations.len()
+            result.violations.len()
         );
     }
-    use_case.execute()?;
+    for v in &result.violations {
+        eprintln!("RULE VIOLATION: {}", v.message());
+    }
+    write_diagram_output(&result.output, args.output.as_deref(), args.split_by_module)?;
 
     Ok(())
 }
 
+fn write_diagram_output(output: &RenderOutput, output_path: Option<&str>, split: bool) -> Result<()> {
+    if split {
+        let dir = std::path::PathBuf::from(output_path.unwrap_or("."));
+        std::fs::create_dir_all(&dir)?;
+        for file in output.files() {
+            std::fs::write(dir.join(file.name()), file.content())?;
+        }
+    } else if let Some(path) = output_path {
+        let p = std::path::Path::new(path);
+        if let Some(parent) = p.parent() {
+            std::fs::create_dir_all(parent)?;
+        }
+        let content = output.files().first().map(|f| f.content()).unwrap_or("");
+        std::fs::write(p, content)?;
+    } else {
+        let content = output.files().first().map(|f| f.content()).unwrap_or("");
+        print!("{content}");
+    }
+    Ok(())
+}
+
 fn load_config(args: &Cli) -> Result<TomlConfigLoader> {
     match &args.config {
         Some(path) => Ok(TomlConfigLoader::from_path(std::path::Path::new(path))?),
@@ -194,15 +215,6 @@ fn create_renderer(
     }
 }
 
-fn format_extension(format: &str) -> &str {
-    match format {
-        "mermaid" => "mmd",
-        "d2" => "d2",
-        "html" => "html",
-        _ => "txt",
-    }
-}
-
 fn run_diff(args: &Cli, existing_path: &std::path::Path) -> Result<()> {
     init_tracing(args.verbose);
 
@@ -332,25 +344,12 @@ fn run_watch(args: Cli) -> Result<()> {
     use std::time::{Duration, Instant};
 
     let level = parse_level(&args.level);
-    let ext = format_extension(&args.format);
     let debounce = Duration::from_millis(500);
 
     let run_once = |args: &Cli| -> Result<()> {
         let config_loader = load_config(args)?;
         let graph = build_graph(args, level)?;
         let renderer = create_renderer(&args.format, level, !args.no_weights)?;
-        let output_dir = args.output.as_ref().map(std::path::PathBuf::from);
-
-        if args.split_by_module {
-            write_split(&graph, &*renderer, &output_dir, ext)?;
-        } else {
-            let rendered = renderer.render(graph.as_graph())?;
-            let content = rendered.files().first().map(|f| f.content()).unwrap_or("");
-            match &output_dir {
-                Some(path) => std::fs::write(path, content)?,
-                None => print!("{content}"),
-            }
-        }
 
         let (raw_allow, raw_deny) = config_loader.load_rules();
         let allow: Vec<BoundaryRule> = raw_allow
@@ -361,19 +360,18 @@ fn run_watch(args: Cli) -> Result<()> {
             .iter()
             .filter_map(|s| BoundaryRule::parse(s))
             .collect();
-        if !allow.is_empty() || !deny.is_empty() {
+
-            let use_case = GenerateDiagram {
+        let use_case = GenerateDiagram {
-                graph,
+            graph,
-                renderer,
+            renderer,
-                allow_rules: allow,
+            allow_rules: allow,
-                deny_rules: deny,
+            deny_rules: deny,
-                split_by_module: false,
+            split_by_module: args.split_by_module,
-                format_ext: ext.to_string(),
+        };
-                output_dir: None,
+        let result = use_case.execute()?;
-            };
+        write_diagram_output(&result.output, args.output.as_deref(), args.split_by_module)?;
-            for v in use_case.check_violations_only() {
+        for v in &result.violations {
-                eprintln!("RULE VIOLATION: {v}");
+            eprintln!("RULE VIOLATION: {}", v.message());
-            }
         }
         Ok(())
     };