From 692a64a6223b5849f2bc70980bbca10d18446359 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Wed, 17 Jun 2026 13:13:27 +0200 Subject: [PATCH] =?UTF-8?q?refactor:=20make=20GenerateDiagram=20pure=20?= =?UTF-8?q?=E2=80=94=20return=20RenderOutput=20instead=20of=20writing=20fi?= =?UTF-8?q?les?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/use_cases/generate_diagram.rs | 112 +++++------------ .../tests/generate_diagram_tests.rs | 113 ++++++++++++++++++ crates/presentation/src/lib.rs | 86 +++++++------ 3 files changed, 184 insertions(+), 127 deletions(-) create mode 100644 crates/application/tests/generate_diagram_tests.rs diff --git a/crates/application/src/use_cases/generate_diagram.rs b/crates/application/src/use_cases/generate_diagram.rs index 5bf8e3a..9267d0e 100644 --- a/crates/application/src/use_cases/generate_diagram.rs +++ b/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, - ports::DiagramRenderer, + BoundaryRule, DomainError, NormalizedGraph, RenderOutput, RenderedFile, RuleViolation, + 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, + pub violations: Vec, 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, pub allow_rules: Vec, pub deny_rules: Vec, pub split_by_module: bool, - pub format_ext: String, - pub output_dir: Option, } impl GenerateDiagram { - pub fn execute(self) -> Result<(), DomainError> { - // Boundary rule checking + pub fn execute(self) -> Result { 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 - if self.split_by_module { - write_split( - &self.graph, - &*self.renderer, - &self.output_dir, - &self.format_ext, - )?; + let output = if self.split_by_module { + render_split(&self.graph, &*self.renderer)? } else { - let rendered = self.renderer.render(self.graph.as_graph())?; - write_to_output(rendered, &self.output_dir)?; - } + self.renderer.render(self.graph.as_graph())? + }; - // Report violations (after writing so the diagram is still produced) - for v in &violations { - eprintln!("RULE VIOLATION: {}", v.message()); - } - - Ok(()) - } - - pub fn check_violations_only(&self) -> Vec { - 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() + Ok(GenerateDiagramResult { violations, output }) } } -pub fn write_split( +fn render_split( graph: &NormalizedGraph, renderer: &dyn DiagramRenderer, - output_dir: &Option, - ext: &str, -) -> Result<(), DomainError> { - let dir = output_dir.clone().unwrap_or_else(|| PathBuf::from(".")); +) -> Result { + let mut files = Vec::new(); let overview = renderer.render(graph.as_graph())?; - let overview_file = RenderedFile::new( - &format!("overview.{ext}"), - overview.files().first().map(|f| f.content()).unwrap_or(""), - )?; - write_file_to_dir(&dir, overview_file)?; + let ext = overview + .files() + .first() + .and_then(|f| std::path::Path::new(f.name()).extension()) + .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( - &format!("{}.{ext}", module.as_str().to_lowercase()), - module_output - .files() - .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) -> 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(()) + if let Some(f) = module_output.files().first() { + files.push(RenderedFile::new( + &format!("{}.{ext}", module.as_str().to_lowercase()), + f.content(), + )?); } } + + Ok(RenderOutput::new(files)) } diff --git a/crates/application/tests/generate_diagram_tests.rs b/crates/application/tests/generate_diagram_tests.rs new file mode 100644 index 0000000..9b5ff07 --- /dev/null +++ b/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); +} diff --git a/crates/presentation/src/lib.rs b/crates/presentation/src/lib.rs index 20495f3..1b0b37e 100644 --- a/crates/presentation/src/lib.rs +++ b/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(); - if args.strict && !violations.is_empty() { + let result = use_case.execute()?; + 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 { 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 = 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 { - graph, - renderer, - allow_rules: allow, - deny_rules: deny, - split_by_module: false, - format_ext: ext.to_string(), - output_dir: None, - }; - for v in use_case.check_violations_only() { - eprintln!("RULE VIOLATION: {v}"); - } + + let use_case = GenerateDiagram { + graph, + renderer, + allow_rules: allow, + deny_rules: deny, + split_by_module: args.split_by_module, + }; + let result = use_case.execute()?; + write_diagram_output(&result.output, args.output.as_deref(), args.split_by_module)?; + for v in &result.violations { + eprintln!("RULE VIOLATION: {}", v.message()); } Ok(()) };