alexion/dev
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(xatu): check let binding order

Browse Source
This commit is contained in:
alexion
2026-09-26 20:04:45 -04:00
parent 16ce11fc52
commit 8b5af32d22
10 changed files with 447 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/xatu/BUILD
View File

@@ -8,9 +8,11 @@ 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",
"scope_graph.rs",
"xatu.rs",
],
deps = [

12
src/xatu/ast.rs
View File

@@ -3,6 +3,7 @@ use crate::FileId;
#[derive(Debug)]
pub struct ParsedFile {
pub file_id: FileId,
pub root: Expr,
pub nodes: Vec<Node>,
pub tokens: Vec<Token>,
pub errors: Vec<ParseError>,
@@ -33,6 +34,7 @@ pub enum NodeData {
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Binding {
pub range: SourceRange,
pub key: Ident,
pub value: Expr,
}
@@ -46,6 +48,16 @@ pub struct Ident {
pub enum Expr {
Ident(Ident),
Select(Select),
Let {
bindings: Vec<Binding>,
body: Box<Expr>,
},
AttrSet(Vec<Binding>),
Lambda {
parameters: Vec<Ident>,
body: Box<Expr>,
},
Other(Vec<Expr>),
}
#[derive(Debug, Clone, PartialEq, Eq)]

78
src/xatu/parser.rs
View File

@@ -1,4 +1,4 @@
use rnix::ast::Attr;
use rnix::ast::{Attr, HasEntry};
use rowan::ast::AstNode;
use crate::{FileId, ast};
@@ -7,10 +7,14 @@ pub fn parse_file(file_id: FileId, source: &str) -> ast::ParsedFile {
let parse = rnix::Root::parse(source);
let source_len = source.len();
let (nodes, tokens) = convert_syntax(parse.syntax());
let syntax = parse.syntax();
let root = rnix::ast::Root::cast(syntax.clone()).expect("parser root is a root node");
let root = convert_expr(root.expr().expect("parser root has an expression"));
let (nodes, tokens) = convert_syntax(syntax);
ast::ParsedFile {
file_id,
root,
nodes,
tokens,
errors: parse
@@ -35,6 +39,70 @@ fn convert_syntax(syntax: rnix::SyntaxNode) -> (Vec<ast::Node>, Vec<ast::Token>)
(nodes, tokens)
}
fn convert_expr(expr: rnix::ast::Expr) -> ast::Expr {
match expr {
rnix::ast::Expr::Ident(ident) => ast::Expr::Ident(ident.into()),
rnix::ast::Expr::LetIn(let_in) => ast::Expr::Let {
bindings: let_in
.attrpath_values()
.filter_map(convert_binding_ast)
.collect(),
body: Box::new(convert_expr(
let_in.body().expect("let expressions have bodies"),
)),
},
rnix::ast::Expr::AttrSet(attr_set) => ast::Expr::AttrSet(
attr_set
.attrpath_values()
.filter_map(convert_binding_ast)
.collect(),
),
rnix::ast::Expr::Lambda(lambda) => ast::Expr::Lambda {
parameters: lambda_parameters(&lambda),
body: Box::new(convert_expr(lambda.body().expect("lambdas have bodies"))),
},
expr => ast::Expr::Other(
expr.syntax()
.children()
.filter_map(rnix::ast::Expr::cast)
.map(convert_expr)
.collect(),
),
}
}
fn convert_binding_ast(binding: rnix::ast::AttrpathValue) -> Option<ast::Binding> {
let mut attrs = binding.attrpath()?.attrs();
let Attr::Ident(key) = attrs.next()? else {
return None;
};
if attrs.next().is_some() {
return None;
}
Some(ast::Binding {
range: binding.syntax().text_range().into(),
key: key.into(),
value: convert_expr(binding.value()?),
})
}
fn lambda_parameters(lambda: &rnix::ast::Lambda) -> Vec<ast::Ident> {
match lambda.param() {
Some(rnix::ast::Param::IdentParam(parameter)) => {
parameter.ident().map(Into::into).into_iter().collect()
}
Some(rnix::ast::Param::Pattern(pattern)) => pattern
.pat_entries()
.filter_map(|entry| entry.ident())
.chain(pattern.pat_bind().and_then(|binding| binding.ident()))
.map(Into::into)
.collect(),
None => Vec::new(),
}
}
fn convert_node(node: rnix::SyntaxNode) -> Option<ast::Node> {
match node.kind() {
rnix::SyntaxKind::NODE_ATTRPATH_VALUE => convert_binding(node),
@@ -69,7 +137,11 @@ impl TryFrom<rnix::ast::AttrpathValue> for ast::Binding {
let value = binding.value().ok_or(())?.try_into()?;
Ok(Self { key, value })
Ok(Self {
range: binding.syntax().text_range().into(),
key,
value,
})
}
}

27
src/xatu/rules.rs
View File

@@ -3,7 +3,9 @@ use crate::ast;
use crate::rules::manual_inherit::ManualInherit;
use crate::rules::manual_inherit_from::ManualInheritFrom;
use crate::rules::unquoted_uri::UnquotedUri;
use crate::scope_graph::ScopeGraph;
pub mod let_order;
pub mod manual_inherit;
pub mod manual_inherit_from;
pub mod unquoted_uri;
@@ -19,15 +21,22 @@ pub struct Finding {
pub rule_id: &'static str,
pub range: ast::SourceRange,
pub message: &'static str,
pub binding: Option<String>,
}
impl std::fmt::Display for Diagnostic {
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
formatter,
"{}: {}: {}",
self.finding.range, self.finding.rule_id, self.finding.message,
)
"[{}] {}",
self.finding.rule_id, self.finding.message
)?;
if let Some(binding) = &self.finding.binding {
write!(formatter, ": {binding}")?;
}
Ok(())
}
}
@@ -49,7 +58,7 @@ pub trait TokenRule {
fn check(token: &Self::Token) -> Option<&'static str>;
}
pub fn run_rules(file: &ast::ParsedFile) -> Vec<Diagnostic> {
pub fn run_rules(file: &ast::ParsedFile, graph: &ScopeGraph) -> Vec<Diagnostic> {
let node_diagnostics = file
.nodes
.iter()
@@ -66,8 +75,16 @@ pub fn run_rules(file: &ast::ParsedFile) -> Vec<Diagnostic> {
.flat_map(|token| [run_token_rule::<UnquotedUri>(token)])
.flatten();
let let_order_diagnostics = let_order::check(graph).into_iter().map(|finding| Finding {
rule_id: "let_order",
range: finding.range,
message: "Let bindings must follow dependency order",
binding: Some(finding.binding),
});
node_diagnostics
.chain(token_diagnostics)
.chain(let_order_diagnostics)
.map(|finding| Diagnostic {
file_id: file.file_id,
finding,
@@ -86,6 +103,7 @@ where
rule_id: R::ID,
range: node.range,
message,
binding: None,
})
}
@@ -100,5 +118,6 @@ where
rule_id: R::ID,
range: token.range,
message,
binding: None,
})
}

71
src/xatu/rules/let_order.rs Normal file
View File

@@ -0,0 +1,71 @@
use crate::{
ast,
scope_graph::{LetScope, NodeId, ScopeGraph},
};
pub struct Finding {
pub range: ast::SourceRange,
pub binding: String,
}
pub fn check(graph: &ScopeGraph) -> Vec<Finding> {
graph
.let_scopes()
.iter()
.flat_map(|let_scope| {
let expected = expected_order(graph, let_scope);
let positions = expected
.iter()
.enumerate()
.map(|(position, declaration)| (*declaration, position))
.collect::<Vec<_>>();
let_scope
.bindings
.iter()
.enumerate()
.filter_map(move |(position, binding)| {
positions
.iter()
.find(|(declaration, _)| *declaration == binding.declaration)
.filter(|(_, expected_position)| position != *expected_position)
.map(|_| Finding {
range: binding.range,
binding: binding.name.clone(),
})
})
})
.collect()
}
fn expected_order(graph: &ScopeGraph, let_scope: &LetScope) -> Vec<NodeId> {
let mut order = Vec::new();
visit_references(graph, &let_scope.entry_references, let_scope, &mut order);
order
}
fn visit_references(
graph: &ScopeGraph,
references: &[NodeId],
let_scope: &LetScope,
order: &mut Vec<NodeId>,
) {
for reference in references {
for declaration in graph.resolve(*reference) {
let Some(binding) = let_scope
.bindings
.iter()
.find(|binding| binding.declaration == declaration)
else {
continue;
};
if order.contains(&declaration) {
continue;
}
order.push(declaration);
visit_references(graph, &binding.references, let_scope, order);
}
}
}

194
src/xatu/scope_graph.rs Normal file
View File

@@ -0,0 +1,194 @@
use crate::ast::{self, ParsedFile};
#[derive(Debug)]
pub(crate) struct ScopeGraph {
nodes: Vec<Node>,
edges: Vec<Edge>,
lets: Vec<LetScope>,
}
impl ScopeGraph {
pub(crate) fn from_file(file: &ParsedFile) -> Self {
let mut graph = Self {
nodes: Vec::new(),
edges: Vec::new(),
lets: Vec::new(),
};
let root = graph.add_scope(None);
graph.build_expr(&file.root, root);
graph
}
pub(crate) fn let_scopes(&self) -> &[LetScope] {
&self.lets
}
fn build_expr(&mut self, expr: &ast::Expr, scope: NodeId) -> Vec<NodeId> {
match expr {
ast::Expr::Ident(ident) => vec![self.add_reference(&ident.name, scope)],
ast::Expr::Let { bindings, body } => self.build_let(bindings, body, scope),
ast::Expr::AttrSet(bindings) => bindings
.iter()
.flat_map(|binding| self.build_expr(&binding.value, scope))
.collect(),
ast::Expr::Lambda { parameters, body } => {
let scope = self.add_scope(Some(scope));
for parameter in parameters {
self.add_declaration(&parameter.name, scope);
}
self.build_expr(body, scope)
}
ast::Expr::Other(expressions) => expressions
.iter()
.flat_map(|expression| self.build_expr(expression, scope))
.collect(),
ast::Expr::Select(_) => Vec::new(),
}
}
fn build_let(
&mut self,
bindings: &[ast::Binding],
body: &ast::Expr,
parent: NodeId,
) -> Vec<NodeId> {
let scope = self.add_scope(Some(parent));
let mut graph_bindings = bindings
.iter()
.map(|binding| Binding {
declaration: self.add_declaration(&binding.key.name, scope),
name: binding.key.name.clone(),
references: Vec::new(),
range: binding.range,
})
.collect::<Vec<_>>();
for (binding, graph_binding) in bindings.iter().zip(&mut graph_bindings) {
graph_binding.references = self.build_expr(&binding.value, scope);
}
let entry_references = self.build_expr(body, scope);
let references = graph_bindings
.iter()
.flat_map(|binding| binding.references.iter().copied())
.chain(entry_references.iter().copied())
.collect();
self.lets.push(LetScope {
bindings: graph_bindings,
entry_references,
});
references
}
pub(crate) fn resolve(&self, reference: NodeId) -> Vec<NodeId> {
let Node::Reference(name) = &self.nodes[reference.0] else {
return Vec::new();
};
let Some(mut scope) = self.edge_target(reference, Node::is_scope) else {
return Vec::new();
};
loop {
let declarations = self
.edge_targets(scope, |node| matches!(node, Node::Declaration(declaration) if declaration.name == *name))
.collect::<Vec<_>>();
if !declarations.is_empty() {
return declarations;
}
let Some(parent) = self.edge_target(scope, Node::is_scope) else {
return Vec::new();
};
scope = parent;
}
}
fn add_scope(&mut self, parent: Option<NodeId>) -> NodeId {
let scope = self.add_node(Node::Scope);
if let Some(parent) = parent {
self.add_edge(scope, parent);
}
scope
}
fn add_declaration(&mut self, name: &str, scope: NodeId) -> NodeId {
let declaration = self.add_node(Node::Declaration(Declaration {
name: name.to_owned(),
}));
self.add_edge(scope, declaration);
declaration
}
fn add_reference(&mut self, name: &str, scope: NodeId) -> NodeId {
let reference = self.add_node(Node::Reference(name.to_owned()));
self.add_edge(reference, scope);
reference
}
fn add_node(&mut self, node: Node) -> NodeId {
let id = NodeId(self.nodes.len());
self.nodes.push(node);
id
}
fn add_edge(&mut self, from: NodeId, to: NodeId) {
self.edges.push(Edge { from, to });
}
fn edge_target(&self, from: NodeId, predicate: fn(&Node) -> bool) -> Option<NodeId> {
self.edge_targets(from, predicate).next()
}
fn edge_targets<'graph>(
&'graph self,
from: NodeId,
predicate: impl Fn(&Node) -> bool + 'graph,
) -> impl Iterator<Item = NodeId> + 'graph {
self.edges
.iter()
.filter(move |edge| edge.from == from && predicate(&self.nodes[edge.to.0]))
.map(|edge| edge.to)
}
}
#[derive(Debug)]
enum Node {
Scope,
Declaration(Declaration),
Reference(String),
}
impl Node {
fn is_scope(&self) -> bool {
matches!(self, Self::Scope)
}
}
#[derive(Debug)]
struct Declaration {
name: String,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) struct NodeId(usize);
#[derive(Debug)]
struct Edge {
from: NodeId,
to: NodeId,
}
#[derive(Debug)]
pub(crate) struct LetScope {
pub(crate) bindings: Vec<Binding>,
pub(crate) entry_references: Vec<NodeId>,
}
#[derive(Debug)]
pub(crate) struct Binding {
pub(crate) declaration: NodeId,
pub(crate) name: String,
pub(crate) references: Vec<NodeId>,
pub(crate) range: ast::SourceRange,
}

18
src/xatu/testdata/clean.nix vendored
View File

@@ -1,6 +1,22 @@
{ pkgs }:
{
let
first = 1;
second = first;
in {
firstField = first;
secondField = second;
nested = let
innerSecond = innerFirst;
innerFirst = first;
in {
result = innerSecond;
};
parameter = first: let
usesParameter = first;
in {
result = usesParameter;
};
path = ./. + "/foo";
url = "https://example.com";
inherit pkgs;

14
src/xatu/testdata/lint_findings.nix vendored
View File

@@ -1,5 +1,17 @@
{
let
second = first;
first = 1;
nested = let
nestedFirst = first;
nestedSecond = nestedFirst;
in {
result = nestedSecond;
};
in {
package = package;
from = pkgs.from;
url = https://example.com;
firstField = first;
secondField = second;
nestedField = nested;
}

29
src/xatu/xatu.rs
View File

@@ -2,6 +2,9 @@ mod ast;
mod file_store;
mod parser;
mod rules;
mod scope_graph;
use scope_graph::ScopeGraph;
pub use file_store::{FileId, FileStore, SourceFile};
pub use rules::{Diagnostic, Finding};
@@ -22,13 +25,34 @@ impl std::fmt::Display for Report {
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
for diagnostic in &self.diagnostics {
let file = self.files.get(diagnostic.file_id);
writeln!(formatter, "{}:{diagnostic}", file.path.display())?;
let (start, end) = line_range(&file.source, diagnostic.finding.range);
let location = if start == end {
start.to_string()
} else {
format!("{start}-{end}")
};
writeln!(formatter, "{}:{location} {diagnostic}", file.path.display())?;
}
Ok(())
}
}
fn line_range(source: &str, range: ast::SourceRange) -> (usize, usize) {
let line_at = |offset| {
source
.bytes()
.take(offset)
.filter(|byte| *byte == b'\n')
.count()
+ 1
};
let start = line_at(range.start);
let end = line_at(range.end);
(start, end)
}
/// Analyzes source files and returns the diagnostics Xatu finds.
pub fn future_sight(files: Vec<SourceFile>) -> status::Result<Report> {
let mut store = FileStore::default();
@@ -46,7 +70,8 @@ pub fn future_sight(files: Vec<SourceFile>) -> status::Result<Report> {
)));
}
diagnostics.extend(rules::run_rules(&parsed));
let graph = ScopeGraph::from_file(&parsed);
diagnostics.extend(rules::run_rules(&parsed, &graph));
}
Ok(Report {

13
src/xatu/xatu_test.rs
View File

@@ -41,6 +41,19 @@ fn xatu_rejects_invalid_source() -> googletest::Result<()> {
Ok(())
}
#[gtest]
fn xatu_reports_out_of_order_let_bindings() -> googletest::Result<()> {
let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?;
let let_order_count = report
.diagnostics
.iter()
.filter(|diagnostic| diagnostic.finding.rule_id == "let_order")
.count();
expect_that!(let_order_count, eq(4));
Ok(())
}
#[gtest]
fn xatu_reports_unquoted_uri() -> googletest::Result<()> {
let report = xatu::future_sight(vec![fixture_source("testdata/lint_findings.nix")?]).or_fail()?;