From d62aab75579b1391587d484301906feaff6deb2a Mon Sep 17 00:00:00 2001 From: alexion Date: Sat, 19 Sep 2026 23:42:25 -0400 Subject: [PATCH 1/5] feat(xatu): add let order rule --- src/xatu/BUILD | 2 + src/xatu/ast.rs | 13 ++++ src/xatu/parser.rs | 77 ++++++++++++++++++- src/xatu/rules.rs | 9 ++- src/xatu/rules/let_order.rs | 104 ++++++++++++++++++++++++++ src/xatu/rules/manual_inherit.rs | 11 +-- src/xatu/rules/manual_inherit_from.rs | 11 +-- src/xatu/rules/unquoted_uri.rs | 4 +- src/xatu/testdata/let_order.nix | 4 + src/xatu/xatu_test.rs | 17 +++++ 10 files changed, 236 insertions(+), 16 deletions(-) create mode 100644 src/xatu/rules/let_order.rs create mode 100644 src/xatu/testdata/let_order.nix diff --git a/src/xatu/BUILD b/src/xatu/BUILD index f60733b..475e1ba 100644 --- a/src/xatu/BUILD +++ b/src/xatu/BUILD @@ -8,6 +8,7 @@ rust_library( "file_store.rs", "parser.rs", "rules.rs", + "rules/let_order.rs", "rules/manual_inherit.rs", "rules/manual_inherit_from.rs", "rules/unquoted_uri.rs", @@ -32,6 +33,7 @@ rust_test( data = [ "testdata/clean.nix", "testdata/invalid.nix", + "testdata/let_order.nix", "testdata/lint_findings.nix", ], deps = [ diff --git a/src/xatu/ast.rs b/src/xatu/ast.rs index 1deb464..35d80d8 100644 --- a/src/xatu/ast.rs +++ b/src/xatu/ast.rs @@ -29,6 +29,7 @@ impl std::fmt::Display for SourceRange { #[derive(Debug, Clone, PartialEq, Eq)] pub enum NodeData { Binding(Binding), + LetBlock(LetBlock), } #[derive(Debug, Clone, PartialEq, Eq)] @@ -37,6 +38,18 @@ pub struct Binding { pub value: Expr, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LetBlock { + pub bindings: Vec, + pub body_references: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LetBinding { + pub name: Ident, + pub references: Vec, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct Ident { pub name: String, diff --git a/src/xatu/parser.rs b/src/xatu/parser.rs index 61545c3..0098771 100644 --- a/src/xatu/parser.rs +++ b/src/xatu/parser.rs @@ -1,4 +1,4 @@ -use rnix::ast::Attr; +use rnix::ast::{Attr, HasEntry}; use rowan::ast::AstNode; use crate::{FileId, ast}; @@ -38,6 +38,7 @@ fn convert_syntax(syntax: rnix::SyntaxNode) -> (Vec, Vec) fn convert_node(node: rnix::SyntaxNode) -> Option { match node.kind() { rnix::SyntaxKind::NODE_ATTRPATH_VALUE => convert_binding(node), + rnix::SyntaxKind::NODE_LET_IN => convert_let_block(node), _ => None, } } @@ -53,6 +54,54 @@ fn convert_binding(node: rnix::SyntaxNode) -> Option { }) } +fn convert_let_block(node: rnix::SyntaxNode) -> Option { + let let_block = rnix::ast::LetIn::cast(node)?; + let range = let_block.syntax().text_range().into(); + let data = let_block.try_into().ok()?; + + Some(ast::Node { + range, + data: ast::NodeData::LetBlock(data), + }) +} + +impl TryFrom for ast::LetBlock { + type Error = (); + + fn try_from(let_block: rnix::ast::LetIn) -> Result { + let bindings = let_block + .attrpath_values() + .map(ast::LetBinding::try_from) + .collect::, _>>()?; + let body_references = let_block.body().ok_or(())?.references(); + + Ok(Self { + bindings, + body_references, + }) + } +} + +impl TryFrom for ast::LetBinding { + type Error = (); + + fn try_from(binding: rnix::ast::AttrpathValue) -> Result { + let mut attrs = binding.attrpath().ok_or(())?.attrs(); + let name = match attrs.next().ok_or(())? { + Attr::Ident(ident) => ident.into(), + _ => return Err(()), + }; + + if attrs.next().is_some() { + return Err(()); + } + + let references = binding.value().ok_or(())?.references(); + + Ok(Self { name, references }) + } +} + impl TryFrom for ast::Binding { type Error = (); @@ -93,6 +142,32 @@ impl From for ast::Ident { } } +trait ExprReferences { + fn references(&self) -> Vec; +} + +impl ExprReferences for rnix::ast::Expr { + fn references(&self) -> Vec { + self.syntax() + .descendants() + .filter_map(rnix::ast::Ident::cast) + .filter(is_reference) + .map(ast::Ident::from) + .collect() + } +} + +fn is_reference(ident: &rnix::ast::Ident) -> bool { + !matches!( + ident.syntax().parent().and_then(|parent| parent.parent()), + Some(grandparent) + if matches!( + grandparent.kind(), + rnix::SyntaxKind::NODE_ATTRPATH_VALUE | rnix::SyntaxKind::NODE_SELECT + ) + ) +} + impl TryFrom for ast::Select { type Error = (); diff --git a/src/xatu/rules.rs b/src/xatu/rules.rs index b27570d..a75907a 100644 --- a/src/xatu/rules.rs +++ b/src/xatu/rules.rs @@ -1,9 +1,11 @@ use crate::FileId; use crate::ast; +use crate::rules::let_order::LetOrder; use crate::rules::manual_inherit::ManualInherit; use crate::rules::manual_inherit_from::ManualInheritFrom; use crate::rules::unquoted_uri::UnquotedUri; +pub mod let_order; pub mod manual_inherit; pub mod manual_inherit_from; pub mod unquoted_uri; @@ -18,7 +20,7 @@ pub struct Diagnostic { pub struct Finding { pub rule_id: &'static str, pub range: ast::SourceRange, - pub message: &'static str, + pub message: String, } impl std::fmt::Display for Diagnostic { @@ -37,7 +39,7 @@ pub trait NodeRule { const ID: &'static str; fn select(data: &ast::NodeData) -> Option<&Self::Node>; - fn check(node: &Self::Node) -> Option<&'static str>; + fn check(node: &Self::Node) -> Option; } pub trait TokenRule { @@ -46,7 +48,7 @@ pub trait TokenRule { const ID: &'static str; fn select(data: &ast::TokenData) -> Option<&Self::Token>; - fn check(token: &Self::Token) -> Option<&'static str>; + fn check(token: &Self::Token) -> Option; } pub fn run_rules(file: &ast::ParsedFile) -> Vec { @@ -55,6 +57,7 @@ pub fn run_rules(file: &ast::ParsedFile) -> Vec { .iter() .flat_map(|node| { [ + run_node_rule::(node), run_node_rule::(node), run_node_rule::(node), ] diff --git a/src/xatu/rules/let_order.rs b/src/xatu/rules/let_order.rs new file mode 100644 index 0000000..d40e000 --- /dev/null +++ b/src/xatu/rules/let_order.rs @@ -0,0 +1,104 @@ +use std::collections::{HashMap, HashSet}; + +use crate::{ast, rules::NodeRule}; + +pub struct LetOrder; + +impl NodeRule for LetOrder { + type Node = ast::LetBlock; + + const ID: &'static str = "let_order"; + + fn select(data: &ast::NodeData) -> Option<&Self::Node> { + match data { + ast::NodeData::LetBlock(let_block) => Some(let_block), + _ => None, + } + } + + fn check(node: &Self::Node) -> Option { + let expected_order = match expected_order(node) { + Some(expected_order) => expected_order, + None => return Some("Cannot order cyclic `let` bindings".to_string()), + }; + let actual_order = reachable_actual_order(node, &expected_order); + + if actual_order == expected_order { + return None; + } + + Some(format!( + "Order reachable `let` bindings by body-first dependency flow: {}", + expected_order.join(", ") + )) + } +} + +fn expected_order(let_block: &ast::LetBlock) -> Option> { + let bindings = let_block + .bindings + .iter() + .map(|binding| (binding.name.name.as_str(), binding)) + .collect::>(); + let mut order = Vec::new(); + let mut permanent_marks = HashSet::new(); + let mut temporary_marks = HashSet::new(); + + for reference in &let_block.body_references { + visit_binding( + &reference.name, + &bindings, + &mut order, + &mut permanent_marks, + &mut temporary_marks, + )?; + } + + Some(order) +} + +fn visit_binding( + name: &str, + bindings: &HashMap<&str, &ast::LetBinding>, + order: &mut Vec, + permanent_marks: &mut HashSet, + temporary_marks: &mut HashSet, +) -> Option<()> { + let Some(binding) = bindings.get(name) else { + return Some(()); + }; + + if permanent_marks.contains(name) { + return Some(()); + } + + if !temporary_marks.insert(name.to_string()) { + return None; + } + + order.push(name.to_string()); + for reference in &binding.references { + visit_binding( + &reference.name, + bindings, + order, + permanent_marks, + temporary_marks, + )?; + } + + temporary_marks.remove(name); + permanent_marks.insert(name.to_string()); + Some(()) +} + +fn reachable_actual_order(let_block: &ast::LetBlock, expected_order: &[String]) -> Vec { + let reachable_names = expected_order.iter().collect::>(); + + let_block + .bindings + .iter() + .filter(|binding| reachable_names.contains(&binding.name.name)) + .map(|binding| binding.name.name.clone()) + .collect() +} diff --git a/src/xatu/rules/manual_inherit.rs b/src/xatu/rules/manual_inherit.rs index 76c71c7..ec5a9ad 100644 --- a/src/xatu/rules/manual_inherit.rs +++ b/src/xatu/rules/manual_inherit.rs @@ -8,15 +8,16 @@ impl NodeRule for ManualInherit { const ID: &'static str = "manual_inherit"; fn select(data: &ast::NodeData) -> Option<&Self::Node> { - let ast::NodeData::Binding(binding) = data; - - Some(binding) + match data { + ast::NodeData::Binding(binding) => Some(binding), + _ => None, + } } - fn check(node: &Self::Node) -> Option<&'static str> { + fn check(node: &Self::Node) -> Option { match &node.value { ast::Expr::Ident(value) if node.key == *value => { - Some("Use `inherit` for this assignment") + Some("Use `inherit` for this assignment".to_string()) } _ => None, } diff --git a/src/xatu/rules/manual_inherit_from.rs b/src/xatu/rules/manual_inherit_from.rs index 5d8717a..1cf78d9 100644 --- a/src/xatu/rules/manual_inherit_from.rs +++ b/src/xatu/rules/manual_inherit_from.rs @@ -8,15 +8,16 @@ impl NodeRule for ManualInheritFrom { const ID: &'static str = "manual_inherit_from"; fn select(data: &ast::NodeData) -> Option<&Self::Node> { - let ast::NodeData::Binding(binding) = data; - - Some(binding) + match data { + ast::NodeData::Binding(binding) => Some(binding), + _ => None, + } } - fn check(node: &Self::Node) -> Option<&'static str> { + fn check(node: &Self::Node) -> Option { match &node.value { ast::Expr::Select(select) if node.key == select.attr => { - Some("Use `inherit` for this selected attribute") + Some("Use `inherit` for this selected attribute".to_string()) } _ => None, } diff --git a/src/xatu/rules/unquoted_uri.rs b/src/xatu/rules/unquoted_uri.rs index cb7410e..0f2bd53 100644 --- a/src/xatu/rules/unquoted_uri.rs +++ b/src/xatu/rules/unquoted_uri.rs @@ -13,7 +13,7 @@ impl TokenRule for UnquotedUri { } } - fn check(_token: &Self::Token) -> Option<&'static str> { - Some("URI literals should be quoted") + fn check(_token: &Self::Token) -> Option { + Some("URI literals should be quoted".to_string()) } } diff --git a/src/xatu/testdata/let_order.nix b/src/xatu/testdata/let_order.nix new file mode 100644 index 0000000..c729421 --- /dev/null +++ b/src/xatu/testdata/let_order.nix @@ -0,0 +1,4 @@ +let + support = "support"; + primary = support; +in primary diff --git a/src/xatu/xatu_test.rs b/src/xatu/xatu_test.rs index 2e82db6..6c29a60 100644 --- a/src/xatu/xatu_test.rs +++ b/src/xatu/xatu_test.rs @@ -65,6 +65,23 @@ fn xatu_reports_unquoted_uri() -> googletest::Result<()> { Ok(()) } +#[gtest] +fn xatu_reports_let_order() -> googletest::Result<()> { + let report = xatu::future_sight(vec![fixture_source("testdata/let_order.nix")?]).or_fail()?; + + let let_order_diagnostics = report + .diagnostics + .iter() + .filter(|diagnostic| diagnostic.finding.rule_id == "let_order") + .collect::>(); + expect_that!(let_order_diagnostics.len(), eq(1)); + expect_that!( + let_order_diagnostics[0].finding.message.as_str(), + eq("Order reachable `let` bindings by body-first dependency flow: primary, support") + ); + Ok(()) +} + #[gtest] fn xatu_reports_manual_inherit() -> googletest::Result<()> { let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?; -- 2.47.3 From a42a9c7064ee71b071b27d6f7d714ebf38264028 Mon Sep 17 00:00:00 2001 From: alexion Date: Sat, 19 Sep 2026 23:44:23 -0400 Subject: [PATCH 2/5] test(xatu): reuse lint findings fixture --- src/xatu/BUILD | 1 - src/xatu/testdata/let_order.nix | 4 ---- src/xatu/testdata/lint_findings.nix | 6 +++++- src/xatu/xatu_test.rs | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) delete mode 100644 src/xatu/testdata/let_order.nix diff --git a/src/xatu/BUILD b/src/xatu/BUILD index 475e1ba..e340ba5 100644 --- a/src/xatu/BUILD +++ b/src/xatu/BUILD @@ -33,7 +33,6 @@ rust_test( data = [ "testdata/clean.nix", "testdata/invalid.nix", - "testdata/let_order.nix", "testdata/lint_findings.nix", ], deps = [ diff --git a/src/xatu/testdata/let_order.nix b/src/xatu/testdata/let_order.nix deleted file mode 100644 index c729421..0000000 --- a/src/xatu/testdata/let_order.nix +++ /dev/null @@ -1,4 +0,0 @@ -let - support = "support"; - primary = support; -in primary diff --git a/src/xatu/testdata/lint_findings.nix b/src/xatu/testdata/lint_findings.nix index 17b4837..de5a287 100644 --- a/src/xatu/testdata/lint_findings.nix +++ b/src/xatu/testdata/lint_findings.nix @@ -1,5 +1,9 @@ -{ +let + support = "support"; + primary = support; +in { package = package; from = pkgs.from; url = https://example.com; + ordered = primary; } diff --git a/src/xatu/xatu_test.rs b/src/xatu/xatu_test.rs index 6c29a60..464f206 100644 --- a/src/xatu/xatu_test.rs +++ b/src/xatu/xatu_test.rs @@ -67,7 +67,7 @@ fn xatu_reports_unquoted_uri() -> googletest::Result<()> { #[gtest] fn xatu_reports_let_order() -> googletest::Result<()> { - let report = xatu::future_sight(vec![fixture_source("testdata/let_order.nix")?]).or_fail()?; + let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?; let let_order_diagnostics = report .diagnostics -- 2.47.3 From 65856156281c07df9fd8a7e28536a0455cabf018 Mon Sep 17 00:00:00 2001 From: alexion Date: Sat, 19 Sep 2026 23:45:46 -0400 Subject: [PATCH 3/5] refactor(xatu): reuse binding model for let order --- src/xatu/ast.rs | 9 ++------ src/xatu/parser.rs | 32 ++++++++--------------------- src/xatu/rules/let_order.rs | 8 ++++---- src/xatu/testdata/lint_findings.nix | 2 +- 4 files changed, 16 insertions(+), 35 deletions(-) diff --git a/src/xatu/ast.rs b/src/xatu/ast.rs index 35d80d8..84695ff 100644 --- a/src/xatu/ast.rs +++ b/src/xatu/ast.rs @@ -36,20 +36,15 @@ pub enum NodeData { pub struct Binding { pub key: Ident, pub value: Expr, + pub references: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] pub struct LetBlock { - pub bindings: Vec, + pub bindings: Vec, pub body_references: Vec, } -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct LetBinding { - pub name: Ident, - pub references: Vec, -} - #[derive(Debug, Clone, PartialEq, Eq)] pub struct Ident { pub name: String, diff --git a/src/xatu/parser.rs b/src/xatu/parser.rs index 0098771..9838d7a 100644 --- a/src/xatu/parser.rs +++ b/src/xatu/parser.rs @@ -71,7 +71,7 @@ impl TryFrom for ast::LetBlock { fn try_from(let_block: rnix::ast::LetIn) -> Result { let bindings = let_block .attrpath_values() - .map(ast::LetBinding::try_from) + .map(ast::Binding::try_from) .collect::, _>>()?; let body_references = let_block.body().ok_or(())?.references(); @@ -82,26 +82,6 @@ impl TryFrom for ast::LetBlock { } } -impl TryFrom for ast::LetBinding { - type Error = (); - - fn try_from(binding: rnix::ast::AttrpathValue) -> Result { - let mut attrs = binding.attrpath().ok_or(())?.attrs(); - let name = match attrs.next().ok_or(())? { - Attr::Ident(ident) => ident.into(), - _ => return Err(()), - }; - - if attrs.next().is_some() { - return Err(()); - } - - let references = binding.value().ok_or(())?.references(); - - Ok(Self { name, references }) - } -} - impl TryFrom for ast::Binding { type Error = (); @@ -116,9 +96,15 @@ impl TryFrom for ast::Binding { return Err(()); } - let value = binding.value().ok_or(())?.try_into()?; + let value_expr = binding.value().ok_or(())?; + let references = value_expr.references(); + let value = value_expr.try_into()?; - Ok(Self { key, value }) + Ok(Self { + key, + value, + references, + }) } } diff --git a/src/xatu/rules/let_order.rs b/src/xatu/rules/let_order.rs index d40e000..127106c 100644 --- a/src/xatu/rules/let_order.rs +++ b/src/xatu/rules/let_order.rs @@ -38,7 +38,7 @@ fn expected_order(let_block: &ast::LetBlock) -> Option> { let bindings = let_block .bindings .iter() - .map(|binding| (binding.name.name.as_str(), binding)) + .map(|binding| (binding.key.name.as_str(), binding)) .collect::>(); let mut order = Vec::new(); let mut permanent_marks = HashSet::new(); @@ -59,7 +59,7 @@ fn expected_order(let_block: &ast::LetBlock) -> Option> { fn visit_binding( name: &str, - bindings: &HashMap<&str, &ast::LetBinding>, + bindings: &HashMap<&str, &ast::Binding>, order: &mut Vec, permanent_marks: &mut HashSet, temporary_marks: &mut HashSet, @@ -98,7 +98,7 @@ fn reachable_actual_order(let_block: &ast::LetBlock, expected_order: &[String]) let_block .bindings .iter() - .filter(|binding| reachable_names.contains(&binding.name.name)) - .map(|binding| binding.name.name.clone()) + .filter(|binding| reachable_names.contains(&binding.key.name)) + .map(|binding| binding.key.name.clone()) .collect() } diff --git a/src/xatu/testdata/lint_findings.nix b/src/xatu/testdata/lint_findings.nix index de5a287..e225eea 100644 --- a/src/xatu/testdata/lint_findings.nix +++ b/src/xatu/testdata/lint_findings.nix @@ -1,5 +1,5 @@ let - support = "support"; + support = dependency; primary = support; in { package = package; -- 2.47.3 From 5fafebad33a481eeebab3548737a5b71acac4af6 Mon Sep 17 00:00:00 2001 From: alexion Date: Sat, 19 Sep 2026 23:47:54 -0400 Subject: [PATCH 4/5] fix(xatu): preserve bindings with unsupported values --- src/xatu/ast.rs | 1 + src/xatu/parser.rs | 2 +- src/xatu/testdata/lint_findings.nix | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/xatu/ast.rs b/src/xatu/ast.rs index 84695ff..2627844 100644 --- a/src/xatu/ast.rs +++ b/src/xatu/ast.rs @@ -54,6 +54,7 @@ pub struct Ident { pub enum Expr { Ident(Ident), Select(Select), + Other, } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/src/xatu/parser.rs b/src/xatu/parser.rs index 9838d7a..ce9c94b 100644 --- a/src/xatu/parser.rs +++ b/src/xatu/parser.rs @@ -115,7 +115,7 @@ impl TryFrom for ast::Expr { match expr { rnix::ast::Expr::Ident(ident) => Ok(Self::Ident(ident.into())), rnix::ast::Expr::Select(select) => Ok(Self::Select(select.try_into()?)), - _ => Err(()), + _ => Ok(Self::Other), } } } diff --git a/src/xatu/testdata/lint_findings.nix b/src/xatu/testdata/lint_findings.nix index e225eea..de5a287 100644 --- a/src/xatu/testdata/lint_findings.nix +++ b/src/xatu/testdata/lint_findings.nix @@ -1,5 +1,5 @@ let - support = dependency; + support = "support"; primary = support; in { package = package; -- 2.47.3 From 90be3f871c7fdb2686fbef7c232a546eb9e19b6c Mon Sep 17 00:00:00 2001 From: alexion Date: Sat, 19 Sep 2026 23:51:45 -0400 Subject: [PATCH 5/5] fix(xatu): count select receivers as references --- src/xatu/parser.rs | 17 +++++++++++------ src/xatu/testdata/lint_findings.nix | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/xatu/parser.rs b/src/xatu/parser.rs index ce9c94b..f8f7624 100644 --- a/src/xatu/parser.rs +++ b/src/xatu/parser.rs @@ -144,13 +144,18 @@ impl ExprReferences for rnix::ast::Expr { } fn is_reference(ident: &rnix::ast::Ident) -> bool { + let parent = ident.syntax().parent(); + let grandparent = parent.as_ref().and_then(rowan::SyntaxNode::parent); + !matches!( - ident.syntax().parent().and_then(|parent| parent.parent()), - Some(grandparent) - if matches!( - grandparent.kind(), - rnix::SyntaxKind::NODE_ATTRPATH_VALUE | rnix::SyntaxKind::NODE_SELECT - ) + ( + parent.map(|node| node.kind()), + grandparent.map(|node| node.kind()) + ), + ( + Some(rnix::SyntaxKind::NODE_ATTRPATH), + Some(rnix::SyntaxKind::NODE_ATTRPATH_VALUE | rnix::SyntaxKind::NODE_SELECT), + ) ) } diff --git a/src/xatu/testdata/lint_findings.nix b/src/xatu/testdata/lint_findings.nix index de5a287..74e396a 100644 --- a/src/xatu/testdata/lint_findings.nix +++ b/src/xatu/testdata/lint_findings.nix @@ -1,6 +1,6 @@ let support = "support"; - primary = support; + primary = support.value; in { package = package; from = pkgs.from; -- 2.47.3