From d08d5fe6f837be33f081ef026672f56000bd56d3 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Sun, 11 Aug 2024 20:22:24 +0800
Subject: [PATCH] nixd/Controller: report error on `textDocument/definition` on
 builtins

Previously the server will crash.
---
 nixd/lib/Controller/AST.h                  |  6 ++
 nixd/lib/Controller/Definition.cpp         | 16 ++++--
 nixd/tools/nixd/test/definition/builtin.md | 67 ++++++++++++++++++++++
 3 files changed, 84 insertions(+), 5 deletions(-)
 create mode 100644 nixd/tools/nixd/test/definition/builtin.md

diff --git a/nixd/lib/Controller/AST.h b/nixd/lib/Controller/AST.h
index 2c6a3152b..6171cd450 100644
--- a/nixd/lib/Controller/AST.h
+++ b/nixd/lib/Controller/AST.h
@@ -39,6 +39,12 @@ struct NotAnIdiomException : IdiomSelectorException {
 
 struct VLAException : std::exception {};
 
+struct NoLocationForBuiltinVariable : std::exception {
+  [[nodiscard]] const char *what() const noexcept override {
+    return "builtin variable is not defined in nix source location";
+  }
+};
+
 /// \brief No such variable.
 struct NoSuchVarException : VLAException {
   [[nodiscard]] const char *what() const noexcept override {
diff --git a/nixd/lib/Controller/Definition.cpp b/nixd/lib/Controller/Definition.cpp
index 0d7a6255b..d64976545 100644
--- a/nixd/lib/Controller/Definition.cpp
+++ b/nixd/lib/Controller/Definition.cpp
@@ -115,6 +115,9 @@ const Definition &findVarDefinition(const ExprVar &Var,
 /// \brief Convert nixf::Definition to lspserver::Location
 Location convertToLocation(llvm::StringRef Src, const Definition &Def,
                            URIForFile URI) {
+  if (!Def.syntax())
+    throw NoLocationForBuiltinVariable();
+  assert(Def.syntax());
   return Location{
       .uri = std::move(URI),
       .range = toLSPRange(Src, Def.syntax()->range()),
@@ -292,9 +295,10 @@ std::vector<T> mergeVec(std::vector<T> A, const std::vector<T> &B) {
   return A;
 }
 
-Locations defineVar(const ExprVar &Var, const VariableLookupAnalysis &VLA,
-                    const ParentMapAnalysis &PM, AttrSetClient &NixpkgsClient,
-                    const URIForFile &URI, llvm::StringRef Src) {
+llvm::Expected<Locations>
+defineVar(const ExprVar &Var, const VariableLookupAnalysis &VLA,
+          const ParentMapAnalysis &PM, AttrSetClient &NixpkgsClient,
+          const URIForFile &URI, llvm::StringRef Src) {
   try {
     Locations StaticLocs = defineVarStatic(Var, VLA, URI, Src);
 
@@ -308,9 +312,11 @@ Locations defineVar(const ExprVar &Var, const VariableLookupAnalysis &VLA,
       return StaticLocs;
     }
   } catch (std::exception &E) {
-    elog("definition/static: {0}", E.what());
+    // Pop a window, notify the user we cannot find static definition.
+    // Likely this will be triggerred by clicking "builtins"
+    return error(E.what());
   }
-  return {};
+  return error("unreachable code! Please submit an issue");
 }
 
 /// \brief Squash a vector into smaller json variant.
diff --git a/nixd/tools/nixd/test/definition/builtin.md b/nixd/tools/nixd/test/definition/builtin.md
new file mode 100644
index 000000000..9e31451cb
--- /dev/null
+++ b/nixd/tools/nixd/test/definition/builtin.md
@@ -0,0 +1,67 @@
+# RUN: nixd --lit-test < %s | FileCheck %s
+
+<-- initialize(0)
+
+```json
+{
+   "jsonrpc":"2.0",
+   "id":0,
+   "method":"initialize",
+   "params":{
+      "processId":123,
+      "rootPath":"",
+      "capabilities":{
+      },
+      "trace":"off"
+   }
+}
+```
+
+
+<-- textDocument/didOpen
+
+```json
+{
+   "jsonrpc":"2.0",
+   "method":"textDocument/didOpen",
+   "params":{
+      "textDocument":{
+         "uri":"file:///basic.nix",
+         "languageId":"nix",
+         "version":1,
+         "text":"builtins"
+      }
+   }
+}
+```
+
+<-- textDocument/definition(2)
+
+
+```json
+{
+   "jsonrpc":"2.0",
+   "id":2,
+   "method":"textDocument/definition",
+   "params":{
+      "textDocument":{
+         "uri":"file:///basic.nix"
+      },
+      "position":{
+        "line": 0,
+        "character":3
+      }
+   }
+}
+```
+
+```
+CHECK:      "message": "builtin variable is not defined in nix source location"
+CHECK-NEXT: },
+CHECK-NEXT: "id": 2,
+```
+
+
+```json
+{"jsonrpc":"2.0","method":"exit"}
+```