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

src/xatu/ast.rs

@@ -29,12 +29,20 @@ impl std::fmt::Display for SourceRange {
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum NodeData {
     Binding(Binding),
+    LetBlock(LetBlock),
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Binding {
     pub key: Ident,
     pub value: Expr,
+    pub references: Vec<Ident>,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct LetBlock {
+    pub bindings: Vec<Binding>,
+    pub body_references: Vec<Ident>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -46,6 +54,7 @@ pub struct Ident {
 pub enum Expr {
     Ident(Ident),
     Select(Select),
+    Other,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]

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<ast::Node>, Vec<ast::Token>)
 fn convert_node(node: rnix::SyntaxNode) -> Option<ast::Node> {
     match node.kind() {
         rnix::SyntaxKind::NODE_ATTRPATH_VALUE => convert_binding(node),
+        rnix::SyntaxKind::NODE_LET_IN => convert_let_block(node),
         _ => None,
     }
 }
@@ -53,6 +54,34 @@ fn convert_binding(node: rnix::SyntaxNode) -> Option<ast::Node> {
     })
 }
 
+fn convert_let_block(node: rnix::SyntaxNode) -> Option<ast::Node> {
+    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<rnix::ast::LetIn> for ast::LetBlock {
+    type Error = ();
+
+    fn try_from(let_block: rnix::ast::LetIn) -> Result<Self, Self::Error> {
+        let bindings = let_block
+            .attrpath_values()
+            .map(ast::Binding::try_from)
+            .collect::<Result<Vec<_>, _>>()?;
+        let body_references = let_block.body().ok_or(())?.references();
+
+        Ok(Self {
+            bindings,
+            body_references,
+        })
+    }
+}
+
 impl TryFrom<rnix::ast::AttrpathValue> for ast::Binding {
     type Error = ();
 
@@ -67,9 +96,15 @@ impl TryFrom<rnix::ast::AttrpathValue> 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,
+        })
     }
 }
 
@@ -80,7 +115,7 @@ impl TryFrom<rnix::ast::Expr> 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),
         }
     }
 }
@@ -93,6 +128,37 @@ impl From<rnix::ast::Ident> for ast::Ident {
     }
 }
 
+trait ExprReferences {
+    fn references(&self) -> Vec<ast::Ident>;
+}
+
+impl ExprReferences for rnix::ast::Expr {
+    fn references(&self) -> Vec<ast::Ident> {
+        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 {
+    let parent = ident.syntax().parent();
+    let grandparent = parent.as_ref().and_then(rowan::SyntaxNode::parent);
+
+    !matches!(
+        (
+            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),
+        )
+    )
+}
+
 impl TryFrom<rnix::ast::Select> for ast::Select {
     type Error = ();
 

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<String>;
 }
 
 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<String>;
 }
 
 pub fn run_rules(file: &ast::ParsedFile) -> Vec<Diagnostic> {
@@ -55,6 +57,7 @@ pub fn run_rules(file: &ast::ParsedFile) -> Vec<Diagnostic> {
         .iter()
         .flat_map(|node| {
             [
+                run_node_rule::<LetOrder>(node),
                 run_node_rule::<ManualInherit>(node),
                 run_node_rule::<ManualInheritFrom>(node),
             ]

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<String> {
+        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<Vec<String>> {
+    let bindings = let_block
+        .bindings
+        .iter()
+        .map(|binding| (binding.key.name.as_str(), binding))
+        .collect::<HashMap<_, _>>();
+    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::Binding>,
+    order: &mut Vec<String>,
+    permanent_marks: &mut HashSet<String>,
+    temporary_marks: &mut HashSet<String>,
+) -> 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<String> {
+    let reachable_names = expected_order.iter().collect::<HashSet<_>>();
+
+    let_block
+        .bindings
+        .iter()
+        .filter(|binding| reachable_names.contains(&binding.key.name))
+        .map(|binding| binding.key.name.clone())
+        .collect()
+}

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;
+        match data {
-
+            ast::NodeData::Binding(binding) => Some(binding),
-        Some(binding)
+            _ => None,
+        }
     }
 
-    fn check(node: &Self::Node) -> Option<&'static str> {
+    fn check(node: &Self::Node) -> Option<String> {
         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,
         }

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;
+        match data {
-
+            ast::NodeData::Binding(binding) => Some(binding),
-        Some(binding)
+            _ => None,
+        }
     }
 
-    fn check(node: &Self::Node) -> Option<&'static str> {
+    fn check(node: &Self::Node) -> Option<String> {
         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,
         }

src/xatu/rules/unquoted_uri.rs

@@ -13,7 +13,7 @@ impl TokenRule for UnquotedUri {
         }
     }
 
-    fn check(_token: &Self::Token) -> Option<&'static str> {
+    fn check(_token: &Self::Token) -> Option<String> {
-        Some("URI literals should be quoted")
+        Some("URI literals should be quoted".to_string())
     }
 }

src/xatu/testdata/lint_findings.nix

@@ -1,5 +1,9 @@
-{
+let
+  support = "support";
+  primary = support.value;
+in {
   package = package;
   from = pkgs.from;
   url = https://example.com;
+  ordered = primary;
 }

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/lint_findings.nix")?]).or_fail()?;
+
+    let let_order_diagnostics = report
+        .diagnostics
+        .iter()
+        .filter(|diagnostic| diagnostic.finding.rule_id == "let_order")
+        .collect::<Vec<_>>();
+    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()?;