From 31a08969a5e39cf3b8e0da23ef28633e3926972d Mon Sep 17 00:00:00 2001 From: Yingchi Long Date: Sun, 11 Aug 2024 20:50:32 +0800 Subject: [PATCH] libnixf: remove `sema-escaping-with` because it is too pedantic This diagnostic highlighted too much. In some cases, people may use "let" expression to override something inside `with pkgs;` If "escaping-with" is diagnosed, there is no good fix to workaround, without completely disable that diagnostic in configuration. --- libnixf/include/nixf/Basic/NoteKinds.inc | 4 +--- libnixf/src/Basic/diagnostic.py | 6 ----- libnixf/src/Sema/VariableLookup.cpp | 16 ------------- libnixf/test/Sema/VariableLookup.cpp | 29 ------------------------ nixd/docs/configuration.md | 4 ++-- 5 files changed, 3 insertions(+), 56 deletions(-) diff --git a/libnixf/include/nixf/Basic/NoteKinds.inc b/libnixf/include/nixf/Basic/NoteKinds.inc index 247d6a830..9ef46eefb 100644 --- a/libnixf/include/nixf/Basic/NoteKinds.inc +++ b/libnixf/include/nixf/Basic/NoteKinds.inc @@ -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 diff --git a/libnixf/src/Basic/diagnostic.py b/libnixf/src/Basic/diagnostic.py index c0a9e6710..c55e11795 100644 --- a/libnixf/src/Basic/diagnostic.py +++ b/libnixf/src/Basic/diagnostic.py @@ -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", - }, ] diff --git a/libnixf/src/Sema/VariableLookup.cpp b/libnixf/src/Sema/VariableLookup.cpp index 9b3b38ab0..dfe045ef1 100644 --- a/libnixf/src/Sema/VariableLookup.cpp +++ b/libnixf/src/Sema/VariableLookup.cpp @@ -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(WithEnv->syntax())->kwWith(); - D.note(Note::NK_EscapingWith, KwWith.range()); - } return; } if (EnclosedWith) { diff --git a/libnixf/test/Sema/VariableLookup.cpp b/libnixf/test/Sema/VariableLookup.cpp index b48a78784..89e596bd5 100644 --- a/libnixf/test/Sema/VariableLookup.cpp +++ b/libnixf/test/Sema/VariableLookup.cpp @@ -273,35 +273,6 @@ TEST_F(VLATest, FormalDef) { ASSERT_EQ(Diags.size(), 0); } -TEST_F(VLATest, EscapingWith) { - std::shared_ptr 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 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 AST = diff --git a/nixd/docs/configuration.md b/nixd/docs/configuration.md index bfad71978..f13866b94 100644 --- a/nixd/docs/configuration.md +++ b/nixd/docs/configuration.md @@ -149,7 +149,7 @@ nvim_lsp.nixd.setup({ // Control the diagnostic system "diagnostic": { "suppress": [ - "sema-escaping-with" + "sema-extra-with" ] } } @@ -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" ] } }