From 46a12cc9ba5aa1e6129365e77f460e4cd3f21cff Mon Sep 17 00:00:00 2001 From: Yingchi Long Date: Tue, 7 Oct 2025 02:10:55 +0800 Subject: [PATCH] libnixf: check 'builtins.' prefix from variable lookups --- libnixf/include/nixf/Basic/Nodes/Expr.h | 10 +++-- libnixf/include/nixf/Sema/VariableLookup.h | 2 + libnixf/src/Basic/diagnostic.py | 12 +++++ libnixf/src/Parse/ParseExpr.cpp | 5 ++- libnixf/src/Sema/SemaActions.cpp | 4 +- libnixf/src/Sema/VariableLookup.cpp | 52 ++++++++++++++++++++++ libnixf/test/Sema/VariableLookup.cpp | 33 ++++++++++++++ 7 files changed, 111 insertions(+), 7 deletions(-) diff --git a/libnixf/include/nixf/Basic/Nodes/Expr.h b/libnixf/include/nixf/Basic/Nodes/Expr.h index 963e094ea..b583eb288 100644 --- a/libnixf/include/nixf/Basic/Nodes/Expr.h +++ b/libnixf/include/nixf/Basic/Nodes/Expr.h @@ -6,14 +6,16 @@ namespace nixf { class ExprSelect : public Expr { const std::shared_ptr E; + const std::shared_ptr Do; const std::shared_ptr Path; const std::shared_ptr Default; public: ExprSelect(LexerCursorRange Range, std::shared_ptr E, - std::shared_ptr Path, std::shared_ptr Default) - : Expr(NK_ExprSelect, Range), E(std::move(E)), Path(std::move(Path)), - Default(std::move(Default)) { + std::shared_ptr Do, std::shared_ptr Path, + std::shared_ptr Default) + : Expr(NK_ExprSelect, Range), E(std::move(E)), Do(std::move(Do)), + Path(std::move(Path)), Default(std::move(Default)) { assert(this->E && "E must not be null"); } @@ -22,6 +24,8 @@ class ExprSelect : public Expr { return *E; } + [[nodiscard]] Dot *dot() const { return Do.get(); } + [[nodiscard]] Expr *defaultExpr() const { return Default.get(); } [[nodiscard]] AttrPath *path() const { return Path.get(); } diff --git a/libnixf/include/nixf/Sema/VariableLookup.h b/libnixf/include/nixf/Sema/VariableLookup.h index d25e2bd4a..9efdbb9d1 100644 --- a/libnixf/include/nixf/Sema/VariableLookup.h +++ b/libnixf/include/nixf/Sema/VariableLookup.h @@ -141,6 +141,8 @@ class VariableLookupAnalysis { void lookupVar(const ExprVar &Var, const std::shared_ptr &Env); + void checkBuiltins(const ExprSelect &Sel); + std::shared_ptr dfsAttrs(const SemaAttrs &SA, const std::shared_ptr &Env, const Node *Syntax, diff --git a/libnixf/src/Basic/diagnostic.py b/libnixf/src/Basic/diagnostic.py index 88cdc0b83..e6e6a787a 100644 --- a/libnixf/src/Basic/diagnostic.py +++ b/libnixf/src/Basic/diagnostic.py @@ -234,6 +234,18 @@ class Diagnostic(TypedDict): # Primary Operation (PrimOp) related diagnostics # We call this "builtin" in messages to match Nix's terminology # "PrimOp" is specific to Nix/nixf implementation + { + "sname": "sema-primop-unknown", + "cname": "PrimOpUnknown", + "severity": "Error", + "message": "unknown builtin `{}`", + }, + { + "sname": "sema-primop-removed-prefix", + "cname": "PrimOpRemovablePrefix", + "severity": "Warning", + "message": "this is a prelude builtin, the `builtins.` prefix can be removed", + }, { "sname": "sema-primop-needs-prefix", "cname": "PrimOpNeedsPrefix", diff --git a/libnixf/src/Parse/ParseExpr.cpp b/libnixf/src/Parse/ParseExpr.cpp index 35303a7de..3f3da2fc5 100644 --- a/libnixf/src/Parse/ParseExpr.cpp +++ b/libnixf/src/Parse/ParseExpr.cpp @@ -18,6 +18,7 @@ std::shared_ptr Parser::parseExprSelect() { // expr_select : expr_simple '.' attrpath // | expr_simple '.' attrpath 'or' expr_select + auto Do = std::make_shared(Tok.range(), Expr.get(), nullptr); consume(); // . auto Path = parseAttrPath(); if (!Path) { @@ -34,7 +35,7 @@ std::shared_ptr Parser::parseExprSelect() { // expr_select : expr_simple '.' attrpath return std::make_shared( LexerCursorRange{Begin, LastToken->rCur()}, std::move(Expr), - std::move(Path), /*Default=*/nullptr); + std::move(Do), std::move(Path), /*Default=*/nullptr); } // expr_select : expr_simple '.' attrpath 'or' expr_select @@ -46,7 +47,7 @@ std::shared_ptr Parser::parseExprSelect() { } return std::make_shared( LexerCursorRange{Begin, LastToken->rCur()}, std::move(Expr), - std::move(Path), std::move(Default)); + std::move(Do), std::move(Path), std::move(Default)); } std::shared_ptr Parser::parseExprApp(int Limit) { diff --git a/libnixf/src/Sema/SemaActions.cpp b/libnixf/src/Sema/SemaActions.cpp index a63f29832..1df72d611 100644 --- a/libnixf/src/Sema/SemaActions.cpp +++ b/libnixf/src/Sema/SemaActions.cpp @@ -344,8 +344,8 @@ Sema::desugarInheritExpr(std::shared_ptr Name, auto Path = std::make_shared( Range, std::vector>{std::move(Name)}, std::vector>{}); - return {std::make_shared(Range, std::move(E), std::move(Path), - nullptr), + return {std::make_shared(Range, std::move(E), nullptr, + std::move(Path), nullptr), Attribute::AttributeKind::InheritFrom}; } diff --git a/libnixf/src/Sema/VariableLookup.cpp b/libnixf/src/Sema/VariableLookup.cpp index 7de472790..e02c2ea15 100644 --- a/libnixf/src/Sema/VariableLookup.cpp +++ b/libnixf/src/Sema/VariableLookup.cpp @@ -325,6 +325,52 @@ void VariableLookupAnalysis::dfs(const ExprWith &With, } } +void VariableLookupAnalysis::checkBuiltins(const ExprSelect &Sel) { + if (!Sel.path()) + return; + + if (Sel.expr().kind() != Node::NK_ExprVar) + return; + + const auto &Builtins = static_cast(Sel.expr()); + if (Builtins.id().name() != "builtins") + return; + + const auto &AP = *Sel.path(); + + if (AP.names().size() != 1) + return; + + AttrName &First = *AP.names()[0]; + if (!First.isStatic()) + return; + + const auto &Name = First.staticName(); + + switch (lookupGlobalPrimOpInfo(Name)) { + case PrimopLookupResult::Found: { + Diagnostic &D = Diags.emplace_back(Diagnostic::DK_PrimOpRemovablePrefix, + Builtins.range()); + Fix &F = + D.fix("remove `builtins.` prefix") + .edit(TextEdit::mkRemoval(Builtins.range())); // remove `builtins` + + if (Sel.dot()) { + // remove the dot also. + F.edit(TextEdit::mkRemoval(Sel.dot()->range())); + } + return; + } + case PrimopLookupResult::PrefixedFound: + return; + case PrimopLookupResult::NotFound: + Diagnostic &D = Diags.emplace_back(Diagnostic::DK_PrimOpUnknown, + AP.names()[0]->range()); + D << AP.names()[0]->name(); + return; + } +} + void VariableLookupAnalysis::dfs(const Node &Root, const std::shared_ptr &Env) { Envs.insert({&Root, Env}); @@ -354,6 +400,12 @@ void VariableLookupAnalysis::dfs(const Node &Root, dfs(With, Env); break; } + case Node::NK_ExprSelect: { + trivialDispatch(Root, Env); + const auto &Sel = static_cast(Root); + checkBuiltins(Sel); + break; + } default: trivialDispatch(Root, Env); } diff --git a/libnixf/test/Sema/VariableLookup.cpp b/libnixf/test/Sema/VariableLookup.cpp index 0d031e34f..e88c429b4 100644 --- a/libnixf/test/Sema/VariableLookup.cpp +++ b/libnixf/test/Sema/VariableLookup.cpp @@ -365,4 +365,37 @@ TEST_F(VLATest, Builtins_Needs_Prefix) { ASSERT_EQ(Diags[0].fixes()[0].edits()[0].newText(), "builtins."); } +TEST_F(VLATest, Builtins_Removable_Prefix) { + const char *Src = R"(builtins.import)"; + + std::shared_ptr AST = parse(Src, Diags); + VariableLookupAnalysis VLA(Diags); + VLA.runOnAST(*AST); + + ASSERT_EQ(Diags.size(), 1); + + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_PrimOpRemovablePrefix); + ASSERT_EQ(Diags[0].fixes().size(), 1); + ASSERT_EQ(Diags[0].fixes()[0].edits().size(), 2); + ASSERT_EQ(Diags[0].fixes()[0].edits()[0].oldRange().lCur().offset(), 0); + ASSERT_EQ(Diags[0].fixes()[0].edits()[0].oldRange().rCur().offset(), 8); + ASSERT_EQ(Diags[0].fixes()[0].edits()[0].newText(), ""); + ASSERT_EQ(Diags[0].fixes()[0].edits()[1].oldRange().lCur().offset(), 8); + ASSERT_EQ(Diags[0].fixes()[0].edits()[1].oldRange().rCur().offset(), 9); + ASSERT_EQ(Diags[0].fixes()[0].edits()[1].newText(), ""); +} + +TEST_F(VLATest, Builtins_Unknown) { + const char *Src = R"(builtins.importaaaa)"; + + std::shared_ptr AST = parse(Src, Diags); + VariableLookupAnalysis VLA(Diags); + VLA.runOnAST(*AST); + + ASSERT_EQ(Diags.size(), 1); + + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_PrimOpUnknown); + ASSERT_EQ(Diags[0].fixes().size(), 0); +} + } // namespace