feat(natu): detect single-use let bindings

src/natu/BUILD

@@ -14,6 +14,7 @@ rust_library(
         "rules/manual_inherit.rs",
         "rules/manual_inherit_from.rs",
         "rules/redundant_parameter_forwarding.rs",
+        "rules/single_use_binding.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\n  cfg = config.example; # NOLINT(let_order, single_use_binding)\n  value = cfg; # NOLINT(single_use_binding)\nin value\n".into(),
     }])
     .or_fail()?;
 
@@ -159,6 +159,49 @@ transform value
     Ok(())
 }
 
+#[gtest]
+fn natu_accepts_value_captured_by_reused_lambda() -> googletest::Result<()> {
+    let report = natu::future_sight(vec![SourceFile {
+        path: PathBuf::from("testdata/captured_value.nix"),
+        source: r#"
+let
+  mapper = _: captured;
+  captured = 1;
+in
+[ (mapper 1) (mapper 2) ]
+"#
+        .into(),
+    }])
+    .or_fail()?;
+
+    expect_that!(report.is_clean(), is_true());
+    Ok(())
+}
+
+#[gtest]
+fn natu_reports_single_use_binding() -> googletest::Result<()> {
+    let report = natu::future_sight(vec![SourceFile {
+        path: PathBuf::from("testdata/single_use_binding.nix"),
+        source: r#"
+let
+  single = 1;
+  recursive = recursive;
+in
+{ inherit single recursive; }
+"#
+        .into(),
+    }])
+    .or_fail()?;
+
+    let count = report
+        .diagnostics
+        .iter()
+        .filter(|diagnostic| diagnostic.finding.rule_id == "single_use_binding")
+        .count();
+    expect_that!(count, eq(1));
+    Ok(())
+}
+
 #[gtest]
 fn natu_reports_list_to_attrs_after_map() -> googletest::Result<()> {
     let report = natu::future_sight(vec![SourceFile {

src/natu/rules.rs

@@ -11,6 +11,7 @@ pub mod list_to_attrs_map;
 pub mod manual_inherit;
 pub mod manual_inherit_from;
 pub mod redundant_parameter_forwarding;
+pub mod single_use_binding;
 pub mod unquoted_uri;
 
 #[derive(Debug)]
@@ -100,11 +101,22 @@ pub fn run_rules(file: &ast::ParsedFile, graph: &ScopeGraph, source: &str) -> Ve
                 binding: None,
                 expected_order: None,
             });
+    let single_use_binding_diagnostics =
+        single_use_binding::check(graph)
+            .into_iter()
+            .map(|finding| Finding {
+                rule_id: "single_use_binding",
+                range: finding.range,
+                message: "Inline this single-use let binding",
+                binding: Some(finding.name),
+                expected_order: None,
+            });
 
     node_diagnostics
         .chain(token_diagnostics)
         .chain(let_order_diagnostics)
         .chain(redundant_parameter_forwarding_diagnostics)
+        .chain(single_use_binding_diagnostics)
         .filter(|finding| !is_suppressed(source, finding.range, finding.rule_id))
         .map(|finding| Diagnostic {
             file_id: file.file_id,

src/natu/rules/single_use_binding.rs

@@ -0,0 +1,45 @@
+use crate::{
+    ast,
+    scope_graph::{Binding, LetScope, ScopeGraph},
+};
+
+pub struct Finding {
+    pub range: ast::SourceRange,
+    pub name: String,
+}
+
+pub fn check(graph: &ScopeGraph) -> Vec<Finding> {
+    graph
+        .let_scopes()
+        .iter()
+        .flat_map(|let_scope| {
+            let_scope.bindings.iter().filter_map(|binding| {
+                is_single_use(graph, let_scope, binding).then(|| Finding {
+                    range: binding.range,
+                    name: binding.name.clone(),
+                })
+            })
+        })
+        .collect()
+}
+
+fn is_single_use(graph: &ScopeGraph, let_scope: &LetScope, binding: &Binding) -> bool {
+    let self_references = binding
+        .references
+        .iter()
+        .filter(|reference| graph.resolve(**reference).contains(&binding.declaration))
+        .count();
+    let external_references = let_scope
+        .bindings
+        .iter()
+        .filter(|candidate| candidate.declaration != binding.declaration)
+        .flat_map(|candidate| candidate.references.iter())
+        .chain(let_scope.entry_references.iter())
+        .filter(|reference| graph.resolve(**reference).contains(&binding.declaration))
+        .collect::<Vec<_>>();
+
+    self_references == 0
+        && external_references.len() == 1
+        && (binding.is_lambda
+            || !graph.crosses_lambda_scope(*external_references[0], binding.declaration))
+}

src/natu/scope_graph.rs

@@ -14,7 +14,7 @@ impl ScopeGraph {
             edges: Vec::new(),
             lets: Vec::new(),
         };
-        let root = graph.add_scope(None);
+        let root = graph.add_scope(None, ScopeKind::Root);
         graph.build_expr(&file.root, root);
         graph
     }
@@ -36,7 +36,7 @@ impl ScopeGraph {
                 .flat_map(|binding| self.build_expr(&binding.value, scope))
                 .collect(),
             ast::Expr::Lambda { parameters, body } => {
-                let scope = self.add_scope(Some(scope));
+                let scope = self.add_scope(Some(scope), ScopeKind::Lambda);
                 for parameter in parameters {
                     self.add_declaration(&parameter.name, scope);
                 }
@@ -62,13 +62,14 @@ impl ScopeGraph {
         body: &ast::Expr,
         parent: NodeId,
     ) -> Vec<NodeId> {
-        let scope = self.add_scope(Some(parent));
+        let scope = self.add_scope(Some(parent), ScopeKind::Let);
         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(),
+                is_lambda: matches!(binding.value, ast::Expr::Lambda { .. }),
                 range: binding.range,
             })
             .collect::<Vec<_>>();
@@ -92,6 +93,33 @@ impl ScopeGraph {
         references
     }
 
+    pub(crate) fn crosses_lambda_scope(&self, reference: NodeId, declaration: NodeId) -> bool {
+        let Some(mut scope) = self.edge_target(reference, Node::is_scope) else {
+            return false;
+        };
+        let Some(declaration_scope) = self
+            .edges
+            .iter()
+            .find(|edge| edge.to == declaration && self.nodes[edge.from.0].is_scope())
+            .map(|edge| edge.from)
+        else {
+            return false;
+        };
+
+        loop {
+            if scope == declaration_scope {
+                return false;
+            }
+            if matches!(self.nodes[scope.0], Node::Scope(ScopeKind::Lambda)) {
+                return true;
+            }
+            let Some(parent) = self.edge_target(scope, Node::is_scope) else {
+                return false;
+            };
+            scope = parent;
+        }
+    }
+
     pub(crate) fn resolve(&self, reference: NodeId) -> Vec<NodeId> {
         let Node::Reference(name) = &self.nodes[reference.0] else {
             return Vec::new();
@@ -115,8 +143,8 @@ impl ScopeGraph {
         }
     }
 
-    fn add_scope(&mut self, parent: Option<NodeId>) -> NodeId {
+    fn add_scope(&mut self, parent: Option<NodeId>, kind: ScopeKind) -> NodeId {
-        let scope = self.add_node(Node::Scope);
+        let scope = self.add_node(Node::Scope(kind));
         if let Some(parent) = parent {
             self.add_edge(scope, parent);
         }
@@ -165,17 +193,24 @@ impl ScopeGraph {
 
 #[derive(Debug)]
 enum Node {
-    Scope,
+    Scope(ScopeKind),
     Declaration(Declaration),
     Reference(String),
 }
 
 impl Node {
     fn is_scope(&self) -> bool {
-        matches!(self, Self::Scope)
+        matches!(self, Self::Scope(_))
     }
 }
 
+#[derive(Debug)]
+enum ScopeKind {
+    Root,
+    Let,
+    Lambda,
+}
+
 #[derive(Debug)]
 struct Declaration {
     name: String,
@@ -202,5 +237,6 @@ pub(crate) struct Binding {
     pub(crate) declaration: NodeId,
     pub(crate) name: String,
     pub(crate) references: Vec<NodeId>,
+    pub(crate) is_lambda: bool,
     pub(crate) range: ast::SourceRange,
 }

src/natu/testdata/clean.nix

@@ -2,18 +2,18 @@
 
 let
   first = 1;
-  second = first;
+  second = first; # NOLINT(single_use_binding)
 in {
   firstField = first;
   secondField = second;
   nested = let
-    innerSecond = innerFirst;
+    innerSecond = innerFirst; # NOLINT(single_use_binding)
-    innerFirst = first;
+    innerFirst = first; # NOLINT(single_use_binding)
   in {
     result = innerSecond;
   };
   parameter = first: let
-    usesParameter = first;
+    usesParameter = first; # NOLINT(single_use_binding)
   in {
     result = usesParameter;
   };

src/natu/testdata/lint_findings.nix

@@ -1,9 +1,9 @@
 let
-  second = first;
+  second = first; # NOLINT(single_use_binding)
   first = 1;
-  nested = let
+  nested = let # NOLINT(single_use_binding)
-    nestedFirst = first;
+    nestedFirst = first; # NOLINT(single_use_binding)
-    nestedSecond = nestedFirst;
+    nestedSecond = nestedFirst; # NOLINT(single_use_binding)
   in {
     result = nestedSecond;
   };