From 88cbe54f7b4c59e2d5819d172a3bb35e71c2b9d5 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Mon, 18 Sep 2023 22:36:35 +0800
Subject: [PATCH 01/24] nixd/Sema: add basic function lowering

---
 nixd/include/nixd/Sema/EvalContext.h          |  14 ++
 nixd/include/nixd/Sema/Lowering.h             |  22 +++
 nixd/lib/Sema/Lowering.cpp                    | 130 ++++++++++++++++++
 nixd/lib/Sema/meson.build                     |   8 +-
 nixd/tools/nixd-lint/meson.build              |   1 +
 nixd/tools/nixd-lint/nixd-lint.cpp            |   7 +
 .../nixd-lint/test/duplicated-fn-arg.nix      |  10 ++
 7 files changed, 186 insertions(+), 6 deletions(-)
 create mode 100644 nixd/include/nixd/Sema/EvalContext.h
 create mode 100644 nixd/include/nixd/Sema/Lowering.h
 create mode 100644 nixd/lib/Sema/Lowering.cpp
 create mode 100644 nixd/tools/nixd-lint/test/duplicated-fn-arg.nix

diff --git a/nixd/include/nixd/Sema/EvalContext.h b/nixd/include/nixd/Sema/EvalContext.h
new file mode 100644
index 000000000..146a26320
--- /dev/null
+++ b/nixd/include/nixd/Sema/EvalContext.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "nixd/Support/GCPool.h"
+
+#include <nix/nixexpr.hh>
+
+namespace nixd {
+
+struct EvalContext {
+  GCPool<nix::Expr> Pool;
+  GCPool<nix::Formals> FormalsPool;
+};
+
+} // namespace nixd
diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
new file mode 100644
index 000000000..48adc00f0
--- /dev/null
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -0,0 +1,22 @@
+#pragma once
+
+#include "nixd/Sema/EvalContext.h"
+#include "nixd/Syntax/Diagnostic.h"
+#include "nixd/Syntax/Nodes.h"
+
+#include <nix/nixexpr.hh>
+#include <nix/symbol-table.hh>
+
+namespace nixd {
+
+struct Lowering {
+  nix::SymbolTable &STable;
+  nix::PosTable &PTable;
+  std::vector<syntax::Diagnostic> &Diags;
+
+  nix::Expr *lower(EvalContext &Ctx, syntax::Node *Root);
+  nix::ExprLambda *lowerFunction(EvalContext &Ctx, syntax::Function *Fn);
+  nix::Formal lowerFormal(EvalContext &Ctx, const syntax::Formal &Formal);
+};
+
+} // namespace nixd
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
new file mode 100644
index 000000000..ae37271bb
--- /dev/null
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -0,0 +1,130 @@
+#include "nixd/Sema/Lowering.h"
+#include "nixd/Sema/EvalContext.h"
+#include "nixd/Syntax/Diagnostic.h"
+#include "nixd/Syntax/Nodes.h"
+
+namespace nixd {
+
+nix::Formal Lowering::lowerFormal(EvalContext &Ctx,
+                                  const syntax::Formal &Formal) {
+  auto *Def = Lowering::lower(Ctx, Formal.Default);
+  nix::Formal F;
+
+  F.pos = Formal.Range.Begin;
+  F.name = Formal.ID->Symbol;
+  F.def = Def;
+  return F;
+}
+
+nix::ExprLambda *Lowering::lowerFunction(EvalContext &Ctx,
+                                         syntax::Function *Fn) {
+  // Implementation note:
+  // The official parser does this in the semantic action, and we deferred it
+  // here, as a part of the progressive lowering process.
+  // We should be consistant with the official implementation, not strictly
+  // required.
+
+  // @toFormals
+  // Link: src/libexpr/parser.y#L156 2a52ec4e928c254338a612a6b40355512298ef38
+
+  if (!Fn)
+    return nullptr;
+
+  auto *Formals = Fn->FunctionFormals;
+
+  nix::Formals *NixFormals = nullptr;
+
+  if (Formals) {
+    std::sort(Formals->Formals.begin(), Formals->Formals.end(),
+              [](syntax::Formal *A, syntax::Formal *B) {
+                auto APos = A->Range.Begin;
+                auto BPos = B->Range.Begin;
+                return std::tie(A->ID->Symbol, APos) <
+                       std::tie(B->ID->Symbol, BPos);
+              });
+
+    // Check if there is a formal duplicated to another, if so, emit a
+    // diagnostic expressively to our user
+    //
+    // We are not using upstream O(n) algorithm because we need to detect all
+    // duplicated stuff, instead of the first one.
+    std::map<nix::Symbol, const nixd::syntax::Formal *> Names;
+    for (const auto *Formal : Formals->Formals) {
+      auto Sym = Formal->ID->Symbol;
+      if (Names.contains(Sym)) {
+        // We have seen this symbol before, so this formal is duplicated
+        const syntax::Formal *Previous = Names[Sym];
+        syntax::Diagnostic Diag;
+        Diag.Kind = syntax::Diagnostic::Error;
+        Diag.Msg = "duplicated function formal declaration";
+        Diag.Range = Formal->Range;
+
+        syntax::Note PrevDef;
+        PrevDef.Msg = "previously declared here";
+        PrevDef.Range = Previous->Range;
+        Diag.Notes.emplace_back(std::move(PrevDef));
+
+        Diags.emplace_back(std::move(Diag));
+      } else {
+        Names[Sym] = Formal;
+      }
+    }
+
+    // Check if the function argument is duplicated with the formal
+    if (Fn->Arg) {
+      auto ArgSym = Fn->Arg->Symbol;
+      if (Names.contains(ArgSym)) {
+        const syntax::Formal *Previous = Names[ArgSym];
+        syntax::Diagnostic Diag;
+        Diag.Kind = syntax::Diagnostic::Error;
+        Diag.Msg = "function argument duplicated to a function formal";
+        Diag.Range = Fn->Arg->Range;
+
+        syntax::Note PrevDef;
+        PrevDef.Msg = "duplicated to this formal";
+        PrevDef.Range = Previous->Range;
+        Diag.Notes.emplace_back(std::move(PrevDef));
+
+        Diags.emplace_back(std::move(Diag));
+      }
+    }
+    // Construct nix::Formals, from ours
+    auto *NixFormals = Ctx.FormalsPool.record(new nix::Formals);
+    NixFormals->ellipsis = Formals->Ellipsis;
+    for (auto [_, Formal] : Names) {
+      nix::Formal F = lowerFormal(Ctx, *Formal);
+      NixFormals->formals.emplace_back(F);
+    }
+  }
+
+  auto *Body = lower(Ctx, Fn->Body);
+
+  nix::ExprLambda *NewLambda;
+  if (Fn->Arg) {
+    NewLambda =
+        new nix::ExprLambda(Fn->Range.Begin, Fn->Arg->Symbol, NixFormals, Body);
+  } else {
+    NewLambda = new nix::ExprLambda(Fn->Range.Begin, NixFormals, Body);
+  }
+  Ctx.Pool.record(NewLambda);
+
+  return NewLambda;
+}
+
+nix::Expr *Lowering::lower(EvalContext &Ctx, nixd::syntax::Node *Root) {
+  if (!Root)
+    return nullptr;
+
+  using syntax::Node;
+
+  switch (Root->getKind()) {
+  case Node::NK_Function: {
+    auto *Fn = dynamic_cast<syntax::Function *>(Root);
+    return lowerFunction(Ctx, Fn);
+  }
+  }
+
+  return nullptr;
+}
+
+} // namespace nixd
diff --git a/nixd/lib/Sema/meson.build b/nixd/lib/Sema/meson.build
index 2313d3d56..c50b309d1 100644
--- a/nixd/lib/Sema/meson.build
+++ b/nixd/lib/Sema/meson.build
@@ -1,11 +1,7 @@
-libnixdSemaDeps = [ nixd_lsp_server
-                  , nix_all
-                  , nixdAST
-                  , llvm
-                  ]
+libnixdSemaDeps = [ nixdSyntax ]
 
 libnixdSema = library('nixdSema'
-, 'CompletionBuilder.cpp'
+, 'Lowering.cpp'
 , include_directories: nixd_inc
 , dependencies: libnixdSemaDeps
 , install: true
diff --git a/nixd/tools/nixd-lint/meson.build b/nixd/tools/nixd-lint/meson.build
index 7484227f5..8dd9795e4 100644
--- a/nixd/tools/nixd-lint/meson.build
+++ b/nixd/tools/nixd-lint/meson.build
@@ -2,6 +2,7 @@ nixd_lint = executable('nixd-lint'
 , 'nixd-lint.cpp'
 , dependencies: [ boost
                 , nixdSyntax
+                , nixdSema
                 ] + nix_all
 , install: true
 )
diff --git a/nixd/tools/nixd-lint/nixd-lint.cpp b/nixd/tools/nixd-lint/nixd-lint.cpp
index 392c2b598..fdca124bc 100644
--- a/nixd/tools/nixd-lint/nixd-lint.cpp
+++ b/nixd/tools/nixd-lint/nixd-lint.cpp
@@ -1,3 +1,5 @@
+#include "nixd/Sema/EvalContext.h"
+#include "nixd/Sema/Lowering.h"
 #include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Parser.h"
 #include "nixd/Syntax/Parser/Require.h"
@@ -97,6 +99,11 @@ int main(int argc, char *argv[]) {
   nixd::syntax::ParseData Data{.State = S, .Origin = Origin};
   nixd::syntax::parse(Buffer, &Data);
 
+  nixd::Lowering Lowering{
+      .STable = *STable, .PTable = *PTable, .Diags = Data.Diags};
+  nixd::EvalContext Ctx;
+  Lowering.lower(Ctx, Data.Result);
+
   for (const auto &Diag : Data.Diags) {
     auto BeginPos = (*PTable)[Diag.Range.Begin];
     auto EndPos = (*PTable)[Diag.Range.End];
diff --git a/nixd/tools/nixd-lint/test/duplicated-fn-arg.nix b/nixd/tools/nixd-lint/test/duplicated-fn-arg.nix
new file mode 100644
index 000000000..cdfb592e5
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/duplicated-fn-arg.nix
@@ -0,0 +1,10 @@
+# RUN: nixd-lint %s | FileCheck %s
+
+# CHECK: duplicated function formal declaration
+a @ { b
+# CHECK: previously declared here
+, a ? 1
+# CHECK: function argument duplicated to a function forma
+, a
+# CHECK: duplicated to this formal
+}: { }

From e51d1b2ba627581cd08e3e43f869cd020459adcf Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Mon, 18 Sep 2023 23:59:41 +0800
Subject: [PATCH 02/24] nixd/Sema: lowering for ExprAssert & ExprWith

---
 nixd/lib/Sema/Lowering.cpp                    | 16 ++++++++++++++++
 nixd/tools/nixd-lint/test/lowering-assert.nix |  4 ++++
 nixd/tools/nixd-lint/test/lowering-with.nix   |  4 ++++
 3 files changed, 24 insertions(+)
 create mode 100644 nixd/tools/nixd-lint/test/lowering-assert.nix
 create mode 100644 nixd/tools/nixd-lint/test/lowering-with.nix

diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index ae37271bb..6db030d68 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -122,6 +122,22 @@ nix::Expr *Lowering::lower(EvalContext &Ctx, nixd::syntax::Node *Root) {
     auto *Fn = dynamic_cast<syntax::Function *>(Root);
     return lowerFunction(Ctx, Fn);
   }
+  case Node::NK_Assert: {
+    auto *Assert = dynamic_cast<syntax::Assert *>(Root);
+    auto *Cond = lower(Ctx, Assert->Cond);
+    auto *Body = lower(Ctx, Assert->Body);
+    auto *NixAssert =
+        Ctx.Pool.record(new nix::ExprAssert(Assert->Range.Begin, Cond, Body));
+    return NixAssert;
+  }
+  case Node::NK_With: {
+    auto *With = dynamic_cast<syntax::With *>(Root);
+    auto *Attrs = lower(Ctx, With->Attrs);
+    auto *Body = lower(Ctx, With->Body);
+    auto *NixWith =
+        Ctx.Pool.record(new nix::ExprWith(With->Range.Begin, Attrs, Body));
+    return NixWith;
+  }
   }
 
   return nullptr;
diff --git a/nixd/tools/nixd-lint/test/lowering-assert.nix b/nixd/tools/nixd-lint/test/lowering-assert.nix
new file mode 100644
index 000000000..5ea4d7a73
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-assert.nix
@@ -0,0 +1,4 @@
+# RUN: nixd-lint %s | FileCheck %s
+
+# CHECK: function argument duplicated to a function formal
+assert 0; a @ { a }: 1
diff --git a/nixd/tools/nixd-lint/test/lowering-with.nix b/nixd/tools/nixd-lint/test/lowering-with.nix
new file mode 100644
index 000000000..94418a533
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-with.nix
@@ -0,0 +1,4 @@
+# RUN: nixd-lint %s | FileCheck %s
+
+# CHECK: function argument duplicated to a function formal
+with 1; { a } @ a : 1

From c18ec1e19e20ee0acbb4767df14c2678c90c3328 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Tue, 19 Sep 2023 14:11:49 +0800
Subject: [PATCH 03/24] nixd/Sema: move lowering ctx into the Lowering class

---
 nixd/include/nixd/Sema/Lowering.h  |  7 ++++---
 nixd/lib/Sema/Lowering.cpp         | 24 +++++++++++-------------
 nixd/tools/nixd-lint/nixd-lint.cpp |  6 +++---
 3 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index 48adc00f0..312c6f976 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -13,10 +13,11 @@ struct Lowering {
   nix::SymbolTable &STable;
   nix::PosTable &PTable;
   std::vector<syntax::Diagnostic> &Diags;
+  EvalContext &Ctx;
 
-  nix::Expr *lower(EvalContext &Ctx, syntax::Node *Root);
-  nix::ExprLambda *lowerFunction(EvalContext &Ctx, syntax::Function *Fn);
-  nix::Formal lowerFormal(EvalContext &Ctx, const syntax::Formal &Formal);
+  nix::Expr *lower(syntax::Node *Root);
+  nix::ExprLambda *lowerFunction(syntax::Function *Fn);
+  nix::Formal lowerFormal(const syntax::Formal &Formal);
 };
 
 } // namespace nixd
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 6db030d68..39c07c9e5 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -5,9 +5,8 @@
 
 namespace nixd {
 
-nix::Formal Lowering::lowerFormal(EvalContext &Ctx,
-                                  const syntax::Formal &Formal) {
-  auto *Def = Lowering::lower(Ctx, Formal.Default);
+nix::Formal Lowering::lowerFormal(const syntax::Formal &Formal) {
+  auto *Def = Lowering::lower(Formal.Default);
   nix::Formal F;
 
   F.pos = Formal.Range.Begin;
@@ -16,8 +15,7 @@ nix::Formal Lowering::lowerFormal(EvalContext &Ctx,
   return F;
 }
 
-nix::ExprLambda *Lowering::lowerFunction(EvalContext &Ctx,
-                                         syntax::Function *Fn) {
+nix::ExprLambda *Lowering::lowerFunction(syntax::Function *Fn) {
   // Implementation note:
   // The official parser does this in the semantic action, and we deferred it
   // here, as a part of the progressive lowering process.
@@ -92,12 +90,12 @@ nix::ExprLambda *Lowering::lowerFunction(EvalContext &Ctx,
     auto *NixFormals = Ctx.FormalsPool.record(new nix::Formals);
     NixFormals->ellipsis = Formals->Ellipsis;
     for (auto [_, Formal] : Names) {
-      nix::Formal F = lowerFormal(Ctx, *Formal);
+      nix::Formal F = lowerFormal(*Formal);
       NixFormals->formals.emplace_back(F);
     }
   }
 
-  auto *Body = lower(Ctx, Fn->Body);
+  auto *Body = lower(Fn->Body);
 
   nix::ExprLambda *NewLambda;
   if (Fn->Arg) {
@@ -111,7 +109,7 @@ nix::ExprLambda *Lowering::lowerFunction(EvalContext &Ctx,
   return NewLambda;
 }
 
-nix::Expr *Lowering::lower(EvalContext &Ctx, nixd::syntax::Node *Root) {
+nix::Expr *Lowering::lower(nixd::syntax::Node *Root) {
   if (!Root)
     return nullptr;
 
@@ -120,20 +118,20 @@ nix::Expr *Lowering::lower(EvalContext &Ctx, nixd::syntax::Node *Root) {
   switch (Root->getKind()) {
   case Node::NK_Function: {
     auto *Fn = dynamic_cast<syntax::Function *>(Root);
-    return lowerFunction(Ctx, Fn);
+    return lowerFunction(Fn);
   }
   case Node::NK_Assert: {
     auto *Assert = dynamic_cast<syntax::Assert *>(Root);
-    auto *Cond = lower(Ctx, Assert->Cond);
-    auto *Body = lower(Ctx, Assert->Body);
+    auto *Cond = lower(Assert->Cond);
+    auto *Body = lower(Assert->Body);
     auto *NixAssert =
         Ctx.Pool.record(new nix::ExprAssert(Assert->Range.Begin, Cond, Body));
     return NixAssert;
   }
   case Node::NK_With: {
     auto *With = dynamic_cast<syntax::With *>(Root);
-    auto *Attrs = lower(Ctx, With->Attrs);
-    auto *Body = lower(Ctx, With->Body);
+    auto *Attrs = lower(With->Attrs);
+    auto *Body = lower(With->Body);
     auto *NixWith =
         Ctx.Pool.record(new nix::ExprWith(With->Range.Begin, Attrs, Body));
     return NixWith;
diff --git a/nixd/tools/nixd-lint/nixd-lint.cpp b/nixd/tools/nixd-lint/nixd-lint.cpp
index fdca124bc..b27eeb4f3 100644
--- a/nixd/tools/nixd-lint/nixd-lint.cpp
+++ b/nixd/tools/nixd-lint/nixd-lint.cpp
@@ -99,10 +99,10 @@ int main(int argc, char *argv[]) {
   nixd::syntax::ParseData Data{.State = S, .Origin = Origin};
   nixd::syntax::parse(Buffer, &Data);
 
-  nixd::Lowering Lowering{
-      .STable = *STable, .PTable = *PTable, .Diags = Data.Diags};
   nixd::EvalContext Ctx;
-  Lowering.lower(Ctx, Data.Result);
+  nixd::Lowering Lowering{
+      .STable = *STable, .PTable = *PTable, .Diags = Data.Diags, .Ctx = Ctx};
+  Lowering.lower(Data.Result);
 
   for (const auto &Diag : Data.Diags) {
     auto BeginPos = (*PTable)[Diag.Range.Begin];

From 910f40f27bffbbea413235257c7cdfaeb5bc6d59 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Wed, 20 Sep 2023 09:58:29 +0800
Subject: [PATCH 04/24] nixd/Sema: lowering for inherited attrs in binds

---
 nixd/include/nixd/Sema/Lowering.h             |  23 ++++
 nixd/lib/Sema/Lowering.cpp                    | 122 +++++++++++++++++-
 .../tools/nixd-lint/test/bind-inherit-dup.nix |   9 ++
 .../nixd-lint/test/bind-inherit-dynamic.nix   |   6 +
 .../nixd-lint/test/bind-inherit-empty.nix     |   6 +
 nixd/tools/nixd-lint/test/bind-inherit.nix    |  10 ++
 6 files changed, 174 insertions(+), 2 deletions(-)
 create mode 100644 nixd/tools/nixd-lint/test/bind-inherit-dup.nix
 create mode 100644 nixd/tools/nixd-lint/test/bind-inherit-dynamic.nix
 create mode 100644 nixd/tools/nixd-lint/test/bind-inherit-empty.nix
 create mode 100644 nixd/tools/nixd-lint/test/bind-inherit.nix

diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index 312c6f976..906bbf356 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -9,6 +9,27 @@
 
 namespace nixd {
 
+struct Lowering;
+
+class ExprAttrsBuilder {
+  // The official parser does this work during the lowering process, via
+  // @addAttr in src/libexpr/parser.y
+
+  Lowering &LW;
+  nix::ExprAttrs *Result;
+
+  // Use a map to detect duplicated fields
+  // Not using the map inside nix::ExprAttrs because we want to preserve the
+  // range
+  std::map<nix::Symbol, syntax::Node *> Fields;
+
+public:
+  ExprAttrsBuilder(Lowering &LW);
+  void addAttribute(const syntax::Attribute &A);
+  void addInherited(const syntax::InheritedAttribute &IA);
+  nix::ExprAttrs *finish();
+};
+
 struct Lowering {
   nix::SymbolTable &STable;
   nix::PosTable &PTable;
@@ -18,6 +39,8 @@ struct Lowering {
   nix::Expr *lower(syntax::Node *Root);
   nix::ExprLambda *lowerFunction(syntax::Function *Fn);
   nix::Formal lowerFormal(const syntax::Formal &Formal);
+  nix::AttrPath lowerAttrPath(const syntax::AttrPath &Path);
+  nix::ExprAttrs *lowerBinds(const syntax::Binds &Binds);
 };
 
 } // namespace nixd
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 39c07c9e5..661b89756 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -3,8 +3,14 @@
 #include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Nodes.h"
 
+#include <nix/nixexpr.hh>
+
+#include <llvm/Support/FormatVariadic.h>
+
 namespace nixd {
 
+using syntax::Node;
+
 nix::Formal Lowering::lowerFormal(const syntax::Formal &Formal) {
   auto *Def = Lowering::lower(Formal.Default);
   nix::Formal F;
@@ -109,12 +115,113 @@ nix::ExprLambda *Lowering::lowerFunction(syntax::Function *Fn) {
   return NewLambda;
 }
 
+static syntax::Diagnostic mkAttrDupDiag(std::string Name, RangeIdx Range,
+                                        RangeIdx Prev) {
+  syntax::Diagnostic Diag;
+  Diag.Kind = syntax::Diagnostic::Error;
+  Diag.Msg = llvm::formatv("duplicated attr `{0}`", Name);
+  Diag.Range = Range;
+
+  syntax::Note Note;
+  Note.Msg = llvm::formatv("previously defined here");
+  Note.Range = Prev;
+  Diag.Notes.emplace_back(std::move(Note));
+  return Diag;
+}
+
+ExprAttrsBuilder::ExprAttrsBuilder(Lowering &LW) : LW(LW) {
+  Result = LW.Ctx.Pool.record(new nix::ExprAttrs);
+}
+
+nix::ExprAttrs *ExprAttrsBuilder::finish() { return Result; }
+
+void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {}
+
+void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) {
+  if (IA.InheritedAttrs->Names.empty()) {
+    syntax::Diagnostic Diag;
+    Diag.Kind = syntax::Diagnostic::Warning;
+    Diag.Msg = "empty inherit expression";
+    Diag.Range = IA.Range;
+
+    LW.Diags.emplace_back(std::move(Diag));
+  }
+
+  // inherit (expr) attr1 attr2
+  // lowering `(expr)` to nix expression
+  nix::Expr *Subject = LW.lower(IA.E);
+
+  for (Node *Name : IA.InheritedAttrs->Names) {
+
+    // Extract nix::Symbol, report error if we encountered dynamic fields
+    nix::Symbol Sym;
+    switch (Name->getKind()) {
+    case Node::NK_Identifier: {
+      Sym = dynamic_cast<syntax::Identifier *>(Name)->Symbol;
+      break;
+    }
+    case Node::NK_String: {
+      Sym = LW.STable.create(dynamic_cast<syntax::String *>(Name)->S);
+      break;
+    }
+    default: {
+      syntax::Diagnostic Diag;
+      Diag.Kind = syntax::Diagnostic::Error;
+      Diag.Msg = "dynamic attributes are not allowed in inherit";
+      Diag.Range = Name->Range;
+
+      LW.Diags.emplace_back(std::move(Diag));
+    }
+    }
+
+    // Check if it is duplicated
+    if (Fields.contains(Sym)) {
+      std::string SymStr = LW.STable[Sym];
+      auto Diag = mkAttrDupDiag(SymStr, Name->Range, Fields[Sym]->Range);
+      LW.Diags.emplace_back(std::move(Diag));
+      continue;
+    }
+
+    // Insert an AttrDef to the result
+    Fields.insert({Sym, Name});
+    nix::Expr *AttrBody;
+    if (Subject) {
+      // inherit (expr) attr1 attr2
+      AttrBody = new nix::ExprSelect(Name->Range.Begin, Subject, Sym);
+    } else {
+      // inherit attr1 attr2 ...
+      AttrBody = new nix::ExprVar(Name->Range.Begin, Sym);
+    }
+    LW.Ctx.Pool.record(AttrBody);
+    nix::ExprAttrs::AttrDef Def(AttrBody, Name->Range.Begin, true);
+    Result->attrs.insert({Sym, Def});
+  }
+}
+
+nix::ExprAttrs *Lowering::lowerBinds(const syntax::Binds &Binds) {
+  ExprAttrsBuilder Builder(*this);
+
+  for (auto *Attr : Binds.Attributes) {
+    switch (Attr->getKind()) {
+    case Node::NK_Attribute: {
+      Builder.addAttribute(*dynamic_cast<syntax::Attribute *>(Attr));
+      break;
+    }
+    case Node::NK_InheritedAttribute: {
+      Builder.addInherited(*dynamic_cast<syntax::InheritedAttribute *>(Attr));
+      break;
+    }
+    default:
+      llvm_unreachable("must be A/IA! incorrect parsing rules?");
+    }
+  }
+  return Builder.finish();
+}
+
 nix::Expr *Lowering::lower(nixd::syntax::Node *Root) {
   if (!Root)
     return nullptr;
 
-  using syntax::Node;
-
   switch (Root->getKind()) {
   case Node::NK_Function: {
     auto *Fn = dynamic_cast<syntax::Function *>(Root);
@@ -136,6 +243,17 @@ nix::Expr *Lowering::lower(nixd::syntax::Node *Root) {
         Ctx.Pool.record(new nix::ExprWith(With->Range.Begin, Attrs, Body));
     return NixWith;
   }
+  case Node::NK_Binds: {
+    auto *Binds = dynamic_cast<syntax::Binds *>(Root);
+    return lowerBinds(*Binds);
+  }
+  case Node::NK_AttrSet: {
+    auto *AttrSet = dynamic_cast<syntax::AttrSet *>(Root);
+    assert(AttrSet->AttrBinds && "null AttrBinds of the AttrSet!");
+    nix::ExprAttrs *Binds = lowerBinds(*AttrSet->AttrBinds);
+    Binds->recursive = AttrSet->Recursive;
+    return Binds;
+  }
   }
 
   return nullptr;
diff --git a/nixd/tools/nixd-lint/test/bind-inherit-dup.nix b/nixd/tools/nixd-lint/test/bind-inherit-dup.nix
new file mode 100644
index 000000000..82040c0ec
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/bind-inherit-dup.nix
@@ -0,0 +1,9 @@
+# RUN: nixd-lint %s | FileCheck %s
+
+{
+  inherit aaa bbb;
+
+  # CHECK: duplicated attr `aaa`
+  # CHECK: previously defined here
+  inherit "aaa";
+}
diff --git a/nixd/tools/nixd-lint/test/bind-inherit-dynamic.nix b/nixd/tools/nixd-lint/test/bind-inherit-dynamic.nix
new file mode 100644
index 000000000..13afae17d
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/bind-inherit-dynamic.nix
@@ -0,0 +1,6 @@
+# RUN: nixd-lint %s | FileCheck %s
+
+{
+  # CHECK: error: dynamic attributes are not allowed in inherit
+  inherit "foo${a}bar";
+}
diff --git a/nixd/tools/nixd-lint/test/bind-inherit-empty.nix b/nixd/tools/nixd-lint/test/bind-inherit-empty.nix
new file mode 100644
index 000000000..c0ed3f850
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/bind-inherit-empty.nix
@@ -0,0 +1,6 @@
+# RUN: nixd-lint %s | FileCheck %s
+
+{
+  # CHECK: empty inherit expression
+  inherit;
+}
diff --git a/nixd/tools/nixd-lint/test/bind-inherit.nix b/nixd/tools/nixd-lint/test/bind-inherit.nix
new file mode 100644
index 000000000..b1d900116
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/bind-inherit.nix
@@ -0,0 +1,10 @@
+# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s
+
+# CHECK: ExprAttrs: { inherit a ; inherit b ; inherit bar ; inherit c ; inherit foo ; }
+{
+  # CHECK: ExprVar: a
+  inherit a b c bar;
+
+  # CHECK: ExprVar: foo
+  inherit "foo";
+}

From d60f2eba19d405a7f2cad5538a7bcc3afdad5be7 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Tue, 19 Sep 2023 23:00:24 +0800
Subject: [PATCH 05/24] nixd-lint: add an option that prints nix AST

---
 nixd/tools/nixd-lint/meson.build   |  1 +
 nixd/tools/nixd-lint/nixd-lint.cpp | 37 +++++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/nixd/tools/nixd-lint/meson.build b/nixd/tools/nixd-lint/meson.build
index 8dd9795e4..8618ec6af 100644
--- a/nixd/tools/nixd-lint/meson.build
+++ b/nixd/tools/nixd-lint/meson.build
@@ -3,6 +3,7 @@ nixd_lint = executable('nixd-lint'
 , dependencies: [ boost
                 , nixdSyntax
                 , nixdSema
+                , nixdExpr
                 ] + nix_all
 , install: true
 )
diff --git a/nixd/tools/nixd-lint/nixd-lint.cpp b/nixd/tools/nixd-lint/nixd-lint.cpp
index b27eeb4f3..2deedd5d8 100644
--- a/nixd/tools/nixd-lint/nixd-lint.cpp
+++ b/nixd/tools/nixd-lint/nixd-lint.cpp
@@ -1,3 +1,4 @@
+#include "nixd/Expr/Expr.h"
 #include "nixd/Sema/EvalContext.h"
 #include "nixd/Sema/Lowering.h"
 #include "nixd/Syntax/Diagnostic.h"
@@ -25,6 +26,9 @@ opt<std::string> Filename(Positional, desc("<input file>"), init("-"),
 
 const OptionCategory *Cat[] = {&Misc};
 
+opt<bool> DumpNixAST("dump-nix-ast", init(false),
+                     desc("Dump the lowered nix AST"), cat(Misc));
+
 static void printCodeLines(std::ostream &Out, const std::string &Prefix,
                            std::shared_ptr<nix::AbstractPos> BeginPos,
                            std::shared_ptr<nix::AbstractPos> EndPos,
@@ -74,6 +78,32 @@ static void printCodeLines(std::ostream &Out, const std::string &Prefix,
   }
 }
 
+struct ASTDump : nixd::RecursiveASTVisitor<ASTDump> {
+  nix::SymbolTable &STable;
+  int Depth = 0;
+
+  bool traverseExpr(const nix::Expr *E) {
+    Depth++;
+    if (!nixd::RecursiveASTVisitor<ASTDump>::traverseExpr(E))
+      return false;
+    Depth--;
+    return true;
+  }
+
+  bool visitExpr(const nix::Expr *E) const {
+    if (!E)
+      return true;
+    for (int I = 0; I < Depth; I++) {
+      std::cout << " ";
+    }
+    std::cout << nixd::getExprName(E) << ": ";
+    E->show(STable, std::cout);
+    std::cout << " ";
+    std::cout << "\n";
+    return true;
+  }
+};
+
 int main(int argc, char *argv[]) {
   HideUnrelatedOptions(Cat);
   ParseCommandLineOptions(argc, argv, "nixd linter", nullptr,
@@ -102,7 +132,12 @@ int main(int argc, char *argv[]) {
   nixd::EvalContext Ctx;
   nixd::Lowering Lowering{
       .STable = *STable, .PTable = *PTable, .Diags = Data.Diags, .Ctx = Ctx};
-  Lowering.lower(Data.Result);
+  nix::Expr *NixTree = Lowering.lower(Data.Result);
+
+  if (DumpNixAST) {
+    ASTDump D{.STable = *STable};
+    D.traverseExpr(NixTree);
+  }
 
   for (const auto &Diag : Data.Diags) {
     auto BeginPos = (*PTable)[Diag.Range.Begin];

From 39a7c253460d69054314a909ca55fbabc0830d9a Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Thu, 21 Sep 2023 01:04:20 +0800
Subject: [PATCH 06/24] nixd/Sema: const-correctness for the Lowering class

---
 nixd/include/nixd/Sema/Lowering.h |  4 ++--
 nixd/lib/Sema/Lowering.cpp        | 14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index 906bbf356..9cdc367d3 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -36,8 +36,8 @@ struct Lowering {
   std::vector<syntax::Diagnostic> &Diags;
   EvalContext &Ctx;
 
-  nix::Expr *lower(syntax::Node *Root);
-  nix::ExprLambda *lowerFunction(syntax::Function *Fn);
+  nix::Expr *lower(const syntax::Node *Root);
+  nix::ExprLambda *lowerFunction(const syntax::Function *Fn);
   nix::Formal lowerFormal(const syntax::Formal &Formal);
   nix::AttrPath lowerAttrPath(const syntax::AttrPath &Path);
   nix::ExprAttrs *lowerBinds(const syntax::Binds &Binds);
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 661b89756..9dc197dc6 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -21,7 +21,7 @@ nix::Formal Lowering::lowerFormal(const syntax::Formal &Formal) {
   return F;
 }
 
-nix::ExprLambda *Lowering::lowerFunction(syntax::Function *Fn) {
+nix::ExprLambda *Lowering::lowerFunction(const syntax::Function *Fn) {
   // Implementation note:
   // The official parser does this in the semantic action, and we deferred it
   // here, as a part of the progressive lowering process.
@@ -218,17 +218,17 @@ nix::ExprAttrs *Lowering::lowerBinds(const syntax::Binds &Binds) {
   return Builder.finish();
 }
 
-nix::Expr *Lowering::lower(nixd::syntax::Node *Root) {
+nix::Expr *Lowering::lower(const nixd::syntax::Node *Root) {
   if (!Root)
     return nullptr;
 
   switch (Root->getKind()) {
   case Node::NK_Function: {
-    auto *Fn = dynamic_cast<syntax::Function *>(Root);
+    const auto *Fn = dynamic_cast<const syntax::Function *>(Root);
     return lowerFunction(Fn);
   }
   case Node::NK_Assert: {
-    auto *Assert = dynamic_cast<syntax::Assert *>(Root);
+    const auto *Assert = dynamic_cast<const syntax::Assert *>(Root);
     auto *Cond = lower(Assert->Cond);
     auto *Body = lower(Assert->Body);
     auto *NixAssert =
@@ -236,7 +236,7 @@ nix::Expr *Lowering::lower(nixd::syntax::Node *Root) {
     return NixAssert;
   }
   case Node::NK_With: {
-    auto *With = dynamic_cast<syntax::With *>(Root);
+    const auto *With = dynamic_cast<const syntax::With *>(Root);
     auto *Attrs = lower(With->Attrs);
     auto *Body = lower(With->Body);
     auto *NixWith =
@@ -244,11 +244,11 @@ nix::Expr *Lowering::lower(nixd::syntax::Node *Root) {
     return NixWith;
   }
   case Node::NK_Binds: {
-    auto *Binds = dynamic_cast<syntax::Binds *>(Root);
+    const auto *Binds = dynamic_cast<const syntax::Binds *>(Root);
     return lowerBinds(*Binds);
   }
   case Node::NK_AttrSet: {
-    auto *AttrSet = dynamic_cast<syntax::AttrSet *>(Root);
+    const auto *AttrSet = dynamic_cast<const syntax::AttrSet *>(Root);
     assert(AttrSet->AttrBinds && "null AttrBinds of the AttrSet!");
     nix::ExprAttrs *Binds = lowerBinds(*AttrSet->AttrBinds);
     Binds->recursive = AttrSet->Recursive;

From 7914c600e42cf51c23060a0b6ebbb39d870a2888 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Thu, 21 Sep 2023 01:12:49 +0800
Subject: [PATCH 07/24] nixd/Sema: add lowering support for attrs

---
 nixd/include/nixd/Sema/Lowering.h             |  19 +-
 nixd/lib/Sema/Lowering.cpp                    | 207 +++++++++++++++---
 .../test/lowering-attrset-dynamic.nix         |   7 +
 .../test/lowering-attrset-merge-inherit.nix   |   7 +
 .../test/lowering-attrset-merging-2.nix       |  10 +
 .../nixd-lint/test/lowering-attrset-rec-2.nix |  14 ++
 .../nixd-lint/test/lowering-attrset-rec-3.nix |  15 ++
 .../nixd-lint/test/lowering-attrset-rec.nix   |  13 ++
 .../test/lowering-attrset-select-merging.nix  |  12 +
 9 files changed, 270 insertions(+), 34 deletions(-)
 create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-dynamic.nix
 create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-merge-inherit.nix
 create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-merging-2.nix
 create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-rec-2.nix
 create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-rec-3.nix
 create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-rec.nix
 create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-select-merging.nix

diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index 9cdc367d3..f6deeb349 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -18,14 +18,28 @@ class ExprAttrsBuilder {
   Lowering &LW;
   nix::ExprAttrs *Result;
 
+  bool Recursive;
+
+  RangeIdx Range;
+
+  /// Nested attributes, we create a new builder for them, and collapse the map
+  /// while finishing
+  std::map<nix::Symbol, std::unique_ptr<ExprAttrsBuilder>> Nested;
+
+  std::map<syntax::Node *, std::unique_ptr<ExprAttrsBuilder>> DynamicNested;
+
   // Use a map to detect duplicated fields
   // Not using the map inside nix::ExprAttrs because we want to preserve the
   // range
-  std::map<nix::Symbol, syntax::Node *> Fields;
+  std::map<nix::Symbol, const syntax::Node *> Fields;
 
 public:
-  ExprAttrsBuilder(Lowering &LW);
+  ExprAttrsBuilder(Lowering &LW, bool Recursive, RangeIdx Range);
+  void addAttr(const syntax::Node *Attr, const syntax::Node *Body,
+               bool Recursive);
   void addAttribute(const syntax::Attribute &A);
+  void addBinds(const syntax::Binds &Binds);
+  void addAttrSet(const syntax::AttrSet &AS);
   void addInherited(const syntax::InheritedAttribute &IA);
   nix::ExprAttrs *finish();
 };
@@ -40,7 +54,6 @@ struct Lowering {
   nix::ExprLambda *lowerFunction(const syntax::Function *Fn);
   nix::Formal lowerFormal(const syntax::Formal &Formal);
   nix::AttrPath lowerAttrPath(const syntax::AttrPath &Path);
-  nix::ExprAttrs *lowerBinds(const syntax::Binds &Binds);
 };
 
 } // namespace nixd
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 9dc197dc6..d4bf9fa84 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -2,14 +2,19 @@
 #include "nixd/Sema/EvalContext.h"
 #include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Nodes.h"
+#include "nixd/Syntax/Range.h"
 
 #include <nix/nixexpr.hh>
 
 #include <llvm/Support/FormatVariadic.h>
 
+#include <memory>
+
 namespace nixd {
 
+using syntax::Diagnostic;
 using syntax::Node;
+using syntax::Note;
 
 nix::Formal Lowering::lowerFormal(const syntax::Formal &Formal) {
   auto *Def = Lowering::lower(Formal.Default);
@@ -129,13 +134,171 @@ static syntax::Diagnostic mkAttrDupDiag(std::string Name, RangeIdx Range,
   return Diag;
 }
 
-ExprAttrsBuilder::ExprAttrsBuilder(Lowering &LW) : LW(LW) {
+ExprAttrsBuilder::ExprAttrsBuilder(Lowering &LW, bool Recursive, RangeIdx Range)
+    : LW(LW), Recursive(Recursive), Range(Range) {
   Result = LW.Ctx.Pool.record(new nix::ExprAttrs);
 }
 
-nix::ExprAttrs *ExprAttrsBuilder::finish() { return Result; }
+nix::ExprAttrs *ExprAttrsBuilder::finish() {
+  Result->recursive = Recursive;
+  for (const auto &[Sym, Builder] : Nested) {
+    auto *NestedAttr = Builder->finish();
+    nix::ExprAttrs::AttrDef Def(NestedAttr, NestedAttr->pos, false);
+    Result->attrs.insert({Sym, Def});
+  }
+  for (const auto &[Node, Builder] : DynamicNested) {
+    nix::Expr *Name = LW.lower(Node);
+    nix::ExprAttrs *Value = Builder->finish();
+    nix::ExprAttrs::DynamicAttrDef DDef(Name, Value, Value->pos);
+    Result->dynamicAttrs.emplace_back(DDef);
+  }
+  return Result;
+}
+
+void ExprAttrsBuilder::addBinds(const syntax::Binds &Binds) {
+  for (Node *Attr : Binds.Attributes) {
+    switch (Attr->getKind()) {
+    case Node::NK_Attribute:
+      addAttribute(*dynamic_cast<syntax::Attribute *>(Attr));
+      break;
+    case Node::NK_InheritedAttribute:
+      addInherited(*dynamic_cast<syntax::InheritedAttribute *>(Attr));
+      break;
+    default:
+      llvm_unreachable("attr in binds must be either A or IA!");
+    }
+  }
+}
+
+static Diagnostic mkRecDiag(RangeIdx ThisRange, RangeIdx MergingRange,
+                            bool ThisRec, bool MergningRec) {
+  assert(ThisRec != MergningRec);
+
+  Diagnostic Diag;
+  Diag.Kind = Diagnostic::Warning;
+  Diag.Msg = "merging two attributes with different `rec` modifiers, the "
+             "latter will be implicitly ignored";
+  Diag.Range = MergingRange;
+
+  Note This;
+  This.Range = ThisRange;
+  This.Msg = llvm::formatv("this attribute set is{0} recursive",
+                           ThisRec ? "" : " not");
+  Diag.Notes.emplace_back(std::move(This));
+
+  Note Merging;
+  Merging.Range = MergingRange;
+  Merging.Msg =
+      llvm::formatv("while this attribute set is marked as {0}recursive, it "
+                    "will be considered as {1}recursive",
+                    MergningRec ? "" : "non-", ThisRec ? "" : "non-");
+  Diag.Notes.emplace_back(std::move(Merging));
+  return Diag;
+}
 
-void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {}
+void ExprAttrsBuilder::addAttrSet(const syntax::AttrSet &AS) {
+  if (AS.Recursive != Recursive) {
+    auto Diag = mkRecDiag(Range, AS.Range, Recursive, AS.Recursive);
+    LW.Diags.emplace_back(std::move(Diag));
+  }
+  assert(AS.AttrBinds && "binds should not be empty! parser error?");
+  addBinds(*AS.AttrBinds);
+}
+
+void ExprAttrsBuilder::addAttr(const syntax::Node *Attr,
+                               const syntax::Node *Body, bool Recursive) {
+  if (Recursive != this->Recursive) {
+    auto Diag = mkRecDiag(Range, Attr->Range, this->Recursive, Recursive);
+    LW.Diags.emplace_back(std::move(Diag));
+  }
+  switch (Attr->getKind()) {
+  case Node::NK_Identifier: {
+    const auto *ID = dynamic_cast<const syntax::Identifier *>(Attr);
+    nix::Symbol Sym = ID->Symbol;
+    if (Fields.contains(Sym)) {
+      // duplicated, but if they are two attrset, we can try to merge them.
+      if (Body->getKind() != Node::NK_AttrSet || !Nested.contains(Sym)) {
+        // the body is not an attribute set, report error.
+        auto Diag =
+            mkAttrDupDiag(LW.STable[Sym], ID->Range, Fields[Sym]->Range);
+        LW.Diags.emplace_back(std::move(Diag));
+        return;
+      }
+      const auto *BodyAttrSet = dynamic_cast<const syntax::AttrSet *>(Body);
+      Nested[Sym]->addAttrSet(*BodyAttrSet);
+    } else {
+      Fields[Sym] = Attr;
+      if (Body->getKind() == Node::NK_AttrSet) {
+        // If the body is an attrset, we want to defer it's lowering process,
+        // because we have chance to merge it latter
+        const auto *BodyAttrSet = dynamic_cast<const syntax::AttrSet *>(Body);
+        Nested[Sym] = std::make_unique<ExprAttrsBuilder>(
+            LW, BodyAttrSet->Recursive, BodyAttrSet->Range);
+        Nested[Sym]->addBinds(*BodyAttrSet->AttrBinds);
+      } else {
+        nix::ExprAttrs::AttrDef Def(LW.lower(Body), Attr->Range.Begin);
+        Result->attrs.insert({Sym, Def});
+      }
+    }
+    break;
+  }
+  default: {
+    nix::Expr *NameExpr = LW.lower(Attr);
+    nix::Expr *ValueExpr = LW.lower(Body);
+    nix::ExprAttrs::DynamicAttrDef DDef(NameExpr, ValueExpr, Attr->Range.Begin);
+    Result->dynamicAttrs.emplace_back(DDef);
+  }
+  }
+}
+
+void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {
+  assert(!A.Path->Names.empty() && "attrpath should not be empty!");
+
+  // The targeting attribute builder we are actually inserting
+  // we will select the attrpath (`a.b.c`) and assign it to this variable
+  // left only one nix::Symbol or nix::Expr*, insert to it.
+  ExprAttrsBuilder *S = this;
+
+  // select attrpath, without the last one.
+  for (auto I = A.Path->Names.begin(); I + 1 != A.Path->Names.end(); I++) {
+    Node *Name = *I;
+
+    switch (Name->getKind()) {
+    case Node::NK_Identifier: {
+      auto *ID = dynamic_cast<syntax::Identifier *>(Name);
+      nix::Symbol Sym = ID->Symbol;
+      if (S->Fields.contains(Sym)) {
+        if (!S->Nested.contains(Sym)) {
+          // contains a field but the body is not an attr, then we cannot
+          // perform merging.
+          Diagnostic Diag =
+              mkAttrDupDiag(LW.STable[Sym], ID->Range, S->Fields[Sym]->Range);
+          LW.Diags.emplace_back(std::move(Diag));
+          return;
+        }
+        // select this symbol, and consider merging it.
+        S = S->Nested[Sym].get();
+      } else {
+        // create a new builder and select to it.
+        S->Nested[Sym] = std::make_unique<ExprAttrsBuilder>(
+            LW, /*Recursive=*/false, Name->Range);
+        S->Fields[Sym] = Name;
+        S = S->Nested[Sym].get();
+      }
+      break; // case Node::NK_Identifier:
+    }
+    default: {
+      DynamicNested[Name] = std::make_unique<ExprAttrsBuilder>(
+          LW, /*Recursive=*/false, Name->Range);
+      S = DynamicNested[Name].get();
+    }
+    }
+  }
+  // Implictly created attribute set, by attrpath selecting, are always
+  // non-recursive
+  S->addAttr(A.Path->Names.back(), A.Body,
+             /*Recursive=*/S != this ? false : this->Recursive);
+}
 
 void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) {
   if (IA.InheritedAttrs->Names.empty()) {
@@ -198,27 +361,7 @@ void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) {
   }
 }
 
-nix::ExprAttrs *Lowering::lowerBinds(const syntax::Binds &Binds) {
-  ExprAttrsBuilder Builder(*this);
-
-  for (auto *Attr : Binds.Attributes) {
-    switch (Attr->getKind()) {
-    case Node::NK_Attribute: {
-      Builder.addAttribute(*dynamic_cast<syntax::Attribute *>(Attr));
-      break;
-    }
-    case Node::NK_InheritedAttribute: {
-      Builder.addInherited(*dynamic_cast<syntax::InheritedAttribute *>(Attr));
-      break;
-    }
-    default:
-      llvm_unreachable("must be A/IA! incorrect parsing rules?");
-    }
-  }
-  return Builder.finish();
-}
-
-nix::Expr *Lowering::lower(const nixd::syntax::Node *Root) {
+nix::Expr *Lowering::lower(const syntax::Node *Root) {
   if (!Root)
     return nullptr;
 
@@ -243,16 +386,18 @@ nix::Expr *Lowering::lower(const nixd::syntax::Node *Root) {
         Ctx.Pool.record(new nix::ExprWith(With->Range.Begin, Attrs, Body));
     return NixWith;
   }
-  case Node::NK_Binds: {
-    const auto *Binds = dynamic_cast<const syntax::Binds *>(Root);
-    return lowerBinds(*Binds);
-  }
   case Node::NK_AttrSet: {
     const auto *AttrSet = dynamic_cast<const syntax::AttrSet *>(Root);
     assert(AttrSet->AttrBinds && "null AttrBinds of the AttrSet!");
-    nix::ExprAttrs *Binds = lowerBinds(*AttrSet->AttrBinds);
-    Binds->recursive = AttrSet->Recursive;
-    return Binds;
+    ExprAttrsBuilder Builder(*this, AttrSet->Recursive, AttrSet->Range);
+    Builder.addBinds(*AttrSet->AttrBinds);
+    return Builder.finish();
+  }
+  case Node::NK_Int: {
+    const auto *Int = dynamic_cast<const syntax::Int *>(Root);
+    auto *NixInt = new nix::ExprInt(Int->N);
+    Ctx.Pool.record(NixInt);
+    return NixInt;
   }
   }
 
diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-dynamic.nix b/nixd/tools/nixd-lint/test/lowering-attrset-dynamic.nix
new file mode 100644
index 000000000..dbc253406
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-attrset-dynamic.nix
@@ -0,0 +1,7 @@
+# RUN: true
+
+# FIXME: we cannot deal with string_parts currently
+# rewrite this test after we can do such thing.
+{
+  "${bar}" = 1;
+}
diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-merge-inherit.nix b/nixd/tools/nixd-lint/test/lowering-attrset-merge-inherit.nix
new file mode 100644
index 000000000..0217c4aed
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-attrset-merge-inherit.nix
@@ -0,0 +1,7 @@
+# RUN: nixd-lint %s | FileCheck %s
+{
+  inherit aa;
+
+  # CHECK: duplicated attr `aa`
+  aa = 1;
+}
diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-merging-2.nix b/nixd/tools/nixd-lint/test/lowering-attrset-merging-2.nix
new file mode 100644
index 000000000..adcf2d53c
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-attrset-merging-2.nix
@@ -0,0 +1,10 @@
+# RUN: nixd-lint %s | FileCheck %s
+{
+  a.bbb = 1;
+
+  # CHECK: error: duplicated attr `b`
+  a.bbb.c = 2;
+
+  # CHECK: duplicated attr `a`
+  a = 3;
+}
diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-rec-2.nix b/nixd/tools/nixd-lint/test/lowering-attrset-rec-2.nix
new file mode 100644
index 000000000..8f3f1eb5c
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-attrset-rec-2.nix
@@ -0,0 +1,14 @@
+# RUN: nixd-lint %s | FileCheck %s
+{
+
+  foo = rec {
+    x = 1;
+    y = 2;
+  };
+
+  # CHECK: merging two attributes with different `rec` modifiers, the latter will be implicitly ignored
+  # CHECK: this attribute set is recursive
+  # CHECK: while this attribute set is marked as non-recursive, it will be considered as recursive
+  foo.bar = 1;
+
+}
diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-rec-3.nix b/nixd/tools/nixd-lint/test/lowering-attrset-rec-3.nix
new file mode 100644
index 000000000..0925de110
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-attrset-rec-3.nix
@@ -0,0 +1,15 @@
+# RUN: nixd-lint %s | FileCheck %s
+{
+
+  foo = rec {
+    x = 1;
+    y = 2;
+  };
+
+  # CHECK: merging two attributes with different `rec` modifiers, the latter will be implicitly ignored
+  # CHECK: this attribute set is recursive
+  # CHECK: while this attribute set is marked as non-recursive, it will be considered as recursive
+  foo = {
+    bar = 1;
+  };
+}
diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix b/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix
new file mode 100644
index 000000000..bea6c7879
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix
@@ -0,0 +1,13 @@
+# RUN: nixd-lint %s | FileCheck %s
+rec {
+  foo.bar = 1;
+
+  # CHECK: merging two attributes with different `rec` modifiers, the latter will be implicitly ignored
+
+  # CHECK: this attribute set is not recursive
+  # CHECK: while this attribute set is marked as recursive, it will be considered as non-recursive
+  foo = rec {
+    x = 1;
+    y = 2;
+  };
+}
diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-select-merging.nix b/nixd/tools/nixd-lint/test/lowering-attrset-select-merging.nix
new file mode 100644
index 000000000..1113f408d
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-attrset-select-merging.nix
@@ -0,0 +1,12 @@
+# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s
+
+# CHECK: ExprAttrs: { a = { b = 1; c = 2; x = 1; }; }
+{
+  # CHECK: ExprAttrs: { b = 1; c = 2; x = 1; }
+  a.b = 1;
+  a.c = 2;
+
+  a = {
+    x = 1;
+  };
+}

From 5cb1dbca4058051e761eb7243119996939c1e11d Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Thu, 21 Sep 2023 18:22:44 +0800
Subject: [PATCH 08/24] nixd/Sema: lowering for `let ... in ...`

---
 nixd/include/nixd/Sema/Lowering.h         | 10 ++++-
 nixd/lib/Sema/Lowering.cpp                | 49 ++++++++++++++++++++---
 nixd/tools/nixd-lint/test/let-dynamic.nix |  8 ++++
 nixd/tools/nixd-lint/test/let-simple.nix  |  9 +++++
 4 files changed, 68 insertions(+), 8 deletions(-)
 create mode 100644 nixd/tools/nixd-lint/test/let-dynamic.nix
 create mode 100644 nixd/tools/nixd-lint/test/let-simple.nix

diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index f6deeb349..f6d20ff7a 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -18,9 +18,14 @@ class ExprAttrsBuilder {
   Lowering &LW;
   nix::ExprAttrs *Result;
 
+  RangeIdx Range;
+
   bool Recursive;
 
-  RangeIdx Range;
+  /// let ... in ...
+  /// It is not allowed to use dynamic binds here, so we want to give diagnostic
+  /// to each occurrence.
+  bool IsLet;
 
   /// Nested attributes, we create a new builder for them, and collapse the map
   /// while finishing
@@ -34,7 +39,8 @@ class ExprAttrsBuilder {
   std::map<nix::Symbol, const syntax::Node *> Fields;
 
 public:
-  ExprAttrsBuilder(Lowering &LW, bool Recursive, RangeIdx Range);
+  ExprAttrsBuilder(Lowering &LW, RangeIdx Range, bool Recursive, bool IsLet);
+
   void addAttr(const syntax::Node *Attr, const syntax::Node *Body,
                bool Recursive);
   void addAttribute(const syntax::Attribute &A);
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index d4bf9fa84..137b49613 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -134,8 +134,9 @@ static syntax::Diagnostic mkAttrDupDiag(std::string Name, RangeIdx Range,
   return Diag;
 }
 
-ExprAttrsBuilder::ExprAttrsBuilder(Lowering &LW, bool Recursive, RangeIdx Range)
-    : LW(LW), Recursive(Recursive), Range(Range) {
+ExprAttrsBuilder::ExprAttrsBuilder(Lowering &LW, RangeIdx Range, bool Recursive,
+                                   bool IsLet)
+    : LW(LW), Range(Range), Recursive(Recursive), IsLet(IsLet) {
   Result = LW.Ctx.Pool.record(new nix::ExprAttrs);
 }
 
@@ -196,6 +197,15 @@ static Diagnostic mkRecDiag(RangeIdx ThisRange, RangeIdx MergingRange,
   return Diag;
 }
 
+static Diagnostic mkLetDynamicDiag(RangeIdx Range) {
+  Diagnostic Diag;
+  Diag.Kind = Diagnostic::Error;
+  Diag.Msg = "dynamic attributes are not allowed in let ... in ... expression";
+  Diag.Range = Range;
+
+  return Diag;
+}
+
 void ExprAttrsBuilder::addAttrSet(const syntax::AttrSet &AS) {
   if (AS.Recursive != Recursive) {
     auto Diag = mkRecDiag(Range, AS.Range, Recursive, AS.Recursive);
@@ -233,7 +243,7 @@ void ExprAttrsBuilder::addAttr(const syntax::Node *Attr,
         // because we have chance to merge it latter
         const auto *BodyAttrSet = dynamic_cast<const syntax::AttrSet *>(Body);
         Nested[Sym] = std::make_unique<ExprAttrsBuilder>(
-            LW, BodyAttrSet->Recursive, BodyAttrSet->Range);
+            LW, BodyAttrSet->Range, BodyAttrSet->Recursive, IsLet);
         Nested[Sym]->addBinds(*BodyAttrSet->AttrBinds);
       } else {
         nix::ExprAttrs::AttrDef Def(LW.lower(Body), Attr->Range.Begin);
@@ -243,6 +253,11 @@ void ExprAttrsBuilder::addAttr(const syntax::Node *Attr,
     break;
   }
   default: {
+    if (IsLet) {
+      // reject dynamic attrs in let expression
+      LW.Diags.emplace_back(mkLetDynamicDiag(Attr->Range));
+      return;
+    }
     nix::Expr *NameExpr = LW.lower(Attr);
     nix::Expr *ValueExpr = LW.lower(Body);
     nix::ExprAttrs::DynamicAttrDef DDef(NameExpr, ValueExpr, Attr->Range.Begin);
@@ -281,15 +296,20 @@ void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {
       } else {
         // create a new builder and select to it.
         S->Nested[Sym] = std::make_unique<ExprAttrsBuilder>(
-            LW, /*Recursive=*/false, Name->Range);
+            LW, Name->Range, /*Recursive=*/false, IsLet);
         S->Fields[Sym] = Name;
         S = S->Nested[Sym].get();
       }
       break; // case Node::NK_Identifier:
     }
     default: {
+      if (IsLet) {
+        // reject dynamic attrs in let expression
+        LW.Diags.emplace_back(mkLetDynamicDiag(Name->Range));
+        return;
+      }
       DynamicNested[Name] = std::make_unique<ExprAttrsBuilder>(
-          LW, /*Recursive=*/false, Name->Range);
+          LW, Name->Range, /*Recursive=*/false, IsLet);
       S = DynamicNested[Name].get();
     }
     }
@@ -389,7 +409,8 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
   case Node::NK_AttrSet: {
     const auto *AttrSet = dynamic_cast<const syntax::AttrSet *>(Root);
     assert(AttrSet->AttrBinds && "null AttrBinds of the AttrSet!");
-    ExprAttrsBuilder Builder(*this, AttrSet->Recursive, AttrSet->Range);
+    ExprAttrsBuilder Builder(*this, AttrSet->Range, AttrSet->Recursive,
+                             /*IsLet=*/false);
     Builder.addBinds(*AttrSet->AttrBinds);
     return Builder.finish();
   }
@@ -399,6 +420,22 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
     Ctx.Pool.record(NixInt);
     return NixInt;
   }
+  case Node::NK_Let: {
+    const auto *Let = dynamic_cast<const syntax::Let *>(Root);
+    ExprAttrsBuilder Builder(*this, Let->Binds->Range, /*Recursive=*/true,
+                             /*IsLet=*/true);
+    Builder.addBinds(*Let->Binds);
+    nix::ExprAttrs *Attrs = Builder.finish();
+    nix::Expr *Body = lower(Let->Body);
+
+    // special work for let expression, the expression there is implictly
+    // recursive, and we should not mark `rec` to the evaluator, because the
+    // official parser does not do this also.
+    Attrs->recursive = false;
+
+    auto *Ret = new nix::ExprLet(Attrs, Body);
+    return Ret;
+  }
   }
 
   return nullptr;
diff --git a/nixd/tools/nixd-lint/test/let-dynamic.nix b/nixd/tools/nixd-lint/test/let-dynamic.nix
new file mode 100644
index 000000000..5d4a8dbd0
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/let-dynamic.nix
@@ -0,0 +1,8 @@
+# RUN: nixd-lint %s | FileCheck %s
+
+# CHECK: dynamic attributes are not allowed in let ... in ... expression
+let
+  ${1} = 3;
+  a = 1;
+in
+1
diff --git a/nixd/tools/nixd-lint/test/let-simple.nix b/nixd/tools/nixd-lint/test/let-simple.nix
new file mode 100644
index 000000000..69ac262c9
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/let-simple.nix
@@ -0,0 +1,9 @@
+# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s
+
+# CHECK: ExprLet: (let x = 1; y = 2; z = 3; in 1)
+let
+  x = 1;
+  y = 2;
+  z = 3;
+in
+1

From 879c8b7d484a33277090a068df2f9a5a6ac203d8 Mon Sep 17 00:00:00 2001
From: Guanghui Hu <120L052208@stu.hit.edu.cn>
Date: Thu, 21 Sep 2023 22:06:56 +0800
Subject: [PATCH 09/24] nixd/Sema: lowering for ExprIf

---
 nixd/lib/Sema/Lowering.cpp | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 137b49613..9bf6724a5 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -436,6 +436,15 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
     auto *Ret = new nix::ExprLet(Attrs, Body);
     return Ret;
   }
+  case Node::NK_If: {
+    const auto *If = dynamic_cast<const syntax::If *>(Root);
+    auto *Cond = lower(If->Cond);
+    auto *Then = lower(If->Then);
+    auto *Else = lower(If->Else);
+    auto *NixIf =
+        Ctx.Pool.record(new nix::ExprIf(If->Range.Begin, Cond, Then, Else));
+    return NixIf;
+  }
   }
 
   return nullptr;

From f76aa03f66cdf9b5e28f482b6a43091946948fb5 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Fri, 22 Sep 2023 23:27:12 +0800
Subject: [PATCH 10/24] nixd-lint: add position support

---
 nixd/tools/nixd-lint/nixd-lint.cpp | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/nixd/tools/nixd-lint/nixd-lint.cpp b/nixd/tools/nixd-lint/nixd-lint.cpp
index 2deedd5d8..b52192c6c 100644
--- a/nixd/tools/nixd-lint/nixd-lint.cpp
+++ b/nixd/tools/nixd-lint/nixd-lint.cpp
@@ -29,6 +29,9 @@ const OptionCategory *Cat[] = {&Misc};
 opt<bool> DumpNixAST("dump-nix-ast", init(false),
                      desc("Dump the lowered nix AST"), cat(Misc));
 
+opt<bool> ShowPosition("show-position", init(false),
+                       desc("Print location information"), cat(Misc));
+
 static void printCodeLines(std::ostream &Out, const std::string &Prefix,
                            std::shared_ptr<nix::AbstractPos> BeginPos,
                            std::shared_ptr<nix::AbstractPos> EndPos,
@@ -80,6 +83,7 @@ static void printCodeLines(std::ostream &Out, const std::string &Prefix,
 
 struct ASTDump : nixd::RecursiveASTVisitor<ASTDump> {
   nix::SymbolTable &STable;
+  nix::PosTable &PTable;
   int Depth = 0;
 
   bool traverseExpr(const nix::Expr *E) {
@@ -98,10 +102,24 @@ struct ASTDump : nixd::RecursiveASTVisitor<ASTDump> {
     }
     std::cout << nixd::getExprName(E) << ": ";
     E->show(STable, std::cout);
+
+    if (ShowPosition) {
+      std::cout << " ";
+      showPosition(*E);
+    }
+
     std::cout << " ";
     std::cout << "\n";
     return true;
   }
+
+  void showPosition(const nix::Expr &E) const {
+    nix::PosIdx PID = E.getPos();
+    if (PID != nix::noPos) {
+      nix::Pos Pos = PTable[PID];
+      std::cout << Pos.line << ":" << Pos.column;
+    }
+  }
 };
 
 int main(int argc, char *argv[]) {
@@ -135,7 +153,7 @@ int main(int argc, char *argv[]) {
   nix::Expr *NixTree = Lowering.lower(Data.Result);
 
   if (DumpNixAST) {
-    ASTDump D{.STable = *STable};
+    ASTDump D{.STable = *STable, .PTable = *PTable};
     D.traverseExpr(NixTree);
   }
 

From 12a479379e772916db29d9344fde3a0b4db3fdda Mon Sep 17 00:00:00 2001
From: Guanghui Hu <120L052208@stu.hit.edu.cn>
Date: Thu, 21 Sep 2023 22:07:45 +0800
Subject: [PATCH 11/24] nixd/Sema: lowering some operators (binary + unary)

Co-authored-by: Yingchi Long <i@lyc.dev>
---
 nixd/include/nixd/Sema/EvalContext.h       |   2 +
 nixd/include/nixd/Sema/Lowering.h          |  59 ++++++++++++
 nixd/lib/Sema/Lowering.cpp                 | 106 +++++++++++++++++++++
 nixd/tools/nixd-lint/test/lowering-ops.nix |  58 +++++++++++
 4 files changed, 225 insertions(+)
 create mode 100644 nixd/tools/nixd-lint/test/lowering-ops.nix

diff --git a/nixd/include/nixd/Sema/EvalContext.h b/nixd/include/nixd/Sema/EvalContext.h
index 146a26320..12260a6e5 100644
--- a/nixd/include/nixd/Sema/EvalContext.h
+++ b/nixd/include/nixd/Sema/EvalContext.h
@@ -7,8 +7,10 @@
 namespace nixd {
 
 struct EvalContext {
+  using ES = std::vector<std::pair<nix::PosIdx, nix::Expr *>>;
   GCPool<nix::Expr> Pool;
   GCPool<nix::Formals> FormalsPool;
+  GCPool<ES> ESPool;
 };
 
 } // namespace nixd
diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index f6d20ff7a..d792be59f 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -60,6 +60,65 @@ struct Lowering {
   nix::ExprLambda *lowerFunction(const syntax::Function *Fn);
   nix::Formal lowerFormal(const syntax::Formal &Formal);
   nix::AttrPath lowerAttrPath(const syntax::AttrPath &Path);
+  nix::Expr *lowerOp(const syntax::Node *Op);
+
+private:
+  constexpr static std::string_view LessThan = "__lessThan";
+  constexpr static std::string_view Sub = "__sub";
+  constexpr static std::string_view Mul = "__mul";
+  constexpr static std::string_view Div = "__div";
+
+  nix::ExprVar *mkVar(std::string_view Sym) {
+    return mkVar(STable.create(Sym));
+  }
+
+  nix::ExprVar *mkVar(nix::Symbol Sym) {
+    auto *Var = new nix::ExprVar(Sym);
+    Ctx.Pool.record(Var);
+    return Var;
+  };
+
+  nix::ExprOpNot *mkNot(nix::Expr *Body) {
+    auto *OpNot = new nix::ExprOpNot(Body);
+    Ctx.Pool.record(OpNot);
+    return OpNot;
+  }
+
+  nix::ExprCall *mkCall(std::string_view FnName, nix::PosIdx P,
+                        std::vector<nix::Expr *> Args) {
+    nix::ExprVar *Fn = mkVar(FnName);
+    auto *Nix = new nix::ExprCall(P, Fn, std::move(Args));
+    return Ctx.Pool.record(Nix);
+  }
+
+  template <class SyntaxTy>
+  nix::Expr *lowerCallOp(std::string_view FnName, const syntax::Node *Op,
+                         bool SwapOperands = false, bool Not = false) {
+    auto *SOP = dynamic_cast<const SyntaxTy *>(Op);
+    assert(SOP && "types are not matching between op pointer & syntax type!");
+    nix::Expr *LHS = lower(SOP->LHS);
+    nix::Expr *RHS = lower(SOP->RHS);
+    if (SwapOperands)
+      std::swap(LHS, RHS);
+    nix::PosIdx P = SOP->OpRange.Begin;
+    nix::ExprCall *Call = mkCall(FnName, P, {LHS, RHS});
+    if (Not)
+      return mkNot(Call);
+    return Call;
+  }
+
+  /// The bin-op expression is actually legal in the nix evaluator
+  /// OpImpl -> nix::ExprOpImpl
+  template <class NixTy, class SyntaxTy>
+  NixTy *lowerLegalOp(const syntax::Node *Op) {
+    auto *SOP = dynamic_cast<const SyntaxTy *>(Op);
+    assert(SOP && "types are not matching between op pointer & syntax type!");
+    nix::Expr *LHS = lower(SOP->LHS);
+    nix::Expr *RHS = lower(SOP->RHS);
+    nix::PosIdx P = SOP->OpRange.Begin;
+    auto *Nix = new NixTy(P, LHS, RHS);
+    return Ctx.Pool.record(Nix);
+  }
 };
 
 } // namespace nixd
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 9bf6724a5..97fa2ab86 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -6,6 +6,7 @@
 
 #include <nix/nixexpr.hh>
 
+#include <llvm/Support/ErrorHandling.h>
 #include <llvm/Support/FormatVariadic.h>
 
 #include <memory>
@@ -381,6 +382,101 @@ void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) {
   }
 }
 
+nix::Expr *Lowering::lowerOp(const syntax::Node *Op) {
+  if (!Op)
+    return nullptr;
+
+  switch (Op->getKind()) {
+  case Node::NK_OpNot: {
+    const auto *OpNot = dynamic_cast<const syntax::OpNot *>(Op);
+    return mkNot(lower(OpNot->Body));
+  }
+
+  // arithmetic
+  case Node::NK_OpAdd: {
+    // A + B
+    // ->
+    // ExprConcatStrings:
+    //   - pos: "+"
+    //   - forceString: false
+    //   - es: {{A.pos, A}, {B.pos, B}}
+    const auto *OpAdd = dynamic_cast<const syntax::OpAdd *>(Op);
+    nix::Expr *A = lower(OpAdd->LHS);
+    nix::Expr *B = lower(OpAdd->RHS);
+    nix::PosIdx APos = OpAdd->LHS->Range.Begin;
+    nix::PosIdx BPos = OpAdd->RHS->Range.Begin;
+    nix::PosIdx Pos = OpAdd->OpRange.Begin;
+    auto *ES = Ctx.ESPool.record(new EvalContext::ES{{APos, A}, {BPos, B}});
+    auto ECS = new nix::ExprConcatStrings(Pos, /*forceString=*/false, ES);
+    return Ctx.Pool.record(ECS);
+  }
+  case Node::NK_OpSub:
+    return lowerCallOp<syntax::OpSub>(Sub, Op);
+  case Node::NK_OpMul:
+    return lowerCallOp<syntax::OpMul>(Mul, Op);
+  case Node::NK_OpDiv:
+    return lowerCallOp<syntax::OpDiv>(Div, Op);
+
+  // comparison
+  case Node::NK_OpNegate: {
+    // -X
+    // ->
+    // (__sub 0 X)
+    const auto *OpNegate = dynamic_cast<const syntax::OpNegate *>(Op);
+    nix::Expr *I0 = Ctx.Pool.record(new nix::ExprInt(0));
+    nix::Expr *X = lower(OpNegate->Body);
+    nix::PosIdx P = OpNegate->OpRange.Begin;
+    return mkCall(Sub, P, {I0, X});
+  }
+  case Node::NK_OpLe: {
+    // A < B
+    // ->
+    // (__lessThan A B)
+    return lowerCallOp<syntax::OpLe>(LessThan, Op);
+  }
+  case Node::NK_OpLeq: {
+    // A <= B
+    // ->
+    // (! (__lessThan B A))
+    return lowerCallOp<syntax::OpLeq>(LessThan, Op, /*SwapOperands=*/true,
+                                      /*Not=*/true);
+  }
+  case Node::NK_OpGe: {
+    // A > B
+    // ->
+    // (__lessThan B A)
+    return lowerCallOp<syntax::OpGe>(LessThan, Op, /*SwapOperands=*/true);
+  }
+  case Node::NK_OpGeq: {
+    // A >= B
+    // ->
+    // ! (__lessThan A B)
+    return lowerCallOp<syntax::OpGeq>(LessThan, Op, /*SwapOperands=*/false,
+                                      /*Not=*/true);
+  }
+
+  // legal nodes, the list here has the same order with official parser.
+  // src/libexpr/nixexpr.hh, `MakeBinOp`
+  case Node::NK_OpEq:
+    return lowerLegalOp<nix::ExprOpEq, syntax::OpEq>(Op);
+  case Node::NK_OpNEq:
+    return lowerLegalOp<nix::ExprOpNEq, syntax::OpNEq>(Op);
+  case Node::NK_OpAnd:
+    return lowerLegalOp<nix::ExprOpAnd, syntax::OpAnd>(Op);
+  case Node::NK_OpOr:
+    return lowerLegalOp<nix::ExprOpOr, syntax::OpOr>(Op);
+  case Node::NK_OpImpl:
+    return lowerLegalOp<nix::ExprOpImpl, syntax::OpImpl>(Op);
+  case Node::NK_OpUpdate:
+    return lowerLegalOp<nix::ExprOpUpdate, syntax::OpUpdate>(Op);
+  case Node::NK_OpConcatLists:
+    return lowerLegalOp<nix::ExprOpConcatLists, syntax::OpConcatLists>(Op);
+  default:
+    llvm_unreachable("do not know how to lower this op!");
+  }
+  return nullptr;
+}
+
 nix::Expr *Lowering::lower(const syntax::Node *Root) {
   if (!Root)
     return nullptr;
@@ -445,6 +541,16 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
         Ctx.Pool.record(new nix::ExprIf(If->Range.Begin, Cond, Then, Else));
     return NixIf;
   }
+
+  // operators
+  case Node::NK_OpNot:
+  case Node::NK_OpNegate:
+#define BIN_OP(NAME, _) case Node::NK_##NAME:
+#include "nixd/Syntax/BinaryOps.inc"
+#undef BIN_OP
+  {
+    return lowerOp(Root);
+  }
   }
 
   return nullptr;
diff --git a/nixd/tools/nixd-lint/test/lowering-ops.nix b/nixd/tools/nixd-lint/test/lowering-ops.nix
new file mode 100644
index 000000000..34f1f2935
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-ops.nix
@@ -0,0 +1,58 @@
+# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s
+# RUN: nixd-lint -dump-nix-ast -show-position %s | FileCheck --check-prefix=POSITION %s
+
+{
+  # POSITION: ExprCall: (__sub 0 1) 6:14
+  Position = -1;
+
+  # CHECK: ExprOpNot: (! 1)
+  Not = !1;
+
+  # CHECK: ExprCall: (__sub 0 1)
+  Negate = -1;
+
+  # CHECK: ExprOpEq: (1 == 1)
+  Eq = 1 == 1;
+
+  # CHECK: ExprOpNEq: (1 != 1)
+  NEq = 1 != 1;
+
+  # CHECK: ExprCall: (__lessThan 2 1)
+  Ge = 1 > 2;
+
+  # CHECK: ExprCall: (__lessThan 1 2)
+  Le = 1 < 2;
+
+  # CHECK: (! (__lessThan 2 1))
+  Leq = 1 <= 2;
+
+  # CHECK: ExprOpNot: (! (__lessThan 1 2))
+  Geq = 1 >= 2;
+
+  # CHECK:  ExprOpAnd: (1 && 2)
+  And = 1 && 2;
+
+  # CHECK: ExprOpOr: (1 || 2)
+  Or = 1 || 2;
+
+  # CHECK: ExprOpImpl: (1 -> 2)
+  Impl = 1 -> 2;
+
+  # CHECK: ExprOpUpdate: (1 // 2)
+  Update = 1 // 2;
+
+  # CHECK: ExprConcatStrings: (1 + 2)
+  Add = 1 + 2;
+
+  # CHECK: ExprCall: (__sub 1 2)
+  Sub = 1 - 2;
+
+  # CHECK: ExprCall: (__mul 1 2)
+  Mul = 1 * 2;
+
+  # CHECK: ExprCall: (__div 1 2)
+  Div = 1 / 2;
+
+  # CHECK: ExprOpConcatLists: (1 ++ 2)
+  Concat = 1 ++ 2;
+}

From 10991da51dd189a05d38316c50572c5183ba6990 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Sat, 23 Sep 2023 14:51:56 +0800
Subject: [PATCH 12/24] nixd/Sema: lowering for `OpHasAttrs`

---
 nixd/lib/Sema/Lowering.cpp                 | 28 ++++++++++++++++++++++
 nixd/tools/nixd-lint/test/lowering-ops.nix |  4 ++++
 2 files changed, 32 insertions(+)

diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 97fa2ab86..12645319a 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -382,6 +382,24 @@ void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) {
   }
 }
 
+nix::AttrPath Lowering::lowerAttrPath(const syntax::AttrPath &Path) {
+  nix::AttrPath Ret;
+  for (Node *Name : Path.Names) {
+    switch (Name->getKind()) {
+    case Node::NK_Identifier: {
+      nix::Symbol Sym = dynamic_cast<syntax::Identifier *>(Name)->Symbol;
+      Ret.emplace_back(Sym);
+      break;
+    }
+    default: {
+      nix::Expr *E = lower(Name);
+      Ret.emplace_back(E);
+    }
+    }
+  }
+  return Ret;
+}
+
 nix::Expr *Lowering::lowerOp(const syntax::Node *Op) {
   if (!Op)
     return nullptr;
@@ -471,6 +489,15 @@ nix::Expr *Lowering::lowerOp(const syntax::Node *Op) {
     return lowerLegalOp<nix::ExprOpUpdate, syntax::OpUpdate>(Op);
   case Node::NK_OpConcatLists:
     return lowerLegalOp<nix::ExprOpConcatLists, syntax::OpConcatLists>(Op);
+
+  case Node::NK_OpHasAttr: {
+    const auto *OpHasAttr = dynamic_cast<const syntax::OpHasAttr *>(Op);
+    assert(OpHasAttr->Path && "OpHasAttr->Path must not be nullptr! (parser?)");
+    nix::AttrPath Path = lowerAttrPath(*OpHasAttr->Path);
+    nix::Expr *Body = lower(OpHasAttr->Operand);
+    auto *NixOpHasAttr = new nix::ExprOpHasAttr(Body, std::move(Path));
+    return Ctx.Pool.record(NixOpHasAttr);
+  }
   default:
     llvm_unreachable("do not know how to lower this op!");
   }
@@ -545,6 +572,7 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
   // operators
   case Node::NK_OpNot:
   case Node::NK_OpNegate:
+  case Node::NK_OpHasAttr:
 #define BIN_OP(NAME, _) case Node::NK_##NAME:
 #include "nixd/Syntax/BinaryOps.inc"
 #undef BIN_OP
diff --git a/nixd/tools/nixd-lint/test/lowering-ops.nix b/nixd/tools/nixd-lint/test/lowering-ops.nix
index 34f1f2935..2e89de1e5 100644
--- a/nixd/tools/nixd-lint/test/lowering-ops.nix
+++ b/nixd/tools/nixd-lint/test/lowering-ops.nix
@@ -55,4 +55,8 @@
 
   # CHECK: ExprOpConcatLists: (1 ++ 2)
   Concat = 1 ++ 2;
+
+  # TODO: check that we can deal with dynamic attrpath
+  # CHECK: ExprOpHasAttr: ((1) ? a.b.c)
+  HasAttr = 1 ? a.b.c;
 }

From cfc6572233e0755db19b1b5d7b76cc8ed04e5871 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Sun, 24 Sep 2023 13:38:59 +0800
Subject: [PATCH 13/24] nixd/Sema: lowering ExprCall

---
 nixd/lib/Sema/Lowering.cpp                  | 12 ++++++++++++
 nixd/tools/nixd-lint/test/lowering-call.nix |  4 ++++
 2 files changed, 16 insertions(+)
 create mode 100644 nixd/tools/nixd-lint/test/lowering-call.nix

diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 12645319a..bac5b31dc 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -579,7 +579,19 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
   {
     return lowerOp(Root);
   }
+
+  case Node::NK_Call: {
+    const auto *Call = dynamic_cast<const syntax::Call *>(Root);
+    nix::Expr *Fn = lower(Call->Fn);
+    std::vector<nix::Expr *> Args;
+
+    for (Node *Arg : Call->Args)
+      Args.emplace_back(lower(Arg));
+
+    auto *NixCall = new nix::ExprCall(Call->Range.Begin, Fn, std::move(Args));
+    return Ctx.Pool.record(NixCall);
   }
+  } // switch
 
   return nullptr;
 }
diff --git a/nixd/tools/nixd-lint/test/lowering-call.nix b/nixd/tools/nixd-lint/test/lowering-call.nix
new file mode 100644
index 000000000..d26a7320c
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-call.nix
@@ -0,0 +1,4 @@
+# RUN: nixd-lint -dump-nix-ast  %s | FileCheck %s
+
+# CHECK: ExprCall: (2 1)
+2 1

From 4cc4f7b4160b16026b647149c6253c01fe931350 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Sun, 24 Sep 2023 13:47:08 +0800
Subject: [PATCH 14/24] nixd/Sema: lowering ExprSelect

---
 nixd/lib/Sema/Lowering.cpp                    | 11 +++++++++++
 nixd/tools/nixd-lint/test/lowering-select.nix |  9 +++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 nixd/tools/nixd-lint/test/lowering-select.nix

diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index bac5b31dc..082bc8d82 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -591,6 +591,17 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
     auto *NixCall = new nix::ExprCall(Call->Range.Begin, Fn, std::move(Args));
     return Ctx.Pool.record(NixCall);
   }
+  case Node::NK_Select: {
+    const auto *Select = dynamic_cast<const syntax::Select *>(Root);
+    nix::Expr *Body = lower(Select->Body);
+    nix::Expr *Default = lower(Select->Default);
+    nix::PosIdx P = Select->Range.Begin;
+    assert(Select->Path && "Select->Path must be non-null!");
+    nix::AttrPath Path = lowerAttrPath(*Select->Path);
+    auto *NixSelect = new nix::ExprSelect(P, Body, std::move(Path), Default);
+    return Ctx.Pool.record(NixSelect);
+  }
+
   } // switch
 
   return nullptr;
diff --git a/nixd/tools/nixd-lint/test/lowering-select.nix b/nixd/tools/nixd-lint/test/lowering-select.nix
new file mode 100644
index 000000000..b5c15c926
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-select.nix
@@ -0,0 +1,9 @@
+# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s
+
+{
+  # CHECK: ExprSelect: (1).a.b.c
+  a = 1 . a.b.c;
+
+  # CHECK: ExprSelect: (1).a.b.c or ((2).foo.bar)
+  b = 1 . a.b.c or 2 . foo.bar;
+}

From 8cc8ac9ed83c192a416feaf0ca31187d86902b6a Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Sun, 24 Sep 2023 14:25:39 +0800
Subject: [PATCH 15/24] nixd/Sema: lowering ExprVar + ExprPos

---
 nixd/include/nixd/Sema/Lowering.h               |  1 +
 nixd/lib/Sema/Lowering.cpp                      | 11 +++++++++++
 nixd/tools/nixd-lint/test/lowering-variable.nix |  9 +++++++++
 3 files changed, 21 insertions(+)
 create mode 100644 nixd/tools/nixd-lint/test/lowering-variable.nix

diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index d792be59f..fa50113f0 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -67,6 +67,7 @@ struct Lowering {
   constexpr static std::string_view Sub = "__sub";
   constexpr static std::string_view Mul = "__mul";
   constexpr static std::string_view Div = "__div";
+  constexpr static std::string_view CurPos = "__curPos";
 
   nix::ExprVar *mkVar(std::string_view Sym) {
     return mkVar(STable.create(Sym));
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 082bc8d82..c67510bcd 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -601,6 +601,17 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
     auto *NixSelect = new nix::ExprSelect(P, Body, std::move(Path), Default);
     return Ctx.Pool.record(NixSelect);
   }
+  case Node::NK_Variable: {
+    const auto *Var = dynamic_cast<const syntax::Variable *>(Root);
+    nix::Expr *Ret;
+    nix::PosIdx P = Var->Range.Begin;
+    nix::Symbol Sym = Var->ID->Symbol;
+    if (Sym == STable.create(CurPos))
+      Ret = new nix::ExprPos(P);
+    else
+      Ret = new nix::ExprVar(P, Sym);
+    return Ctx.Pool.record(Ret);
+  }
 
   } // switch
 
diff --git a/nixd/tools/nixd-lint/test/lowering-variable.nix b/nixd/tools/nixd-lint/test/lowering-variable.nix
new file mode 100644
index 000000000..20d62b7d1
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-variable.nix
@@ -0,0 +1,9 @@
+# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s
+
+{
+  # CHECK: ExprVar: a
+  a = a;
+
+  # CHECK: ExprPos: __curPos
+  p = __curPos;
+}

From ac9225375db9fde4911cf24879e40534914063c4 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Sun, 24 Sep 2023 16:48:51 +0800
Subject: [PATCH 16/24] nixd/Sema: lowering for strings

---
 nixd/lib/Sema/Lowering.cpp                    | 20 +++++++++++++++++++
 nixd/tools/nixd-lint/test/lowering-string.nix |  8 ++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 nixd/tools/nixd-lint/test/lowering-string.nix

diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index c67510bcd..3265bdb13 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -612,6 +612,26 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
       Ret = new nix::ExprVar(P, Sym);
     return Ctx.Pool.record(Ret);
   }
+  case Node::NK_String: {
+    const auto *String = dynamic_cast<const syntax::String *>(Root);
+    auto *NixString = new nix::ExprString(std::string(String->S));
+    return Ctx.Pool.record(NixString);
+  }
+  case Node::NK_ConcatStrings: {
+    const auto *CS = dynamic_cast<const syntax::ConcatStrings *>(Root);
+    nix::PosIdx P = CS->Range.Begin;
+    auto *ES = Ctx.ESPool.record(new EvalContext::ES);
+    for (Node *SubStr : CS->SubStrings) {
+      auto *NixSubStr = lower(SubStr);
+      ES->emplace_back(std::pair{SubStr->Range.Begin, NixSubStr});
+    }
+    auto *NixConcatStrings = new nix::ExprConcatStrings(P, CS->ForceString, ES);
+    return Ctx.Pool.record(NixConcatStrings);
+  }
+  case Node::NK_InterpExpr: {
+    const auto *IE = dynamic_cast<const syntax::InterpExpr *>(Root);
+    return lower(IE->Body);
+  }
 
   } // switch
 
diff --git a/nixd/tools/nixd-lint/test/lowering-string.nix b/nixd/tools/nixd-lint/test/lowering-string.nix
new file mode 100644
index 000000000..5930a872f
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-string.nix
@@ -0,0 +1,8 @@
+# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s
+{
+  # CHECK: ExprConcatStrings: (aa + "bbb" + cc)
+  s1 = "${aa}bbb${cc}";
+
+  # CHECK: ExprString: "aabbcc"
+  s2 = "aabbcc";
+}

From 4e5a2136304f1c799d0468431259ea2e53625676 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Tue, 26 Sep 2023 23:02:31 +0800
Subject: [PATCH 17/24] nixd/Sema: lowering for indented attrs

---
 nixd/include/nixd/Sema/Lowering.h             |   2 +
 nixd/lib/Sema/Lowering.cpp                    | 112 ++++++++++++++++++
 .../nixd-lint/test/lowering-indstring.nix     |  16 +++
 3 files changed, 130 insertions(+)
 create mode 100644 nixd/tools/nixd-lint/test/lowering-indstring.nix

diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index fa50113f0..befe13d65 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -69,6 +69,8 @@ struct Lowering {
   constexpr static std::string_view Div = "__div";
   constexpr static std::string_view CurPos = "__curPos";
 
+  nix::Expr *stripIndentation(const syntax::IndStringParts &ISP);
+
   nix::ExprVar *mkVar(std::string_view Sym) {
     return mkVar(STable.create(Sym));
   }
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 3265bdb13..a2b07cba2 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -504,6 +504,114 @@ nix::Expr *Lowering::lowerOp(const syntax::Node *Op) {
   return nullptr;
 }
 
+nix::Expr *Lowering::stripIndentation(const syntax::IndStringParts &ISP) {
+  // Note: this function is adopted from the offical implementation, see the
+  // comments there for what they want to do.
+  // @stripIndentation,  src/libexpr/parser.y
+  if (ISP.SubStrings.empty())
+    return Ctx.Pool.record(new nix::ExprString(""));
+
+  bool AtStartOfLine = true;
+  std::size_t MinIndent = 1000000;
+  std::size_t CurIndent = 0;
+  for (Node *SubStr : ISP.SubStrings) {
+    auto *IndStr = dynamic_cast<syntax::IndString *>(SubStr);
+    if (!IndStr || !IndStr->HasIndentation) {
+      if (AtStartOfLine) {
+        AtStartOfLine = false;
+        MinIndent = std::min(CurIndent, MinIndent);
+      }
+      continue;
+    }
+    for (char C : IndStr->S) {
+      if (AtStartOfLine) {
+        if (C == ' ') {
+          CurIndent++;
+        } else if (C == '\n') {
+          CurIndent = 0;
+        } else {
+          AtStartOfLine = false;
+          MinIndent = std::min(CurIndent, MinIndent);
+        }
+      } else if (C == '\n') {
+        AtStartOfLine = true;
+        CurIndent = 0;
+      }
+    }
+  }
+
+  auto NewES = new EvalContext::ES();
+  AtStartOfLine = true;
+  std::size_t CurDropped = 0;
+  auto I = ISP.SubStrings.begin();
+  auto N = ISP.SubStrings.size();
+
+  const auto TrimExpr = [&](nix::Expr *E) {
+    AtStartOfLine = false;
+    CurDropped = 0;
+    NewES->emplace_back(std::pair{(*I)->Range.Begin, E});
+  };
+
+  const auto TrimString = [&](std::string &Before) {
+    std::string After;
+    for (char C : Before) {
+      if (AtStartOfLine) {
+        if (C == ' ') {
+          if (CurDropped >= MinIndent)
+            After += C;
+        } else if (C == '\n') {
+          CurDropped = 0;
+          After += C;
+        } else {
+          AtStartOfLine = false;
+          CurDropped = 0;
+          After += C;
+        }
+      } else {
+        After += C;
+        if (C == '\n')
+          AtStartOfLine = true;
+      }
+    }
+
+    if (N == 1) {
+      auto P = After.find_last_of('\n');
+      if (P != std::string::npos &&
+          After.find_first_not_of(' ', P + 1) == std::string::npos)
+        After = std::string(After, 0, P + 1);
+    }
+
+    NewES->emplace_back((*I)->Range.Begin,
+                        Ctx.Pool.record(new nix::ExprString(std::move(After))));
+  };
+
+  for (; I != ISP.SubStrings.end(); ++I, --N) {
+    switch ((*I)->getKind()) {
+    case Node::NK_IndString: {
+      auto *IndStr = dynamic_cast<syntax::IndString *>(*I);
+      TrimString(IndStr->S);
+      break;
+    }
+    case Node::NK_InterpExpr: {
+      auto *IE = dynamic_cast<syntax::InterpExpr *>(*I);
+      TrimExpr(lower(IE->Body));
+      break;
+    }
+    default:
+      llvm_unreachable("encountered neither of string nor interpolated expre "
+                       "in indented strings!");
+    }
+  }
+
+  if (NewES->size() == 1) {
+    if (auto *EStr = dynamic_cast<nix::ExprString *>((*NewES)[0].second)) {
+      return EStr;
+    }
+  }
+  return Ctx.Pool.record(
+      new nix::ExprConcatStrings(ISP.Range.Begin, /*forceString=*/true, NewES));
+}
+
 nix::Expr *Lowering::lower(const syntax::Node *Root) {
   if (!Root)
     return nullptr;
@@ -632,6 +740,10 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
     const auto *IE = dynamic_cast<const syntax::InterpExpr *>(Root);
     return lower(IE->Body);
   }
+  case Node::NK_IndStringParts: {
+    const auto *ISP = dynamic_cast<const syntax::IndStringParts *>(Root);
+    return stripIndentation(*ISP);
+  }
 
   } // switch
 
diff --git a/nixd/tools/nixd-lint/test/lowering-indstring.nix b/nixd/tools/nixd-lint/test/lowering-indstring.nix
new file mode 100644
index 000000000..ee51ca832
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-indstring.nix
@@ -0,0 +1,16 @@
+# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s
+
+{
+  # CHECK: ExprString: "asdada\nasdasd\n"
+  foo = ''
+        asdada
+        asdasd
+        '';
+
+  # CHECK: ExprConcatStrings: ("asdasd\nasdasdasd\nasdasdasd\n" + a)
+  bar = ''
+asdasd
+asdasdasd
+asdasdasd
+${a}'';
+}

From 03f3fbda36d8741e71832c13ed1d07a5ccc4d8fc Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Tue, 26 Sep 2023 23:47:53 +0800
Subject: [PATCH 18/24] nixd/Sema: lowering paths

---
 nixd/include/nixd/Sema/Lowering.h           |  4 +++
 nixd/lib/Sema/Lowering.cpp                  | 32 +++++++++++++++++++++
 nixd/tools/nixd-lint/nixd-lint.cpp          |  6 +++-
 nixd/tools/nixd-lint/test/lowering-path.nix | 13 +++++++++
 4 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 nixd/tools/nixd-lint/test/lowering-path.nix

diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index befe13d65..684fcbc6f 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -4,6 +4,7 @@
 #include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Nodes.h"
 
+#include <nix/input-accessor.hh>
 #include <nix/nixexpr.hh>
 #include <nix/symbol-table.hh>
 
@@ -55,6 +56,7 @@ struct Lowering {
   nix::PosTable &PTable;
   std::vector<syntax::Diagnostic> &Diags;
   EvalContext &Ctx;
+  const nix::SourcePath &BasePath;
 
   nix::Expr *lower(const syntax::Node *Root);
   nix::ExprLambda *lowerFunction(const syntax::Function *Fn);
@@ -68,6 +70,8 @@ struct Lowering {
   constexpr static std::string_view Mul = "__mul";
   constexpr static std::string_view Div = "__div";
   constexpr static std::string_view CurPos = "__curPos";
+  constexpr static std::string_view FindFile = "__findFile";
+  constexpr static std::string_view NixPath = "__nixPath";
 
   nix::Expr *stripIndentation(const syntax::IndStringParts &ISP);
 
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index a2b07cba2..7b1cb0576 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -3,6 +3,7 @@
 #include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Nodes.h"
 #include "nixd/Syntax/Range.h"
+#include "util.hh"
 
 #include <nix/nixexpr.hh>
 
@@ -744,6 +745,37 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
     const auto *ISP = dynamic_cast<const syntax::IndStringParts *>(Root);
     return stripIndentation(*ISP);
   }
+  case Node::NK_Path: {
+    const auto *Path = dynamic_cast<const syntax::Path *>(Root);
+    const std::string &S = Path->S;
+
+    // conformance hack for offical parser.
+    // add back in the trailing '/' to the first segment
+    nix::Path NixPath(nix::absPath(S, BasePath.path.abs()));
+    if (S.length() > 1 && S.back() == '/')
+      NixPath += "/";
+
+    return Ctx.Pool.record(new nix::ExprPath(std::move(NixPath)));
+  }
+  case Node::NK_SPath: {
+    // <nixpkgs> -> (__findPath __nixPath nixpkgs)
+    const auto *SPath = dynamic_cast<const syntax::SPath *>(Root);
+
+    // strip '<' '>'
+    std::string Path = SPath->S.substr(1, SPath->S.length() - 2);
+
+    nix::PosIdx Pos = SPath->Range.Begin;
+    nix::ExprVar *Fn = mkVar(FindFile);
+    nix::ExprVar *NP = mkVar(NixPath);
+    auto *Arg = Ctx.Pool.record(new nix::ExprString(std::move(Path)));
+    return Ctx.Pool.record(new nix::ExprCall(Pos, Fn, {NP, Arg}));
+  }
+  case Node::NK_HPath: {
+    // HPath,  ~/. -> $HOME
+    const auto *HPath = dynamic_cast<const syntax::HPath *>(Root);
+    nix::Path Path(nix::getHome() + HPath->S.substr(1, HPath->S.length() - 1));
+    return Ctx.Pool.record(new nix::ExprPath(std::move(Path)));
+  }
 
   } // switch
 
diff --git a/nixd/tools/nixd-lint/nixd-lint.cpp b/nixd/tools/nixd-lint/nixd-lint.cpp
index b52192c6c..9e005a759 100644
--- a/nixd/tools/nixd-lint/nixd-lint.cpp
+++ b/nixd/tools/nixd-lint/nixd-lint.cpp
@@ -149,7 +149,11 @@ int main(int argc, char *argv[]) {
 
   nixd::EvalContext Ctx;
   nixd::Lowering Lowering{
-      .STable = *STable, .PTable = *PTable, .Diags = Data.Diags, .Ctx = Ctx};
+      .STable = *STable,
+      .PTable = *PTable,
+      .Diags = Data.Diags,
+      .Ctx = Ctx,
+      .BasePath = nix::SourcePath(nix::CanonPath(BasePath.string()))};
   nix::Expr *NixTree = Lowering.lower(Data.Result);
 
   if (DumpNixAST) {
diff --git a/nixd/tools/nixd-lint/test/lowering-path.nix b/nixd/tools/nixd-lint/test/lowering-path.nix
new file mode 100644
index 000000000..90c59829e
--- /dev/null
+++ b/nixd/tools/nixd-lint/test/lowering-path.nix
@@ -0,0 +1,13 @@
+# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s
+
+{
+  # CHECK: lowering-path.nix
+  a = ./.;
+
+  # CHECK: ExprCall: (__findFile __nixPath "nixpkgs")
+  spath = <nixpkgs>;
+
+  # CHECK-NOT: ~/foo.nix
+  # CHECK: foo.nix
+  hpath = ~/foo.nix;
+}

From 1dadc608ca10c708b577f090c49722c384a020b3 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Wed, 27 Sep 2023 00:05:18 +0800
Subject: [PATCH 19/24] nixd/Sema: misc nodes, add llvm_unreachable

---
 nixd/lib/Sema/Lowering.cpp               | 30 ++++++++++++++++++++++++
 nixd/tools/nixd-lint/test/legacy-let.nix |  2 ++
 2 files changed, 32 insertions(+)

diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 7b1cb0576..3f0a3db01 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -668,6 +668,16 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
     auto *Ret = new nix::ExprLet(Attrs, Body);
     return Ret;
   }
+  case Node::NK_LegacyLet: {
+    // let { ..., .body = ... } -> rec { ..., body = ... }.body
+    const auto *LegacyLet = dynamic_cast<const syntax::LegacyLet *>(Root);
+    ExprAttrsBuilder Builder(*this, LegacyLet->AttrBinds->Range,
+                             /*Recursive=*/true, /*IsLet=*/false);
+    Builder.addBinds(*LegacyLet->AttrBinds);
+    nix::ExprAttrs *Attrs = Builder.finish();
+    return Ctx.Pool.record(
+        new nix::ExprSelect(nix::noPos, Attrs, STable.create("body")));
+  }
   case Node::NK_If: {
     const auto *If = dynamic_cast<const syntax::If *>(Root);
     auto *Cond = lower(If->Cond);
@@ -776,6 +786,26 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
     nix::Path Path(nix::getHome() + HPath->S.substr(1, HPath->S.length() - 1));
     return Ctx.Pool.record(new nix::ExprPath(std::move(Path)));
   }
+  case Node::NK_List: {
+    // [ a b c ]
+    const auto *List = dynamic_cast<const syntax::List *>(Root);
+    auto *Ret = Ctx.Pool.record(new nix::ExprList);
+    for (Node *Elem : List->Body->Elems) {
+      Ret->elems.emplace_back(lower(Elem));
+    }
+    return Ret;
+  }
+  case Node::NK_URI: {
+    const auto *URI = dynamic_cast<const syntax::URI *>(Root);
+    auto *NixString = new nix::ExprString(std::string(URI->S));
+    return Ctx.Pool.record(NixString);
+  }
+  case Node::NK_Braced: {
+    const auto *Braced = dynamic_cast<const syntax::Braced *>(Root);
+    return lower(Braced->Body);
+  }
+  default:
+    llvm_unreachable("don't know how to lowering this node!");
 
   } // switch
 
diff --git a/nixd/tools/nixd-lint/test/legacy-let.nix b/nixd/tools/nixd-lint/test/legacy-let.nix
index 84c4fe578..498691a06 100644
--- a/nixd/tools/nixd-lint/test/legacy-let.nix
+++ b/nixd/tools/nixd-lint/test/legacy-let.nix
@@ -1,5 +1,7 @@
 # RUN: nixd-lint %s | FileCheck %s
+# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s --check-prefix=AST
 
+# AST: (rec { a = 1; }).body
 let { a = 1; } # CHECK:  using deprecated `let'
 
 ; # CHECK: syntax error, unexpected ';', expecting end of file

From 2cd3fe88b4acf638ae69044cf225a4c74b93befc Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Wed, 27 Sep 2023 00:18:44 +0800
Subject: [PATCH 20/24] nixd/Syntax: use Binds for syntax nodes

---
 nixd/include/nixd/Syntax/Nodes.inc |  4 ++--
 nixd/lib/Sema/Lowering.cpp         | 14 +++++++-------
 nixd/lib/Syntax/Parser/Parser.y    |  6 +++---
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/nixd/include/nixd/Syntax/Nodes.inc b/nixd/include/nixd/Syntax/Nodes.inc
index b6ee215c6..47c5dd06a 100644
--- a/nixd/include/nixd/Syntax/Nodes.inc
+++ b/nixd/include/nixd/Syntax/Nodes.inc
@@ -171,13 +171,13 @@ NODE(Braced, {
 })
 
 NODE(AttrSet, {
-  Binds *AttrBinds;
+  syntax::Binds *Binds;
   bool Recursive;
   COMMON_METHOD
 })
 
 NODE(LegacyLet, {
-  Binds *AttrBinds;
+  syntax::Binds *Binds;
   COMMON_METHOD
 })
 
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 3f0a3db01..30471a559 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -213,8 +213,8 @@ void ExprAttrsBuilder::addAttrSet(const syntax::AttrSet &AS) {
     auto Diag = mkRecDiag(Range, AS.Range, Recursive, AS.Recursive);
     LW.Diags.emplace_back(std::move(Diag));
   }
-  assert(AS.AttrBinds && "binds should not be empty! parser error?");
-  addBinds(*AS.AttrBinds);
+  assert(AS.Binds && "binds should not be empty! parser error?");
+  addBinds(*AS.Binds);
 }
 
 void ExprAttrsBuilder::addAttr(const syntax::Node *Attr,
@@ -246,7 +246,7 @@ void ExprAttrsBuilder::addAttr(const syntax::Node *Attr,
         const auto *BodyAttrSet = dynamic_cast<const syntax::AttrSet *>(Body);
         Nested[Sym] = std::make_unique<ExprAttrsBuilder>(
             LW, BodyAttrSet->Range, BodyAttrSet->Recursive, IsLet);
-        Nested[Sym]->addBinds(*BodyAttrSet->AttrBinds);
+        Nested[Sym]->addBinds(*BodyAttrSet->Binds);
       } else {
         nix::ExprAttrs::AttrDef Def(LW.lower(Body), Attr->Range.Begin);
         Result->attrs.insert({Sym, Def});
@@ -640,10 +640,10 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
   }
   case Node::NK_AttrSet: {
     const auto *AttrSet = dynamic_cast<const syntax::AttrSet *>(Root);
-    assert(AttrSet->AttrBinds && "null AttrBinds of the AttrSet!");
+    assert(AttrSet->Binds && "null Binds of the AttrSet!");
     ExprAttrsBuilder Builder(*this, AttrSet->Range, AttrSet->Recursive,
                              /*IsLet=*/false);
-    Builder.addBinds(*AttrSet->AttrBinds);
+    Builder.addBinds(*AttrSet->Binds);
     return Builder.finish();
   }
   case Node::NK_Int: {
@@ -671,9 +671,9 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
   case Node::NK_LegacyLet: {
     // let { ..., .body = ... } -> rec { ..., body = ... }.body
     const auto *LegacyLet = dynamic_cast<const syntax::LegacyLet *>(Root);
-    ExprAttrsBuilder Builder(*this, LegacyLet->AttrBinds->Range,
+    ExprAttrsBuilder Builder(*this, LegacyLet->Binds->Range,
                              /*Recursive=*/true, /*IsLet=*/false);
-    Builder.addBinds(*LegacyLet->AttrBinds);
+    Builder.addBinds(*LegacyLet->Binds);
     nix::ExprAttrs *Attrs = Builder.finish();
     return Ctx.Pool.record(
         new nix::ExprSelect(nix::noPos, Attrs, STable.create("body")));
diff --git a/nixd/lib/Syntax/Parser/Parser.y b/nixd/lib/Syntax/Parser/Parser.y
index 0fde0431e..b3ea9ec6e 100644
--- a/nixd/lib/Syntax/Parser/Parser.y
+++ b/nixd/lib/Syntax/Parser/Parser.y
@@ -288,7 +288,7 @@ expr_simple
   }
   | LET '{' binds '}' {
     auto N = decorateNode(new LegacyLet, *yylocp, *Data);
-    N->AttrBinds = $3;
+    N->Binds = $3;
     $$ = N;
 
     Diagnostic Diag;
@@ -299,13 +299,13 @@ expr_simple
   }
   | REC '{' binds '}' {
     auto N = decorateNode(new AttrSet, *yylocp, *Data);
-    N->AttrBinds = $3;
+    N->Binds = $3;
     N->Recursive = true;
     $$ = N;
   }
   | '{' binds '}' {
     auto N = decorateNode(new AttrSet, *yylocp, *Data);
-    N->AttrBinds = $2;
+    N->Binds = $2;
     N->Recursive = false;
     $$ = N;
   }

From de5cf366e7c84d3b6932dbedc2664a877e50100a Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Wed, 27 Sep 2023 10:20:34 +0800
Subject: [PATCH 21/24] nixd/Sema: general fixup, addressing review comments

---
 nixd/lib/Sema/Lowering.cpp | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index 30471a559..eb8d4aba7 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -3,9 +3,9 @@
 #include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Nodes.h"
 #include "nixd/Syntax/Range.h"
-#include "util.hh"
 
 #include <nix/nixexpr.hh>
+#include <nix/util.hh>
 
 #include <llvm/Support/ErrorHandling.h>
 #include <llvm/Support/FormatVariadic.h>
@@ -65,7 +65,7 @@ nix::ExprLambda *Lowering::lowerFunction(const syntax::Function *Fn) {
       if (Names.contains(Sym)) {
         // We have seen this symbol before, so this formal is duplicated
         const syntax::Formal *Previous = Names[Sym];
-        syntax::Diagnostic Diag;
+        Diagnostic Diag;
         Diag.Kind = syntax::Diagnostic::Error;
         Diag.Msg = "duplicated function formal declaration";
         Diag.Range = Formal->Range;
@@ -426,7 +426,7 @@ nix::Expr *Lowering::lowerOp(const syntax::Node *Op) {
     nix::PosIdx BPos = OpAdd->RHS->Range.Begin;
     nix::PosIdx Pos = OpAdd->OpRange.Begin;
     auto *ES = Ctx.ESPool.record(new EvalContext::ES{{APos, A}, {BPos, B}});
-    auto ECS = new nix::ExprConcatStrings(Pos, /*forceString=*/false, ES);
+    auto *ECS = new nix::ExprConcatStrings(Pos, /*forceString=*/false, ES);
     return Ctx.Pool.record(ECS);
   }
   case Node::NK_OpSub:
@@ -541,7 +541,7 @@ nix::Expr *Lowering::stripIndentation(const syntax::IndStringParts &ISP) {
     }
   }
 
-  auto NewES = new EvalContext::ES();
+  auto *NewES = new EvalContext::ES();
   AtStartOfLine = true;
   std::size_t CurDropped = 0;
   auto I = ISP.SubStrings.begin();
@@ -599,7 +599,7 @@ nix::Expr *Lowering::stripIndentation(const syntax::IndStringParts &ISP) {
       break;
     }
     default:
-      llvm_unreachable("encountered neither of string nor interpolated expre "
+      llvm_unreachable("encountered neither string nor interpolated expr"
                        "in indented strings!");
     }
   }
@@ -664,9 +664,7 @@ nix::Expr *Lowering::lower(const syntax::Node *Root) {
     // recursive, and we should not mark `rec` to the evaluator, because the
     // official parser does not do this also.
     Attrs->recursive = false;
-
-    auto *Ret = new nix::ExprLet(Attrs, Body);
-    return Ret;
+    return Ctx.Pool.record(new nix::ExprLet(Attrs, Body));
   }
   case Node::NK_LegacyLet: {
     // let { ..., .body = ... } -> rec { ..., body = ... }.body

From 8e431caae38e5100036f08e681b5ee0fa72a7b8c Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Mon, 16 Oct 2023 16:28:33 +0800
Subject: [PATCH 22/24] nixd/Basic: diagnostic declarations

---
 nixd/include/nixd/Basic/Diagnostic.h          |  99 +++++++++++++++
 nixd/include/nixd/Basic/DiagnosticKinds.inc   | 101 ++++++++++++++++
 nixd/include/nixd/Basic/DiagnosticMerge.inc   |  23 ++++
 nixd/include/nixd/{Syntax => Basic}/Range.h   |   0
 nixd/include/nixd/Sema/Lowering.h             |   4 +-
 nixd/include/nixd/Support/Position.h          |   2 +-
 nixd/include/nixd/Syntax/Diagnostic.h         |  33 -----
 nixd/include/nixd/Syntax/Nodes.h              |   2 +-
 nixd/include/nixd/Syntax/Parser/Require.h     |   4 +-
 nixd/lib/Basic/Diagnostic.cpp                 |  52 ++++++++
 nixd/lib/Basic/meson.build                    |  13 ++
 nixd/lib/Sema/Lowering.cpp                    | 114 +++++-------------
 nixd/lib/Sema/meson.build                     |   2 +-
 nixd/lib/Syntax/Lexer/Lexer.l                 |  18 +--
 nixd/lib/Syntax/Lexer/Prologue.cpp            |   3 +-
 nixd/lib/Syntax/Parser/Parser.y               |  21 +---
 nixd/lib/Syntax/Parser/Prologue.cpp           |   9 +-
 nixd/lib/Syntax/meson.build                   |   2 +-
 nixd/lib/meson.build                          |   1 +
 nixd/tools/nixd-lint/nixd-lint.cpp            |  50 ++++----
 .../nixd-lint/test/duplicated-fn-arg.nix      |   6 +-
 .../nixd-lint/test/lowering-attrset-rec.nix   |   2 +-
 22 files changed, 365 insertions(+), 196 deletions(-)
 create mode 100644 nixd/include/nixd/Basic/Diagnostic.h
 create mode 100644 nixd/include/nixd/Basic/DiagnosticKinds.inc
 create mode 100644 nixd/include/nixd/Basic/DiagnosticMerge.inc
 rename nixd/include/nixd/{Syntax => Basic}/Range.h (100%)
 delete mode 100644 nixd/include/nixd/Syntax/Diagnostic.h
 create mode 100644 nixd/lib/Basic/Diagnostic.cpp
 create mode 100644 nixd/lib/Basic/meson.build

diff --git a/nixd/include/nixd/Basic/Diagnostic.h b/nixd/include/nixd/Basic/Diagnostic.h
new file mode 100644
index 000000000..c5f92926e
--- /dev/null
+++ b/nixd/include/nixd/Basic/Diagnostic.h
@@ -0,0 +1,99 @@
+/// Diagnostic.h, diagnostic types and definitions
+///
+/// Diagnostics are structures with a main message,
+/// and optionally some additional information (body).
+///
+/// For diagnostics with a body,
+/// they may need a special overrided function to format the message.
+///
+#pragma once
+
+#include "Range.h"
+#include <optional>
+#include <string>
+
+namespace nixd {
+
+/// The super class for all diagnostics.
+/// concret diagnostic types are defined in Diagnostic*.inc
+struct Diagnostic {
+
+  /// Location of this diagnostic
+  RangeIdx Range;
+
+  Diagnostic(RangeIdx Range) : Range(Range) {}
+
+  /// Each diagnostic contains a severity field,
+  /// 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 eval the code.
+  /// "Warning" -- just a warning.
+  /// "Note"    -- some additional information about the error.
+  enum Severity { DS_Fatal, DS_Error, DS_Warning, DS_Note };
+
+  /// Internal diagnostic kind
+  enum Kind {
+#define DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE) DK_##CNAME,
+#include "DiagnosticMerge.inc"
+#undef DIAG_ALL
+  };
+
+  [[nodiscard]] virtual Kind kind() const = 0;
+
+  [[nodiscard]] virtual Severity severity() const = 0;
+
+  /// Format a printable diagnostic
+  [[nodiscard]] virtual std::string_view format() const = 0;
+
+  /// Short name, switch name.
+  /// There might be a human readable short name that controls the diagnostic
+  /// For example, one may pass -Wno-dup-formal to suppress duplicated formals.
+  /// A special case for parsing errors, generated from bison
+  /// have the sname "bison"
+  [[nodiscard]] virtual const char *sname() const = 0;
+
+  virtual ~Diagnostic() = default;
+};
+
+struct NotableDiagnostic : Diagnostic {
+  NotableDiagnostic(RangeIdx Range) : Diagnostic(Range) {}
+  using NotesTy = std::vector<std::unique_ptr<Diagnostic>>;
+  NotesTy Notes;
+  NotesTy &notes() { return Notes; }
+};
+
+#define DIAG_SIMPLE(SNAME, CNAME, SEVERITY, MESSAGE)                           \
+  struct Diag##CNAME : Diagnostic {                                            \
+    Diag##CNAME(RangeIdx Range) : Diagnostic(Range) {}                         \
+    std::string_view format() const override { return MESSAGE; }               \
+    const char *sname() const override { return SNAME; }                       \
+    Severity severity() const override { return DS_##SEVERITY; }               \
+    Kind kind() const override { return DK_##CNAME; }                          \
+  };
+#include "DiagnosticKinds.inc"
+#undef DIAG_SIMPLE
+
+#define DIAG_NOTE(SNAME, CNAME, SEVERITY, MESSAGE)                             \
+  struct Diag##CNAME : NotableDiagnostic {                                     \
+    Diag##CNAME(RangeIdx Range) : NotableDiagnostic(Range) {}                  \
+    std::string_view format() const override { return MESSAGE; }               \
+    const char *sname() const override { return SNAME; }                       \
+    Severity severity() const override { return DS_##SEVERITY; }               \
+    Kind kind() const override { return DK_##CNAME; }                          \
+  };
+#include "DiagnosticKinds.inc"
+#undef DIAG_NOTE
+
+#define DIAG_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                       \
+  struct Diag##CNAME : Diagnostic BODY;
+#include "DiagnosticKinds.inc"
+#undef DIAG_BODY
+
+#define DIAG_NOTE_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                  \
+  struct Diag##CNAME : NotableDiagnostic BODY;
+#include "DiagnosticKinds.inc"
+#undef DIAG_NOTE_BODY
+
+} // namespace nixd
diff --git a/nixd/include/nixd/Basic/DiagnosticKinds.inc b/nixd/include/nixd/Basic/DiagnosticKinds.inc
new file mode 100644
index 000000000..92df0ccff
--- /dev/null
+++ b/nixd/include/nixd/Basic/DiagnosticKinds.inc
@@ -0,0 +1,101 @@
+/// DiagnosticKinds.inc, diagnostic declarations
+
+// provides: DIAG_SIMPLE(SNAME, CNAME, SEVERITY, MESSAGE)
+// provides: DIAG_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)
+// provides: DIAG_NOTE(SNAME, CNAME, SEVERITY, MESSAGE)
+// provides: DIAG_NOTE_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)
+
+//=============================================================================/
+// DIAG_SIMPLE(SName, ClassName, Severity, Message)
+// "simple" means they have no additional body
+//=============================================================================/
+
+#ifdef DIAG_SIMPLE
+DIAG_SIMPLE("note-prev", PrevDeclared, Note, "previously declared here")
+DIAG_SIMPLE("let-dynamic", LetDynamic, Error,
+            "dynamic attributes are not allowed in let ... in ... expression")
+DIAG_SIMPLE("inherit-dynamic", InheritDynamic, Error,
+            "dynamic attributes are not allowed in inherit")
+DIAG_SIMPLE("empty-inherit", EmptyInherit, Warning, "empty inherit expression")
+DIAG_SIMPLE("or-identifier", OrIdentifier, Warning,
+            "keyword `or` used as an identifier")
+DIAG_SIMPLE("deprecated-url-literal", DeprecatedURL, Warning,
+            "URL literal is deprecated")
+DIAG_SIMPLE("deprecated-let", DeprecatedLet, Warning,
+            "using deprecated `let' syntactic sugar `let {..., body = ...}' -> "
+            "(rec {..., body = ...}).body'")
+DIAG_SIMPLE("path-trailing-slash", PathTrailingSlash, Fatal,
+            "path has a trailing slash")
+#endif // DIAG_SIMPLE
+
+//=============================================================================/
+// DIAG_NOTE(SName, ClassName, Severity, Message)
+// "simple", but with additional note support
+//=============================================================================/
+#ifdef DIAG_NOTE
+DIAG_NOTE("dup-formal", DuplicatedFormal, Error,
+          "duplicated function formal declaration")
+DIAG_NOTE("dup-formal-arg", DuplicatedFormalToArg, Error,
+          "function argument duplicated to a function formal")
+DIAG_NOTE("merge-diff-rec", MergeDiffRec, Warning,
+          "merging two attributes with different `rec` modifiers, the latter "
+          "will be implicitly ignored")
+#endif // DIAG_NOTE
+
+#define COMMON_METHOD                                                          \
+  Severity severity() const override;                                          \
+  Kind kind() const override;                                                  \
+  std::string_view format() const override;                                    \
+  const char *sname() const override;                                          \
+  static constexpr const char *message();
+
+//=============================================================================/
+// DIAG_BODY(SName, ClassName, Severity, Message, Body)
+// diagnostics with special body
+//=============================================================================/
+#ifdef DIAG_BODY
+DIAG_BODY("bison", BisonParse, Fatal, "{0}", {
+  DiagBisonParse(RangeIdx Range, std::string Str);
+  std::string Str;
+  COMMON_METHOD
+})
+DIAG_BODY("merge-diff-rec-this-rec", ThisRecursive, Note,
+          "this attribute set is {0}recursive", {
+            DiagThisRecursive(RangeIdx Range, bool Recursive);
+            std::string Str;
+            COMMON_METHOD
+          })
+DIAG_BODY("merge-diff-rec-consider", RecConsider, Note,
+          "while this attribute set is marked as {0}recursive, it "
+          "will be considered as {1}recursive",
+          {
+            DiagRecConsider(RangeIdx Range, bool MarkedRecursive);
+            std::string Str;
+            COMMON_METHOD
+          })
+DIAG_BODY("invalid-float", InvalidFloat, Note, "invalid float {0}", {
+  DiagInvalidFloat(RangeIdx Range, std::string_view Content);
+  std::string Str;
+  COMMON_METHOD
+})
+DIAG_BODY("invalid-integer", InvalidInteger, Note, "invalid integer {0}", {
+  DiagInvalidInteger(RangeIdx Range, std::string_view Content);
+  std::string Str;
+  COMMON_METHOD
+})
+#endif // DIAG_BODY
+
+//=============================================================================/
+// DIAG_NOTE_BODY(SName, ClassName, Severity, Message, Body)
+// diagnostics with special body, but with additional note support
+//=============================================================================/
+#ifdef DIAG_NOTE_BODY
+DIAG_NOTE_BODY("dup-formal-arg", DuplicatedAttr, Error, "duplicated attr `{0}`",
+               {
+                 DiagDuplicatedAttr(RangeIdx Range, std::string AttrName);
+                 std::string Str;
+                 COMMON_METHOD
+               })
+#endif // DIAG_NOTE_BODY
+
+#undef COMMON_METHOD
diff --git a/nixd/include/nixd/Basic/DiagnosticMerge.inc b/nixd/include/nixd/Basic/DiagnosticMerge.inc
new file mode 100644
index 000000000..b5df7b926
--- /dev/null
+++ b/nixd/include/nixd/Basic/DiagnosticMerge.inc
@@ -0,0 +1,23 @@
+// DiagnosticMerge.inc, merge all declarations
+
+// provides: DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE)
+
+#ifdef DIAG_ALL
+
+#define DIAG_SIMPLE(SNAME, CNAME, SEVERITY, MESSAGE)                           \
+  DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE)
+#define DIAG_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                       \
+  DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE)
+#define DIAG_NOTE_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                  \
+  DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE)
+#define DIAG_NOTE(SNAME, CNAME, SEVERITY, MESSAGE)                             \
+  DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE)
+
+#include "DiagnosticKinds.inc"
+
+#undef DIAG_NOTE
+#undef DIAG_NOTE_BODY
+#undef DIAG_BODY
+#undef DIAG_SIMPLE
+
+#endif
diff --git a/nixd/include/nixd/Syntax/Range.h b/nixd/include/nixd/Basic/Range.h
similarity index 100%
rename from nixd/include/nixd/Syntax/Range.h
rename to nixd/include/nixd/Basic/Range.h
diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index 684fcbc6f..0614384b5 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -1,7 +1,7 @@
 #pragma once
 
+#include "nixd/Basic/Diagnostic.h"
 #include "nixd/Sema/EvalContext.h"
-#include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Nodes.h"
 
 #include <nix/input-accessor.hh>
@@ -54,7 +54,7 @@ class ExprAttrsBuilder {
 struct Lowering {
   nix::SymbolTable &STable;
   nix::PosTable &PTable;
-  std::vector<syntax::Diagnostic> &Diags;
+  std::vector<std::unique_ptr<Diagnostic>> &Diags;
   EvalContext &Ctx;
   const nix::SourcePath &BasePath;
 
diff --git a/nixd/include/nixd/Support/Position.h b/nixd/include/nixd/Support/Position.h
index f7ff86d73..88fe7d5e3 100644
--- a/nixd/include/nixd/Support/Position.h
+++ b/nixd/include/nixd/Support/Position.h
@@ -1,7 +1,7 @@
 #pragma once
 
+#include "nixd/Basic/Range.h"
 #include "nixd/Nix/PosAdapter.h"
-#include "nixd/Syntax/Range.h"
 
 #include "lspserver/Protocol.h"
 
diff --git a/nixd/include/nixd/Syntax/Diagnostic.h b/nixd/include/nixd/Syntax/Diagnostic.h
deleted file mode 100644
index 17f13fd19..000000000
--- a/nixd/include/nixd/Syntax/Diagnostic.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/// Nixd diagnostic types. Provide structured diagnostic with location, FixIt
-/// hints, annotation, and notes
-#pragma once
-
-#include "nixd/Syntax/Range.h"
-
-namespace nixd::syntax {
-
-// The note.
-struct Note {
-  nixd::RangeIdx Range;
-
-  std::string Msg;
-};
-
-struct Edit {
-  nixd::RangeIdx Range;
-
-  std::string NewText;
-};
-
-struct Diagnostic {
-  nixd::RangeIdx Range;
-
-  std::string Msg;
-
-  enum DiagKind { Warning, Error } Kind;
-
-  std::vector<Note> Notes;
-  std::vector<Edit> Edits;
-};
-
-} // namespace nixd::syntax
diff --git a/nixd/include/nixd/Syntax/Nodes.h b/nixd/include/nixd/Syntax/Nodes.h
index 44738962b..16d76d8ac 100644
--- a/nixd/include/nixd/Syntax/Nodes.h
+++ b/nixd/include/nixd/Syntax/Nodes.h
@@ -1,7 +1,7 @@
 /// FIXME: comment for this file.
 #pragma once
 
-#include "Range.h"
+#include "nixd/Basic/Range.h"
 
 namespace nixd::syntax {
 
diff --git a/nixd/include/nixd/Syntax/Parser/Require.h b/nixd/include/nixd/Syntax/Parser/Require.h
index 034a8886f..bf51c655f 100644
--- a/nixd/include/nixd/Syntax/Parser/Require.h
+++ b/nixd/include/nixd/Syntax/Parser/Require.h
@@ -1,7 +1,7 @@
 #pragma once
 
+#include "nixd/Basic/Diagnostic.h"
 #include "nixd/Support/GCPool.h"
-#include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Nodes.h"
 
 #include <cstddef>
@@ -26,7 +26,7 @@ struct ParseData {
 
   nixd::GCPool<Node> Nodes;
 
-  std::vector<Diagnostic> Diags;
+  std::vector<std::unique_ptr<Diagnostic>> Diags;
 };
 
 // Note: copied from
diff --git a/nixd/lib/Basic/Diagnostic.cpp b/nixd/lib/Basic/Diagnostic.cpp
new file mode 100644
index 000000000..c55361a83
--- /dev/null
+++ b/nixd/lib/Basic/Diagnostic.cpp
@@ -0,0 +1,52 @@
+#include "nixd/Basic/Diagnostic.h"
+#include "nixd/Basic/Range.h"
+
+#include "llvm/Support/FormatVariadic.h"
+
+namespace nixd {
+
+#define DIAG_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                       \
+  const char *Diag##CNAME::sname() const { return SNAME; }                     \
+  Diagnostic::Severity Diag##CNAME::severity() const { return DS_##SEVERITY; } \
+  Diagnostic::Kind Diag##CNAME::kind() const { return DK_##CNAME; }            \
+  constexpr const char *Diag##CNAME::message() { return MESSAGE; }             \
+  std::string_view Diag##CNAME::format() const { return Str; }
+#define DIAG_NOTE_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                  \
+  DIAG_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)
+#include "nixd/Basic/DiagnosticKinds.inc"
+#undef DIAG_BODY
+#undef DIAG_NOTE_BODY
+
+DiagBisonParse::DiagBisonParse(RangeIdx Range, std::string Str)
+    : Diagnostic(Range), Str(std::move(Str)) {}
+
+DiagDuplicatedAttr::DiagDuplicatedAttr(RangeIdx Range, std::string AttrName)
+    : NotableDiagnostic(Range) {
+  Str = llvm::formatv(message(), AttrName);
+}
+
+static const char *getPrefix(bool P) { return P ? "" : "non-"; }
+
+DiagThisRecursive::DiagThisRecursive(RangeIdx Range, bool Recursive)
+    : Diagnostic(Range) {
+  Str = llvm::formatv(message(), getPrefix(Recursive));
+}
+
+DiagRecConsider::DiagRecConsider(RangeIdx Range, bool MarkedRecursive)
+    : Diagnostic(Range) {
+  Str = llvm::formatv(message(), getPrefix(MarkedRecursive),
+                      getPrefix(!MarkedRecursive));
+}
+
+DiagInvalidInteger::DiagInvalidInteger(RangeIdx Range, std::string_view Content)
+    : Diagnostic(Range) {
+  Str = llvm::formatv(message(), Content);
+}
+
+DiagInvalidFloat::DiagInvalidFloat(RangeIdx Range,
+                                   const std::string_view Content)
+    : Diagnostic(Range) {
+  Str = llvm::formatv(message(), Content);
+}
+
+} // namespace nixd
diff --git a/nixd/lib/Basic/meson.build b/nixd/lib/Basic/meson.build
new file mode 100644
index 000000000..e94c002d7
--- /dev/null
+++ b/nixd/lib/Basic/meson.build
@@ -0,0 +1,13 @@
+libnixdBasicDeps = [ nix_all, llvm ]
+libnixdBasic = library('nixdBasic'
+, [ 'Diagnostic.cpp'
+  ]
+, include_directories: nixd_inc
+, dependencies: libnixdBasicDeps
+, install: true
+)
+
+nixdBasic = declare_dependency( include_directories: nixd_inc
+                            , dependencies: libnixdBasicDeps
+                            , link_with: libnixdBasic
+                            )
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index eb8d4aba7..bb94d549f 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -1,8 +1,8 @@
 #include "nixd/Sema/Lowering.h"
+#include "nixd/Basic/Diagnostic.h"
+#include "nixd/Basic/Range.h"
 #include "nixd/Sema/EvalContext.h"
-#include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Nodes.h"
-#include "nixd/Syntax/Range.h"
 
 #include <nix/nixexpr.hh>
 #include <nix/util.hh>
@@ -14,9 +14,7 @@
 
 namespace nixd {
 
-using syntax::Diagnostic;
 using syntax::Node;
-using syntax::Note;
 
 nix::Formal Lowering::lowerFormal(const syntax::Formal &Formal) {
   auto *Def = Lowering::lower(Formal.Default);
@@ -65,16 +63,9 @@ nix::ExprLambda *Lowering::lowerFunction(const syntax::Function *Fn) {
       if (Names.contains(Sym)) {
         // We have seen this symbol before, so this formal is duplicated
         const syntax::Formal *Previous = Names[Sym];
-        Diagnostic Diag;
-        Diag.Kind = syntax::Diagnostic::Error;
-        Diag.Msg = "duplicated function formal declaration";
-        Diag.Range = Formal->Range;
-
-        syntax::Note PrevDef;
-        PrevDef.Msg = "previously declared here";
-        PrevDef.Range = Previous->Range;
-        Diag.Notes.emplace_back(std::move(PrevDef));
-
+        auto Diag = std::make_unique<DiagDuplicatedFormal>(Formal->Range);
+        auto PrevDefNote = std::make_unique<DiagPrevDeclared>(Previous->Range);
+        Diag->Notes.emplace_back(std::move(PrevDefNote));
         Diags.emplace_back(std::move(Diag));
       } else {
         Names[Sym] = Formal;
@@ -86,16 +77,9 @@ nix::ExprLambda *Lowering::lowerFunction(const syntax::Function *Fn) {
       auto ArgSym = Fn->Arg->Symbol;
       if (Names.contains(ArgSym)) {
         const syntax::Formal *Previous = Names[ArgSym];
-        syntax::Diagnostic Diag;
-        Diag.Kind = syntax::Diagnostic::Error;
-        Diag.Msg = "function argument duplicated to a function formal";
-        Diag.Range = Fn->Arg->Range;
-
-        syntax::Note PrevDef;
-        PrevDef.Msg = "duplicated to this formal";
-        PrevDef.Range = Previous->Range;
-        Diag.Notes.emplace_back(std::move(PrevDef));
-
+        auto Diag = std::make_unique<DiagDuplicatedFormalToArg>(Fn->Arg->Range);
+        auto PrevDefNote = std::make_unique<DiagPrevDeclared>(Previous->Range);
+        Diag->Notes.emplace_back(std::move(PrevDefNote));
         Diags.emplace_back(std::move(Diag));
       }
     }
@@ -122,17 +106,11 @@ nix::ExprLambda *Lowering::lowerFunction(const syntax::Function *Fn) {
   return NewLambda;
 }
 
-static syntax::Diagnostic mkAttrDupDiag(std::string Name, RangeIdx Range,
-                                        RangeIdx Prev) {
-  syntax::Diagnostic Diag;
-  Diag.Kind = syntax::Diagnostic::Error;
-  Diag.Msg = llvm::formatv("duplicated attr `{0}`", Name);
-  Diag.Range = Range;
-
-  syntax::Note Note;
-  Note.Msg = llvm::formatv("previously defined here");
-  Note.Range = Prev;
-  Diag.Notes.emplace_back(std::move(Note));
+static std::unique_ptr<Diagnostic>
+mkAttrDupDiag(const std::string &Name, RangeIdx Range, RangeIdx Prev) {
+  auto Diag = std::make_unique<DiagDuplicatedAttr>(Range, Name);
+  auto PrevDefNote = std::make_unique<DiagPrevDeclared>(Prev);
+  Diag->Notes.emplace_back(std::move(PrevDefNote));
   return Diag;
 }
 
@@ -173,44 +151,19 @@ void ExprAttrsBuilder::addBinds(const syntax::Binds &Binds) {
   }
 }
 
-static Diagnostic mkRecDiag(RangeIdx ThisRange, RangeIdx MergingRange,
-                            bool ThisRec, bool MergningRec) {
-  assert(ThisRec != MergningRec);
-
-  Diagnostic Diag;
-  Diag.Kind = Diagnostic::Warning;
-  Diag.Msg = "merging two attributes with different `rec` modifiers, the "
-             "latter will be implicitly ignored";
-  Diag.Range = MergingRange;
-
-  Note This;
-  This.Range = ThisRange;
-  This.Msg = llvm::formatv("this attribute set is{0} recursive",
-                           ThisRec ? "" : " not");
-  Diag.Notes.emplace_back(std::move(This));
-
-  Note Merging;
-  Merging.Range = MergingRange;
-  Merging.Msg =
-      llvm::formatv("while this attribute set is marked as {0}recursive, it "
-                    "will be considered as {1}recursive",
-                    MergningRec ? "" : "non-", ThisRec ? "" : "non-");
-  Diag.Notes.emplace_back(std::move(Merging));
-  return Diag;
-}
-
-static Diagnostic mkLetDynamicDiag(RangeIdx Range) {
-  Diagnostic Diag;
-  Diag.Kind = Diagnostic::Error;
-  Diag.Msg = "dynamic attributes are not allowed in let ... in ... expression";
-  Diag.Range = Range;
-
+static std::unique_ptr<DiagMergeDiffRec>
+mkRecDiag(RangeIdx ThisRange, RangeIdx MergingRange, bool ThisRec) {
+  auto Diag = std::make_unique<DiagMergeDiffRec>(MergingRange);
+  Diag->Notes.emplace_back(
+      std::make_unique<DiagThisRecursive>(ThisRange, ThisRec));
+  Diag->Notes.emplace_back(
+      std::make_unique<DiagRecConsider>(MergingRange, !ThisRec));
   return Diag;
 }
 
 void ExprAttrsBuilder::addAttrSet(const syntax::AttrSet &AS) {
   if (AS.Recursive != Recursive) {
-    auto Diag = mkRecDiag(Range, AS.Range, Recursive, AS.Recursive);
+    auto Diag = mkRecDiag(Range, AS.Range, Recursive);
     LW.Diags.emplace_back(std::move(Diag));
   }
   assert(AS.Binds && "binds should not be empty! parser error?");
@@ -220,7 +173,7 @@ void ExprAttrsBuilder::addAttrSet(const syntax::AttrSet &AS) {
 void ExprAttrsBuilder::addAttr(const syntax::Node *Attr,
                                const syntax::Node *Body, bool Recursive) {
   if (Recursive != this->Recursive) {
-    auto Diag = mkRecDiag(Range, Attr->Range, this->Recursive, Recursive);
+    auto Diag = mkRecDiag(Range, Attr->Range, this->Recursive);
     LW.Diags.emplace_back(std::move(Diag));
   }
   switch (Attr->getKind()) {
@@ -257,7 +210,7 @@ void ExprAttrsBuilder::addAttr(const syntax::Node *Attr,
   default: {
     if (IsLet) {
       // reject dynamic attrs in let expression
-      LW.Diags.emplace_back(mkLetDynamicDiag(Attr->Range));
+      LW.Diags.emplace_back(std::make_unique<DiagLetDynamic>(Attr->Range));
       return;
     }
     nix::Expr *NameExpr = LW.lower(Attr);
@@ -288,7 +241,7 @@ void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {
         if (!S->Nested.contains(Sym)) {
           // contains a field but the body is not an attr, then we cannot
           // perform merging.
-          Diagnostic Diag =
+          std::unique_ptr<Diagnostic> Diag =
               mkAttrDupDiag(LW.STable[Sym], ID->Range, S->Fields[Sym]->Range);
           LW.Diags.emplace_back(std::move(Diag));
           return;
@@ -307,7 +260,7 @@ void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {
     default: {
       if (IsLet) {
         // reject dynamic attrs in let expression
-        LW.Diags.emplace_back(mkLetDynamicDiag(Name->Range));
+        LW.Diags.emplace_back(std::make_unique<DiagLetDynamic>(Name->Range));
         return;
       }
       DynamicNested[Name] = std::make_unique<ExprAttrsBuilder>(
@@ -323,14 +276,8 @@ void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {
 }
 
 void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) {
-  if (IA.InheritedAttrs->Names.empty()) {
-    syntax::Diagnostic Diag;
-    Diag.Kind = syntax::Diagnostic::Warning;
-    Diag.Msg = "empty inherit expression";
-    Diag.Range = IA.Range;
-
-    LW.Diags.emplace_back(std::move(Diag));
-  }
+  if (IA.InheritedAttrs->Names.empty())
+    LW.Diags.emplace_back(std::make_unique<DiagEmptyInherit>(IA.Range));
 
   // inherit (expr) attr1 attr2
   // lowering `(expr)` to nix expression
@@ -350,12 +297,7 @@ void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) {
       break;
     }
     default: {
-      syntax::Diagnostic Diag;
-      Diag.Kind = syntax::Diagnostic::Error;
-      Diag.Msg = "dynamic attributes are not allowed in inherit";
-      Diag.Range = Name->Range;
-
-      LW.Diags.emplace_back(std::move(Diag));
+      LW.Diags.emplace_back(std::make_unique<DiagInheritDynamic>(Name->Range));
     }
     }
 
diff --git a/nixd/lib/Sema/meson.build b/nixd/lib/Sema/meson.build
index c50b309d1..39f5d638b 100644
--- a/nixd/lib/Sema/meson.build
+++ b/nixd/lib/Sema/meson.build
@@ -1,4 +1,4 @@
-libnixdSemaDeps = [ nixdSyntax ]
+libnixdSemaDeps = [ nixdSyntax, nixdBasic ]
 
 libnixdSema = library('nixdSema'
 , 'Lowering.cpp'
diff --git a/nixd/lib/Syntax/Lexer/Lexer.l b/nixd/lib/Syntax/Lexer/Lexer.l
index 2fe5c259c..29fcbd5f1 100644
--- a/nixd/lib/Syntax/Lexer/Lexer.l
+++ b/nixd/lib/Syntax/Lexer/Lexer.l
@@ -63,11 +63,7 @@ or          { return OR_KW; }
                 yylval->N = boost::lexical_cast<int64_t>(yytext);
               } catch (const boost::bad_lexical_cast &) {
                 auto Range = mkRange(*yylloc, *Data);
-                Diagnostic Diag;
-                Diag.Msg = llvm::formatv("invalid integer {0}", yytext);
-                Diag.Kind = Diagnostic::Error;
-                Diag.Range = Range;
-                Data->Diags.emplace_back(std::move(Diag));
+                Data->Diags.emplace_back(std::make_unique<nixd::DiagInvalidInteger>(Range, yytext));
                 yyterminate();
               }
               return INT;
@@ -77,11 +73,7 @@ or          { return OR_KW; }
               yylval->NF = strtod(yytext, 0);
               if (errno != 0) {
                 auto Range = mkRange(*yylloc, *Data);
-                Diagnostic Diag;
-                Diag.Msg = llvm::formatv("invalid float {0}", yytext);
-                Diag.Kind = Diagnostic::Error;
-                Diag.Range = Range;
-                Data->Diags.emplace_back(std::move(Diag));
+                Data->Diags.emplace_back(std::make_unique<nixd::DiagInvalidFloat>(Range, yytext));
                 yyterminate();
               }
               return FLOAT;
@@ -210,11 +202,7 @@ or          { return OR_KW; }
 <INPATH_SLASH>{ANY} |
 <INPATH_SLASH><<EOF>> {
   auto Range = mkRange(*yylloc, *Data);
-  Diagnostic Diag;
-  Diag.Msg = llvm::formatv("path has a trailing slash");
-  Diag.Kind = Diagnostic::Error;
-  Diag.Range = Range;
-  Data->Diags.emplace_back(std::move(Diag));
+  Data->Diags.emplace_back(std::make_unique<nixd::DiagPathTrailingSlash>(Range));
   yyterminate();
 }
 
diff --git a/nixd/lib/Syntax/Lexer/Prologue.cpp b/nixd/lib/Syntax/Lexer/Prologue.cpp
index 98ee05420..67b4294b3 100644
--- a/nixd/lib/Syntax/Lexer/Prologue.cpp
+++ b/nixd/lib/Syntax/Lexer/Prologue.cpp
@@ -1,7 +1,6 @@
 #include "Parser.tab.h"
 
 #include "nixd/Support/Position.h"
-#include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Nodes.h"
 #include "nixd/Syntax/Parser/Require.h"
 
@@ -10,7 +9,7 @@
 
 #include <boost/lexical_cast.hpp>
 
-using nixd::syntax::Diagnostic;
+using nixd::Diagnostic;
 
 #ifdef __clang__
 #pragma clang diagnostic ignored "-Wunneeded-internal-declaration"
diff --git a/nixd/lib/Syntax/Parser/Parser.y b/nixd/lib/Syntax/Parser/Parser.y
index b3ea9ec6e..06c9d1abb 100644
--- a/nixd/lib/Syntax/Parser/Parser.y
+++ b/nixd/lib/Syntax/Parser/Parser.y
@@ -290,12 +290,7 @@ expr_simple
     auto N = decorateNode(new LegacyLet, *yylocp, *Data);
     N->Binds = $3;
     $$ = N;
-
-    Diagnostic Diag;
-    Diag.Msg = "using deprecated `let' syntactic sugar `let {..., body = ...}' -> (rec {..., body = ...}).body'";
-    Diag.Kind = Diagnostic::Warning;
-    Diag.Range = N->Range;
-    Data->Diags.emplace_back(std::move(Diag));
+    Data->Diags.emplace_back(std::make_unique<nixd::DiagDeprecatedLet>(N->Range));
   }
   | REC '{' binds '}' {
     auto N = decorateNode(new AttrSet, *yylocp, *Data);
@@ -416,24 +411,14 @@ uri
   : URI {
     $$ = decorateNode(new nixd::syntax::URI, *yylocp, *Data);
     $$->S = std::string($1);
-
-    Diagnostic Diag;
-    Diag.Msg = "URL literal is deprecated";
-    Diag.Kind = Diagnostic::Warning;
-    Diag.Range = $$->Range;
-    Data->Diags.emplace_back(std::move(Diag));
+    Data->Diags.emplace_back(std::make_unique<nixd::DiagDeprecatedURL>($$->Range));
   }
 
 identifier_or
   : OR_KW {
     $$ = decorateNode(new Identifier, *yylocp, *Data);
     $$->Symbol = Data->State.Symbols.create("or");
-
-    Diagnostic Diag;
-    Diag.Msg = "keyword `or` used as an identifier";
-    Diag.Kind = Diagnostic::Warning;
-    Diag.Range = $$->Range;
-    Data->Diags.emplace_back(std::move(Diag));
+    Data->Diags.emplace_back(std::make_unique<nixd::DiagOrIdentifier>($$->Range));
   }
 
 var_or: identifier_or {
diff --git a/nixd/lib/Syntax/Parser/Prologue.cpp b/nixd/lib/Syntax/Parser/Prologue.cpp
index c1ff1246e..7cab05015 100644
--- a/nixd/lib/Syntax/Parser/Prologue.cpp
+++ b/nixd/lib/Syntax/Parser/Prologue.cpp
@@ -2,7 +2,7 @@
 
 #include "Lexer.tab.h"
 
-#include "nixd/Syntax/Diagnostic.h"
+#include "nixd/Basic/Diagnostic.h"
 #include "nixd/Syntax/Nodes.h"
 #include "nixd/Syntax/Parser/Require.h"
 
@@ -28,11 +28,8 @@ static nixd::RangeIdx mkRange(YYLTYPE YL, nixd::syntax::ParseData &Data) {
 void yyerror(YYLTYPE *YL, yyscan_t Scanner, nixd::syntax::ParseData *Data,
              const char *Error) {
   auto Range = mkRange(*YL, *Data);
-  Diagnostic Diag;
-  Diag.Msg = llvm::formatv("{0}", Error);
-  Diag.Kind = Diagnostic::Error;
-  Diag.Range = Range;
-  Data->Diags.emplace_back(std::move(Diag));
+  Data->Diags.emplace_back(
+      std::make_unique<nixd::DiagBisonParse>(Range, Error));
 }
 
 template <class T>
diff --git a/nixd/lib/Syntax/meson.build b/nixd/lib/Syntax/meson.build
index c00af5097..4550f573c 100644
--- a/nixd/lib/Syntax/meson.build
+++ b/nixd/lib/Syntax/meson.build
@@ -19,7 +19,7 @@ parser = custom_target('parser'
 )
 
 #------------------------------------------------------------------------------#
-libnixdSyntaxDeps = [ nix_all, nixdExpr, nixdSupport ]
+libnixdSyntaxDeps = [ nix_all, nixdBasic, nixdExpr, nixdSupport ]
 libnixdSyntaxInc = [ nixd_inc, './' ]
 libnixdSyntax = library('nixdSyntax'
 , lexer
diff --git a/nixd/lib/meson.build b/nixd/lib/meson.build
index 7d0f8f3e9..bede995f0 100644
--- a/nixd/lib/meson.build
+++ b/nixd/lib/meson.build
@@ -1,4 +1,5 @@
 subdir('Expr')
+subdir('Basic')
 subdir('Nix')
 subdir('Support')
 subdir('Syntax')
diff --git a/nixd/tools/nixd-lint/nixd-lint.cpp b/nixd/tools/nixd-lint/nixd-lint.cpp
index 9e005a759..1251f7e1a 100644
--- a/nixd/tools/nixd-lint/nixd-lint.cpp
+++ b/nixd/tools/nixd-lint/nixd-lint.cpp
@@ -1,7 +1,7 @@
+#include "nixd/Basic/Diagnostic.h"
 #include "nixd/Expr/Expr.h"
 #include "nixd/Sema/EvalContext.h"
 #include "nixd/Sema/Lowering.h"
-#include "nixd/Syntax/Diagnostic.h"
 #include "nixd/Syntax/Parser.h"
 #include "nixd/Syntax/Parser/Require.h"
 
@@ -66,7 +66,7 @@ static void printCodeLines(std::ostream &Out, const std::string &Prefix,
       if (BeginPos->line == EndPos->line) {
         Out << ANSI_RED;
         for (auto I = BeginPos->column + 1; I < EndPos->column; I++) {
-          Out << (I == EndPos->column - 1 ? "^" : "~");
+          Out << "~";
         }
         Out << ANSI_NORMAL;
       }
@@ -162,17 +162,17 @@ int main(int argc, char *argv[]) {
   }
 
   for (const auto &Diag : Data.Diags) {
-    auto BeginPos = (*PTable)[Diag.Range.Begin];
-    auto EndPos = (*PTable)[Diag.Range.End];
-    switch (Diag.Kind) {
-    case nixd::syntax::Diagnostic::Warning:
+    auto BeginPos = (*PTable)[Diag->Range.Begin];
+    auto EndPos = (*PTable)[Diag->Range.End];
+    switch (Diag->severity()) {
+    case nixd::Diagnostic::DS_Warning:
       std::cout << ANSI_WARNING "warning: " ANSI_NORMAL;
       break;
-    case nixd::syntax::Diagnostic::Error:
+    case nixd::Diagnostic::DS_Error:
       std::cout << ANSI_RED "error: " ANSI_NORMAL;
       break;
     }
-    std::cout << Diag.Msg << "\n";
+    std::cout << Diag->format() << "\n";
     if (BeginPos) {
       std::cout << "\n"
                 << ANSI_BLUE << "at " ANSI_WARNING << BeginPos << ANSI_NORMAL
@@ -185,22 +185,24 @@ int main(int argc, char *argv[]) {
       }
     }
 
-    for (const auto &Note : Diag.Notes) {
-      auto NoteBegin = (*PTable)[Note.Range.Begin];
-      auto NoteEnd = (*PTable)[Note.Range.End];
-      std::cout << "\n";
-      std::cout << ANSI_CYAN "note: " ANSI_NORMAL;
-      std::cout << Note.Msg << "\n";
-
-      if (NoteBegin) {
-        std::cout << "\n"
-                  << ANSI_BLUE << "at " ANSI_WARNING << NoteBegin << ANSI_NORMAL
-                  << ":";
-        if (auto Lines =
-                std::shared_ptr<nix::AbstractPos>(NoteBegin)->getCodeLines()) {
-          std::cout << "\n";
-          printCodeLines(std::cout, "", NoteBegin, NoteEnd, *Lines);
-          std::cout << "\n";
+    if (auto *ND = dynamic_cast<nixd::NotableDiagnostic *>(Diag.get())) {
+      for (const auto &Note : ND->Notes) {
+        auto NoteBegin = (*PTable)[Note->Range.Begin];
+        auto NoteEnd = (*PTable)[Note->Range.End];
+        std::cout << "\n";
+        std::cout << ANSI_CYAN "note: " ANSI_NORMAL;
+        std::cout << Note->format() << "\n";
+
+        if (NoteBegin) {
+          std::cout << "\n"
+                    << ANSI_BLUE << "at " ANSI_WARNING << NoteBegin
+                    << ANSI_NORMAL << ":";
+          if (auto Lines = std::shared_ptr<nix::AbstractPos>(NoteBegin)
+                               ->getCodeLines()) {
+            std::cout << "\n";
+            printCodeLines(std::cout, "", NoteBegin, NoteEnd, *Lines);
+            std::cout << "\n";
+          }
         }
       }
     }
diff --git a/nixd/tools/nixd-lint/test/duplicated-fn-arg.nix b/nixd/tools/nixd-lint/test/duplicated-fn-arg.nix
index cdfb592e5..f5bc93c97 100644
--- a/nixd/tools/nixd-lint/test/duplicated-fn-arg.nix
+++ b/nixd/tools/nixd-lint/test/duplicated-fn-arg.nix
@@ -2,9 +2,9 @@
 
 # CHECK: duplicated function formal declaration
 a @ { b
-# CHECK: previously declared here
+  # CHECK: previously declared here
 , a ? 1
-# CHECK: function argument duplicated to a function forma
+  # CHECK: function argument duplicated to a function forma
 , a
-# CHECK: duplicated to this formal
+  # CHECK: previously declared here
 }: { }
diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix b/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix
index bea6c7879..9dce853d5 100644
--- a/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix
+++ b/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix
@@ -4,7 +4,7 @@ rec {
 
   # CHECK: merging two attributes with different `rec` modifiers, the latter will be implicitly ignored
 
-  # CHECK: this attribute set is not recursive
+  # CHECK: this attribute set is non-recursive
   # CHECK: while this attribute set is marked as recursive, it will be considered as non-recursive
   foo = rec {
     x = 1;

From e6df00b4fe00d608caea47df8b420dd4261d7322 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Thu, 19 Oct 2023 01:22:34 +0800
Subject: [PATCH 23/24] fixup! nixd/Basic: diagnostic declarations

---
 nixd/include/nixd/Basic/DiagnosticMerge.inc | 2 +-
 nixd/lib/Basic/meson.build                  | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/nixd/include/nixd/Basic/DiagnosticMerge.inc b/nixd/include/nixd/Basic/DiagnosticMerge.inc
index b5df7b926..ab4da8e40 100644
--- a/nixd/include/nixd/Basic/DiagnosticMerge.inc
+++ b/nixd/include/nixd/Basic/DiagnosticMerge.inc
@@ -20,4 +20,4 @@
 #undef DIAG_BODY
 #undef DIAG_SIMPLE
 
-#endif
+#endif // DIAG_ALL
diff --git a/nixd/lib/Basic/meson.build b/nixd/lib/Basic/meson.build
index e94c002d7..10674b0d8 100644
--- a/nixd/lib/Basic/meson.build
+++ b/nixd/lib/Basic/meson.build
@@ -8,6 +8,6 @@ libnixdBasic = library('nixdBasic'
 )
 
 nixdBasic = declare_dependency( include_directories: nixd_inc
-                            , dependencies: libnixdBasicDeps
-                            , link_with: libnixdBasic
-                            )
+                              , dependencies: libnixdBasicDeps
+                              , link_with: libnixdBasic
+                              )

From 5745fbffe323d9e87928807d8ebc07eff6dbee20 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Thu, 19 Oct 2023 04:34:10 +0800
Subject: [PATCH 24/24] nixd/Basic: fmt-based diagnostics

---
 default.nix                                 |   2 +
 meson.build                                 |   1 +
 nixd/include/nixd/Basic/Diagnostic.h        | 161 +++++++++++++-------
 nixd/include/nixd/Basic/DiagnosticKinds.inc | 127 ++++-----------
 nixd/include/nixd/Basic/DiagnosticMerge.inc |  23 ---
 nixd/include/nixd/Basic/NoteKinds.inc       |  10 ++
 nixd/include/nixd/Sema/Lowering.h           |   2 +-
 nixd/include/nixd/Syntax/Parser/Require.h   |   2 +-
 nixd/lib/Basic/Diagnostic.cpp               |  71 ++++-----
 nixd/lib/Basic/meson.build                  |   2 +-
 nixd/lib/Sema/Lowering.cpp                  |  67 ++++----
 nixd/lib/Syntax/Lexer/Lexer.l               |   6 +-
 nixd/lib/Syntax/Parser/Parser.y             |   6 +-
 nixd/lib/Syntax/Parser/Prologue.cpp         |   5 +-
 nixd/tools/nixd-lint/nixd-lint.cpp          | 126 ++++++++-------
 nixd/tools/nixd-lint/test/legacy-let.nix    |   4 +-
 16 files changed, 300 insertions(+), 315 deletions(-)
 delete mode 100644 nixd/include/nixd/Basic/DiagnosticMerge.inc
 create mode 100644 nixd/include/nixd/Basic/NoteKinds.inc

diff --git a/default.nix b/default.nix
index d0e09641c..96728bbe7 100644
--- a/default.nix
+++ b/default.nix
@@ -3,6 +3,7 @@
 , bison
 , boost182
 , flex
+, fmt
 , gtest
 , libbacktrace
 , lit
@@ -41,6 +42,7 @@ stdenv.mkDerivation {
     gtest
     boost182
     llvmPackages.llvm
+    fmt
   ];
 
   env.CXXFLAGS = "-include ${nix.dev}/include/nix/config.h";
diff --git a/meson.build b/meson.build
index fd5d8c698..d0d4a91b0 100644
--- a/meson.build
+++ b/meson.build
@@ -31,6 +31,7 @@ gtest_main = dependency('gtest_main')
 
 llvm = dependency('llvm')
 boost = dependency('boost')
+fmt = dependency('fmt')
 
 cpp = meson.get_compiler('cpp')
 backtrace = cpp.find_library('backtrace')
diff --git a/nixd/include/nixd/Basic/Diagnostic.h b/nixd/include/nixd/Basic/Diagnostic.h
index c5f92926e..3512d5854 100644
--- a/nixd/include/nixd/Basic/Diagnostic.h
+++ b/nixd/include/nixd/Basic/Diagnostic.h
@@ -12,17 +12,67 @@
 #include <optional>
 #include <string>
 
+#include <fmt/args.h>
+
 namespace nixd {
 
-/// The super class for all diagnostics.
-/// concret diagnostic types are defined in Diagnostic*.inc
-struct Diagnostic {
+class PartialDiagnostic {
+public:
+  virtual const char *format() const {
+    if (Result.empty())
+      Result = fmt::vformat(message(), Args);
+    return Result.c_str();
+  };
+
+  [[nodiscard]] virtual const char *message() const = 0;
 
+  virtual ~PartialDiagnostic() = default;
+
+  template <class T> PartialDiagnostic &operator<<(const T &Var) {
+    Args.push_back(Var);
+    return *this;
+  }
+
+protected:
+  mutable std::string Result;
+  fmt::dynamic_format_arg_store<fmt::format_context> Args;
   /// Location of this diagnostic
   RangeIdx Range;
+};
+
+class Note : public PartialDiagnostic {
+public:
+  /// Internal kind
+  enum NoteKind {
+#define DIAG_NOTE(SNAME, CNAME, MESSAGE) NK_##CNAME,
+#include "NoteKinds.inc"
+#undef DIAG_NOTE
+  };
+
+  Note(RangeIdx Range, NoteKind Kind) : Range(Range), Kind(Kind) {}
+
+  template <class T> PartialDiagnostic &operator<<(const T &Var) {
+    Args.push_back(Var);
+    return *this;
+  }
+
+  NoteKind kind() const { return Kind; }
+
+  [[nodiscard]] static const char *message(NoteKind Kind);
 
-  Diagnostic(RangeIdx Range) : Range(Range) {}
+  [[nodiscard]] const char *message() const override { return message(kind()); }
 
+  RangeIdx range() const { return Range; }
+
+private:
+  RangeIdx Range;
+  NoteKind Kind;
+};
+
+/// The super class for all diagnostics.
+/// concret diagnostic types are defined in Diagnostic*.inc
+class Diagnostic : public PartialDiagnostic {
+public:
   /// Each diagnostic contains a severity field,
   /// should be "Fatal", "Error" or "Warning"
   /// this will affect the eval process.
@@ -31,69 +81,76 @@ struct Diagnostic {
   /// "Error"   -- trigger an error in nix, but we can eval the code.
   /// "Warning" -- just a warning.
   /// "Note"    -- some additional information about the error.
-  enum Severity { DS_Fatal, DS_Error, DS_Warning, DS_Note };
+  enum Severity { DS_Fatal, DS_Error, DS_Warning };
 
-  /// Internal diagnostic kind
-  enum Kind {
-#define DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE) DK_##CNAME,
-#include "DiagnosticMerge.inc"
-#undef DIAG_ALL
+  /// Internal kind
+  enum DiagnosticKind {
+#define DIAG(SNAME, CNAME, SEVERITY, MESSAGE) DK_##CNAME,
+#include "DiagnosticKinds.inc"
+#undef DIAG
   };
 
-  [[nodiscard]] virtual Kind kind() const = 0;
+  Diagnostic(RangeIdx Range, DiagnosticKind Kind) : Range(Range), Kind(Kind) {}
+
+  [[nodiscard]] DiagnosticKind kind() const { return Kind; };
 
-  [[nodiscard]] virtual Severity severity() const = 0;
+  [[nodiscard]] static Severity severity(DiagnosticKind Kind);
 
-  /// Format a printable diagnostic
-  [[nodiscard]] virtual std::string_view format() const = 0;
+  [[nodiscard]] static const char *message(DiagnosticKind Kind);
+
+  [[nodiscard]] const char *message() const override { return message(kind()); }
 
   /// Short name, switch name.
   /// There might be a human readable short name that controls the diagnostic
   /// For example, one may pass -Wno-dup-formal to suppress duplicated formals.
   /// A special case for parsing errors, generated from bison
   /// have the sname "bison"
-  [[nodiscard]] virtual const char *sname() const = 0;
+  [[nodiscard]] static const char *sname(DiagnosticKind Kind);
 
-  virtual ~Diagnostic() = default;
-};
+  [[nodiscard]] virtual const char *sname() const { return sname(kind()); }
 
-struct NotableDiagnostic : Diagnostic {
-  NotableDiagnostic(RangeIdx Range) : Diagnostic(Range) {}
-  using NotesTy = std::vector<std::unique_ptr<Diagnostic>>;
-  NotesTy Notes;
-  NotesTy &notes() { return Notes; }
-};
+  Note &note(RangeIdx Range, Note::NoteKind Kind) {
+    return *Notes.emplace_back(std::make_unique<Note>(Range, Kind));
+  }
 
-#define DIAG_SIMPLE(SNAME, CNAME, SEVERITY, MESSAGE)                           \
-  struct Diag##CNAME : Diagnostic {                                            \
-    Diag##CNAME(RangeIdx Range) : Diagnostic(Range) {}                         \
-    std::string_view format() const override { return MESSAGE; }               \
-    const char *sname() const override { return SNAME; }                       \
-    Severity severity() const override { return DS_##SEVERITY; }               \
-    Kind kind() const override { return DK_##CNAME; }                          \
-  };
-#include "DiagnosticKinds.inc"
-#undef DIAG_SIMPLE
-
-#define DIAG_NOTE(SNAME, CNAME, SEVERITY, MESSAGE)                             \
-  struct Diag##CNAME : NotableDiagnostic {                                     \
-    Diag##CNAME(RangeIdx Range) : NotableDiagnostic(Range) {}                  \
-    std::string_view format() const override { return MESSAGE; }               \
-    const char *sname() const override { return SNAME; }                       \
-    Severity severity() const override { return DS_##SEVERITY; }               \
-    Kind kind() const override { return DK_##CNAME; }                          \
-  };
-#include "DiagnosticKinds.inc"
-#undef DIAG_NOTE
+  std::vector<std::unique_ptr<Note>> &notes() { return Notes; }
 
-#define DIAG_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                       \
-  struct Diag##CNAME : Diagnostic BODY;
-#include "DiagnosticKinds.inc"
-#undef DIAG_BODY
+  RangeIdx range() const { return Range; }
 
-#define DIAG_NOTE_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                  \
-  struct Diag##CNAME : NotableDiagnostic BODY;
-#include "DiagnosticKinds.inc"
-#undef DIAG_NOTE_BODY
+private:
+  /// Location of this diagnostic
+  RangeIdx Range;
+
+  std::vector<std::unique_ptr<Note>> Notes;
+
+  DiagnosticKind Kind;
+};
+
+class DiagnosticEngine {
+public:
+  Diagnostic &diag(RangeIdx Range, Diagnostic::DiagnosticKind Kind) {
+    auto Diag = std::make_unique<Diagnostic>(Range, Kind);
+    switch (getServerity(Diag->kind())) {
+    case Diagnostic::DS_Fatal:
+      return *Fatals.emplace_back(std::move(Diag));
+    case Diagnostic::DS_Error:
+      return *Errors.emplace_back(std::move(Diag));
+    case Diagnostic::DS_Warning:
+      return *Warnings.emplace_back(std::move(Diag));
+    }
+  }
+
+  std::vector<std::unique_ptr<Diagnostic>> &warnings() { return Warnings; }
+  std::vector<std::unique_ptr<Diagnostic>> &errors() { return Errors; }
+  std::vector<std::unique_ptr<Diagnostic>> &fatals() { return Fatals; }
+
+private:
+  Diagnostic::Severity getServerity(Diagnostic::DiagnosticKind Kind) {
+    return Diagnostic::severity(Kind);
+  }
+  std::vector<std::unique_ptr<Diagnostic>> Warnings;
+  std::vector<std::unique_ptr<Diagnostic>> Errors;
+  std::vector<std::unique_ptr<Diagnostic>> Fatals;
+};
 
 } // namespace nixd
diff --git a/nixd/include/nixd/Basic/DiagnosticKinds.inc b/nixd/include/nixd/Basic/DiagnosticKinds.inc
index 92df0ccff..a37f6bea5 100644
--- a/nixd/include/nixd/Basic/DiagnosticKinds.inc
+++ b/nixd/include/nixd/Basic/DiagnosticKinds.inc
@@ -1,101 +1,30 @@
 /// DiagnosticKinds.inc, diagnostic declarations
 
-// provides: DIAG_SIMPLE(SNAME, CNAME, SEVERITY, MESSAGE)
-// provides: DIAG_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)
-// provides: DIAG_NOTE(SNAME, CNAME, SEVERITY, MESSAGE)
-// provides: DIAG_NOTE_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)
-
-//=============================================================================/
-// DIAG_SIMPLE(SName, ClassName, Severity, Message)
-// "simple" means they have no additional body
-//=============================================================================/
-
-#ifdef DIAG_SIMPLE
-DIAG_SIMPLE("note-prev", PrevDeclared, Note, "previously declared here")
-DIAG_SIMPLE("let-dynamic", LetDynamic, Error,
-            "dynamic attributes are not allowed in let ... in ... expression")
-DIAG_SIMPLE("inherit-dynamic", InheritDynamic, Error,
-            "dynamic attributes are not allowed in inherit")
-DIAG_SIMPLE("empty-inherit", EmptyInherit, Warning, "empty inherit expression")
-DIAG_SIMPLE("or-identifier", OrIdentifier, Warning,
-            "keyword `or` used as an identifier")
-DIAG_SIMPLE("deprecated-url-literal", DeprecatedURL, Warning,
-            "URL literal is deprecated")
-DIAG_SIMPLE("deprecated-let", DeprecatedLet, Warning,
-            "using deprecated `let' syntactic sugar `let {..., body = ...}' -> "
-            "(rec {..., body = ...}).body'")
-DIAG_SIMPLE("path-trailing-slash", PathTrailingSlash, Fatal,
-            "path has a trailing slash")
-#endif // DIAG_SIMPLE
-
-//=============================================================================/
-// DIAG_NOTE(SName, ClassName, Severity, Message)
-// "simple", but with additional note support
-//=============================================================================/
-#ifdef DIAG_NOTE
-DIAG_NOTE("dup-formal", DuplicatedFormal, Error,
-          "duplicated function formal declaration")
-DIAG_NOTE("dup-formal-arg", DuplicatedFormalToArg, Error,
-          "function argument duplicated to a function formal")
-DIAG_NOTE("merge-diff-rec", MergeDiffRec, Warning,
-          "merging two attributes with different `rec` modifiers, the latter "
-          "will be implicitly ignored")
-#endif // DIAG_NOTE
-
-#define COMMON_METHOD                                                          \
-  Severity severity() const override;                                          \
-  Kind kind() const override;                                                  \
-  std::string_view format() const override;                                    \
-  const char *sname() const override;                                          \
-  static constexpr const char *message();
-
-//=============================================================================/
-// DIAG_BODY(SName, ClassName, Severity, Message, Body)
-// diagnostics with special body
-//=============================================================================/
-#ifdef DIAG_BODY
-DIAG_BODY("bison", BisonParse, Fatal, "{0}", {
-  DiagBisonParse(RangeIdx Range, std::string Str);
-  std::string Str;
-  COMMON_METHOD
-})
-DIAG_BODY("merge-diff-rec-this-rec", ThisRecursive, Note,
-          "this attribute set is {0}recursive", {
-            DiagThisRecursive(RangeIdx Range, bool Recursive);
-            std::string Str;
-            COMMON_METHOD
-          })
-DIAG_BODY("merge-diff-rec-consider", RecConsider, Note,
-          "while this attribute set is marked as {0}recursive, it "
-          "will be considered as {1}recursive",
-          {
-            DiagRecConsider(RangeIdx Range, bool MarkedRecursive);
-            std::string Str;
-            COMMON_METHOD
-          })
-DIAG_BODY("invalid-float", InvalidFloat, Note, "invalid float {0}", {
-  DiagInvalidFloat(RangeIdx Range, std::string_view Content);
-  std::string Str;
-  COMMON_METHOD
-})
-DIAG_BODY("invalid-integer", InvalidInteger, Note, "invalid integer {0}", {
-  DiagInvalidInteger(RangeIdx Range, std::string_view Content);
-  std::string Str;
-  COMMON_METHOD
-})
-#endif // DIAG_BODY
-
-//=============================================================================/
-// DIAG_NOTE_BODY(SName, ClassName, Severity, Message, Body)
-// diagnostics with special body, but with additional note support
-//=============================================================================/
-#ifdef DIAG_NOTE_BODY
-DIAG_NOTE_BODY("dup-formal-arg", DuplicatedAttr, Error, "duplicated attr `{0}`",
-               {
-                 DiagDuplicatedAttr(RangeIdx Range, std::string AttrName);
-                 std::string Str;
-                 COMMON_METHOD
-               })
-#endif // DIAG_NOTE_BODY
-
-#undef COMMON_METHOD
+#ifdef DIAG
+DIAG("let-dynamic", LetDynamic, Error,
+     "dynamic attributes are not allowed in let ... in ... expression")
+DIAG("inherit-dynamic", InheritDynamic, Error,
+     "dynamic attributes are not allowed in inherit")
+DIAG("empty-inherit", EmptyInherit, Warning, "empty inherit expression")
+DIAG("or-identifier", OrIdentifier, Warning,
+     "keyword `or` used as an identifier")
+DIAG("deprecated-url-literal", DeprecatedURL, Warning,
+     "URL literal is deprecated")
+DIAG("deprecated-let", DeprecatedLet, Warning,
+     "using deprecated `let' syntactic sugar `let {{..., body = ...}}' -> "
+     "(rec {{..., body = ...}}).body'")
+DIAG("path-trailing-slash", PathTrailingSlash, Fatal,
+     "path has a trailing slash")
+DIAG("dup-formal", DuplicatedFormal, Error,
+     "duplicated function formal declaration")
+DIAG("dup-formal-arg", DuplicatedFormalToArg, Error,
+     "function argument duplicated to a function formal")
+DIAG("merge-diff-rec", MergeDiffRec, Warning,
+     "merging two attributes with different `rec` modifiers, the latter "
+     "will be implicitly ignored")
+DIAG("bison", BisonParse, Fatal, "{}")
+DIAG("invalid-float", InvalidFloat, Fatal, "invalid float {}")
+DIAG("invalid-integer", InvalidInteger, Fatal, "invalid integer {}")
+DIAG("dup-formal-arg", DuplicatedAttr, Error, "duplicated attr `{}`")
+
+#endif // DIAG
diff --git a/nixd/include/nixd/Basic/DiagnosticMerge.inc b/nixd/include/nixd/Basic/DiagnosticMerge.inc
deleted file mode 100644
index ab4da8e40..000000000
--- a/nixd/include/nixd/Basic/DiagnosticMerge.inc
+++ /dev/null
@@ -1,23 +0,0 @@
-// DiagnosticMerge.inc, merge all declarations
-
-// provides: DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE)
-
-#ifdef DIAG_ALL
-
-#define DIAG_SIMPLE(SNAME, CNAME, SEVERITY, MESSAGE)                           \
-  DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE)
-#define DIAG_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                       \
-  DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE)
-#define DIAG_NOTE_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                  \
-  DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE)
-#define DIAG_NOTE(SNAME, CNAME, SEVERITY, MESSAGE)                             \
-  DIAG_ALL(SNAME, CNAME, SEVERITY, MESSAGE)
-
-#include "DiagnosticKinds.inc"
-
-#undef DIAG_NOTE
-#undef DIAG_NOTE_BODY
-#undef DIAG_BODY
-#undef DIAG_SIMPLE
-
-#endif // DIAG_ALL
diff --git a/nixd/include/nixd/Basic/NoteKinds.inc b/nixd/include/nixd/Basic/NoteKinds.inc
new file mode 100644
index 000000000..e9e159183
--- /dev/null
+++ b/nixd/include/nixd/Basic/NoteKinds.inc
@@ -0,0 +1,10 @@
+/// DiagnosticNodes.inc, note declarations
+
+#ifdef DIAG_NOTE
+DIAG_NOTE("note-prev", PrevDeclared, "previously declared here")
+DIAG_NOTE("merge-diff-rec-this-rec", ThisRecursive,
+          "this attribute set is {}recursive")
+DIAG_NOTE("merge-diff-rec-consider", RecConsider,
+          "while this attribute set is marked as {}recursive, it "
+          "will be considered as {}recursive")
+#endif // DIAG_NOTE
diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h
index 0614384b5..bfa7b3741 100644
--- a/nixd/include/nixd/Sema/Lowering.h
+++ b/nixd/include/nixd/Sema/Lowering.h
@@ -54,7 +54,7 @@ class ExprAttrsBuilder {
 struct Lowering {
   nix::SymbolTable &STable;
   nix::PosTable &PTable;
-  std::vector<std::unique_ptr<Diagnostic>> &Diags;
+  DiagnosticEngine &Diags;
   EvalContext &Ctx;
   const nix::SourcePath &BasePath;
 
diff --git a/nixd/include/nixd/Syntax/Parser/Require.h b/nixd/include/nixd/Syntax/Parser/Require.h
index bf51c655f..95a044cee 100644
--- a/nixd/include/nixd/Syntax/Parser/Require.h
+++ b/nixd/include/nixd/Syntax/Parser/Require.h
@@ -26,7 +26,7 @@ struct ParseData {
 
   nixd::GCPool<Node> Nodes;
 
-  std::vector<std::unique_ptr<Diagnostic>> Diags;
+  DiagnosticEngine Diags;
 };
 
 // Note: copied from
diff --git a/nixd/lib/Basic/Diagnostic.cpp b/nixd/lib/Basic/Diagnostic.cpp
index c55361a83..f8f3efa5c 100644
--- a/nixd/lib/Basic/Diagnostic.cpp
+++ b/nixd/lib/Basic/Diagnostic.cpp
@@ -1,52 +1,43 @@
 #include "nixd/Basic/Diagnostic.h"
 #include "nixd/Basic/Range.h"
 
-#include "llvm/Support/FormatVariadic.h"
-
 namespace nixd {
 
-#define DIAG_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                       \
-  const char *Diag##CNAME::sname() const { return SNAME; }                     \
-  Diagnostic::Severity Diag##CNAME::severity() const { return DS_##SEVERITY; } \
-  Diagnostic::Kind Diag##CNAME::kind() const { return DK_##CNAME; }            \
-  constexpr const char *Diag##CNAME::message() { return MESSAGE; }             \
-  std::string_view Diag##CNAME::format() const { return Str; }
-#define DIAG_NOTE_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)                  \
-  DIAG_BODY(SNAME, CNAME, SEVERITY, MESSAGE, BODY)
+nixd::Diagnostic::Severity nixd::Diagnostic::severity(DiagnosticKind Kind) {
+  switch (Kind) {
+#define DIAG(SNAME, CNAME, SEVERITY, MESSAGE)                                  \
+  case DK_##CNAME:                                                             \
+    return DS_##SEVERITY;
 #include "nixd/Basic/DiagnosticKinds.inc"
-#undef DIAG_BODY
-#undef DIAG_NOTE_BODY
-
-DiagBisonParse::DiagBisonParse(RangeIdx Range, std::string Str)
-    : Diagnostic(Range), Str(std::move(Str)) {}
-
-DiagDuplicatedAttr::DiagDuplicatedAttr(RangeIdx Range, std::string AttrName)
-    : NotableDiagnostic(Range) {
-  Str = llvm::formatv(message(), AttrName);
-}
-
-static const char *getPrefix(bool P) { return P ? "" : "non-"; }
-
-DiagThisRecursive::DiagThisRecursive(RangeIdx Range, bool Recursive)
-    : Diagnostic(Range) {
-  Str = llvm::formatv(message(), getPrefix(Recursive));
+#undef DIAG
+  }
 }
-
-DiagRecConsider::DiagRecConsider(RangeIdx Range, bool MarkedRecursive)
-    : Diagnostic(Range) {
-  Str = llvm::formatv(message(), getPrefix(MarkedRecursive),
-                      getPrefix(!MarkedRecursive));
+const char *nixd::Diagnostic::message(DiagnosticKind Kind) {
+  switch (Kind) {
+#define DIAG(SNAME, CNAME, SEVERITY, MESSAGE)                                  \
+  case DK_##CNAME:                                                             \
+    return MESSAGE;
+#include "nixd/Basic/DiagnosticKinds.inc"
+#undef DIAG
+  }
 }
-
-DiagInvalidInteger::DiagInvalidInteger(RangeIdx Range, std::string_view Content)
-    : Diagnostic(Range) {
-  Str = llvm::formatv(message(), Content);
+const char *nixd::Diagnostic::sname(DiagnosticKind Kind) {
+  switch (Kind) {
+#define DIAG(SNAME, CNAME, SEVERITY, MESSAGE)                                  \
+  case DK_##CNAME:                                                             \
+    return SNAME;
+#include "nixd/Basic/DiagnosticKinds.inc"
+#undef DIAG
+  }
 }
-
-DiagInvalidFloat::DiagInvalidFloat(RangeIdx Range,
-                                   const std::string_view Content)
-    : Diagnostic(Range) {
-  Str = llvm::formatv(message(), Content);
+const char *nixd::Note::message(NoteKind Kind) {
+  switch (Kind) {
+#define DIAG_NOTE(SNAME, CNAME, MESSAGE)                                       \
+  case NK_##CNAME:                                                             \
+    return MESSAGE;
+#include "nixd/Basic/NoteKinds.inc"
+#undef DIAG_NOTE
+  }
 }
 
 } // namespace nixd
diff --git a/nixd/lib/Basic/meson.build b/nixd/lib/Basic/meson.build
index 10674b0d8..440f04078 100644
--- a/nixd/lib/Basic/meson.build
+++ b/nixd/lib/Basic/meson.build
@@ -1,4 +1,4 @@
-libnixdBasicDeps = [ nix_all, llvm ]
+libnixdBasicDeps = [ nix_all, llvm, fmt ]
 libnixdBasic = library('nixdBasic'
 , [ 'Diagnostic.cpp'
   ]
diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp
index bb94d549f..aef6aab80 100644
--- a/nixd/lib/Sema/Lowering.cpp
+++ b/nixd/lib/Sema/Lowering.cpp
@@ -15,6 +15,8 @@
 namespace nixd {
 
 using syntax::Node;
+using DK = Diagnostic::DiagnosticKind;
+using NK = Note::NoteKind;
 
 nix::Formal Lowering::lowerFormal(const syntax::Formal &Formal) {
   auto *Def = Lowering::lower(Formal.Default);
@@ -63,10 +65,8 @@ nix::ExprLambda *Lowering::lowerFunction(const syntax::Function *Fn) {
       if (Names.contains(Sym)) {
         // We have seen this symbol before, so this formal is duplicated
         const syntax::Formal *Previous = Names[Sym];
-        auto Diag = std::make_unique<DiagDuplicatedFormal>(Formal->Range);
-        auto PrevDefNote = std::make_unique<DiagPrevDeclared>(Previous->Range);
-        Diag->Notes.emplace_back(std::move(PrevDefNote));
-        Diags.emplace_back(std::move(Diag));
+        Diags.diag(Formal->Range, Diagnostic::DK_DuplicatedFormal)
+            .note(Previous->Range, NK::NK_PrevDeclared);
       } else {
         Names[Sym] = Formal;
       }
@@ -77,10 +77,8 @@ nix::ExprLambda *Lowering::lowerFunction(const syntax::Function *Fn) {
       auto ArgSym = Fn->Arg->Symbol;
       if (Names.contains(ArgSym)) {
         const syntax::Formal *Previous = Names[ArgSym];
-        auto Diag = std::make_unique<DiagDuplicatedFormalToArg>(Fn->Arg->Range);
-        auto PrevDefNote = std::make_unique<DiagPrevDeclared>(Previous->Range);
-        Diag->Notes.emplace_back(std::move(PrevDefNote));
-        Diags.emplace_back(std::move(Diag));
+        Diags.diag(Fn->Arg->Range, Diagnostic::DK_DuplicatedFormalToArg)
+            .note(Previous->Range, NK::NK_PrevDeclared);
       }
     }
     // Construct nix::Formals, from ours
@@ -106,12 +104,11 @@ nix::ExprLambda *Lowering::lowerFunction(const syntax::Function *Fn) {
   return NewLambda;
 }
 
-static std::unique_ptr<Diagnostic>
-mkAttrDupDiag(const std::string &Name, RangeIdx Range, RangeIdx Prev) {
-  auto Diag = std::make_unique<DiagDuplicatedAttr>(Range, Name);
-  auto PrevDefNote = std::make_unique<DiagPrevDeclared>(Prev);
-  Diag->Notes.emplace_back(std::move(PrevDefNote));
-  return Diag;
+static void diagAttrDup(DiagnosticEngine &DE, const std::string &Name,
+                        RangeIdx Range, RangeIdx Prev) {
+  auto &Diag = DE.diag(Range, DK::DK_DuplicatedAttr);
+  Diag << Name;
+  Diag.note(Prev, NK::NK_PrevDeclared);
 }
 
 ExprAttrsBuilder::ExprAttrsBuilder(Lowering &LW, RangeIdx Range, bool Recursive,
@@ -151,20 +148,19 @@ void ExprAttrsBuilder::addBinds(const syntax::Binds &Binds) {
   }
 }
 
-static std::unique_ptr<DiagMergeDiffRec>
-mkRecDiag(RangeIdx ThisRange, RangeIdx MergingRange, bool ThisRec) {
-  auto Diag = std::make_unique<DiagMergeDiffRec>(MergingRange);
-  Diag->Notes.emplace_back(
-      std::make_unique<DiagThisRecursive>(ThisRange, ThisRec));
-  Diag->Notes.emplace_back(
-      std::make_unique<DiagRecConsider>(MergingRange, !ThisRec));
-  return Diag;
+static const char *nonPrefix(bool P) { return P ? "" : "non-"; }
+
+static void diagRec(DiagnosticEngine &DE, RangeIdx ThisRange,
+                    RangeIdx MergingRange, bool ThisRec) {
+  Diagnostic &Diag = DE.diag(MergingRange, DK::DK_MergeDiffRec);
+  Diag.note(ThisRange, NK::NK_ThisRecursive) << nonPrefix(ThisRec);
+  Diag.note(MergingRange, NK::NK_RecConsider)
+      << nonPrefix(!ThisRec) << nonPrefix(ThisRec);
 }
 
 void ExprAttrsBuilder::addAttrSet(const syntax::AttrSet &AS) {
   if (AS.Recursive != Recursive) {
-    auto Diag = mkRecDiag(Range, AS.Range, Recursive);
-    LW.Diags.emplace_back(std::move(Diag));
+    diagRec(LW.Diags, Range, AS.Range, Recursive);
   }
   assert(AS.Binds && "binds should not be empty! parser error?");
   addBinds(*AS.Binds);
@@ -173,8 +169,7 @@ void ExprAttrsBuilder::addAttrSet(const syntax::AttrSet &AS) {
 void ExprAttrsBuilder::addAttr(const syntax::Node *Attr,
                                const syntax::Node *Body, bool Recursive) {
   if (Recursive != this->Recursive) {
-    auto Diag = mkRecDiag(Range, Attr->Range, this->Recursive);
-    LW.Diags.emplace_back(std::move(Diag));
+    diagRec(LW.Diags, Range, Attr->Range, this->Recursive);
   }
   switch (Attr->getKind()) {
   case Node::NK_Identifier: {
@@ -184,9 +179,7 @@ void ExprAttrsBuilder::addAttr(const syntax::Node *Attr,
       // duplicated, but if they are two attrset, we can try to merge them.
       if (Body->getKind() != Node::NK_AttrSet || !Nested.contains(Sym)) {
         // the body is not an attribute set, report error.
-        auto Diag =
-            mkAttrDupDiag(LW.STable[Sym], ID->Range, Fields[Sym]->Range);
-        LW.Diags.emplace_back(std::move(Diag));
+        diagAttrDup(LW.Diags, LW.STable[Sym], ID->Range, Fields[Sym]->Range);
         return;
       }
       const auto *BodyAttrSet = dynamic_cast<const syntax::AttrSet *>(Body);
@@ -210,7 +203,7 @@ void ExprAttrsBuilder::addAttr(const syntax::Node *Attr,
   default: {
     if (IsLet) {
       // reject dynamic attrs in let expression
-      LW.Diags.emplace_back(std::make_unique<DiagLetDynamic>(Attr->Range));
+      LW.Diags.diag(Attr->Range, Diagnostic::DK_LetDynamic);
       return;
     }
     nix::Expr *NameExpr = LW.lower(Attr);
@@ -241,9 +234,8 @@ void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {
         if (!S->Nested.contains(Sym)) {
           // contains a field but the body is not an attr, then we cannot
           // perform merging.
-          std::unique_ptr<Diagnostic> Diag =
-              mkAttrDupDiag(LW.STable[Sym], ID->Range, S->Fields[Sym]->Range);
-          LW.Diags.emplace_back(std::move(Diag));
+          diagAttrDup(LW.Diags, LW.STable[Sym], ID->Range,
+                      S->Fields[Sym]->Range);
           return;
         }
         // select this symbol, and consider merging it.
@@ -260,7 +252,7 @@ void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {
     default: {
       if (IsLet) {
         // reject dynamic attrs in let expression
-        LW.Diags.emplace_back(std::make_unique<DiagLetDynamic>(Name->Range));
+        LW.Diags.diag(Name->Range, Diagnostic::DK_LetDynamic);
         return;
       }
       DynamicNested[Name] = std::make_unique<ExprAttrsBuilder>(
@@ -277,7 +269,7 @@ void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {
 
 void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) {
   if (IA.InheritedAttrs->Names.empty())
-    LW.Diags.emplace_back(std::make_unique<DiagEmptyInherit>(IA.Range));
+    LW.Diags.diag(IA.Range, Diagnostic::DK_EmptyInherit);
 
   // inherit (expr) attr1 attr2
   // lowering `(expr)` to nix expression
@@ -297,15 +289,14 @@ void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) {
       break;
     }
     default: {
-      LW.Diags.emplace_back(std::make_unique<DiagInheritDynamic>(Name->Range));
+      LW.Diags.diag(Name->Range, Diagnostic::DK_InheritDynamic);
     }
     }
 
     // Check if it is duplicated
     if (Fields.contains(Sym)) {
       std::string SymStr = LW.STable[Sym];
-      auto Diag = mkAttrDupDiag(SymStr, Name->Range, Fields[Sym]->Range);
-      LW.Diags.emplace_back(std::move(Diag));
+      diagAttrDup(LW.Diags, SymStr, Name->Range, Fields[Sym]->Range);
       continue;
     }
 
diff --git a/nixd/lib/Syntax/Lexer/Lexer.l b/nixd/lib/Syntax/Lexer/Lexer.l
index 29fcbd5f1..6c575b648 100644
--- a/nixd/lib/Syntax/Lexer/Lexer.l
+++ b/nixd/lib/Syntax/Lexer/Lexer.l
@@ -63,7 +63,7 @@ or          { return OR_KW; }
                 yylval->N = boost::lexical_cast<int64_t>(yytext);
               } catch (const boost::bad_lexical_cast &) {
                 auto Range = mkRange(*yylloc, *Data);
-                Data->Diags.emplace_back(std::make_unique<nixd::DiagInvalidInteger>(Range, yytext));
+                Data->Diags.diag(Range, Diagnostic::DK_InvalidInteger) << std::string(yytext);
                 yyterminate();
               }
               return INT;
@@ -73,7 +73,7 @@ or          { return OR_KW; }
               yylval->NF = strtod(yytext, 0);
               if (errno != 0) {
                 auto Range = mkRange(*yylloc, *Data);
-                Data->Diags.emplace_back(std::make_unique<nixd::DiagInvalidFloat>(Range, yytext));
+                Data->Diags.diag(Range, Diagnostic::DK_InvalidFloat) << std::string(yytext);
                 yyterminate();
               }
               return FLOAT;
@@ -202,7 +202,7 @@ or          { return OR_KW; }
 <INPATH_SLASH>{ANY} |
 <INPATH_SLASH><<EOF>> {
   auto Range = mkRange(*yylloc, *Data);
-  Data->Diags.emplace_back(std::make_unique<nixd::DiagPathTrailingSlash>(Range));
+  Data->Diags.diag(Range, Diagnostic::DK_PathTrailingSlash);
   yyterminate();
 }
 
diff --git a/nixd/lib/Syntax/Parser/Parser.y b/nixd/lib/Syntax/Parser/Parser.y
index 06c9d1abb..d1ae03342 100644
--- a/nixd/lib/Syntax/Parser/Parser.y
+++ b/nixd/lib/Syntax/Parser/Parser.y
@@ -290,7 +290,7 @@ expr_simple
     auto N = decorateNode(new LegacyLet, *yylocp, *Data);
     N->Binds = $3;
     $$ = N;
-    Data->Diags.emplace_back(std::make_unique<nixd::DiagDeprecatedLet>(N->Range));
+    Data->Diags.diag($$->Range, Diagnostic::DK_DeprecatedLet);
   }
   | REC '{' binds '}' {
     auto N = decorateNode(new AttrSet, *yylocp, *Data);
@@ -411,14 +411,14 @@ uri
   : URI {
     $$ = decorateNode(new nixd::syntax::URI, *yylocp, *Data);
     $$->S = std::string($1);
-    Data->Diags.emplace_back(std::make_unique<nixd::DiagDeprecatedURL>($$->Range));
+    Data->Diags.diag($$->Range, Diagnostic::DK_DeprecatedURL);
   }
 
 identifier_or
   : OR_KW {
     $$ = decorateNode(new Identifier, *yylocp, *Data);
     $$->Symbol = Data->State.Symbols.create("or");
-    Data->Diags.emplace_back(std::make_unique<nixd::DiagOrIdentifier>($$->Range));
+    Data->Diags.diag($$->Range, Diagnostic::DK_OrIdentifier);
   }
 
 var_or: identifier_or {
diff --git a/nixd/lib/Syntax/Parser/Prologue.cpp b/nixd/lib/Syntax/Parser/Prologue.cpp
index 7cab05015..c692a16ff 100644
--- a/nixd/lib/Syntax/Parser/Prologue.cpp
+++ b/nixd/lib/Syntax/Parser/Prologue.cpp
@@ -10,6 +10,8 @@
 
 using namespace nixd::syntax;
 
+using nixd::Diagnostic;
+
 YY_DECL;
 
 /// Convert a yacc location (yylloc) to nix::PosIdx
@@ -28,8 +30,7 @@ static nixd::RangeIdx mkRange(YYLTYPE YL, nixd::syntax::ParseData &Data) {
 void yyerror(YYLTYPE *YL, yyscan_t Scanner, nixd::syntax::ParseData *Data,
              const char *Error) {
   auto Range = mkRange(*YL, *Data);
-  Data->Diags.emplace_back(
-      std::make_unique<nixd::DiagBisonParse>(Range, Error));
+  Data->Diags.diag(Range, Diagnostic::DK_BisonParse) << std::string(Error);
 }
 
 template <class T>
diff --git a/nixd/tools/nixd-lint/nixd-lint.cpp b/nixd/tools/nixd-lint/nixd-lint.cpp
index 1251f7e1a..338352ee0 100644
--- a/nixd/tools/nixd-lint/nixd-lint.cpp
+++ b/nixd/tools/nixd-lint/nixd-lint.cpp
@@ -40,15 +40,15 @@ static void printCodeLines(std::ostream &Out, const std::string &Prefix,
   // previous line of code.
   if (LOC.prevLineOfCode.has_value()) {
     Out << std::endl
-        << fmt("%1% %|2$5d|| %3%", Prefix, (BeginPos->line - 1),
-               *LOC.prevLineOfCode);
+        << nix::fmt("%1% %|2$5d|| %3%", Prefix, (BeginPos->line - 1),
+                    *LOC.prevLineOfCode);
   }
 
   if (LOC.errLineOfCode.has_value()) {
     // line of code containing the error.
     Out << std::endl
-        << fmt("%1% %|2$5d|| %3%", Prefix, (BeginPos->line),
-               *LOC.errLineOfCode);
+        << nix::fmt("%1% %|2$5d|| %3%", Prefix, (BeginPos->line),
+                    *LOC.errLineOfCode);
     // error arrows for the column range.
     if (BeginPos->column > 0) {
       auto Start = BeginPos->column;
@@ -60,8 +60,8 @@ static void printCodeLines(std::ostream &Out, const std::string &Prefix,
       std::string arrows("^");
 
       Out << std::endl
-          << fmt("%1%      |%2%" ANSI_RED "%3%" ANSI_NORMAL, Prefix, Spaces,
-                 arrows);
+          << nix::fmt("%1%      |%2%" ANSI_RED "%3%" ANSI_NORMAL, Prefix,
+                      Spaces, arrows);
 
       if (BeginPos->line == EndPos->line) {
         Out << ANSI_RED;
@@ -76,8 +76,8 @@ static void printCodeLines(std::ostream &Out, const std::string &Prefix,
   // next line of code.
   if (LOC.nextLineOfCode.has_value()) {
     Out << std::endl
-        << fmt("%1% %|2$5d|| %3%", Prefix, (BeginPos->line + 1),
-               *LOC.nextLineOfCode);
+        << nix::fmt("%1% %|2$5d|| %3%", Prefix, (BeginPos->line + 1),
+                    *LOC.nextLineOfCode);
   }
 }
 
@@ -122,6 +122,60 @@ struct ASTDump : nixd::RecursiveASTVisitor<ASTDump> {
   }
 };
 
+void printNote(nixd::Note &Note, nix::SymbolTable &STable,
+               nix::PosTable &PTable) {
+  auto NoteBegin = PTable[Note.range().Begin];
+  auto NoteEnd = PTable[Note.range().End];
+  std::cout << "\n";
+  std::cout << ANSI_CYAN "note: " ANSI_NORMAL;
+  std::cout << Note.format() << "\n";
+
+  if (NoteBegin) {
+    std::cout << "\n"
+              << ANSI_BLUE << "at " ANSI_WARNING << NoteBegin << ANSI_NORMAL
+              << ":";
+    if (auto Lines =
+            std::shared_ptr<nix::AbstractPos>(NoteBegin)->getCodeLines()) {
+      std::cout << "\n";
+      printCodeLines(std::cout, "", NoteBegin, NoteEnd, *Lines);
+      std::cout << "\n";
+    }
+  }
+}
+
+void printDiag(nixd::Diagnostic::Severity Serverity, nixd::Diagnostic &Diag,
+               nix::SymbolTable &STable, nix::PosTable &PTable) {
+  auto BeginPos = PTable[Diag.range().Begin];
+  auto EndPos = PTable[Diag.range().End];
+  switch (Serverity) {
+  case nixd::Diagnostic::DS_Warning:
+    std::cout << ANSI_WARNING "warning: " ANSI_NORMAL;
+    break;
+  case nixd::Diagnostic::DS_Error:
+    std::cout << ANSI_RED "error: " ANSI_NORMAL;
+    break;
+  case nixd::Diagnostic::DS_Fatal:
+    std::cout << ANSI_RED "fatal: " ANSI_NORMAL;
+    break;
+  }
+  std::cout << Diag.format() << "\n";
+  if (BeginPos) {
+    std::cout << "\n"
+              << ANSI_BLUE << "at " ANSI_WARNING << BeginPos << ANSI_NORMAL
+              << ":";
+    if (auto Lines =
+            std::shared_ptr<nix::AbstractPos>(BeginPos)->getCodeLines()) {
+      std::cout << "\n";
+      printCodeLines(std::cout, "", BeginPos, EndPos, *Lines);
+      std::cout << "\n";
+    }
+  }
+
+  for (const auto &Note : Diag.notes()) {
+    printNote(*Note, STable, PTable);
+  }
+}
+
 int main(int argc, char *argv[]) {
   HideUnrelatedOptions(Cat);
   ParseCommandLineOptions(argc, argv, "nixd linter", nullptr,
@@ -161,51 +215,21 @@ int main(int argc, char *argv[]) {
     D.traverseExpr(NixTree);
   }
 
+  for (const auto &Err : Data.Diags.fatals()) {
+    printDiag(nixd::Diagnostic::Severity::DS_Fatal, *Err, *STable, *PTable);
+  }
+
+  for (const auto &Err : Data.Diags.errors()) {
+    printDiag(nixd::Diagnostic::Severity::DS_Error, *Err, *STable, *PTable);
+  }
+
+  for (const auto &Err : Data.Diags.warnings()) {
+    printDiag(nixd::Diagnostic::Severity::DS_Warning, *Err, *STable, *PTable);
+  }
+  /*
   for (const auto &Diag : Data.Diags) {
-    auto BeginPos = (*PTable)[Diag->Range.Begin];
-    auto EndPos = (*PTable)[Diag->Range.End];
-    switch (Diag->severity()) {
-    case nixd::Diagnostic::DS_Warning:
-      std::cout << ANSI_WARNING "warning: " ANSI_NORMAL;
-      break;
-    case nixd::Diagnostic::DS_Error:
-      std::cout << ANSI_RED "error: " ANSI_NORMAL;
-      break;
-    }
-    std::cout << Diag->format() << "\n";
-    if (BeginPos) {
-      std::cout << "\n"
-                << ANSI_BLUE << "at " ANSI_WARNING << BeginPos << ANSI_NORMAL
-                << ":";
-      if (auto Lines =
-              std::shared_ptr<nix::AbstractPos>(BeginPos)->getCodeLines()) {
-        std::cout << "\n";
-        printCodeLines(std::cout, "", BeginPos, EndPos, *Lines);
-        std::cout << "\n";
-      }
-    }
 
-    if (auto *ND = dynamic_cast<nixd::NotableDiagnostic *>(Diag.get())) {
-      for (const auto &Note : ND->Notes) {
-        auto NoteBegin = (*PTable)[Note->Range.Begin];
-        auto NoteEnd = (*PTable)[Note->Range.End];
-        std::cout << "\n";
-        std::cout << ANSI_CYAN "note: " ANSI_NORMAL;
-        std::cout << Note->format() << "\n";
-
-        if (NoteBegin) {
-          std::cout << "\n"
-                    << ANSI_BLUE << "at " ANSI_WARNING << NoteBegin
-                    << ANSI_NORMAL << ":";
-          if (auto Lines = std::shared_ptr<nix::AbstractPos>(NoteBegin)
-                               ->getCodeLines()) {
-            std::cout << "\n";
-            printCodeLines(std::cout, "", NoteBegin, NoteEnd, *Lines);
-            std::cout << "\n";
-          }
-        }
-      }
-    }
   }
+  */
   return 0;
 }
diff --git a/nixd/tools/nixd-lint/test/legacy-let.nix b/nixd/tools/nixd-lint/test/legacy-let.nix
index 498691a06..a94b0e52d 100644
--- a/nixd/tools/nixd-lint/test/legacy-let.nix
+++ b/nixd/tools/nixd-lint/test/legacy-let.nix
@@ -1,7 +1,9 @@
 # RUN: nixd-lint %s | FileCheck %s
 # RUN: nixd-lint -dump-nix-ast %s | FileCheck %s --check-prefix=AST
 
+# CHECK: syntax error, unexpected ';', expecting end of file
+
 # AST: (rec { a = 1; }).body
 let { a = 1; } # CHECK:  using deprecated `let'
 
-; # CHECK: syntax error, unexpected ';', expecting end of file
+;