From 9ba33c3e746fa001d4b49bdd29e577dddac58b57 Mon Sep 17 00:00:00 2001 From: alexion Date: Thu, 17 Sep 2026 18:34:43 -0400 Subject: [PATCH] refactor(xatu): split lint architecture modules --- src/xatu/BUILD | 11 +- src/xatu/file_store.rs | 27 +++ src/xatu/finding.rs | 75 ++++++++ src/xatu/node_rule.rs | 26 +++ src/xatu/reporting.rs | 60 +++++++ src/xatu/rules.rs | 5 + src/xatu/rules/empty_inherit.rs | 33 ++++ src/xatu/syntax.rs | 135 +++++++++++++++ src/xatu/xatu.rs | 292 +++----------------------------- src/xatu/xatu_test.rs | 10 +- 10 files changed, 395 insertions(+), 279 deletions(-) create mode 100644 src/xatu/file_store.rs create mode 100644 src/xatu/finding.rs create mode 100644 src/xatu/node_rule.rs create mode 100644 src/xatu/reporting.rs create mode 100644 src/xatu/rules.rs create mode 100644 src/xatu/rules/empty_inherit.rs create mode 100644 src/xatu/syntax.rs diff --git a/src/xatu/BUILD b/src/xatu/BUILD index 502fff2..fb1f77a 100644 --- a/src/xatu/BUILD +++ b/src/xatu/BUILD @@ -3,7 +3,16 @@ load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rust_test") rust_library( name = "xatu_lib", crate_name = "xatu", - srcs = ["xatu.rs"], + srcs = [ + "file_store.rs", + "finding.rs", + "node_rule.rs", + "reporting.rs", + "rules.rs", + "rules/empty_inherit.rs", + "syntax.rs", + "xatu.rs", + ], deps = [ "//src/status:status", "//third_party/crates:rnix", diff --git a/src/xatu/file_store.rs b/src/xatu/file_store.rs new file mode 100644 index 0000000..e4c792a --- /dev/null +++ b/src/xatu/file_store.rs @@ -0,0 +1,27 @@ +use std::path::PathBuf; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct FileId(usize); + +#[derive(Debug, Default)] +pub struct FileStore { + files: Vec, +} + +impl FileStore { + pub fn add(&mut self, file: SourceFile) -> FileId { + let id = FileId(self.files.len()); + self.files.push(file); + id + } + + pub fn get(&self, id: FileId) -> &SourceFile { + &self.files[id.0] + } +} + +#[derive(Debug, Clone)] +pub struct SourceFile { + pub path: PathBuf, + pub source: String, +} diff --git a/src/xatu/finding.rs b/src/xatu/finding.rs new file mode 100644 index 0000000..c347eaa --- /dev/null +++ b/src/xatu/finding.rs @@ -0,0 +1,75 @@ +use crate::file_store::FileId; +use std::ops::Range; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Severity { + Error, + Warning, +} + +impl Severity { + pub fn as_str(self) -> &'static str { + match self { + Self::Error => "error", + Self::Warning => "warning", + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RuleId { + ParseError, + EmptyInherit, +} + +impl RuleId { + pub fn metadata(self) -> RuleMetadata { + match self { + Self::ParseError => RuleMetadata { + id: self, + code: "parse_error", + severity: Severity::Error, + }, + Self::EmptyInherit => RuleMetadata { + id: self, + code: "empty_inherit", + severity: Severity::Warning, + }, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct RuleMetadata { + pub id: RuleId, + pub code: &'static str, + pub severity: Severity, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Finding { + pub file_id: FileId, + pub range: Range, + pub rule_id: RuleId, + pub message: String, +} + +impl Finding { + pub fn new( + file_id: FileId, + range: Range, + rule_id: RuleId, + message: impl Into, + ) -> Self { + Self { + file_id, + range, + rule_id, + message: message.into(), + } + } + + pub fn metadata(&self) -> RuleMetadata { + self.rule_id.metadata() + } +} diff --git a/src/xatu/node_rule.rs b/src/xatu/node_rule.rs new file mode 100644 index 0000000..3c04ad0 --- /dev/null +++ b/src/xatu/node_rule.rs @@ -0,0 +1,26 @@ +use crate::file_store::FileId; +use crate::finding::{Finding, RuleMetadata}; +use crate::syntax::{NodeKind, SyntaxNode}; + +pub trait NodeRule: Sync { + fn metadata(&self) -> RuleMetadata; + fn node_kinds(&self) -> &'static [NodeKind]; + fn check(&self, file_id: FileId, node: &SyntaxNode, findings: &mut Vec); +} + +pub fn dispatch_node_rules( + rules: &[&dyn NodeRule], + file_id: FileId, + node: &SyntaxNode, + findings: &mut Vec, +) { + let Some(kind) = node.kind() else { + return; + }; + + for rule in rules { + if rule.node_kinds().contains(&kind) { + rule.check(file_id, node, findings); + } + } +} diff --git a/src/xatu/reporting.rs b/src/xatu/reporting.rs new file mode 100644 index 0000000..443e6fb --- /dev/null +++ b/src/xatu/reporting.rs @@ -0,0 +1,60 @@ +use crate::Report; +use crate::file_store::FileStore; +use crate::finding::Finding; +use std::path::Path; + +pub fn format_findings(report: &Report) -> Vec { + report + .findings + .iter() + .map(|finding| format_finding(&report.files, finding)) + .collect() +} + +fn format_finding(files: &FileStore, finding: &Finding) -> String { + let file = files.get(finding.file_id); + let position = line_column(&file.source, finding.range.start); + let metadata = finding.metadata(); + + format!( + "{}:{}:{}: {}[{}]: {}", + display_path(&file.path), + position.line, + position.column, + metadata.severity.as_str(), + metadata.code, + finding.message + ) +} + +fn display_path(path: &Path) -> String { + path.display().to_string() +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct Position { + line: usize, + column: usize, +} + +fn line_column(source: &str, offset: usize) -> Position { + let bounded_offset = offset.min(source.len()); + let mut line = 1; + let mut line_start = 0; + + for (index, byte) in source.bytes().enumerate() { + if index >= bounded_offset { + break; + } + + if byte == b'\n' { + line += 1; + line_start = index + 1; + } + } + + Position { + line, + column: bounded_offset.saturating_sub(line_start) + 1, + } +} diff --git a/src/xatu/rules.rs b/src/xatu/rules.rs new file mode 100644 index 0000000..a432ddb --- /dev/null +++ b/src/xatu/rules.rs @@ -0,0 +1,5 @@ +mod empty_inherit; + +use crate::node_rule::NodeRule; + +pub static NODE_RULES: &[&dyn NodeRule] = &[&empty_inherit::EMPTY_INHERIT]; diff --git a/src/xatu/rules/empty_inherit.rs b/src/xatu/rules/empty_inherit.rs new file mode 100644 index 0000000..bb42223 --- /dev/null +++ b/src/xatu/rules/empty_inherit.rs @@ -0,0 +1,33 @@ +use crate::file_store::FileId; +use crate::finding::{Finding, RuleId, RuleMetadata}; +use crate::node_rule::NodeRule; +use crate::syntax::{NodeKind, SyntaxNode}; + +pub struct EmptyInherit; + +pub static EMPTY_INHERIT: EmptyInherit = EmptyInherit; + +impl NodeRule for EmptyInherit { + fn metadata(&self) -> RuleMetadata { + RuleId::EmptyInherit.metadata() + } + + fn node_kinds(&self) -> &'static [NodeKind] { + &[NodeKind::Inherit] + } + + fn check(&self, file_id: FileId, node: &SyntaxNode, findings: &mut Vec) { + let Some(inherit) = node.as_inherit() else { + return; + }; + + if !inherit.has_attrs() { + findings.push(Finding::new( + file_id, + inherit.text_range(), + self.metadata().id, + "empty inherit statement", + )); + } + } +} diff --git a/src/xatu/syntax.rs b/src/xatu/syntax.rs new file mode 100644 index 0000000..b3b8344 --- /dev/null +++ b/src/xatu/syntax.rs @@ -0,0 +1,135 @@ +use std::ops::Range; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NodeKind { + Inherit, +} + +impl NodeKind { + fn from_rnix(kind: rnix::SyntaxKind) -> Option { + match kind { + rnix::SyntaxKind::NODE_INHERIT => Some(Self::Inherit), + _ => None, + } + } +} + +#[derive(Debug, Clone)] +pub struct ParseError { + range: Range, + message: String, +} + +impl ParseError { + pub fn range(&self) -> Range { + self.range.clone() + } + + pub fn message(&self) -> &str { + &self.message + } +} + +#[derive(Debug, Clone)] +pub struct ParsedSource { + root: SyntaxNode, + errors: Vec, +} + +impl ParsedSource { + pub fn errors(&self) -> &[ParseError] { + &self.errors + } + + pub fn descendants(&self) -> Vec { + self.root.raw.descendants().map(SyntaxNode::new).collect() + } +} + +#[derive(Debug, Clone)] +pub struct SyntaxNode { + raw: rnix::SyntaxNode, +} + +impl SyntaxNode { + fn new(raw: rnix::SyntaxNode) -> Self { + Self { raw } + } + + pub fn kind(&self) -> Option { + NodeKind::from_rnix(self.raw.kind()) + } + + pub fn as_inherit(&self) -> Option { + if self.kind() == Some(NodeKind::Inherit) { + Some(InheritNode { + raw: self.raw.clone(), + }) + } else { + None + } + } +} + +#[derive(Debug, Clone)] +pub struct InheritNode { + raw: rnix::SyntaxNode, +} + +impl InheritNode { + pub fn has_attrs(&self) -> bool { + self.attr_count() > 0 + } + + pub fn attr_count(&self) -> usize { + self.raw + .children() + .filter(|child| { + matches!( + child.kind(), + rnix::SyntaxKind::NODE_IDENT + | rnix::SyntaxKind::NODE_DYNAMIC + | rnix::SyntaxKind::NODE_STRING + ) + }) + .count() + } + + pub fn text_range(&self) -> Range { + let range = self.raw.text_range(); + usize::from(range.start())..usize::from(range.end()) + } +} + +pub fn parse(source: &str) -> ParsedSource { + let parse = rnix::Root::parse(source); + let errors = parse + .errors() + .iter() + .map(|error| ParseError { + range: parse_error_range(error, source.len()), + message: error.to_string(), + }) + .collect(); + + ParsedSource { + root: SyntaxNode::new(parse.syntax()), + errors, + } +} + +fn parse_error_range(error: &rnix::ParseError, source_len: usize) -> Range { + match error { + rnix::ParseError::Unexpected(range) + | rnix::ParseError::UnexpectedExtra(range) + | rnix::ParseError::UnexpectedWanted(_, range, _) + | rnix::ParseError::UnexpectedDoubleBind(range) + | rnix::ParseError::DuplicatedArgs(range, _) => { + usize::from(range.start())..usize::from(range.end()) + } + rnix::ParseError::UnexpectedEOF + | rnix::ParseError::UnexpectedEOFWanted(_) + | rnix::ParseError::RecursionLimitExceeded => source_len..source_len, + _ => source_len..source_len, + } +} diff --git a/src/xatu/xatu.rs b/src/xatu/xatu.rs index 8be3adf..8c0e385 100644 --- a/src/xatu/xatu.rs +++ b/src/xatu/xatu.rs @@ -1,174 +1,14 @@ -use std::ops::Range; -use std::path::{Path, PathBuf}; +mod file_store; +mod finding; +mod node_rule; +mod reporting; +mod rules; +mod syntax; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct FileId(usize); - -#[derive(Debug, Default)] -pub struct FileStore { - files: Vec, -} - -impl FileStore { - pub fn add(&mut self, file: SourceFile) -> FileId { - let id = FileId(self.files.len()); - self.files.push(file); - id - } - - pub fn get(&self, id: FileId) -> &SourceFile { - &self.files[id.0] - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum Severity { - Error, - Warning, -} - -impl Severity { - fn as_str(self) -> &'static str { - match self { - Self::Error => "error", - Self::Warning => "warning", - } - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct RuleMetadata { - pub id: &'static str, - pub severity: Severity, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Finding { - pub file_id: FileId, - pub range: Range, - pub rule_id: &'static str, - pub message: String, -} - -#[derive(Debug, Clone)] -pub struct SourceFile { - pub path: PathBuf, - pub source: String, -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum NodeKind { - Inherit, -} - -impl NodeKind { - fn from_rnix(kind: rnix::SyntaxKind) -> Option { - match kind { - rnix::SyntaxKind::NODE_INHERIT => Some(Self::Inherit), - _ => None, - } - } -} - -#[derive(Debug, Clone)] -pub struct SyntaxNode { - raw: rnix::SyntaxNode, -} - -impl SyntaxNode { - fn new(raw: rnix::SyntaxNode) -> Self { - Self { raw } - } - - pub fn kind(&self) -> Option { - NodeKind::from_rnix(self.raw.kind()) - } - - pub fn text_range(&self) -> Range { - let range = self.raw.text_range(); - usize::from(range.start())..usize::from(range.end()) - } - - pub fn as_inherit(&self) -> Option { - if self.kind() == Some(NodeKind::Inherit) { - Some(InheritNode { - raw: self.raw.clone(), - }) - } else { - None - } - } -} - -#[derive(Debug, Clone)] -pub struct InheritNode { - raw: rnix::SyntaxNode, -} - -impl InheritNode { - pub fn is_empty(&self) -> bool { - !self.raw.children().any(|child| { - matches!( - child.kind(), - rnix::SyntaxKind::NODE_IDENT - | rnix::SyntaxKind::NODE_DYNAMIC - | rnix::SyntaxKind::NODE_STRING - ) - }) - } - - pub fn text_range(&self) -> Range { - let range = self.raw.text_range(); - usize::from(range.start())..usize::from(range.end()) - } -} - -trait NodeRule: Sync { - fn metadata(&self) -> RuleMetadata; - fn node_kinds(&self) -> &'static [NodeKind]; - fn check(&self, file_id: FileId, node: &SyntaxNode, findings: &mut Vec); -} - -struct EmptyInherit; - -impl EmptyInherit { - const METADATA: RuleMetadata = RuleMetadata { - id: "empty_inherit", - severity: Severity::Warning, - }; -} - -impl NodeRule for EmptyInherit { - fn metadata(&self) -> RuleMetadata { - Self::METADATA - } - - fn node_kinds(&self) -> &'static [NodeKind] { - &[NodeKind::Inherit] - } - - fn check(&self, file_id: FileId, node: &SyntaxNode, findings: &mut Vec) { - let Some(inherit) = node.as_inherit() else { - return; - }; - - if inherit.is_empty() { - findings.push(Finding { - file_id, - range: inherit.text_range(), - rule_id: Self::METADATA.id, - message: "empty inherit statement".to_owned(), - }); - } - } -} - -const PARSE_ERROR: RuleMetadata = RuleMetadata { - id: "parse_error", - severity: Severity::Error, -}; - -static NODE_RULES: &[&dyn NodeRule] = &[&EmptyInherit]; +pub use file_store::{FileId, FileStore, SourceFile}; +pub use finding::{Finding, RuleId, RuleMetadata, Severity}; +pub use reporting::format_findings; +pub use syntax::{InheritNode, NodeKind, ParsedSource, SyntaxNode}; #[derive(Debug)] pub struct Report { @@ -193,113 +33,19 @@ pub fn analyze(files: Vec) -> Report { fn analyze_file(file_id: FileId, store: &FileStore, findings: &mut Vec) { let file = store.get(file_id); - let parse = rnix::Root::parse(&file.source); + let parsed = syntax::parse(&file.source); - for error in parse.errors() { - findings.push(Finding { + for error in parsed.errors() { + findings.push(Finding::new( file_id, - range: parse_error_range(error, file.source.len()), - rule_id: PARSE_ERROR.id, - message: error.to_string(), - }); + error.range(), + RuleId::ParseError, + error.message(), + )); } - for raw_node in parse.syntax().descendants() { - let node = SyntaxNode::new(raw_node); - let Some(kind) = node.kind() else { - continue; - }; - - for rule in NODE_RULES { - if rule.node_kinds().contains(&kind) { - rule.check(file_id, &node, findings); - } - } - } -} - -fn parse_error_range(error: &rnix::ParseError, source_len: usize) -> Range { - match error { - rnix::ParseError::Unexpected(range) - | rnix::ParseError::UnexpectedExtra(range) - | rnix::ParseError::UnexpectedWanted(_, range, _) - | rnix::ParseError::UnexpectedDoubleBind(range) - | rnix::ParseError::DuplicatedArgs(range, _) => { - usize::from(range.start())..usize::from(range.end()) - } - rnix::ParseError::UnexpectedEOF - | rnix::ParseError::UnexpectedEOFWanted(_) - | rnix::ParseError::RecursionLimitExceeded => source_len..source_len, - _ => source_len..source_len, - } -} - -fn rule_severity(rule_id: &str) -> Severity { - if rule_id == PARSE_ERROR.id { - return PARSE_ERROR.severity; - } - - NODE_RULES - .iter() - .map(|rule| rule.metadata()) - .find(|metadata| metadata.id == rule_id) - .map(|metadata| metadata.severity) - .unwrap_or(Severity::Error) -} - -pub fn format_findings(report: &Report) -> Vec { - report - .findings - .iter() - .map(|finding| format_finding(&report.files, finding)) - .collect() -} - -fn format_finding(files: &FileStore, finding: &Finding) -> String { - let file = files.get(finding.file_id); - let position = line_column(&file.source, finding.range.start); - let severity = rule_severity(finding.rule_id); - - format!( - "{}:{}:{}: {} {} {}", - display_path(&file.path), - position.line, - position.column, - severity.as_str(), - finding.rule_id, - finding.message - ) -} - -fn display_path(path: &Path) -> String { - path.display().to_string() -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -struct Position { - line: usize, - column: usize, -} - -fn line_column(source: &str, offset: usize) -> Position { - let bounded_offset = offset.min(source.len()); - let mut line = 1; - let mut line_start = 0; - - for (index, byte) in source.bytes().enumerate() { - if index >= bounded_offset { - break; - } - - if byte == b'\n' { - line += 1; - line_start = index + 1; - } - } - - Position { - line, - column: bounded_offset.saturating_sub(line_start) + 1, + for node in parsed.descendants() { + node_rule::dispatch_node_rules(rules::NODE_RULES, file_id, &node, findings); } } diff --git a/src/xatu/xatu_test.rs b/src/xatu/xatu_test.rs index 5ee982d..0f22645 100644 --- a/src/xatu/xatu_test.rs +++ b/src/xatu/xatu_test.rs @@ -1,5 +1,5 @@ use std::path::PathBuf; -use xatu::{SourceFile, analyze, exit_code_for_findings, format_findings}; +use xatu::{RuleId, SourceFile, analyze, exit_code_for_findings, format_findings}; fn source(source: &str) -> SourceFile { SourceFile { @@ -15,7 +15,7 @@ fn parse_errors_become_findings() { assert!(report .findings .iter() - .any(|finding| finding.rule_id == "parse_error")); + .any(|finding| finding.rule_id == RuleId::ParseError)); } #[test] @@ -23,7 +23,7 @@ fn empty_inherit_reports_finding() { let report = analyze(vec![source("{ inherit; }")]); assert_eq!(report.findings.len(), 1); - assert_eq!(report.findings[0].rule_id, "empty_inherit"); + assert_eq!(report.findings[0].rule_id, RuleId::EmptyInherit); } #[test] @@ -31,7 +31,7 @@ fn inherit_from_without_attrs_reports_finding() { let report = analyze(vec![source("{ inherit (pkgs); }")]); assert_eq!(report.findings.len(), 1); - assert_eq!(report.findings[0].rule_id, "empty_inherit"); + assert_eq!(report.findings[0].rule_id, RuleId::EmptyInherit); } #[test] @@ -48,7 +48,7 @@ fn findings_use_compact_one_line_format() { assert_eq!( lines, - vec!["sample.nix:2:3: warning empty_inherit empty inherit statement"] + vec!["sample.nix:2:3: warning[empty_inherit]: empty inherit statement"] ); }