feat(xatu): check let binding order

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/rules.rs

@@ -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,
     })
 }

src/xatu/rules/let_order.rs

@@ -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);
+        }
+    }
+}

src/xatu/testdata/clean.nix

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

src/xatu/testdata/lint_findings.nix

@@ -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;
 }

src/xatu/xatu.rs

@@ -4,6 +4,8 @@ mod parser;
 mod rules;
 mod scope_graph;
 
+use scope_graph::ScopeGraph;
+
 pub use file_store::{FileId, FileStore, SourceFile};
 pub use rules::{Diagnostic, Finding};
 
@@ -23,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();
@@ -47,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 {

src/xatu/xatu_test.rs

@@ -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()?;