feat(natu): detect redundant let expressions

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

@@ -101,6 +101,52 @@ 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
+  value = 1;
+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/nonredundant_let.nix"),
+        source: r#"
+let
+  recursive = recursive;
+in
+recursive
+"#
+        .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 single-binding let expression",
+                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,91 @@
+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 [binding] = bindings else {
+        return false;
+    };
+
+    is_ident(body, &binding.key.name) && !references_name(&binding.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,
+    }
+}