feat(xatu): add node-local rule dispatch

Cargo.lock

@@ -307,6 +307,7 @@ dependencies = [
  "clap",
  "monorepo",
  "rnix",
+ "rowan",
  "status",
 ]
 

Cargo.toml

@@ -12,6 +12,7 @@ resolver = "3"
 clap = "4"
 monorepo = { path = "src/monorepo" }
 rnix = "=0.14.0"
+rowan = "0.16"
 serde = "1.0"
 serde_json = "1.0"
 status = { path = "src/status" }

src/xatu/BUILD

@@ -4,21 +4,26 @@ rust_library(
     name = "xatu_lib",
     crate_name = "xatu",
     srcs = [
+        "ast.rs",
         "file_store.rs",
-        "finding.rs",
-        "node_rule.rs",
-        "reporting.rs",
+        "parser.rs",
         "rules.rs",
         "rules/empty_inherit.rs",
-        "syntax.rs",
         "xatu.rs",
     ],
     deps = [
         "//src/status:status",
         "//third_party/crates:rnix",
+        "//third_party/crates/rowan-0.16.1:rowan",
     ],
 )
 
+rust_test(
+    name = "file_store_test",
+    srcs = ["file_store_test.rs"],
+    deps = [":xatu_lib"],
+)
+
 rust_binary(
     name = "xatu",
     crate_name = "xatu_cli",
@@ -30,9 +35,3 @@ rust_binary(
         "//third_party/crates:clap",
     ],
 )
-
-rust_test(
-    name = "xatu_test",
-    srcs = ["xatu_test.rs"],
-    deps = [":xatu_lib"],
-)

src/xatu/Cargo.toml

@@ -14,4 +14,5 @@ path = "xatu_main.rs"
 clap = { workspace = true, features = ["derive"] }
 monorepo = { workspace = true }
 rnix = { workspace = true }
+rowan = { workspace = true }
 status = { workspace = true }

src/xatu/ast.rs

@@ -0,0 +1,36 @@
+use crate::FileId;
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Inherit {
+    pub attr_count: usize,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum NodeData {
+    Inherit(Inherit),
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct SourceRange {
+    pub start: usize,
+    pub end: usize,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Node {
+    pub range: SourceRange,
+    pub data: NodeData,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ParseError {
+    pub range: SourceRange,
+    pub message: String,
+}
+
+#[derive(Debug)]
+pub struct ParsedFile {
+    pub file_id: FileId,
+    pub nodes: Vec<Node>,
+    pub errors: Vec<ParseError>,
+}

src/xatu/file_store.rs

@@ -3,6 +3,12 @@ use std::path::PathBuf;
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct FileId(usize);
 
+#[derive(Debug)]
+pub struct SourceFile {
+    pub path: PathBuf,
+    pub source: String,
+}
+
 #[derive(Debug, Default)]
 pub struct FileStore {
     files: Vec<SourceFile>,
@@ -20,8 +26,8 @@ impl FileStore {
     }
 }
 
-#[derive(Debug, Clone)]
-pub struct SourceFile {
-    pub path: PathBuf,
-    pub source: String,
+impl From<Vec<SourceFile>> for FileStore {
+    fn from(files: Vec<SourceFile>) -> Self {
+        Self { files }
+    }
 }

src/xatu/file_store_test.rs

@@ -0,0 +1,33 @@
+use std::path::PathBuf;
+
+use xatu::{FileStore, SourceFile};
+
+fn source_file(path: &str, source: &str) -> SourceFile {
+    SourceFile {
+        path: PathBuf::from(path),
+        source: source.to_owned(),
+    }
+}
+
+#[test]
+fn added_file_can_be_retrieved_by_id() {
+    let mut store = FileStore::default();
+
+    let id = store.add(source_file("first.nix", "{ inherit foo; }"));
+    let file = store.get(id);
+
+    assert_eq!(PathBuf::from("first.nix"), file.path);
+    assert_eq!("{ inherit foo; }", file.source);
+}
+
+#[test]
+fn file_ids_refer_to_their_added_files() {
+    let mut store = FileStore::default();
+
+    let first = store.add(source_file("first.nix", "first"));
+    let second = store.add(source_file("second.nix", "second"));
+
+    assert_ne!(first, second);
+    assert_eq!("first", store.get(first).source);
+    assert_eq!("second", store.get(second).source);
+}

src/xatu/finding.rs

@@ -1,75 +0,0 @@
-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

@@ -1,26 +0,0 @@
-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/parser.rs

@@ -0,0 +1,84 @@
+use rowan::ast::AstNode;
+
+use crate::{FileId, ast};
+
+pub fn parse_file(file_id: FileId, source: &str) -> ast::ParsedFile {
+    let parse = rnix::Root::parse(source);
+    let source_len = source.len();
+
+    ast::ParsedFile {
+        file_id,
+        nodes: parse
+            .syntax()
+            .descendants()
+            .filter_map(convert_node)
+            .collect(),
+        errors: parse
+            .errors()
+            .iter()
+            .map(|error| convert_parse_error(error, source_len))
+            .collect(),
+    }
+}
+
+fn convert_parse_error(error: &rnix::ParseError, source_len: usize) -> ast::ParseError {
+    ast::ParseError {
+        range: parse_error_range(error, source_len),
+        message: error.to_string(),
+    }
+}
+
+fn parse_error_range(error: &rnix::ParseError, source_len: usize) -> ast::SourceRange {
+    match error {
+        rnix::ParseError::Unexpected(range)
+        | rnix::ParseError::UnexpectedExtra(range)
+        | rnix::ParseError::UnexpectedWanted(_, range, _)
+        | rnix::ParseError::UnexpectedDoubleBind(range)
+        | rnix::ParseError::DuplicatedArgs(range, _) => (*range).into(),
+
+        rnix::ParseError::UnexpectedEOF
+        | rnix::ParseError::UnexpectedEOFWanted(_)
+        | rnix::ParseError::RecursionLimitExceeded => ast::SourceRange {
+            start: source_len,
+            end: source_len,
+        },
+
+        _ => ast::SourceRange {
+            start: source_len,
+            end: source_len,
+        },
+    }
+}
+
+fn convert_node(node: rnix::SyntaxNode) -> Option<ast::Node> {
+    match node.kind() {
+        rnix::SyntaxKind::NODE_INHERIT => convert_inherit(node),
+        _ => None,
+    }
+}
+
+fn convert_inherit(node: rnix::SyntaxNode) -> Option<ast::Node> {
+    let inherit = rnix::ast::Inherit::cast(node)?;
+
+    Some(ast::Node {
+        range: inherit.syntax().text_range().into(),
+        data: ast::NodeData::Inherit(inherit.into()),
+    })
+}
+
+impl From<rnix::TextRange> for ast::SourceRange {
+    fn from(range: rnix::TextRange) -> Self {
+        Self {
+            start: usize::from(range.start()),
+            end: usize::from(range.end()),
+        }
+    }
+}
+
+impl From<rnix::ast::Inherit> for ast::Inherit {
+    fn from(inherit: rnix::ast::Inherit) -> Self {
+        Self {
+            attr_count: inherit.attrs().count(),
+        }
+    }
+}

src/xatu/reporting.rs

@@ -1,60 +0,0 @@
-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

@@ -1,5 +1,54 @@
-mod empty_inherit;
+use crate::FileId;
+use crate::ast;
+use crate::rules::empty_inherit::EmptyInherit;
 
-use crate::node_rule::NodeRule;
+pub mod empty_inherit;
 
-pub static NODE_RULES: &[&dyn NodeRule] = &[&empty_inherit::EMPTY_INHERIT];
+pub struct Finding {
+    pub rule_id: &'static str,
+    pub range: ast::SourceRange,
+    pub message: &'static str,
+}
+
+pub struct Diagnostic {
+    pub file_id: FileId,
+    pub finding: Finding,
+}
+
+pub trait NodeRule {
+    type Node;
+
+    const ID: &'static str;
+
+    fn select(data: &ast::NodeData) -> Option<&Self::Node>;
+    fn check(node: &Self::Node) -> Option<&'static str>;
+}
+
+fn run_node_rule<R>(node: &ast::Node) -> Option<Finding>
+where
+    R: NodeRule,
+{
+    let typed_node = R::select(&node.data)?;
+    let message = R::check(typed_node)?;
+
+    Some(Finding {
+        rule_id: R::ID,
+        range: node.range,
+        message,
+    })
+}
+
+pub fn run_rules(file: &ast::ParsedFile) -> Vec<Diagnostic> {
+    file.nodes
+        .iter()
+        .flat_map(|node| {
+            [run_node_rule::<EmptyInherit>(node)]
+                .into_iter()
+                .flatten()
+                .map(|finding| Diagnostic {
+                    file_id: file.file_id,
+                    finding,
+                })
+        })
+        .collect()
+}

src/xatu/rules/empty_inherit.rs

@@ -1,33 +1,23 @@
-use crate::file_store::FileId;
-use crate::finding::{Finding, RuleId, RuleMetadata};
-use crate::node_rule::NodeRule;
-use crate::syntax::{NodeKind, SyntaxNode};
+use crate::{ast, rules::NodeRule};
 
 pub struct EmptyInherit;
 
-pub static EMPTY_INHERIT: EmptyInherit = EmptyInherit;
-
 impl NodeRule for EmptyInherit {
-    fn metadata(&self) -> RuleMetadata {
-        RuleId::EmptyInherit.metadata()
+    type Node = ast::Inherit;
+
+    const ID: &'static str = "empty_inherit";
+
+    fn select(data: &ast::NodeData) -> Option<&Self::Node> {
+        match data {
+            ast::NodeData::Inherit(inherit) => Some(inherit),
+        }
     }
 
-    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",
-            ));
+    fn check(node: &Self::Node) -> Option<&'static str> {
+        if node.attr_count == 0 {
+            Some("inherit statement has no attributes")
+        } else {
+            None
         }
     }
 }

src/xatu/syntax.rs

@@ -1,135 +0,0 @@
-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,64 +1,29 @@
+mod ast;
 mod file_store;
-mod finding;
-mod node_rule;
-mod reporting;
+mod parser;
 mod rules;
-mod syntax;
 
 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 {
-    pub files: FileStore,
-    pub findings: Vec<Finding>,
-}
-
-pub fn analyze(files: Vec<SourceFile>) -> Report {
+pub fn run(files: Vec<SourceFile>) -> status::Result<()> {
     let mut store = FileStore::default();
-    let mut findings = Vec::new();
 
     for file in files {
         let file_id = store.add(file);
-        analyze_file(file_id, &store, &mut findings);
-    }
-
-    Report {
-        files: store,
-        findings,
-    }
-}
-
-fn analyze_file(file_id: FileId, store: &FileStore, findings: &mut Vec<Finding>) {
         let file = store.get(file_id);
-    let parsed = syntax::parse(&file.source);
+        let parsed = parser::parse_file(file_id, &file.source);
 
-    for error in parsed.errors() {
-        findings.push(Finding::new(
-            file_id,
-            error.range(),
-            RuleId::ParseError,
-            error.message(),
-        ));
+        for diagnostic in rules::run_rules(&parsed) {
+            println!(
+                "{}:{}..{}: {}: {}",
+                file.path.display(),
+                diagnostic.finding.range.start,
+                diagnostic.finding.range.end,
+                diagnostic.finding.rule_id,
+                diagnostic.finding.message,
+            );
+        }
     }
 
-    for node in parsed.descendants() {
-        node_rule::dispatch_node_rules(rules::NODE_RULES, file_id, &node, findings);
-    }
-}
-
-pub fn exit_code_for_findings(findings: &[Finding]) -> i32 {
-    if findings.is_empty() { 0 } else { 1 }
-}
-
-pub fn run(files: Vec<SourceFile>) -> status::Result<i32> {
-    let report = analyze(files);
-
-    for line in format_findings(&report) {
-        println!("{line}");
-    }
-
-    Ok(exit_code_for_findings(&report.findings))
+    Ok(())
 }

src/xatu/xatu_main.rs

@@ -37,18 +37,12 @@ fn collect_source_files(path: &Path) -> status::Result<Vec<SourceFile>> {
 
         let mut files = Vec::new();
 
-        let mut entries: Vec<PathBuf> = std::fs::read_dir(path)
+        for entry in std::fs::read_dir(path)
             .with_context(|| format!("failed to read directory {}", path.display()))?
-            .map(|entry| {
-                entry
-                    .map(|entry| entry.path())
-                    .with_context(|| format!("failed to read entry in {}", path.display()))
-            })
-            .collect::<status::Result<Vec<_>>>()?;
-        entries.sort();
-
-        for entry in entries {
-            files.extend(collect_source_files(&entry)?);
+        {
+            let entry =
+                entry.with_context(|| format!("failed to read entry in {}", path.display()))?;
+            files.extend(collect_source_files(&entry.path())?);
         }
 
         return Ok(files);
@@ -71,7 +65,7 @@ struct Args {
     paths: Vec<PathBuf>,
 }
 
-fn try_main() -> status::Result<i32> {
+fn main() -> status::Result<()> {
     let args = Args::parse();
 
     let paths: Vec<PathBuf> = args
@@ -90,13 +84,3 @@ fn try_main() -> status::Result<i32> {
 
     xatu::run(files)
 }
-
-fn main() {
-    match try_main() {
-        Ok(code) => std::process::exit(code),
-        Err(error) => {
-            eprintln!("{error}");
-            std::process::exit(2);
-        }
-    }
-}

src/xatu/xatu_test.rs

@@ -1,67 +0,0 @@
-use std::path::PathBuf;
-use xatu::{RuleId, SourceFile, analyze, exit_code_for_findings, format_findings};
-
-fn source(source: &str) -> SourceFile {
-    SourceFile {
-        path: PathBuf::from("sample.nix"),
-        source: source.to_owned(),
-    }
-}
-
-#[test]
-fn parse_errors_become_findings() {
-    let report = analyze(vec![source("{")]);
-
-    assert!(report
-        .findings
-        .iter()
-        .any(|finding| finding.rule_id == RuleId::ParseError));
-}
-
-#[test]
-fn empty_inherit_reports_finding() {
-    let report = analyze(vec![source("{ inherit; }")]);
-
-    assert_eq!(report.findings.len(), 1);
-    assert_eq!(report.findings[0].rule_id, RuleId::EmptyInherit);
-}
-
-#[test]
-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, RuleId::EmptyInherit);
-}
-
-#[test]
-fn non_empty_inherit_does_not_report_finding() {
-    let report = analyze(vec![source("{ inherit foo; inherit (pkgs) bar; }")]);
-
-    assert!(report.findings.is_empty());
-}
-
-#[test]
-fn findings_use_compact_one_line_format() {
-    let report = analyze(vec![source("{\n  inherit;\n}")]);
-    let lines = format_findings(&report);
-
-    assert_eq!(
-        lines,
-        vec!["sample.nix:2:3: warning[empty_inherit]: empty inherit statement"]
-    );
-}
-
-#[test]
-fn exit_code_zero_means_no_findings() {
-    let report = analyze(vec![source("{ inherit foo; }")]);
-
-    assert_eq!(exit_code_for_findings(&report.findings), 0);
-}
-
-#[test]
-fn exit_code_one_means_findings_exist() {
-    let report = analyze(vec![source("{ inherit; }")]);
-
-    assert_eq!(exit_code_for_findings(&report.findings), 1);
-}