Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

libnixf: remove sema-escaping-with because it is too pedantic #582

Merged
merged 1 commit into from
Aug 11, 2024
Merged

libnixf: remove sema-escaping-with because it is too pedantic #582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions libnixf/include/nixf/Basic/NoteKinds.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,5 @@ DIAG_NOTE("merge-diff-rec-consider", RecConsider,
"will be considered as {}recursive")
DIAG_NOTE("note-bcomment-begin", BCommentBegin, "/* comment begins at here")
DIAG_NOTE("to-match-this", ToMachThis, "to match this {}")
DIAG_NOTE("var-bind-to-this", VarBindToThis,
"variable will bind to this definition")
DIAG_NOTE("escaping-this-with", EscapingWith, "escaping this with expression")

#endif // DIAG_NOTE
6 changes: 0 additions & 6 deletions libnixf/src/Basic/diagnostic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,4 @@ class Diagnostic(TypedDict):
"severity": "Warning",
"message": "unused `with` expression",
},
{
"sname": "sema-escaping-with",
"cname": "EscapingWith",
"severity": "Hint",
"message": "this variable comes from the scope outside of the `with` expression",
},
]
16 changes: 0 additions & 16 deletions libnixf/src/Sema/VariableLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,22 +97,6 @@ void VariableLookupAnalysis::lookupVar(const ExprVar &Var,
if (Def) {
Def->usedBy(Var);
Results.insert({&Var, LookupResult{LookupResultKind::Defined, Def}});

if (EnclosedWith && !Def->isBuiltin()) {
// Escaping from "with" to outer scope.
// https://github.com/NixOS/nix/issues/490

assert(WithEnv && "EnclosedWith -> WithEnv");
// Make a diagnostic.
Diagnostic &D =
Diags.emplace_back(Diagnostic::DK_EscapingWith, Var.range());
if (Def->syntax()) {
D.note(Note::NK_VarBindToThis, Def->syntax()->range());
}
const auto &KwWith =
static_cast<const nixf::ExprWith *>(WithEnv->syntax())->kwWith();
D.note(Note::NK_EscapingWith, KwWith.range());
}
return;
}
if (EnclosedWith) {
Expand Down
29 changes: 0 additions & 29 deletions libnixf/test/Sema/VariableLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,35 +273,6 @@ TEST_F(VLATest, FormalDef) {
ASSERT_EQ(Diags.size(), 0);
}

TEST_F(VLATest, EscapingWith) {
std::shared_ptr<Node> AST = parse("a: with { a = 1; b = 2; }; a + b", Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_EQ(Diags.size(), 1);

const Diagnostic &D = Diags[0];

ASSERT_EQ(D.notes().size(), 2);

ASSERT_EQ(D.notes()[0].kind(), Note::NK_VarBindToThis);
ASSERT_EQ(D.notes()[0].range().lCur().offset(), 0);
ASSERT_EQ(D.notes()[0].range().rCur().offset(), 1);

ASSERT_EQ(D.notes()[1].kind(), Note::NK_EscapingWith);
ASSERT_EQ(D.notes()[1].range().lCur().offset(), 3);
ASSERT_EQ(D.notes()[1].range().rCur().offset(), 7);
}

TEST_F(VLATest, EscapingWithButBuiltin) {
std::shared_ptr<Node> AST =
parse("with { a = 1; }; [ a true false null ]", Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_EQ(Diags.size(), 0);
}

TEST_F(VLATest, InheritRec) {
// Make sure inheirt (expr), the expression is binded to "NewEnv".
std::shared_ptr<Node> AST =
Expand Down
4 changes: 2 additions & 2 deletions nixd/docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ nvim_lsp.nixd.setup({
// Control the diagnostic system
"diagnostic": {
"suppress": [
"sema-escaping-with"
"sema-extra-with"
]
}
}
Expand All @@ -167,7 +167,7 @@ prefer to suppress diagnostics altogether. This can be achieved by utilizing the
"diagnostic": {
// A list of diagnostic short names
"suppress": [
"sema-escaping-with"
"sema-extra-with"
]
}
}
Expand Down
Loading