Skip to content

Commit a03bc42

Browse files
committed
libnixf: check builtins. prefix from variable lookups (#721)
1 parent d7f6516 commit a03bc42

File tree

7 files changed

+111
-7
lines changed

7 files changed

+111
-7
lines changed

libnixf/include/nixf/Basic/Nodes/Expr.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,16 @@ namespace nixf {
66

77
class ExprSelect : public Expr {
88
const std::shared_ptr<Expr> E;
9+
const std::shared_ptr<Dot> Do;
910
const std::shared_ptr<AttrPath> Path;
1011
const std::shared_ptr<Expr> Default;
1112

1213
public:
1314
ExprSelect(LexerCursorRange Range, std::shared_ptr<Expr> E,
14-
std::shared_ptr<AttrPath> Path, std::shared_ptr<Expr> Default)
15-
: Expr(NK_ExprSelect, Range), E(std::move(E)), Path(std::move(Path)),
16-
Default(std::move(Default)) {
15+
std::shared_ptr<Dot> Do, std::shared_ptr<AttrPath> Path,
16+
std::shared_ptr<Expr> Default)
17+
: Expr(NK_ExprSelect, Range), E(std::move(E)), Do(std::move(Do)),
18+
Path(std::move(Path)), Default(std::move(Default)) {
1719
assert(this->E && "E must not be null");
1820
}
1921

@@ -22,6 +24,8 @@ class ExprSelect : public Expr {
2224
return *E;
2325
}
2426

27+
[[nodiscard]] Dot *dot() const { return Do.get(); }
28+
2529
[[nodiscard]] Expr *defaultExpr() const { return Default.get(); }
2630

2731
[[nodiscard]] AttrPath *path() const { return Path.get(); }

libnixf/include/nixf/Sema/VariableLookup.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ class VariableLookupAnalysis {
141141

142142
void lookupVar(const ExprVar &Var, const std::shared_ptr<EnvNode> &Env);
143143

144+
void checkBuiltins(const ExprSelect &Sel);
145+
144146
std::shared_ptr<EnvNode> dfsAttrs(const SemaAttrs &SA,
145147
const std::shared_ptr<EnvNode> &Env,
146148
const Node *Syntax,

libnixf/src/Basic/diagnostic.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,18 @@ class Diagnostic(TypedDict):
234234
# Primary Operation (PrimOp) related diagnostics
235235
# We call this "builtin" in messages to match Nix's terminology
236236
# "PrimOp" is specific to Nix/nixf implementation
237+
{
238+
"sname": "sema-primop-unknown",
239+
"cname": "PrimOpUnknown",
240+
"severity": "Error",
241+
"message": "unknown builtin `{}`",
242+
},
243+
{
244+
"sname": "sema-primop-removed-prefix",
245+
"cname": "PrimOpRemovablePrefix",
246+
"severity": "Warning",
247+
"message": "this is a prelude builtin, the `builtins.` prefix can be removed",
248+
},
237249
{
238250
"sname": "sema-primop-needs-prefix",
239251
"cname": "PrimOpNeedsPrefix",

libnixf/src/Parse/ParseExpr.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ std::shared_ptr<Expr> Parser::parseExprSelect() {
1818

1919
// expr_select : expr_simple '.' attrpath
2020
// | expr_simple '.' attrpath 'or' expr_select
21+
auto Do = std::make_shared<Dot>(Tok.range(), Expr.get(), nullptr);
2122
consume(); // .
2223
auto Path = parseAttrPath();
2324
if (!Path) {
@@ -34,7 +35,7 @@ std::shared_ptr<Expr> Parser::parseExprSelect() {
3435
// expr_select : expr_simple '.' attrpath
3536
return std::make_shared<ExprSelect>(
3637
LexerCursorRange{Begin, LastToken->rCur()}, std::move(Expr),
37-
std::move(Path), /*Default=*/nullptr);
38+
std::move(Do), std::move(Path), /*Default=*/nullptr);
3839
}
3940

4041
// expr_select : expr_simple '.' attrpath 'or' expr_select
@@ -46,7 +47,7 @@ std::shared_ptr<Expr> Parser::parseExprSelect() {
4647
}
4748
return std::make_shared<ExprSelect>(
4849
LexerCursorRange{Begin, LastToken->rCur()}, std::move(Expr),
49-
std::move(Path), std::move(Default));
50+
std::move(Do), std::move(Path), std::move(Default));
5051
}
5152

5253
std::shared_ptr<Expr> Parser::parseExprApp(int Limit) {

libnixf/src/Sema/SemaActions.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,8 @@ Sema::desugarInheritExpr(std::shared_ptr<AttrName> Name,
344344
auto Path = std::make_shared<AttrPath>(
345345
Range, std::vector<std::shared_ptr<AttrName>>{std::move(Name)},
346346
std::vector<std::shared_ptr<Dot>>{});
347-
return {std::make_shared<ExprSelect>(Range, std::move(E), std::move(Path),
348-
nullptr),
347+
return {std::make_shared<ExprSelect>(Range, std::move(E), nullptr,
348+
std::move(Path), nullptr),
349349
Attribute::AttributeKind::InheritFrom};
350350
}
351351

libnixf/src/Sema/VariableLookup.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,52 @@ void VariableLookupAnalysis::dfs(const ExprWith &With,
325325
}
326326
}
327327

328+
void VariableLookupAnalysis::checkBuiltins(const ExprSelect &Sel) {
329+
if (!Sel.path())
330+
return;
331+
332+
if (Sel.expr().kind() != Node::NK_ExprVar)
333+
return;
334+
335+
const auto &Builtins = static_cast<const ExprVar &>(Sel.expr());
336+
if (Builtins.id().name() != "builtins")
337+
return;
338+
339+
const auto &AP = *Sel.path();
340+
341+
if (AP.names().size() != 1)
342+
return;
343+
344+
AttrName &First = *AP.names()[0];
345+
if (!First.isStatic())
346+
return;
347+
348+
const auto &Name = First.staticName();
349+
350+
switch (lookupGlobalPrimOpInfo(Name)) {
351+
case PrimopLookupResult::Found: {
352+
Diagnostic &D = Diags.emplace_back(Diagnostic::DK_PrimOpRemovablePrefix,
353+
Builtins.range());
354+
Fix &F =
355+
D.fix("remove `builtins.` prefix")
356+
.edit(TextEdit::mkRemoval(Builtins.range())); // remove `builtins`
357+
358+
if (Sel.dot()) {
359+
// remove the dot also.
360+
F.edit(TextEdit::mkRemoval(Sel.dot()->range()));
361+
}
362+
return;
363+
}
364+
case PrimopLookupResult::PrefixedFound:
365+
return;
366+
case PrimopLookupResult::NotFound:
367+
Diagnostic &D = Diags.emplace_back(Diagnostic::DK_PrimOpUnknown,
368+
AP.names()[0]->range());
369+
D << Name;
370+
return;
371+
}
372+
}
373+
328374
void VariableLookupAnalysis::dfs(const Node &Root,
329375
const std::shared_ptr<EnvNode> &Env) {
330376
Envs.insert({&Root, Env});
@@ -354,6 +400,12 @@ void VariableLookupAnalysis::dfs(const Node &Root,
354400
dfs(With, Env);
355401
break;
356402
}
403+
case Node::NK_ExprSelect: {
404+
trivialDispatch(Root, Env);
405+
const auto &Sel = static_cast<const ExprSelect &>(Root);
406+
checkBuiltins(Sel);
407+
break;
408+
}
357409
default:
358410
trivialDispatch(Root, Env);
359411
}

libnixf/test/Sema/VariableLookup.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,4 +365,37 @@ TEST_F(VLATest, Builtins_Needs_Prefix) {
365365
ASSERT_EQ(Diags[0].fixes()[0].edits()[0].newText(), "builtins.");
366366
}
367367

368+
TEST_F(VLATest, Builtins_Removable_Prefix) {
369+
const char *Src = R"(builtins.import)";
370+
371+
std::shared_ptr<Node> AST = parse(Src, Diags);
372+
VariableLookupAnalysis VLA(Diags);
373+
VLA.runOnAST(*AST);
374+
375+
ASSERT_EQ(Diags.size(), 1);
376+
377+
ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_PrimOpRemovablePrefix);
378+
ASSERT_EQ(Diags[0].fixes().size(), 1);
379+
ASSERT_EQ(Diags[0].fixes()[0].edits().size(), 2);
380+
ASSERT_EQ(Diags[0].fixes()[0].edits()[0].oldRange().lCur().offset(), 0);
381+
ASSERT_EQ(Diags[0].fixes()[0].edits()[0].oldRange().rCur().offset(), 8);
382+
ASSERT_EQ(Diags[0].fixes()[0].edits()[0].newText(), "");
383+
ASSERT_EQ(Diags[0].fixes()[0].edits()[1].oldRange().lCur().offset(), 8);
384+
ASSERT_EQ(Diags[0].fixes()[0].edits()[1].oldRange().rCur().offset(), 9);
385+
ASSERT_EQ(Diags[0].fixes()[0].edits()[1].newText(), "");
386+
}
387+
388+
TEST_F(VLATest, Builtins_Unknown) {
389+
const char *Src = R"(builtins.importaaaa)";
390+
391+
std::shared_ptr<Node> AST = parse(Src, Diags);
392+
VariableLookupAnalysis VLA(Diags);
393+
VLA.runOnAST(*AST);
394+
395+
ASSERT_EQ(Diags.size(), 1);
396+
397+
ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_PrimOpUnknown);
398+
ASSERT_EQ(Diags[0].fixes().size(), 0);
399+
}
400+
368401
} // namespace

0 commit comments

Comments
 (0)