From c1d9c34600c2ec90a72b11c6634a06a406b1a908 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Sat, 18 May 2024 17:40:17 +0800
Subject: [PATCH] libnixf: fix variable lookup not visiting empty let ... in
 ... expr

Emtpy let ... in ... expression, leaving a nullptr in libnixf's "Let" struct,
causes the code not continue visiting the body at all.

Fixes: #500
---
 libnixf/src/Sema/VariableLookup.cpp  | 27 +++++++++++++++++++--------
 libnixf/test/Sema/VariableLookup.cpp | 10 ++++++++++
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/libnixf/src/Sema/VariableLookup.cpp b/libnixf/src/Sema/VariableLookup.cpp
index 6d0dac263..52ddb7b08 100644
--- a/libnixf/src/Sema/VariableLookup.cpp
+++ b/libnixf/src/Sema/VariableLookup.cpp
@@ -220,15 +220,26 @@ void VariableLookupAnalysis::dfs(const ExprAttrs &Attrs,
 
 void VariableLookupAnalysis::dfs(const ExprLet &Let,
                                  const std::shared_ptr<EnvNode> &Env) {
-  if (!Let.attrs())
-    return;
-  const SemaAttrs &SA = Let.attrs()->sema();
-  assert(SA.isRecursive() && "let ... in ... attrset must be recursive");
-  std::shared_ptr<EnvNode> NewEnv = dfsAttrs(SA, Env, &Let, Definition::DS_Let);
-  if (Let.expr())
-    dfs(*Let.expr(), NewEnv);
 
-  emitEnvLivenessWarning(NewEnv);
+  // Obtain the env object suitable for "in" expression.
+  auto GetLetEnv = [&Env, &Let, this]() -> std::shared_ptr<EnvNode> {
+    // This is an empty let ... in ... expr, definitely anti-pattern in
+    // nix language. We want to passthrough the env then.
+    if (!Let.attrs()) {
+      return Env;
+    }
+
+    // If there are some attributes actually, create a new env.
+    const SemaAttrs &SA = Let.attrs()->sema();
+    assert(SA.isRecursive() && "let ... in ... attrset must be recursive");
+    return dfsAttrs(SA, Env, &Let, Definition::DS_Let);
+  };
+
+  auto LetEnv = GetLetEnv();
+
+  if (Let.expr())
+    dfs(*Let.expr(), LetEnv);
+  emitEnvLivenessWarning(LetEnv);
 }
 
 void VariableLookupAnalysis::trivialDispatch(
diff --git a/libnixf/test/Sema/VariableLookup.cpp b/libnixf/test/Sema/VariableLookup.cpp
index b154847b8..e52fd89c8 100644
--- a/libnixf/test/Sema/VariableLookup.cpp
+++ b/libnixf/test/Sema/VariableLookup.cpp
@@ -280,4 +280,14 @@ TEST_F(VLATest, InheritRec) {
   ASSERT_EQ(Diags.size(), 0);
 }
 
+TEST_F(VLATest, EmptyLetIn) {
+  // Test that emtpy let ... in ... expression is considered.
+  // https://github.com/nix-community/nixd/issues/500
+  std::shared_ptr<Node> AST = parse("{ config }: let in config", Diags);
+  VariableLookupAnalysis VLA(Diags);
+  VLA.runOnAST(*AST);
+
+  ASSERT_EQ(Diags.size(), 0);
+}
+
 } // namespace