refactor(xatu): split lint architecture modules

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",

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<SourceFile>,
+}
+
+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,
+}

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<usize>,
+    pub rule_id: RuleId,
+    pub message: String,
+}
+
+impl Finding {
+    pub fn new(
+        file_id: FileId,
+        range: Range<usize>,
+        rule_id: RuleId,
+        message: impl Into<String>,
+    ) -> Self {
+        Self {
+            file_id,
+            range,
+            rule_id,
+            message: message.into(),
+        }
+    }
+
+    pub fn metadata(&self) -> RuleMetadata {
+        self.rule_id.metadata()
+    }
+}

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<Finding>);
+}
+
+pub fn dispatch_node_rules(
+    rules: &[&dyn NodeRule],
+    file_id: FileId,
+    node: &SyntaxNode,
+    findings: &mut Vec<Finding>,
+) {
+    let Some(kind) = node.kind() else {
+        return;
+    };
+
+    for rule in rules {
+        if rule.node_kinds().contains(&kind) {
+            rule.check(file_id, node, findings);
+        }
+    }
+}

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<String> {
+    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,
+    }
+}

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];

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<Finding>) {
+        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",
+            ));
+        }
+    }
+}

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<Self> {
+        match kind {
+            rnix::SyntaxKind::NODE_INHERIT => Some(Self::Inherit),
+            _ => None,
+        }
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct ParseError {
+    range: Range<usize>,
+    message: String,
+}
+
+impl ParseError {
+    pub fn range(&self) -> Range<usize> {
+        self.range.clone()
+    }
+
+    pub fn message(&self) -> &str {
+        &self.message
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct ParsedSource {
+    root: SyntaxNode,
+    errors: Vec<ParseError>,
+}
+
+impl ParsedSource {
+    pub fn errors(&self) -> &[ParseError] {
+        &self.errors
+    }
+
+    pub fn descendants(&self) -> Vec<SyntaxNode> {
+        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> {
+        NodeKind::from_rnix(self.raw.kind())
+    }
+
+    pub fn as_inherit(&self) -> Option<InheritNode> {
+        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<usize> {
+        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<usize> {
+    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,
+    }
+}

src/xatu/xatu.rs

@@ -1,174 +1,14 @@
-use std::ops::Range;
+mod file_store;
-use std::path::{Path, PathBuf};
+mod finding;
+mod node_rule;
+mod reporting;
+mod rules;
+mod syntax;
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub use file_store::{FileId, FileStore, SourceFile};
-pub struct FileId(usize);
+pub use finding::{Finding, RuleId, RuleMetadata, Severity};
-
+pub use reporting::format_findings;
-#[derive(Debug, Default)]
+pub use syntax::{InheritNode, NodeKind, ParsedSource, SyntaxNode};
-pub struct FileStore {
-    files: Vec<SourceFile>,
-}
-
-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<usize>,
-    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<Self> {
-        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> {
-        NodeKind::from_rnix(self.raw.kind())
-    }
-
-    pub fn text_range(&self) -> Range<usize> {
-        let range = self.raw.text_range();
-        usize::from(range.start())..usize::from(range.end())
-    }
-
-    pub fn as_inherit(&self) -> Option<InheritNode> {
-        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<usize> {
-        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<Finding>);
-}
-
-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<Finding>) {
-        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];
 
 #[derive(Debug)]
 pub struct Report {
@@ -193,113 +33,19 @@ pub fn analyze(files: Vec<SourceFile>) -> Report {
 
 fn analyze_file(file_id: FileId, store: &FileStore, findings: &mut Vec<Finding>) {
     let file = store.get(file_id);
-    let parse = rnix::Root::parse(&file.source);
+    let parsed = syntax::parse(&file.source);
 
-    for error in parse.errors() {
+    for error in parsed.errors() {
-        findings.push(Finding {
+        findings.push(Finding::new(
             file_id,
-            range: parse_error_range(error, file.source.len()),
+            error.range(),
-            rule_id: PARSE_ERROR.id,
+            RuleId::ParseError,
-            message: error.to_string(),
+            error.message(),
-        });
+        ));
     }
 
-    for raw_node in parse.syntax().descendants() {
+    for node in parsed.descendants() {
-        let node = SyntaxNode::new(raw_node);
+        node_rule::dispatch_node_rules(rules::NODE_RULES, file_id, &node, findings);
-        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<usize> {
-    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<String> {
-    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,
     }
 }
 

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"]
     );
 }