AGENTS.md

@@ -108,6 +108,8 @@
 - During the Nix refactor, address each issue that comes up by adding either a Natu rule or a new Nix style rule.
 - Git pathspec searches started in a subdirectory default to that subtree.
   Use `git -C <repository-root>` or top-anchored pathspecs for repository-wide history searches.
+- A `**/*.nix` Git pathspec does not include Nix files directly in its starting directory.
+  Include both `*.nix` and `**/*.nix`, or use a recursive `*.nix` pathspec, when linting every Nix file.
 - Ariadne extraction must preserve the game-root season prefix for archived source.
   Different game installs can contain compiled scripts with the same internal path.
 - Clew owns Ren'Py extraction, preparation, and compiled-script AST export. Ariadne owns analysis over Clew-produced data.

src/dotfiles/registry/validation.nix

@@ -3,17 +3,6 @@
   lib,
 }:
 let
-  assertValid =
-    data:
-    let
-      assertions = validationAssertions data;
-      failures = lib.filter (assertion: !assertion.assertion) assertions;
-    in
-    assert lib.assertMsg (lib.all (assertion: assertion.assertion) assertions) (
-      lib.concatMapStringsSep "\n" (assertion: assertion.message) failures
-    );
-    data;
-
   validationAssertions =
     data:
     lib.concatLists (
@@ -41,4 +30,12 @@ let
       ) data
     );
 in
-assertValid
+data:
+let
+  assertions = validationAssertions data;
+  failures = lib.filter (assertion: !assertion.assertion) assertions;
+in
+assert lib.assertMsg (lib.all (assertion: assertion.assertion) assertions) (
+  lib.concatMapStringsSep "\n" (assertion: assertion.message) failures
+);
+data

src/natu/BUILD

@@ -13,6 +13,7 @@ rust_library(
         "rules/list_to_attrs_map.rs",
         "rules/manual_inherit.rs",
         "rules/manual_inherit_from.rs",
+        "rules/redundant_let.rs",
         "rules/redundant_parameter_forwarding.rs",
         "rules/unquoted_uri.rs",
         "scope_graph.rs",

src/natu/natu_test.rs

@@ -69,7 +69,7 @@ fn natu_reports_out_of_order_let_bindings() -> googletest::Result<()> {
 fn natu_preserves_suppressed_bindings() -> googletest::Result<()> {
     let report = natu::future_sight(vec![SourceFile {
         path: PathBuf::from("testdata/nolint.nix"),
-        source: "{ config, ... }: let\n  cfg = config.example; # NOLINT(let_order)\n  value = cfg;\nin value\n".into(),
+        source: "{ config, ... }: let # NOLINT(redundant_let)\n  cfg = config.example; # NOLINT(let_order)\n  value = cfg;\nin value\n".into(),
     }])
     .or_fail()?;
 
@@ -101,6 +101,66 @@ entry
     Ok(())
 }
 
+#[gtest]
+fn natu_reports_redundant_let() -> googletest::Result<()> {
+    let report = natu::future_sight(vec![SourceFile {
+        path: PathBuf::from("testdata/redundant_let.nix"),
+        source: r#"
+let
+  helper = 1;
+  value = helper;
+in
+value
+"#
+        .into(),
+    }])
+    .or_fail()?;
+
+    let count = report
+        .diagnostics
+        .iter()
+        .filter(|diagnostic| diagnostic.finding.rule_id == "redundant_let")
+        .count();
+    expect_that!(count, eq(1));
+    Ok(())
+}
+
+#[gtest]
+fn natu_accepts_nonredundant_let() -> googletest::Result<()> {
+    let report = natu::future_sight(vec![
+        SourceFile {
+            path: PathBuf::from("testdata/recursive_let.nix"),
+            source: r#"
+let
+  recursive = recursive;
+in
+recursive
+"#
+            .into(),
+        },
+        SourceFile {
+            path: PathBuf::from("testdata/dependent_let.nix"),
+            source: r#"
+let
+  value = 1;
+  dependent = value;
+in
+value
+"#
+            .into(),
+        },
+    ])
+    .or_fail()?;
+
+    let count = report
+        .diagnostics
+        .iter()
+        .filter(|diagnostic| diagnostic.finding.rule_id == "redundant_let")
+        .count();
+    expect_that!(count, eq(0));
+    Ok(())
+}
+
 #[gtest]
 fn natu_reports_redundant_parameter_forwarding() -> googletest::Result<()> {
     let report = natu::future_sight(vec![

src/natu/rules.rs

@@ -10,6 +10,7 @@ pub mod let_order;
 pub mod list_to_attrs_map;
 pub mod manual_inherit;
 pub mod manual_inherit_from;
+pub mod redundant_let;
 pub mod redundant_parameter_forwarding;
 pub mod unquoted_uri;
 
@@ -90,6 +91,16 @@ pub fn run_rules(file: &ast::ParsedFile, graph: &ScopeGraph, source: &str) -> Ve
                 binding: None,
                 expected_order: Some(finding.expected_order),
             });
+    let redundant_let_diagnostics =
+        redundant_let::check(&file.root)
+            .into_iter()
+            .map(|finding| Finding {
+                rule_id: "redundant_let",
+                range: finding.range,
+                message: "Inline this returned let binding",
+                binding: None,
+                expected_order: None,
+            });
     let redundant_parameter_forwarding_diagnostics =
         redundant_parameter_forwarding::check(&file.root)
             .into_iter()
@@ -103,6 +114,7 @@ pub fn run_rules(file: &ast::ParsedFile, graph: &ScopeGraph, source: &str) -> Ve
     node_diagnostics
         .chain(token_diagnostics)
         .chain(let_order_diagnostics)
+        .chain(redundant_let_diagnostics)
         .chain(redundant_parameter_forwarding_diagnostics)
         .filter(|finding| !is_suppressed(source, finding.range, finding.rule_id))
         .map(|finding| Diagnostic {

src/natu/rules/redundant_let.rs

@@ -0,0 +1,98 @@
+use crate::ast;
+
+pub struct Finding {
+    pub range: ast::SourceRange,
+}
+
+pub fn check(expr: &ast::Expr) -> Vec<Finding> {
+    let mut findings = Vec::new();
+    check_expr(expr, &mut findings);
+    findings
+}
+
+fn check_expr(expr: &ast::Expr, findings: &mut Vec<Finding>) {
+    match expr {
+        ast::Expr::Let {
+            range,
+            bindings,
+            body,
+        } => {
+            if is_redundant(bindings, body) {
+                findings.push(Finding { range: *range });
+            }
+            for binding in bindings {
+                check_expr(&binding.value, findings);
+            }
+            check_expr(body, findings);
+        }
+        ast::Expr::AttrSet(bindings) => {
+            for binding in bindings {
+                check_expr(&binding.value, findings);
+            }
+        }
+        ast::Expr::Lambda { body, .. } => check_expr(body, findings),
+        ast::Expr::Apply(application) => {
+            check_expr(&application.lambda, findings);
+            check_expr(&application.argument, findings);
+        }
+        ast::Expr::Other(expressions) => {
+            for expression in expressions {
+                check_expr(expression, findings);
+            }
+        }
+        ast::Expr::Ident(_) | ast::Expr::Select(_) => { }
+    }
+}
+
+fn is_redundant(bindings: &[ast::Binding], body: &ast::Expr) -> bool {
+    let Some((binding_index, binding)) = bindings
+        .iter()
+        .enumerate()
+        .find(|(_, binding)| is_ident(body, &binding.key.name))
+    else {
+        return false;
+    };
+
+    !references_name(&binding.value, &binding.key.name)
+        && !bindings.iter().enumerate().any(|(candidate_index, candidate)| {
+            candidate_index != binding_index && references_name(&candidate.value, &binding.key.name)
+        })
+}
+
+fn is_ident(expr: &ast::Expr, name: &str) -> bool {
+    matches!(unwrap_expr(expr), ast::Expr::Ident(ident) if ident.name == name)
+}
+
+fn references_name(expr: &ast::Expr, name: &str) -> bool {
+    match expr {
+        ast::Expr::Ident(ident) => ident.name == name,
+        ast::Expr::Select(select) => select.receiver == name,
+        ast::Expr::Apply(application) => {
+            references_name(&application.lambda, name) || references_name(&application.argument, name)
+        }
+        ast::Expr::Let { bindings, body, .. } => {
+            !bindings.iter().any(|binding| binding.key.name == name)
+                && (bindings
+                    .iter()
+                    .any(|binding| references_name(&binding.value, name))
+                    || references_name(body, name))
+        }
+        ast::Expr::AttrSet(bindings) => bindings
+            .iter()
+            .any(|binding| references_name(&binding.value, name)),
+        ast::Expr::Lambda { parameters, body } => {
+            !parameters.iter().any(|parameter| parameter.name == name)
+                && references_name(body, name)
+        }
+        ast::Expr::Other(expressions) => expressions
+            .iter()
+            .any(|expression| references_name(expression, name)),
+    }
+}
+
+fn unwrap_expr(expr: &ast::Expr) -> &ast::Expr {
+    match expr {
+        ast::Expr::Other(expressions) if expressions.len() == 1 => unwrap_expr(&expressions[0]),
+        _ => expr,
+    }
+}