Merge branch 'main' into release/2.x

nix-community · Jun 23, 2024 · e31a270 · e31a270
2 parents 6fc32fe + 16beb7b
commit e31a270
Show file tree

Hide file tree

Showing 11 changed files with 182 additions and 16 deletions.
diff --git a/libnixf/include/nixf/Basic/Diagnostic.h b/libnixf/include/nixf/Basic/Diagnostic.h
@@ -149,11 +149,22 @@ class Diagnostic : public PartialDiagnostic {
   /// should be "Fatal", "Error" or "Warning"
   /// this will affect the eval process.
   ///
-  /// "Fatal"   -- shouldn't eval the code, e.g. parsing error.
-  /// "Error"   -- trigger an error in nix, but we can recover & eval the code.
   /// "Warning" -- just a warning.
-  /// "Note"    -- some additional information about the error.
-  enum Severity { DS_Fatal, DS_Error, DS_Warning };
+  enum Severity {
+    /// shouldn't eval the code, e.g. parsing error.
+    DS_Fatal,
+    /// trigger an error in nix, but we can recover & eval the code.
+    DS_Error,
+    /// A warning.
+    DS_Warning,
+    /// An information.
+    DS_Info,
+
+    /// A hint. Hints are usually not rendered directly in some editor GUI
+    /// So this is suitable for liveness analysis results.
+    /// For example, "unused xxx"
+    DS_Hint,
+  };
 
   /// Internal kind
   enum DiagnosticKind {

diff --git a/libnixf/include/nixf/Basic/DiagnosticKinds.inc b/libnixf/include/nixf/Basic/DiagnosticKinds.inc
@@ -54,15 +54,15 @@ DIAG("sema-dup-formal-arg", DuplicatedFormalToArg, Error,
      "function argument duplicated to a function formal")
 DIAG("sema-undefined-variable", UndefinedVariable, Error,
      "undefined variable `{}`")
-DIAG("sema-def-not-used", DefinitionNotUsed, Warning,
+DIAG("sema-def-not-used", DefinitionNotUsed, Hint,
      "definition `{}` is not used")
-DIAG("sema-extra-rec", ExtraRecursive, Warning,
+DIAG("sema-extra-rec", ExtraRecursive, Hint,
      "attrset is not necessary to be `rec`ursive ")
-DIAG("sema-extra-with", ExtraWith, Warning, "unused `with` expression")
+DIAG("sema-extra-with", ExtraWith, Hint, "unused `with` expression")
 
 // let a = 1; b = { a = 2; }; with b;  a
 // "a" will bind to "let", not "with", that is not very intuitive.
-DIAG("sema-escaping-with", EscapingWith, Warning,
+DIAG("sema-escaping-with", EscapingWith, Hint,
      "this variable comes from the scope outside of the `with` expression")
 
 #endif // DIAG
diff --git a/libnixf/src/Sema/VariableLookup.cpp b/libnixf/src/Sema/VariableLookup.cpp
@@ -81,7 +81,7 @@ void VariableLookupAnalysis::lookupVar(const ExprVar &Var,
     Def->usedBy(Var);
     Results.insert({&Var, LookupResult{LookupResultKind::Defined, Def}});
 
-    if (EnclosedWith) {
+    if (EnclosedWith && !Def->isBuiltin()) {
       // Escaping from "with" to outer scope.
       // https://github.com/NixOS/nix/issues/490
 

diff --git a/libnixf/test/Sema/VariableLookup.cpp b/libnixf/test/Sema/VariableLookup.cpp
@@ -270,6 +270,14 @@ TEST_F(VLATest, EscapingWith) {
   ASSERT_EQ(D.notes()[1].range().rCur().offset(), 7);
 }
 
+TEST_F(VLATest, EscapingWithButBuiltin) {
+  std::shared_ptr<Node> AST = parse("with { a = 1; }; [ a true false null ]", Diags);
+  VariableLookupAnalysis VLA(Diags);
+  VLA.runOnAST(*AST);
+
+  ASSERT_EQ(Diags.size(), 0);
+}
+
 TEST_F(VLATest, InheritRec) {
   // Make sure inheirt (expr), the expression is binded to "NewEnv".
   std::shared_ptr<Node> AST =

diff --git a/nixd/include/nixd/Controller/Controller.h b/nixd/include/nixd/Controller/Controller.h
@@ -111,7 +111,7 @@ class Controller : public lspserver::LSPServer {
 
   template <class T>
   std::shared_ptr<NixTU> getTU(std::string File, lspserver::Callback<T> &Reply,
-                               bool Ignore = false) {
+                               bool Ignore = true) {
     using lspserver::error;
     std::lock_guard G(TUsLock);
     if (!TUs.count(File)) [[unlikely]] {

diff --git a/nixd/lib/Controller/Convert.cpp b/nixd/lib/Controller/Convert.cpp
@@ -1,4 +1,5 @@
 #include "Convert.h"
+#include "nixf/Basic/Diagnostic.h"
 
 using namespace lspserver;
 
@@ -11,6 +12,10 @@ int getLSPSeverity(nixf::Diagnostic::DiagnosticKind Kind) {
     return 1;
   case nixf::Diagnostic::DS_Warning:
     return 2;
+  case nixf::Diagnostic::DS_Info:
+    return 3;
+  case nixf::Diagnostic::DS_Hint:
+    return 4;
   }
   assert(false && "Invalid severity");
   __builtin_unreachable();

diff --git a/nixd/lib/Controller/Definition.cpp b/nixd/lib/Controller/Definition.cpp
@@ -125,7 +125,7 @@ class NixpkgsDefinitionProvider {
       };
     }
     int PosL = std::stoi(std::string(Position.substr(Pos + 1)));
-    lspserver::Position P{PosL, PosL};
+    lspserver::Position P{PosL, 0};
     std::string_view File = Position.substr(0, Pos);
     return Location{
         .uri = URIForFile::canonicalize(File, File),

diff --git a/nixd/lib/Controller/Hover.cpp b/nixd/lib/Controller/Hover.cpp
@@ -22,6 +22,30 @@ using namespace lspserver;
 
 namespace {
 
+class OptionsHoverProvider {
+  AttrSetClient &Client;
+
+public:
+  OptionsHoverProvider(AttrSetClient &Client) : Client(Client) {}
+  std::optional<OptionDescription>
+  resolveHover(const std::vector<std::string> &Scope) {
+    std::binary_semaphore Ready(0);
+    std::optional<OptionDescription> Desc;
+    auto OnReply = [&Ready, &Desc](llvm::Expected<OptionInfoResponse> Resp) {
+      if (Resp)
+        Desc = *Resp;
+      else
+        elog("options hover: {0}", Resp.takeError());
+      Ready.release();
+    };
+
+    Client.optionInfo(Scope, std::move(OnReply));
+    Ready.acquire();
+
+    return Desc;
+  }
+};
+
 /// \brief Provide package information, library information ... , from nixpkgs.
 class NixpkgsHoverProvider {
   AttrSetClient &NixpkgsClient;
@@ -46,8 +70,7 @@ class NixpkgsHoverProvider {
     }
 
     if (Package.Description) {
-      OS << "## Description"
-         << "\n\n";
+      OS << "## Description" << "\n\n";
       OS << *Package.Description;
       OS << "\n\n";
 
@@ -122,6 +145,41 @@ void Controller::onHover(const TextDocumentPositionParams &Params,
             return;
           }
         }
+
+        std::vector<std::string> Scope;
+        auto R = findAttrPath(*N, PM, Scope);
+        if (R == FindAttrPathResult::OK) {
+          std::lock_guard _(OptionsLock);
+          for (const auto &[_, Client] : Options) {
+            if (AttrSetClient *C = Client->client()) {
+              OptionsHoverProvider OHP(*C);
+              std::optional<OptionDescription> Desc = OHP.resolveHover(Scope);
+              std::string Docs;
+              if (Desc) {
+                if (Desc->Type) {
+                  std::string TypeName = Desc->Type->Name.value_or("");
+                  std::string TypeDesc = Desc->Type->Description.value_or("");
+                  Docs += llvm::formatv("{0} ({1})", TypeName, TypeDesc);
+                } else {
+                  Docs += "? (missing type)";
+                }
+                if (Desc->Description) {
+                  Docs += "\n\n" + Desc->Description.value_or("");
+                }
+                Reply(Hover{
+                    .contents =
+                        MarkupContent{
+                            .kind = MarkupKind::Markdown,
+                            .value = std::move(Docs),
+                        },
+                    .range = toLSPRange(N->range()),
+                });
+                return;
+              }
+            }
+          }
+        }
+
         // Reply it's kind by static analysis
         // FIXME: support more.
         Reply(Hover{

diff --git a/nixd/tools/nixd/test/definition-package.md b/nixd/tools/nixd/test/definition-package.md
@@ -62,11 +62,11 @@ CHECK-NEXT: "result": [
 CHECK-NEXT:   {
 CHECK-NEXT:     "range": {
 CHECK-NEXT:       "end": {
-CHECK-NEXT:         "character": 33,
+CHECK-NEXT:         "character": 0,
 CHECK-NEXT:         "line": 33
 CHECK-NEXT:       },
 CHECK-NEXT:       "start": {
-CHECK-NEXT:         "character": 33,
+CHECK-NEXT:         "character": 0,
 CHECK-NEXT:         "line": 33
 CHECK-NEXT:       }
 CHECK-NEXT:     },

diff --git a/nixd/tools/nixd/test/diagnostic-liveness-formal.md b/nixd/tools/nixd/test/diagnostic-liveness-formal.md
@@ -51,7 +51,7 @@ CHECK-NEXT:         "line": 0
 CHECK-NEXT:       }
 CHECK-NEXT:     },
 CHECK-NEXT:     "relatedInformation": [],
-CHECK-NEXT:     "severity": 2,
+CHECK-NEXT:     "severity": 4,
 CHECK-NEXT:     "source": "nixf",
 CHECK-NEXT:     "tags": [
 CHECK-NEXT:       1

diff --git a/nixd/tools/nixd/test/hover-options.md b/nixd/tools/nixd/test/hover-options.md
@@ -0,0 +1,84 @@
+# RUN: nixd --lit-test \
+# RUN: --nixos-options-expr="{ foo.bar = { _type = \"option\";  description = \"test\"; type.description = \"hello type\"; }; }" \
+# RUN: < %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":"{ foo.bar = 1 }"
+      }
+   }
+}
+```
+
+<-- textDocument/hover(2)
+
+
+```json
+{
+   "jsonrpc":"2.0",
+   "id":2,
+   "method":"textDocument/hover",
+   "params":{
+      "textDocument":{
+         "uri":"file:///basic.nix"
+      },
+      "position":{
+         "line":0,
+         "character":6
+      }
+   }
+}
+```
+
+```
+     CHECK:  "id": 2,
+CHECK-NEXT:  "jsonrpc": "2.0",
+CHECK-NEXT:  "result": {
+CHECK-NEXT:    "contents": {
+CHECK-NEXT:      "kind": "markdown",
+CHECK-NEXT:      "value": " (hello type)\n\ntest"
+CHECK-NEXT:    },
+CHECK-NEXT:    "range": {
+CHECK-NEXT:      "end": {
+CHECK-NEXT:        "character": 9,
+CHECK-NEXT:        "line": 0
+CHECK-NEXT:      },
+CHECK-NEXT:      "start": {
+CHECK-NEXT:        "character": 6,
+CHECK-NEXT:        "line": 0
+CHECK-NEXT:      }
+CHECK-NEXT:    }
+CHECK-NEXT:  }
+```
+
+```json
+{"jsonrpc":"2.0","method":"exit"}
+```