From b23dde37fbf09796b39b336353853dceba9d0e1f Mon Sep 17 00:00:00 2001 From: Christoph Wolf Date: Sun, 12 Apr 2026 09:28:19 +0200 Subject: [PATCH] feat(diagnostics): add wrong/empty enum Value diagnostic --- nixd/include/nixd/Controller/Controller.h | 9 +- nixd/include/nixd/Controller/NixTU.h | 25 +++ nixd/include/nixd/Protocol/AttrSet.h | 1 + nixd/lib/Controller/Configuration.cpp | 3 +- nixd/lib/Controller/Diagnostics.cpp | 14 +- nixd/lib/Controller/OptionDiagnostics.cpp | 165 ++++++++++++++++++ nixd/lib/Controller/Support.cpp | 20 ++- nixd/lib/Eval/AttrSetProvider.cpp | 25 +++ nixd/lib/Protocol/AttrSet.cpp | 6 +- nixd/lib/meson.build | 1 + .../diagnostic/option-enum-empty-value.md | 38 ++++ .../nixd/test/diagnostic/option-enum-value.md | 38 ++++ 12 files changed, 329 insertions(+), 16 deletions(-) create mode 100644 nixd/lib/Controller/OptionDiagnostics.cpp create mode 100644 nixd/tools/nixd/test/diagnostic/option-enum-empty-value.md create mode 100644 nixd/tools/nixd/test/diagnostic/option-enum-value.md diff --git a/nixd/include/nixd/Controller/Controller.h b/nixd/include/nixd/Controller/Controller.h index f49fac53b..f3c5c2bb4 100644 --- a/nixd/include/nixd/Controller/Controller.h +++ b/nixd/include/nixd/Controller/Controller.h @@ -230,9 +230,12 @@ class Controller : public lspserver::LSPServer { /// Determine whether or not this diagnostic is suppressed. bool isSuppressed(nixf::Diagnostic::DiagnosticKind Kind); - void publishDiagnostics(lspserver::PathRef File, - std::optional Version, std::string_view Src, - const std::vector &Diagnostics); + std::vector collectOptionDiagnostics(const NixTU &TU); + void + publishDiagnostics(lspserver::PathRef File, std::optional Version, + std::string_view Src, + const std::vector &Diagnostics, + const std::vector &NixdDiagnostics = {}); void onRename(const lspserver::RenameParams &Params, lspserver::Callback Reply); diff --git a/nixd/include/nixd/Controller/NixTU.h b/nixd/include/nixd/Controller/NixTU.h index 8a4a84a39..dfa626716 100644 --- a/nixd/include/nixd/Controller/NixTU.h +++ b/nixd/include/nixd/Controller/NixTU.h @@ -9,15 +9,32 @@ #include #include +#include #include namespace nixd { +enum class NixdDiagnosticSeverity { + Error = 1, + Warning = 2, + Information = 3, + Hint = 4, +}; + +struct NixdDiagnostic { + nixf::LexerCursorRange Range; + NixdDiagnosticSeverity Severity = NixdDiagnosticSeverity::Error; + std::string Code; + std::string Source; + std::string Message; +}; + /// \brief Holds analyzed information about a document. /// /// TU stands for "Translation Unit". class NixTU { std::vector Diagnostics; + std::vector NixdDiagnostics; std::shared_ptr AST; std::optional ASTByteCode; std::unique_ptr VLA; @@ -36,6 +53,14 @@ class NixTU { return Diagnostics; } + [[nodiscard]] const std::vector &nixdDiagnostics() const { + return NixdDiagnostics; + } + + void setNixdDiagnostics(std::vector Diagnostics) { + NixdDiagnostics = std::move(Diagnostics); + } + [[nodiscard]] const std::shared_ptr &ast() const { return AST; } [[nodiscard]] const nixf::ParentMapAnalysis *parentMap() const { diff --git a/nixd/include/nixd/Protocol/AttrSet.h b/nixd/include/nixd/Protocol/AttrSet.h index 7db9c8255..7c292e406 100644 --- a/nixd/include/nixd/Protocol/AttrSet.h +++ b/nixd/include/nixd/Protocol/AttrSet.h @@ -118,6 +118,7 @@ using AttrPathCompleteResponse = std::vector; struct OptionType { std::optional Description; + std::optional> EnumValues; std::optional Name; }; diff --git a/nixd/lib/Controller/Configuration.cpp b/nixd/lib/Controller/Configuration.cpp index 3bb28e8a9..7fac653e4 100644 --- a/nixd/lib/Controller/Configuration.cpp +++ b/nixd/lib/Controller/Configuration.cpp @@ -84,7 +84,8 @@ void Controller::updateConfig(Configuration NewConfig) { // After all, notify all AST modules the diagnostic set has been updated. std::lock_guard TUsGuard(TUsLock); for (const auto &[File, TU] : TUs) { - publishDiagnostics(File, std::nullopt, TU->src(), TU->diagnostics()); + publishDiagnostics(File, std::nullopt, TU->src(), TU->diagnostics(), + TU->nixdDiagnostics()); } } diff --git a/nixd/lib/Controller/Diagnostics.cpp b/nixd/lib/Controller/Diagnostics.cpp index 4557888c8..1e9b930f4 100644 --- a/nixd/lib/Controller/Diagnostics.cpp +++ b/nixd/lib/Controller/Diagnostics.cpp @@ -41,9 +41,10 @@ bool Controller::isSuppressed(nixf::Diagnostic::DiagnosticKind Kind) { void Controller::publishDiagnostics( PathRef File, std::optional Version, std::string_view Src, - const std::vector &Diagnostics) { + const std::vector &Diagnostics, + const std::vector &NixdDiagnostics) { std::vector LSPDiags; - LSPDiags.reserve(Diagnostics.size()); + LSPDiags.reserve(Diagnostics.size() + NixdDiagnostics.size()); for (const nixf::Diagnostic &D : Diagnostics) { // Before actually doing anything, // let's check if the diagnostic is suppressed. @@ -112,6 +113,15 @@ void Controller::publishDiagnostics( }); } } + for (const NixdDiagnostic &D : NixdDiagnostics) { + LSPDiags.emplace_back(Diagnostic{ + .range = toLSPRange(Src, D.Range), + .severity = static_cast(D.Severity), + .code = D.Code, + .source = D.Source, + .message = D.Message, + }); + } PublishDiagnostic({ .uri = URIForFile::canonicalize(File, File), .diagnostics = std::move(LSPDiags), diff --git a/nixd/lib/Controller/OptionDiagnostics.cpp b/nixd/lib/Controller/OptionDiagnostics.cpp new file mode 100644 index 000000000..981af805c --- /dev/null +++ b/nixd/lib/Controller/OptionDiagnostics.cpp @@ -0,0 +1,165 @@ +#include "AST.h" + +#include "nixd/Controller/Controller.h" +#include "nixd/Protocol/AttrSet.h" + +#include "lspserver/Logger.h" + +#include +#include +#include + +#include +#include +#include + +#include +#include +#include + +using namespace lspserver; +using namespace nixd; +using namespace nixf; + +namespace { + +constexpr llvm::StringLiteral OptionValueTypeCode = "option-value-type"; + +std::optional +resolveOptionInfo(AttrSetClient &Client, + const std::vector &Scope) { + std::binary_semaphore Ready(0); + std::optional Desc; + auto OnReply = [&Ready, &Desc](llvm::Expected Resp) { + if (Resp) + Desc = *Resp; + else + llvm::consumeError(Resp.takeError()); + Ready.release(); + }; + + Client.optionInfo(Scope, std::move(OnReply)); + Ready.acquire(); + return Desc; +} + +std::string joinOptionPath(const std::vector &Scope) { + std::ostringstream OS; + for (std::size_t I = 0; I < Scope.size(); ++I) { + if (I != 0) + OS << "."; + OS << Scope[I]; + } + return OS.str(); +} + +bool enumContains(const OptionType &Type, llvm::StringRef Value) { + assert(Type.EnumValues && "must have enum values"); + return llvm::is_contained(*Type.EnumValues, Value); +} + +std::string sourceText(llvm::StringRef Src, LexerCursorRange Range) { + return Src + .substr(Range.lCur().offset(), + Range.rCur().offset() - Range.lCur().offset()) + .str(); +} + +std::optional getLiteralString(const ExprString &String) { + if (String.parts().fragments().empty()) + return ""; + + if (!String.isLiteral()) + return std::nullopt; + + return String.literal(); +} + +std::optional +validateOptionBinding(const Binding &Binding, const ParentMapAnalysis &PM, + llvm::StringRef Src, Controller::OptionMapTy &Options) { + const auto &Value = Binding.value(); + if (!Value || Value->kind() != Node::NK_ExprString) + return std::nullopt; + + const auto &String = static_cast(*Value); + std::optional LiteralString = getLiteralString(String); + if (!LiteralString) + return std::nullopt; + + const auto &Names = Binding.path().names(); + if (Names.empty()) + return std::nullopt; + + std::vector Scope; + if (findAttrPathForOptions(*Names.back(), PM, Scope) != + FindAttrPathResult::OK || + Scope.empty()) + return std::nullopt; + + for (const auto &[Name, Provider] : Options) { + AttrSetClient *Client = Provider->client(); + if (!Client) { + elog("skipped option client {0} as it is dead", Name); + continue; + } + + std::optional Desc = resolveOptionInfo(*Client, Scope); + if (!Desc || !Desc->Type) + continue; + + const OptionType &Type = *Desc->Type; + if (Type.Name != "enum") + continue; + + if (!Type.EnumValues) + return std::nullopt; + + if (enumContains(Type, *LiteralString)) + return std::nullopt; + + std::string TypeDescription = + Type.Description.value_or(Type.Name.value_or("expected option type")); + return NixdDiagnostic{ + .Range = Value->range(), + .Severity = NixdDiagnosticSeverity::Error, + .Code = std::string(OptionValueTypeCode), + .Source = "nixd", + .Message = "definition `" + sourceText(Src, Value->range()) + + "` for option `" + joinOptionPath(Scope) + + "` is not of type `" + TypeDescription + "`.", + }; + } + + return std::nullopt; +} + +void collectOptionDiagnosticsFromNode( + const Node &N, const ParentMapAnalysis &PM, llvm::StringRef Src, + Controller::OptionMapTy &Options, + std::vector &Diagnostics) { + if (N.kind() == Node::NK_Binding) { + if (std::optional Diag = validateOptionBinding( + static_cast(N), PM, Src, Options)) + Diagnostics.emplace_back(std::move(*Diag)); + } + + for (const Node *Child : N.children()) { + if (Child) + collectOptionDiagnosticsFromNode(*Child, PM, Src, Options, Diagnostics); + } +} + +} // namespace + +std::vector +Controller::collectOptionDiagnostics(const NixTU &TU) { + std::vector Diagnostics; + if (!TU.ast() || !TU.parentMap()) + return Diagnostics; + + std::lock_guard _(OptionsLock); + collectOptionDiagnosticsFromNode(*TU.ast(), *TU.parentMap(), TU.src(), + Options, Diagnostics); + return Diagnostics; +} diff --git a/nixd/lib/Controller/Support.cpp b/nixd/lib/Controller/Support.cpp index d9c1670b2..ae7a54cb4 100644 --- a/nixd/lib/Controller/Support.cpp +++ b/nixd/lib/Controller/Support.cpp @@ -32,25 +32,27 @@ void Controller::actOnDocumentAdd(PathRef File, nixf::parse(*Draft->Contents, Diagnostics); if (!AST) { - std::lock_guard G(TUsLock); publishDiagnostics(File, Version, *Src, Diagnostics); - TUs.insert_or_assign(File, - std::make_shared(std::move(Diagnostics), - std::move(AST), std::nullopt, - /*VLA=*/nullptr, Src)); + auto TU = std::make_shared(std::move(Diagnostics), std::move(AST), + std::nullopt, /*VLA=*/nullptr, Src); + std::lock_guard G(TUsLock); + TUs.insert_or_assign(File, std::move(TU)); return; } auto VLA = std::make_unique(Diagnostics); VLA->runOnAST(*AST); - publishDiagnostics(File, Version, *Src, Diagnostics); + auto TU = std::make_shared(std::move(Diagnostics), std::move(AST), + std::nullopt, std::move(VLA), Src); + TU->setNixdDiagnostics(collectOptionDiagnostics(*TU)); + + publishDiagnostics(File, Version, *Src, TU->diagnostics(), + TU->nixdDiagnostics()); { std::lock_guard G(TUsLock); - TUs.insert_or_assign( - File, std::make_shared(std::move(Diagnostics), std::move(AST), - std::nullopt, std::move(VLA), Src)); + TUs.insert_or_assign(File, std::move(TU)); return; } }; diff --git a/nixd/lib/Eval/AttrSetProvider.cpp b/nixd/lib/Eval/AttrSetProvider.cpp index 01c0a5d19..c590da07f 100644 --- a/nixd/lib/Eval/AttrSetProvider.cpp +++ b/nixd/lib/Eval/AttrSetProvider.cpp @@ -125,9 +125,34 @@ void fillOptionDeclarations(nix::EvalState &State, nix::Value &V, } } +std::optional> +getEnumValuesFromFunctor(nix::EvalState &State, nix::Value &VType) { + try { + nix::Value &Values = + nixt::selectStringViews(State, VType, {"functor", "payload", "values"}); + State.forceValue(Values, nix::noPos); + if (Values.type() != nix::ValueType::nList) + return std::nullopt; + + std::vector EnumValues; + for (nix::Value *Item : Values.listView()) { + State.forceValue(*Item, nix::noPos); + if (Item->type() != nix::ValueType::nString) + return std::nullopt; + EnumValues.emplace_back(Item->string_view()); + } + return EnumValues; + } catch (std::exception &E) { + return std::nullopt; + } +} + void fillOptionType(nix::EvalState &State, nix::Value &VType, OptionType &R) { fillString(State, VType, {"description"}, R.Description); fillString(State, VType, {"name"}, R.Name); + + if (R.Name && *R.Name == "enum") + R.EnumValues = getEnumValuesFromFunctor(State, VType); } void fillOptionDescription(nix::EvalState &State, nix::Value &V, diff --git a/nixd/lib/Protocol/AttrSet.cpp b/nixd/lib/Protocol/AttrSet.cpp index cf3dc2e8a..d427092a6 100644 --- a/nixd/lib/Protocol/AttrSet.cpp +++ b/nixd/lib/Protocol/AttrSet.cpp @@ -4,16 +4,20 @@ using namespace nixd; using namespace llvm::json; Value nixd::toJSON(const OptionType &Params) { - return Object{ + Object Result{ {"Description", Params.Description}, {"Name", Params.Name}, }; + if (Params.EnumValues) + Result["EnumValues"] = *Params.EnumValues; + return Result; } bool nixd::fromJSON(const Value &Params, OptionType &R, Path P) { ObjectMapper O(Params, P); return O // && O.mapOptional("Description", R.Description) // + && O.mapOptional("EnumValues", R.EnumValues) // && O.mapOptional("Name", R.Name); } diff --git a/nixd/lib/meson.build b/nixd/lib/meson.build index 9edbfaee1..c496cce4b 100644 --- a/nixd/lib/meson.build +++ b/nixd/lib/meson.build @@ -35,6 +35,7 @@ libnixd_lib = library( 'Controller/InlayHints.cpp', 'Controller/LifeTime.cpp', 'Controller/NixTU.cpp', + 'Controller/OptionDiagnostics.cpp', 'Controller/Rename.cpp', 'Controller/SemanticTokens.cpp', 'Controller/Support.cpp', diff --git a/nixd/tools/nixd/test/diagnostic/option-enum-empty-value.md b/nixd/tools/nixd/test/diagnostic/option-enum-empty-value.md new file mode 100644 index 000000000..66d979e44 --- /dev/null +++ b/nixd/tools/nixd/test/diagnostic/option-enum-empty-value.md @@ -0,0 +1,38 @@ +# RUN: nixd --lit-test \ +# RUN: --nixos-options-expr='let roleType = { name = "enum"; description = "one of \"server\", \"agent\""; functor.payload.values = [ "server" "agent" ]; }; in { demo.role = { _type = "option"; type = roleType; }; }' \ +# RUN: < %s | FileCheck %s + +<-- initialize(0) + +```json +{ + "jsonrpc":"2.0", + "id":0, + "method":"initialize", + "params":{ + "processId":123, + "rootPath":"", + "capabilities":{ + }, + "trace":"off" + } +} +``` + +<-- textDocument/didOpen + +```nix file:///basic.nix +{ demo.role = ""; } +``` + +``` + CHECK: "method": "textDocument/publishDiagnostics", + CHECK: "code": "option-value-type", + CHECK: "message": "definition `\"\"` for option `demo.role` is not of type `one of \"server\", \"agent\"`.", + CHECK: "severity": 1, + CHECK: "source": "nixd" +``` + +```json +{"jsonrpc":"2.0","method":"exit"} +``` diff --git a/nixd/tools/nixd/test/diagnostic/option-enum-value.md b/nixd/tools/nixd/test/diagnostic/option-enum-value.md new file mode 100644 index 000000000..6b027f6e3 --- /dev/null +++ b/nixd/tools/nixd/test/diagnostic/option-enum-value.md @@ -0,0 +1,38 @@ +# RUN: nixd --lit-test \ +# RUN: --nixos-options-expr='let roleType = { name = "enum"; description = "one of \"server\", \"agent\""; functor.payload.values = [ "server" "agent" ]; }; in { demo.role = { _type = "option"; type = roleType; }; }' \ +# RUN: < %s | FileCheck %s + +<-- initialize(0) + +```json +{ + "jsonrpc":"2.0", + "id":0, + "method":"initialize", + "params":{ + "processId":123, + "rootPath":"", + "capabilities":{ + }, + "trace":"off" + } +} +``` + +<-- textDocument/didOpen + +```nix file:///basic.nix +{ demo.role = "wrongValue"; } +``` + +``` + CHECK: "method": "textDocument/publishDiagnostics", + CHECK: "code": "option-value-type", + CHECK: "message": "definition `\"wrongValue\"` for option `demo.role` is not of type `one of \"server\", \"agent\"`.", + CHECK: "severity": 1, + CHECK: "source": "nixd" +``` + +```json +{"jsonrpc":"2.0","method":"exit"} +```