From ce4cd6c3970e2623354eab4f25154921f5b4df69 Mon Sep 17 00:00:00 2001 From: Aleksana Date: Wed, 31 Jul 2024 17:15:25 +0800 Subject: [PATCH 01/10] libnixf/Sema: differentiate lambda argument with and without formals (#561) Fixes https://github.com/nix-community/nixd/issues/560 --- libnixf/include/nixf/Sema/VariableLookup.h | 3 +++ libnixf/src/Sema/VariableLookup.cpp | 11 ++++++++--- libnixf/test/Sema/VariableLookup.cpp | 13 +++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/libnixf/include/nixf/Sema/VariableLookup.h b/libnixf/include/nixf/Sema/VariableLookup.h index eae96c2a9..e470415cd 100644 --- a/libnixf/include/nixf/Sema/VariableLookup.h +++ b/libnixf/include/nixf/Sema/VariableLookup.h @@ -39,6 +39,9 @@ class Definition { /// \brief From lambda formal, e.g. { a }: a + 1 DS_LambdaFormal, + /// \brief From lambda arg with formal, e.g. { foo, bar }@a: a + 1 + DS_LambdaArgWithFormal, + /// \brief From recursive attribute set. e.g. rec { } DS_Rec, diff --git a/libnixf/src/Sema/VariableLookup.cpp b/libnixf/src/Sema/VariableLookup.cpp index 294ea5beb..d25d3c3d4 100644 --- a/libnixf/src/Sema/VariableLookup.cpp +++ b/libnixf/src/Sema/VariableLookup.cpp @@ -129,11 +129,16 @@ void VariableLookupAnalysis::dfs(const ExprLambda &Lambda, // foo: body // ^~~<------- add function argument. if (Arg.id()) { - // Function arg cannot duplicate to it's formal. - // If it this unluckily happens, we would like to skip this definition. - if (!Arg.formals() || !Arg.formals()->dedup().contains(Arg.id()->name())) + if (!Arg.formals()) { ToDef.insert_or_assign(Arg.id(), DBuilder.add(Arg.id()->name(), Arg.id(), Definition::DS_LambdaArg)); + // Function arg cannot duplicate to it's formal. + // If it this unluckily happens, we would like to skip this definition. + } else if (!Arg.formals()->dedup().contains(Arg.id()->name())) { + ToDef.insert_or_assign(Arg.id(), + DBuilder.add(Arg.id()->name(), Arg.id(), + Definition::DS_LambdaArgWithFormal)); + } } // { foo, bar, ... } : body diff --git a/libnixf/test/Sema/VariableLookup.cpp b/libnixf/test/Sema/VariableLookup.cpp index 41d0de70b..4e1b0f409 100644 --- a/libnixf/test/Sema/VariableLookup.cpp +++ b/libnixf/test/Sema/VariableLookup.cpp @@ -172,6 +172,19 @@ TEST_F(VLATest, LivenessDupSymbol) { ASSERT_EQ(Diags[0].tags().size(), 0); } +TEST_F(VLATest, LivenessArgWithFormal) { + std::shared_ptr AST = parse("{ foo }@bar: foo", Diags); + VariableLookupAnalysis VLA(Diags); + VLA.runOnAST(*AST); + + ASSERT_EQ(Diags.size(), 1); + + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed); + ASSERT_EQ(Diags[0].range().lCur().column(), 8); + ASSERT_EQ(Diags[0].tags().size(), 1); + ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded); +} + TEST_F(VLATest, ToDefAttrs) { std::shared_ptr AST = parse("rec { x = 1; y = x; z = x; }", Diags); VariableLookupAnalysis VLA(Diags); From a9a8638ce3235b7bb3d3290206dc3ae52ac2a15d Mon Sep 17 00:00:00 2001 From: aleksana Date: Wed, 31 Jul 2024 18:07:23 +0800 Subject: [PATCH 02/10] libnixf: remove trailing space in sema-extra-rec message --- libnixf/src/Basic/diagnostic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnixf/src/Basic/diagnostic.py b/libnixf/src/Basic/diagnostic.py index 295ecd2ac..57d4fe837 100644 --- a/libnixf/src/Basic/diagnostic.py +++ b/libnixf/src/Basic/diagnostic.py @@ -193,7 +193,7 @@ class Diagnostic(TypedDict): "sname": "sema-extra-rec", "cname": "ExtraRecursive", "severity": "Hint", - "message": "attrset is not necessary to be `rec`ursive ", + "message": "attrset is not necessary to be `rec`ursive", }, { "sname": "sema-extra-with", From 41c499a453d523387314241227c78958fea643d9 Mon Sep 17 00:00:00 2001 From: aleksana Date: Wed, 31 Jul 2024 18:10:07 +0800 Subject: [PATCH 03/10] gitignore: add nix-build result links --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 154bdc5f4..e39d0cf46 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,6 @@ /html/ __pycache__ + +result +result-* From 4883bf9352cd5921f98b4136176aa9780eaa6b46 Mon Sep 17 00:00:00 2001 From: Yingchi Long Date: Wed, 31 Jul 2024 21:23:14 +0800 Subject: [PATCH 04/10] nixd/Controller: support completion for `pkgs.subpackages` (#563) Fixes: #555 Add auto-completion code-snippet like this: ```nix with pkgs.vimPlugins; [ | ] ``` i.e. with a "select" expression. ![Screenshot_20240731_211203](https://github.com/user-attachments/assets/7532baca-e20b-45bf-a44f-0270da5be1fb) --- nixd/lib/Controller/AST.cpp | 68 ++++++++++-------- nixd/lib/Controller/AST.h | 56 +++++++-------- nixd/lib/Controller/Completion.cpp | 5 +- nixd/lib/Controller/Definition.cpp | 3 +- .../nixd/test/completion/with-expr-select.md | 71 +++++++++++++++++++ 5 files changed, 143 insertions(+), 60 deletions(-) create mode 100644 nixd/tools/nixd/test/completion/with-expr-select.md diff --git a/nixd/lib/Controller/AST.cpp b/nixd/lib/Controller/AST.cpp index ba45c5c89..e941b1319 100644 --- a/nixd/lib/Controller/AST.cpp +++ b/nixd/lib/Controller/AST.cpp @@ -144,10 +144,10 @@ IdiomSetT IdiomSet{nixd::idioms::Pkgs, nixd::idioms::Lib}; auto ItLib = IdiomSet.find(nixd::idioms::Lib); auto ItPkgs = IdiomSet.find(nixd::idioms::Pkgs); -nixd::Selector idiomItSelector(IdiomSetT::iterator It) { +nixd::Selector getKnownIdiomSelector(IdiomSetT::iterator It) { // Unknown name, cannot deal with it. if (It == IdiomSet.end()) - throw nixd::NotAnIdiomException(); + throw nixd::idioms::NotAnIdiomException(); return [&]() -> nixd::Selector { if (It == ItLib) { @@ -162,25 +162,43 @@ nixd::Selector idiomItSelector(IdiomSetT::iterator It) { }(); } -IdiomSetT::iterator varIdiomIt(const nixf::ExprVar &Var) { - return IdiomSet.find(Var.id().name()); -}; - nixd::Selector varSelector(const nixf::ExprVar &Var) { - return idiomItSelector(varIdiomIt(Var)); + return getKnownIdiomSelector(IdiomSet.find(Var.id().name())); }; -nixd::Selector withSelector(const nixf::ExprWith &With) { - if (!With.with() || With.with()->kind() != Node::NK_ExprVar) - throw nixd::NotAnIdiomException(); - return varSelector(static_cast(*With.with())); +nixd::Selector withSelector(const nixf::ExprWith &With, + const nixf::VariableLookupAnalysis &VLA, + const nixf::ParentMapAnalysis &PM) { + if (!With.with()) + throw nixd::idioms::NotAnIdiomException(); + switch (With.with()->kind()) { + case Node::NK_ExprVar: + return nixd::idioms::mkVarSelector( + static_cast(*With.with()), VLA, PM); + case Node::NK_ExprSelect: + return nixd::idioms::mkSelector( + static_cast(*With.with()), VLA, PM); + default: + break; + } + throw nixd::idioms::NotAnIdiomException(); } } // namespace -nixd::Selector nixd::mkIdiomSelector(const nixf::ExprVar &Var, - const nixf::VariableLookupAnalysis &VLA, - const nixf::ParentMapAnalysis &PM) { +nixd::Selector nixd::idioms::mkSelector(const nixf::ExprSelect &Select, + nixd::Selector BaseSelector) { + if (Select.path()) + return nixd::idioms::mkSelector( + *static_cast(Select.path()), + std::move(BaseSelector)); + return BaseSelector; +} + +nixd::Selector +nixd::idioms::mkVarSelector(const nixf::ExprVar &Var, + const nixf::VariableLookupAnalysis &VLA, + const nixf::ParentMapAnalysis &PM) { // Only check if the variable can be recogonized by some idiom. using ResultKind = VariableLookupAnalysis::LookupResultKind; @@ -208,7 +226,7 @@ nixd::Selector nixd::mkIdiomSelector(const nixf::ExprVar &Var, assert(With && "parent of kwWith should be the with expression"); assert(With->kind() == nixf::Node::NK_ExprWith); Selector WithSelector = - withSelector(static_cast(*With)); + withSelector(static_cast(*With), VLA, PM); // Append variable name after "with" expression selector. // e.g. @@ -228,8 +246,8 @@ nixd::Selector nixd::mkIdiomSelector(const nixf::ExprVar &Var, return {}; } -nixd::Selector nixd::mkSelector(const nixf::AttrPath &AP, - Selector BaseSelector) { +nixd::Selector nixd::idioms::mkSelector(const nixf::AttrPath &AP, + Selector BaseSelector) { const auto &Names = AP.names(); for (const auto &Name : Names) { if (!Name->isStatic()) @@ -239,23 +257,15 @@ nixd::Selector nixd::mkSelector(const nixf::AttrPath &AP, return BaseSelector; } -nixd::Selector nixd::mkSelector(const nixf::ExprSelect &Select, - nixd::Selector BaseSelector) { - if (Select.path()) - return nixd::mkSelector(*static_cast(Select.path()), - std::move(BaseSelector)); - return BaseSelector; -} - -nixd::Selector nixd::mkSelector(const nixf::ExprSelect &Sel, - const nixf::VariableLookupAnalysis &VLA, - const nixf::ParentMapAnalysis &PM) { +nixd::Selector nixd::idioms::mkSelector(const nixf::ExprSelect &Sel, + const nixf::VariableLookupAnalysis &VLA, + const nixf::ParentMapAnalysis &PM) { if (Sel.expr().kind() != Node::NK_ExprVar) throw NotVariableSelect(); const auto &Var = static_cast(Sel.expr()); - auto BaseSelector = mkIdiomSelector(Var, VLA, PM); + auto BaseSelector = mkVarSelector(Var, VLA, PM); return mkSelector(Sel, std::move(BaseSelector)); } diff --git a/nixd/lib/Controller/AST.h b/nixd/lib/Controller/AST.h index fd04e0366..2c6a3152b 100644 --- a/nixd/lib/Controller/AST.h +++ b/nixd/lib/Controller/AST.h @@ -25,31 +25,6 @@ constexpr inline std::string_view Pkgs = "pkgs"; /// e.g. lib.genAttrs. constexpr inline std::string_view Lib = "lib"; -} // namespace idioms - -/// \brief Search up until there are some node associated with "EnvNode". -[[nodiscard]] const nixf::EnvNode * -upEnv(const nixf::Node &Desc, const nixf::VariableLookupAnalysis &VLA, - const nixf::ParentMapAnalysis &PM); - -/// \brief Determine whether or not some node has enclosed "with pkgs; [ ]" -/// -/// Yes, this evaluation isn't flawless. What if the identifier isn't "pkgs"? We -/// can't dynamically evaluate everything each time and invalidate them -/// immediately after document updates. Therefore, this heuristic method -/// represents a trade-off between performance considerations. -[[nodiscard]] bool havePackageScope(const nixf::Node &N, - const nixf::VariableLookupAnalysis &VLA, - const nixf::ParentMapAnalysis &PM); - -/// \brief get variable scope, and it's prefix name. -/// -/// Nixpkgs has some packages scoped in "nested" attrs. -/// e.g. llvmPackages, pythonPackages. -/// Try to find these name as a pre-selected scope, the last value is "prefix". -std::pair, std::string> -getScopeAndPrefix(const nixf::Node &N, const nixf::ParentMapAnalysis &PM); - struct IdiomException : std::exception {}; /// \brief Exceptions scoped in nixd::mkIdiomSelector @@ -81,9 +56,9 @@ struct UndefinedVarException : VLAException { /// /// Try to heuristically find a selector of a variable, based on some known /// idioms. -Selector mkIdiomSelector(const nixf::ExprVar &Var, - const nixf::VariableLookupAnalysis &VLA, - const nixf::ParentMapAnalysis &PM); +Selector mkVarSelector(const nixf::ExprVar &Var, + const nixf::VariableLookupAnalysis &VLA, + const nixf::ParentMapAnalysis &PM); /// \brief The attrpath has a dynamic name, thus it cannot be trivially /// transformed to "static" selector. @@ -110,6 +85,31 @@ Selector mkSelector(const nixf::ExprSelect &Select, const nixf::VariableLookupAnalysis &VLA, const nixf::ParentMapAnalysis &PM); +} // namespace idioms + +/// \brief Search up until there are some node associated with "EnvNode". +[[nodiscard]] const nixf::EnvNode * +upEnv(const nixf::Node &Desc, const nixf::VariableLookupAnalysis &VLA, + const nixf::ParentMapAnalysis &PM); + +/// \brief Determine whether or not some node has enclosed "with pkgs; [ ]" +/// +/// Yes, this evaluation isn't flawless. What if the identifier isn't "pkgs"? We +/// can't dynamically evaluate everything each time and invalidate them +/// immediately after document updates. Therefore, this heuristic method +/// represents a trade-off between performance considerations. +[[nodiscard]] bool havePackageScope(const nixf::Node &N, + const nixf::VariableLookupAnalysis &VLA, + const nixf::ParentMapAnalysis &PM); + +/// \brief get variable scope, and it's prefix name. +/// +/// Nixpkgs has some packages scoped in "nested" attrs. +/// e.g. llvmPackages, pythonPackages. +/// Try to find these name as a pre-selected scope, the last value is "prefix". +std::pair, std::string> +getScopeAndPrefix(const nixf::Node &N, const nixf::ParentMapAnalysis &PM); + enum class FindAttrPathResult { OK, Inherit, diff --git a/nixd/lib/Controller/Completion.cpp b/nixd/lib/Controller/Completion.cpp index 334d23c73..93dc2343c 100644 --- a/nixd/lib/Controller/Completion.cpp +++ b/nixd/lib/Controller/Completion.cpp @@ -313,7 +313,7 @@ void completeVarName(const VariableLookupAnalysis &VLA, // Try to complete the name by known idioms. try { - Selector Sel = mkIdiomSelector(N, VLA, PM); + Selector Sel = idioms::mkVarSelector(N, VLA, PM); // Clickling "pkgs" does not make sense for variable completion if (Sel.empty()) @@ -359,7 +359,8 @@ void completeSelect(const nixf::ExprSelect &Select, AttrSetClient &Client, NixpkgsCompletionProvider NCP(Client); try { - Selector Sel = mkSelector(Select, mkIdiomSelector(Var, VLA, PM)); + Selector Sel = + idioms::mkSelector(Select, idioms::mkVarSelector(Var, VLA, PM)); NCP.completePackages(mkParams(Sel, IsComplete), List); } catch (ExceedSizeError &) { // Let "onCompletion" catch this exception to set "inComplete" field. diff --git a/nixd/lib/Controller/Definition.cpp b/nixd/lib/Controller/Definition.cpp index 5a35503d6..908a58195 100644 --- a/nixd/lib/Controller/Definition.cpp +++ b/nixd/lib/Controller/Definition.cpp @@ -27,6 +27,7 @@ #include using namespace nixd; +using namespace nixd::idioms; using namespace nixf; using namespace lspserver; using namespace llvm; @@ -298,7 +299,7 @@ Locations defineVar(const ExprVar &Var, const VariableLookupAnalysis &VLA, // Nixpkgs locations. try { - Selector Sel = mkIdiomSelector(Var, VLA, PM); + Selector Sel = mkVarSelector(Var, VLA, PM); Locations NixpkgsLocs = defineNixpkgsSelector(Sel, NixpkgsClient); return mergeVec(std::move(StaticLocs), NixpkgsLocs); } catch (std::exception &E) { diff --git a/nixd/tools/nixd/test/completion/with-expr-select.md b/nixd/tools/nixd/test/completion/with-expr-select.md new file mode 100644 index 000000000..9eff2dade --- /dev/null +++ b/nixd/tools/nixd/test/completion/with-expr-select.md @@ -0,0 +1,71 @@ +# RUN: nixd --nixpkgs-expr='{ ax.bx = 1; ax.by = 2; }' --lit-test < %s | FileCheck %s + +<-- initialize(0) + +```json +{ + "jsonrpc":"2.0", + "id":0, + "method":"initialize", + "params":{ + "processId":123, + "rootPath":"", + "capabilities":{ + }, + "trace":"off" + } +} +``` + + +<-- textDocument/didOpen + + +```json +{ + "jsonrpc":"2.0", + "method":"textDocument/didOpen", + "params":{ + "textDocument":{ + "uri":"file:///completion.nix", + "languageId":"nix", + "version":1, + "text":"with pkgs.ax; [ b ]" + } + } +} +``` + +```json +{ + "jsonrpc": "2.0", + "id": 1, + "method": "textDocument/completion", + "params": { + "textDocument": { + "uri": "file:///completion.nix" + }, + "position": { + "line": 0, + "character": 16 + }, + "context": { + "triggerKind": 1 + } + } +} +``` + +``` + CHECK: "data": "{\"Prefix\":\"b\",\"Scope\":[\"ax\"]}", +CHECK-NEXT: "kind": 5, +CHECK-NEXT: "label": "bx", +CHECK-NEXT: "score": 0 +CHECK-NEXT: }, +CHECK-NEXT: { +CHECK-NEXT: "data": "{\"Prefix\":\"b\",\"Scope\":[\"ax\"]}", +CHECK-NEXT: "kind": 5, +CHECK-NEXT: "label": "by", +CHECK-NEXT: "score": 0 +CHECK-NEXT: } +``` From a53ba5915754a76d0420bb39398e4b49351d6c4f Mon Sep 17 00:00:00 2001 From: Aleksana Date: Wed, 31 Jul 2024 21:41:36 +0800 Subject: [PATCH 05/10] flake: add treefmt-nix (#562) --- .github/workflows/treefmt.yml | 43 +++++++++++++++++++++++++++++++++++ flake.lock | 23 ++++++++++++++++++- flake.nix | 9 +++++++- treefmt.nix | 10 ++++++++ 4 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/treefmt.yml create mode 100644 treefmt.nix diff --git a/.github/workflows/treefmt.yml b/.github/workflows/treefmt.yml new file mode 100644 index 000000000..0aa84f681 --- /dev/null +++ b/.github/workflows/treefmt.yml @@ -0,0 +1,43 @@ +name: Check formatting with treefmt + +on: + [push, pull_request] + +jobs: + treefmt: + runs-on: ubuntu-latest + steps: + - uses: cachix/install-nix-action@v22 + - uses: actions/checkout@v3 + - id: files + uses: tj-actions/changed-files@v44 + - name: Run treefmt + run: nix fmt + - name: Check diff + run: | + unformatted_touched=() + unformatted_untouched=() + while IFS= read -r unformatted_file; do + matched= + for changed_file in ${{ steps.files.outputs.all }}; do + if [[ "$changed_file" == "$unformatted_file" ]]; then + unformatted_touched+=("$unformatted_file") + matched=1 + break + fi + done + if [[ -z "$matched" ]]; then + unformatted_untouched+=("$unformatted_file") + fi + done <<< "$(git diff --name-only)" + + if (( ${#unformatted_untouched[@]} )); then + echo "These files are not formatted, but out of scope of this PR:" + printf '%s\n' "${unformatted_untouched[@]}" + fi + + if (( ${#unformatted_touched[@]} )); then + echo "These files are created/edited but not formatted:" + printf '%s\n' "${unformatted_touched[@]}" + exit 1 + fi diff --git a/flake.lock b/flake.lock index 21a9a7e86..5b7dcf117 100644 --- a/flake.lock +++ b/flake.lock @@ -71,7 +71,28 @@ "inputs": { "flake-parts": "flake-parts", "flake-root": "flake-root", - "nixpkgs": "nixpkgs" + "nixpkgs": "nixpkgs", + "treefmt-nix": "treefmt-nix" + } + }, + "treefmt-nix": { + "inputs": { + "nixpkgs": [ + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1722330636, + "narHash": "sha256-uru7JzOa33YlSRwf9sfXpJG+UAV+bnBEYMjrzKrQZFw=", + "owner": "numtide", + "repo": "treefmt-nix", + "rev": "768acdb06968e53aa1ee8de207fd955335c754b7", + "type": "github" + }, + "original": { + "owner": "numtide", + "repo": "treefmt-nix", + "type": "github" } } }, diff --git a/flake.nix b/flake.nix index 32eafae73..2fcfe2020 100644 --- a/flake.nix +++ b/flake.nix @@ -5,9 +5,14 @@ flake-parts.url = "github:hercules-ci/flake-parts"; flake-root.url = "github:srid/flake-root"; + + treefmt-nix = { + url = "github:numtide/treefmt-nix"; + inputs.nixpkgs.follows = "nixpkgs"; + }; }; - outputs = { nixpkgs, flake-parts, ... }@inputs: flake-parts.lib.mkFlake { inherit inputs; } { + outputs = { nixpkgs, flake-parts, treefmt-nix, ... }@inputs: flake-parts.lib.mkFlake { inherit inputs; } { imports = [ inputs.flake-parts.flakeModules.easyOverlay inputs.flake-root.flakeModule @@ -45,6 +50,7 @@ ''; hardeningDisable = [ "fortify" ]; }; + treefmtEval = treefmt-nix.lib.evalModule pkgs ./treefmt.nix; in { packages.default = nixd; @@ -103,6 +109,7 @@ (import ./nixd/docs/editors/vscodium.nix { inherit pkgs; }) ]; }; + formatter = treefmtEval.config.build.wrapper; }; systems = nixpkgs.lib.systems.flakeExposed; }; diff --git a/treefmt.nix b/treefmt.nix new file mode 100644 index 000000000..00a70fc3f --- /dev/null +++ b/treefmt.nix @@ -0,0 +1,10 @@ +{ ... }: + +{ + projectRootFile = "flake.nix"; + programs = { + clang-format.enable = true; + nixpkgs-fmt.enable = true; + black.enable = true; + }; +} From 5c3f5cad9f5ad30eca6dca66b396a1bfac4c889e Mon Sep 17 00:00:00 2001 From: aleksana Date: Wed, 31 Jul 2024 22:16:55 +0800 Subject: [PATCH 06/10] ci: minor fix to treefmt check --- .github/workflows/treefmt.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/treefmt.yml b/.github/workflows/treefmt.yml index 0aa84f681..0a5ef7c6d 100644 --- a/.github/workflows/treefmt.yml +++ b/.github/workflows/treefmt.yml @@ -8,18 +8,20 @@ jobs: runs-on: ubuntu-latest steps: - uses: cachix/install-nix-action@v22 - - uses: actions/checkout@v3 - - id: files + - uses: actions/checkout@v4 + - id: changed-files uses: tj-actions/changed-files@v44 - name: Run treefmt run: nix fmt - name: Check diff + env: + ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }} run: | unformatted_touched=() unformatted_untouched=() while IFS= read -r unformatted_file; do matched= - for changed_file in ${{ steps.files.outputs.all }}; do + for changed_file in ${ALL_CHANGED_FILES}; do if [[ "$changed_file" == "$unformatted_file" ]]; then unformatted_touched+=("$unformatted_file") matched=1 @@ -36,6 +38,8 @@ jobs: printf '%s\n' "${unformatted_untouched[@]}" fi + echo # blank line + if (( ${#unformatted_touched[@]} )); then echo "These files are created/edited but not formatted:" printf '%s\n' "${unformatted_touched[@]}" From 187aabe2cadb5eff43d862134f4ac99e33edaf4e Mon Sep 17 00:00:00 2001 From: Yingchi Long Date: Fri, 2 Aug 2024 15:13:58 +0800 Subject: [PATCH 07/10] libnixf/test/Sema: format VariableLookup.cpp --- libnixf/test/Sema/VariableLookup.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libnixf/test/Sema/VariableLookup.cpp b/libnixf/test/Sema/VariableLookup.cpp index 4e1b0f409..ef11bde0c 100644 --- a/libnixf/test/Sema/VariableLookup.cpp +++ b/libnixf/test/Sema/VariableLookup.cpp @@ -284,10 +284,11 @@ TEST_F(VLATest, EscapingWith) { } TEST_F(VLATest, EscapingWithButBuiltin) { - std::shared_ptr AST = parse("with { a = 1; }; [ a true false null ]", Diags); + std::shared_ptr AST = + parse("with { a = 1; }; [ a true false null ]", Diags); VariableLookupAnalysis VLA(Diags); VLA.runOnAST(*AST); - + ASSERT_EQ(Diags.size(), 0); } From d687efae4a9931ac08e718712cf69de322473847 Mon Sep 17 00:00:00 2001 From: Aleksana Date: Fri, 2 Aug 2024 15:42:40 +0800 Subject: [PATCH 08/10] libnixf: split sema-unused-def into let, arg and formal (#565) Related https://github.com/NixOS/nixpkgs/issues/331085 Fixes: #558 --------- Co-authored-by: Yingchi Long --- libnixf/include/nixf/Sema/VariableLookup.h | 13 ++++-- libnixf/src/Basic/diagnostic.py | 24 ++++++++-- libnixf/src/Sema/VariableLookup.cpp | 45 +++++++++++++++---- libnixf/test/Sema/VariableLookup.cpp | 24 +++++++--- .../nixd/test/diagnostic/liveness-formal.md | 6 +-- 5 files changed, 86 insertions(+), 26 deletions(-) diff --git a/libnixf/include/nixf/Sema/VariableLookup.h b/libnixf/include/nixf/Sema/VariableLookup.h index e470415cd..7b0843f3e 100644 --- a/libnixf/include/nixf/Sema/VariableLookup.h +++ b/libnixf/include/nixf/Sema/VariableLookup.h @@ -36,11 +36,16 @@ class Definition { /// \brief From ambda arg e.g. a: a + 1 DS_LambdaArg, - /// \brief From lambda formal, e.g. { a }: a + 1 - DS_LambdaFormal, + /// \brief From lambda (noarg) formal, e.g. { a }: a + 1 + DS_LambdaNoArg_Formal, - /// \brief From lambda arg with formal, e.g. { foo, bar }@a: a + 1 - DS_LambdaArgWithFormal, + /// \brief From lambda (with `@arg`) `arg`, + /// e.g. `a` in `{ foo }@a: foo + 1` + DS_LambdaWithArg_Arg, + + /// \brief From lambda (with `@arg`) formal, + /// e.g. `foo` in `{ foo }@a: foo + 1` + DS_LambdaWithArg_Formal, /// \brief From recursive attribute set. e.g. rec { } DS_Rec, diff --git a/libnixf/src/Basic/diagnostic.py b/libnixf/src/Basic/diagnostic.py index 57d4fe837..ecaef03ea 100644 --- a/libnixf/src/Basic/diagnostic.py +++ b/libnixf/src/Basic/diagnostic.py @@ -184,10 +184,28 @@ class Diagnostic(TypedDict): "message": "undefined variable `{}`", }, { - "sname": "sema-def-not-used", - "cname": "DefinitionNotUsed", + "sname": "sema-unused-def-let", + "cname": "UnusedDefLet", + "severity": "Warning", + "message": "definition `{}` in let-expression is not used", + }, + { + "sname": "sema-unused-def-lambda-noarg-formal", + "cname": "UnusedDefLambdaNoArg_Formal", + "severity": "Warning", + "message": "attribute `{}` of argument is not used", + }, + { + "sname": "sema-unused-def-lambda-witharg-formal", + "cname": "UnusedDefLambdaWithArg_Formal", + "severity": "Warning", + "message": "argument `{}` in `@`-pattern is not used", + }, + { + "sname": "sema-unused-def-lambda-witharg-arg", + "cname": "UnusedDefLambdaWithArg_Arg", "severity": "Hint", - "message": "definition `{}` is not used", + "message": "attribute `{}` of `@`-pattern argument is not used, but may be referenced from the argument", }, { "sname": "sema-extra-rec", diff --git a/libnixf/src/Sema/VariableLookup.cpp b/libnixf/src/Sema/VariableLookup.cpp index d25d3c3d4..9b3b38ab0 100644 --- a/libnixf/src/Sema/VariableLookup.cpp +++ b/libnixf/src/Sema/VariableLookup.cpp @@ -52,8 +52,22 @@ void VariableLookupAnalysis::emitEnvLivenessWarning( if (!Def->syntax()) continue; if (Def->uses().empty()) { - Diagnostic &D = Diags.emplace_back(Diagnostic::DK_DefinitionNotUsed, - Def->syntax()->range()); + Diagnostic::DiagnosticKind Kind = [&]() { + switch (Def->source()) { + case Definition::DS_Let: + return Diagnostic::DK_UnusedDefLet; + case Definition::DS_LambdaNoArg_Formal: + return Diagnostic::DK_UnusedDefLambdaNoArg_Formal; + case Definition::DS_LambdaWithArg_Formal: + return Diagnostic::DK_UnusedDefLambdaWithArg_Formal; + case Definition::DS_LambdaWithArg_Arg: + return Diagnostic::DK_UnusedDefLambdaWithArg_Arg; + default: + assert(false && "liveness diagnostic encountered an unknown source!"); + __builtin_unreachable(); + } + }(); + Diagnostic &D = Diags.emplace_back(Kind, Def->syntax()->range()); D << Name; D.tag(DiagnosticTag::Faded); } @@ -137,17 +151,30 @@ void VariableLookupAnalysis::dfs(const ExprLambda &Lambda, } else if (!Arg.formals()->dedup().contains(Arg.id()->name())) { ToDef.insert_or_assign(Arg.id(), DBuilder.add(Arg.id()->name(), Arg.id(), - Definition::DS_LambdaArgWithFormal)); + Definition::DS_LambdaWithArg_Arg)); } } // { foo, bar, ... } : body - /// ^~~~~~~~~<-------------- add function formals. - if (Arg.formals()) - for (const auto &[Name, Formal] : Arg.formals()->dedup()) - ToDef.insert_or_assign( - Formal->id(), - DBuilder.add(Name, Formal->id(), Definition::DS_LambdaFormal)); + // ^~~~~~~~~<-------------- add function formals. + + // This section differentiates between formal parameters with an argument and + // without. Example: + // + // { foo }@arg : use arg + // + // In this case, the definition of `foo` is not used directly; however, it + // might be accessed via arg.foo. Therefore, the severity of an unused formal + // parameter is reduced in this scenario. + if (Arg.formals()) { + for (const auto &[Name, Formal] : Arg.formals()->dedup()) { + Definition::DefinitionSource Source = + Arg.id() ? Definition::DS_LambdaWithArg_Formal + : Definition::DS_LambdaNoArg_Formal; + ToDef.insert_or_assign(Formal->id(), + DBuilder.add(Name, Formal->id(), Source)); + } + } auto NewEnv = std::make_shared(Env, DBuilder.finish(), &Lambda); diff --git a/libnixf/test/Sema/VariableLookup.cpp b/libnixf/test/Sema/VariableLookup.cpp index ef11bde0c..b48a78784 100644 --- a/libnixf/test/Sema/VariableLookup.cpp +++ b/libnixf/test/Sema/VariableLookup.cpp @@ -135,29 +135,27 @@ TEST_F(VLATest, LivenessRec) { ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_ExtraRecursive); } -TEST_F(VLATest, LivenessArg) { +TEST_F(VLATest, LivenessFormal) { std::shared_ptr AST = parse("{ foo }: 1", Diags); VariableLookupAnalysis VLA(Diags); VLA.runOnAST(*AST); ASSERT_EQ(Diags.size(), 1); - ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed); + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaNoArg_Formal); ASSERT_EQ(Diags[0].tags().size(), 1); - ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded); } -TEST_F(VLATest, LivenessNested) { +TEST_F(VLATest, LivenessLet) { std::shared_ptr AST = parse("let y = 1; in x: y: x + y", Diags); VariableLookupAnalysis VLA(Diags); VLA.runOnAST(*AST); ASSERT_EQ(Diags.size(), 1); - ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed); + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLet); ASSERT_EQ(Diags[0].range().lCur().column(), 4); ASSERT_EQ(Diags[0].tags().size(), 1); - ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded); } TEST_F(VLATest, LivenessDupSymbol) { @@ -179,9 +177,21 @@ TEST_F(VLATest, LivenessArgWithFormal) { ASSERT_EQ(Diags.size(), 1); - ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed); + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaWithArg_Arg); ASSERT_EQ(Diags[0].range().lCur().column(), 8); ASSERT_EQ(Diags[0].tags().size(), 1); +} + +TEST_F(VLATest, LivenessFormalWithArg) { + std::shared_ptr AST = parse("{ foo }@bar: bar", Diags); + VariableLookupAnalysis VLA(Diags); + VLA.runOnAST(*AST); + + ASSERT_EQ(Diags.size(), 1); + + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaWithArg_Formal); + ASSERT_EQ(Diags[0].range().lCur().column(), 2); + ASSERT_EQ(Diags[0].tags().size(), 1); ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded); } diff --git a/nixd/tools/nixd/test/diagnostic/liveness-formal.md b/nixd/tools/nixd/test/diagnostic/liveness-formal.md index f90dd5b71..6391524cb 100644 --- a/nixd/tools/nixd/test/diagnostic/liveness-formal.md +++ b/nixd/tools/nixd/test/diagnostic/liveness-formal.md @@ -38,8 +38,8 @@ ``` CHECK: "diagnostics": [ CHECK-NEXT: { -CHECK-NEXT: "code": "sema-def-not-used", -CHECK-NEXT: "message": "definition `y` is not used", +CHECK-NEXT: "code": "sema-unused-def-lambda-noarg-formal", +CHECK-NEXT: "message": "attribute `y` of argument is not used", CHECK-NEXT: "range": { CHECK-NEXT: "end": { CHECK-NEXT: "character": 5, @@ -51,7 +51,7 @@ CHECK-NEXT: "line": 0 CHECK-NEXT: } CHECK-NEXT: }, CHECK-NEXT: "relatedInformation": [], -CHECK-NEXT: "severity": 4, +CHECK-NEXT: "severity": 2, CHECK-NEXT: "source": "nixf", CHECK-NEXT: "tags": [ CHECK-NEXT: 1 From d938026c55c7c36a6e79afd9627459160b4924ed Mon Sep 17 00:00:00 2001 From: Aleksana Date: Fri, 2 Aug 2024 15:44:15 +0800 Subject: [PATCH 09/10] libnixf: change extra-{rec,with} diagnostics to warning (#564) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because if you don’t remove such unnecessary patterns in time, you may make subtle mistakes later --------- Co-authored-by: Yingchi Long --- libnixf/src/Basic/diagnostic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libnixf/src/Basic/diagnostic.py b/libnixf/src/Basic/diagnostic.py index ecaef03ea..c0a9e6710 100644 --- a/libnixf/src/Basic/diagnostic.py +++ b/libnixf/src/Basic/diagnostic.py @@ -210,13 +210,13 @@ class Diagnostic(TypedDict): { "sname": "sema-extra-rec", "cname": "ExtraRecursive", - "severity": "Hint", + "severity": "Warning", "message": "attrset is not necessary to be `rec`ursive", }, { "sname": "sema-extra-with", "cname": "ExtraWith", - "severity": "Hint", + "severity": "Warning", "message": "unused `with` expression", }, { From af1255ebad535dfc4b96705bc0628d5e43be5f95 Mon Sep 17 00:00:00 2001 From: Yingchi Long Date: Tue, 6 Aug 2024 10:37:41 +0800 Subject: [PATCH 10/10] docs: remove project matrix room Please always submit issues on github. --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index af7771790..de7b6b6f8 100644 --- a/README.md +++ b/README.md @@ -29,4 +29,3 @@ We have tested some working & reproducible [editor environments](/nixd/docs/edit - [Configuration](nixd/docs/configuration.md) (see how to, and which options are tunable) - [Features](nixd/docs/features.md) (features explanation) - [Developers' Manual](nixd/docs/dev.md) (internal design, contributing) -- Project matrix room: https://matrix.to/#/#nixd:matrix.org