diff --git a/libnixf/include/nixf/Basic/Diagnostic.h b/libnixf/include/nixf/Basic/Diagnostic.h index 3c8b14652..cbb9eb5b2 100644 --- a/libnixf/include/nixf/Basic/Diagnostic.h +++ b/libnixf/include/nixf/Basic/Diagnostic.h @@ -57,6 +57,7 @@ class TextEdit { class Fix { std::vector Edits; std::string Message; + bool Preferred = false; public: Fix(std::vector Edits, std::string Message) @@ -69,6 +70,11 @@ class Fix { [[nodiscard]] const std::vector &edits() const { return Edits; } [[nodiscard]] const std::string &message() const { return Message; } + Fix &prefer() { + Preferred = true; + return *this; + } + [[nodiscard]] bool isPreferred() const { return Preferred; } }; enum class DiagnosticTag { diff --git a/libnixf/src/Sema/VariableLookup.cpp b/libnixf/src/Sema/VariableLookup.cpp index dad02dad1..0651d5ea7 100644 --- a/libnixf/src/Sema/VariableLookup.cpp +++ b/libnixf/src/Sema/VariableLookup.cpp @@ -4,6 +4,7 @@ #include "nixf/Basic/Nodes/Expr.h" #include "nixf/Basic/Nodes/Lambda.h" #include "nixf/Sema/PrimOpInfo.h" +#include "nixf/Sema/SemaActions.h" #include @@ -157,13 +158,33 @@ void VariableLookupAnalysis::emitEnvLivenessWarning( if (D.range().lCur() == FirstName->range().lCur() && D.range().rCur() == FirstName->range().rCur()) { D.fix("remove unused binding") - .edit(TextEdit::mkRemoval(Bind.range())); + .edit(TextEdit::mkRemoval(Bind.range())) + .prefer(); break; } } } } } + // Attach fix for unused formal parameters + if (Def->source() == Definition::DS_LambdaNoArg_Formal || + Def->source() == Definition::DS_LambdaWithArg_Formal) { + // Def->source() guarantees NewEnv->syntax() is ExprLambda* + const auto *Lambda = static_cast(NewEnv->syntax()); + if (Lambda->arg() && Lambda->arg()->formals()) { + const auto &FV = Lambda->arg()->formals()->members(); + for (auto It = FV.begin(); It != FV.end(); ++It) { + if (!(*It)->id()) // skip ellipsis + continue; + if ((*It)->id() == Def->syntax()) { + Fix &F = D.fix("remove unused formal `" + Name + "`"); + Sema::removeFormal(F, It, FV); + F.prefer(); + break; + } + } + } + } } } } diff --git a/libnixf/test/Sema/VariableLookup.cpp b/libnixf/test/Sema/VariableLookup.cpp index 18224caf6..682f02b11 100644 --- a/libnixf/test/Sema/VariableLookup.cpp +++ b/libnixf/test/Sema/VariableLookup.cpp @@ -195,6 +195,63 @@ TEST_F(VLATest, LivenessFormalWithArg) { ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded); } +TEST_F(VLATest, UnusedFormalFix) { + std::shared_ptr AST = parse("{x, y}: x", Diags); + VariableLookupAnalysis VLA(Diags); + VLA.runOnAST(*AST); + + ASSERT_EQ(Diags.size(), 1); + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaNoArg_Formal); + ASSERT_EQ(Diags[0].fixes().size(), 1); + ASSERT_EQ(Diags[0].fixes()[0].message(), "remove unused formal `y`"); + ASSERT_TRUE(Diags[0].fixes()[0].isPreferred()); +} + +TEST_F(VLATest, UnusedFormalFixSingle) { + std::shared_ptr AST = parse("{ x }: 1", Diags); + VariableLookupAnalysis VLA(Diags); + VLA.runOnAST(*AST); + + ASSERT_EQ(Diags.size(), 1); + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaNoArg_Formal); + ASSERT_EQ(Diags[0].fixes().size(), 1); + ASSERT_EQ(Diags[0].fixes()[0].edits().size(), 1); + ASSERT_TRUE(Diags[0].fixes()[0].isPreferred()); +} + +TEST_F(VLATest, UnusedFormalFixFirst) { + std::shared_ptr AST = parse("{x, y}: y", Diags); + VariableLookupAnalysis VLA(Diags); + VLA.runOnAST(*AST); + + ASSERT_EQ(Diags.size(), 1); + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaNoArg_Formal); + ASSERT_EQ(Diags[0].fixes().size(), 1); + ASSERT_EQ(Diags[0].fixes()[0].message(), "remove unused formal `x`"); + // First formal removal needs 2 edits: remove formal + remove next comma + ASSERT_EQ(Diags[0].fixes()[0].edits().size(), 2); + ASSERT_TRUE(Diags[0].fixes()[0].isPreferred()); +} + +TEST_F(VLATest, UnusedFormalFixWithArg) { + // {x, y}@args: args — formals are DS_LambdaWithArg_Formal + std::shared_ptr AST = parse("{x, y}@args: args", Diags); + VariableLookupAnalysis VLA(Diags); + VLA.runOnAST(*AST); + + // Both x and y are unused formals (args is used via @-pattern) + ASSERT_EQ(Diags.size(), 2); + ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaWithArg_Formal); + ASSERT_EQ(Diags[0].fixes().size(), 1); + ASSERT_EQ(Diags[0].fixes()[0].message(), "remove unused formal `x`"); + ASSERT_TRUE(Diags[0].fixes()[0].isPreferred()); + + ASSERT_EQ(Diags[1].kind(), Diagnostic::DK_UnusedDefLambdaWithArg_Formal); + ASSERT_EQ(Diags[1].fixes().size(), 1); + ASSERT_EQ(Diags[1].fixes()[0].message(), "remove unused formal `y`"); + ASSERT_TRUE(Diags[1].fixes()[0].isPreferred()); +} + TEST_F(VLATest, ToDefAttrs) { std::shared_ptr AST = parse("rec { x = 1; y = x; z = x; }", Diags); VariableLookupAnalysis VLA(Diags); diff --git a/nixd/lib/Controller/CodeAction.cpp b/nixd/lib/Controller/CodeAction.cpp index 6e737116f..00099e050 100644 --- a/nixd/lib/Controller/CodeAction.cpp +++ b/nixd/lib/Controller/CodeAction.cpp @@ -47,16 +47,6 @@ void Controller::onCodeAction(const lspserver::CodeActionParams &Params, if (!Range.overlap(DRange)) continue; - // Determine if this diagnostic's fixes should be preferred - bool IsPreferred = false; - switch (D.kind()) { - case nixf::Diagnostic::DK_UnusedDefLet: - IsPreferred = true; - break; - default: - break; - } - // Add fixes. for (const nixf::Fix &F : D.fixes()) { std::vector Edits; @@ -74,7 +64,7 @@ void Controller::onCodeAction(const lspserver::CodeActionParams &Params, Actions.emplace_back(CodeAction{ .title = F.message(), .kind = std::string(CodeAction::QUICKFIX_KIND), - .isPreferred = IsPreferred, + .isPreferred = F.isPreferred(), .edit = std::move(WE), }); } diff --git a/nixd/tools/nixd/test/code-action/remove-unused-formal/all-used-negative.md b/nixd/tools/nixd/test/code-action/remove-unused-formal/all-used-negative.md new file mode 100644 index 000000000..84e046aab --- /dev/null +++ b/nixd/tools/nixd/test/code-action/remove-unused-formal/all-used-negative.md @@ -0,0 +1,69 @@ +# RUN: nixd --lit-test < %s | FileCheck %s + +Test that no action is offered when all formals are used. + +<-- initialize(0) + +```json +{ + "jsonrpc":"2.0", + "id":0, + "method":"initialize", + "params":{ + "processId":123, + "rootPath":"", + "capabilities":{ + }, + "trace":"off" + } +} +``` + + +<-- textDocument/didOpen + +```nix file:///all-used.nix +{x, y}: x + y +``` + +<-- textDocument/codeAction(2) + + +```json +{ + "jsonrpc":"2.0", + "id":2, + "method":"textDocument/codeAction", + "params":{ + "textDocument":{ + "uri":"file:///all-used.nix" + }, + "range":{ + "start":{ + "line": 0, + "character": 4 + }, + "end":{ + "line":0, + "character": 5 + } + }, + "context":{ + "diagnostics":[], + "triggerKind":2 + } + } +} +``` + +No remove unused formal action should be offered since all formals are used. + +``` + CHECK: "id": 2, +CHECK-NEXT: "jsonrpc": "2.0", +CHECK-NEXT: "result": [] +``` + +```json +{"jsonrpc":"2.0","method":"exit"} +``` diff --git a/nixd/tools/nixd/test/code-action/remove-unused-formal/basic.md b/nixd/tools/nixd/test/code-action/remove-unused-formal/basic.md new file mode 100644 index 000000000..c638d2f25 --- /dev/null +++ b/nixd/tools/nixd/test/code-action/remove-unused-formal/basic.md @@ -0,0 +1,70 @@ +# RUN: nixd --lit-test < %s | FileCheck %s + +Test basic `remove unused formal` action. + +<-- initialize(0) + +```json +{ + "jsonrpc":"2.0", + "id":0, + "method":"initialize", + "params":{ + "processId":123, + "rootPath":"", + "capabilities":{ + }, + "trace":"off" + } +} +``` + + +<-- textDocument/didOpen + +```nix file:///basic.nix +{x, y}: x +``` + +<-- textDocument/codeAction(2) + + +```json +{ + "jsonrpc":"2.0", + "id":2, + "method":"textDocument/codeAction", + "params":{ + "textDocument":{ + "uri":"file:///basic.nix" + }, + "range":{ + "start":{ + "line": 0, + "character": 4 + }, + "end":{ + "line":0, + "character": 5 + } + }, + "context":{ + "diagnostics":[], + "triggerKind":2 + } + } +} +``` + +The action should remove `, y` from the formals since `y` is unused. + +``` + CHECK: "id": 2, + CHECK: "isPreferred": true, + CHECK: "kind": "quickfix", + CHECK: "title": "remove unused formal `y`" +``` + +```json +{"jsonrpc":"2.0","method":"exit"} +``` diff --git a/nixd/tools/nixd/test/code-action/remove-unused-formal/first-formal.md b/nixd/tools/nixd/test/code-action/remove-unused-formal/first-formal.md new file mode 100644 index 000000000..c06595943 --- /dev/null +++ b/nixd/tools/nixd/test/code-action/remove-unused-formal/first-formal.md @@ -0,0 +1,70 @@ +# RUN: nixd --lit-test < %s | FileCheck %s + +Test removing the first formal (special case for comma handling). + +<-- initialize(0) + +```json +{ + "jsonrpc":"2.0", + "id":0, + "method":"initialize", + "params":{ + "processId":123, + "rootPath":"", + "capabilities":{ + }, + "trace":"off" + } +} +``` + + +<-- textDocument/didOpen + +```nix file:///first-formal.nix +{x, y}: y +``` + +<-- textDocument/codeAction(2) + + +```json +{ + "jsonrpc":"2.0", + "id":2, + "method":"textDocument/codeAction", + "params":{ + "textDocument":{ + "uri":"file:///first-formal.nix" + }, + "range":{ + "start":{ + "line": 0, + "character": 1 + }, + "end":{ + "line":0, + "character": 2 + } + }, + "context":{ + "diagnostics":[], + "triggerKind":2 + } + } +} +``` + +The action should remove `x, ` from the formals since `x` is unused. + +``` + CHECK: "id": 2, + CHECK: "isPreferred": true, + CHECK: "kind": "quickfix", + CHECK: "title": "remove unused formal `x`" +``` + +```json +{"jsonrpc":"2.0","method":"exit"} +``` diff --git a/nixd/tools/nixd/test/code-action/remove-unused-formal/middle-formal.md b/nixd/tools/nixd/test/code-action/remove-unused-formal/middle-formal.md new file mode 100644 index 000000000..5481c1ac7 --- /dev/null +++ b/nixd/tools/nixd/test/code-action/remove-unused-formal/middle-formal.md @@ -0,0 +1,70 @@ +# RUN: nixd --lit-test < %s | FileCheck %s + +Test removing middle formal (special case for comma handling). + +<-- initialize(0) + +```json +{ + "jsonrpc":"2.0", + "id":0, + "method":"initialize", + "params":{ + "processId":123, + "rootPath":"", + "capabilities":{ + }, + "trace":"off" + } +} +``` + + +<-- textDocument/didOpen + +```nix file:///middle-formal.nix +{a, b, c}: a + c +``` + +<-- textDocument/codeAction(2) + + +```json +{ + "jsonrpc":"2.0", + "id":2, + "method":"textDocument/codeAction", + "params":{ + "textDocument":{ + "uri":"file:///middle-formal.nix" + }, + "range":{ + "start":{ + "line": 0, + "character": 4 + }, + "end":{ + "line":0, + "character": 5 + } + }, + "context":{ + "diagnostics":[], + "triggerKind":2 + } + } +} +``` + +The action should remove `, b` from the formals since `b` is unused. + +``` + CHECK: "id": 2, + CHECK: "isPreferred": true, + CHECK: "kind": "quickfix", + CHECK: "title": "remove unused formal `b`" +``` + +```json +{"jsonrpc":"2.0","method":"exit"} +``` diff --git a/nixd/tools/nixd/test/code-action/remove-unused-formal/with-default.md b/nixd/tools/nixd/test/code-action/remove-unused-formal/with-default.md new file mode 100644 index 000000000..bdb3de99c --- /dev/null +++ b/nixd/tools/nixd/test/code-action/remove-unused-formal/with-default.md @@ -0,0 +1,70 @@ +# RUN: nixd --lit-test < %s | FileCheck %s + +Test removing unused formal with default value. + +<-- initialize(0) + +```json +{ + "jsonrpc":"2.0", + "id":0, + "method":"initialize", + "params":{ + "processId":123, + "rootPath":"", + "capabilities":{ + }, + "trace":"off" + } +} +``` + + +<-- textDocument/didOpen + +```nix file:///with-default.nix +{x, y ? 1}: x +``` + +<-- textDocument/codeAction(2) + + +```json +{ + "jsonrpc":"2.0", + "id":2, + "method":"textDocument/codeAction", + "params":{ + "textDocument":{ + "uri":"file:///with-default.nix" + }, + "range":{ + "start":{ + "line": 0, + "character": 4 + }, + "end":{ + "line":0, + "character": 5 + } + }, + "context":{ + "diagnostics":[], + "triggerKind":2 + } + } +} +``` + +The action should remove `, y ? 1` from the formals since `y` is unused. + +``` + CHECK: "id": 2, + CHECK: "isPreferred": true, + CHECK: "kind": "quickfix", + CHECK: "title": "remove unused formal `y`" +``` + +```json +{"jsonrpc":"2.0","method":"exit"} +``` diff --git a/nixd/tools/nixd/test/code-action/remove-unused-formal/with-ellipsis.md b/nixd/tools/nixd/test/code-action/remove-unused-formal/with-ellipsis.md new file mode 100644 index 000000000..5657ab8f2 --- /dev/null +++ b/nixd/tools/nixd/test/code-action/remove-unused-formal/with-ellipsis.md @@ -0,0 +1,70 @@ +# RUN: nixd --lit-test < %s | FileCheck %s + +Test removing unused formal with ellipsis present. + +<-- initialize(0) + +```json +{ + "jsonrpc":"2.0", + "id":0, + "method":"initialize", + "params":{ + "processId":123, + "rootPath":"", + "capabilities":{ + }, + "trace":"off" + } +} +``` + + +<-- textDocument/didOpen + +```nix file:///with-ellipsis.nix +{x, y, ...}: x +``` + +<-- textDocument/codeAction(2) + + +```json +{ + "jsonrpc":"2.0", + "id":2, + "method":"textDocument/codeAction", + "params":{ + "textDocument":{ + "uri":"file:///with-ellipsis.nix" + }, + "range":{ + "start":{ + "line": 0, + "character": 4 + }, + "end":{ + "line":0, + "character": 5 + } + }, + "context":{ + "diagnostics":[], + "triggerKind":2 + } + } +} +``` + +The action should remove `, y` from the formals since `y` is unused. + +``` + CHECK: "id": 2, + CHECK: "isPreferred": true, + CHECK: "kind": "quickfix", + CHECK: "title": "remove unused formal `y`" +``` + +```json +{"jsonrpc":"2.0","method":"exit"} +``` diff --git a/nixd/tools/nixd/test/diagnostic/liveness-formal.md b/nixd/tools/nixd/test/diagnostic/liveness-formal.md index 3c5f59c9f..63accd637 100644 --- a/nixd/tools/nixd/test/diagnostic/liveness-formal.md +++ b/nixd/tools/nixd/test/diagnostic/liveness-formal.md @@ -28,7 +28,7 @@ CHECK: "diagnostics": [ CHECK-NEXT: { CHECK-NEXT: "code": "sema-unused-def-lambda-noarg-formal", -CHECK-NEXT: "message": "attribute `y` of argument is not used", +CHECK-NEXT: "message": "attribute `y` of argument is not used (fix available)", CHECK-NEXT: "range": { CHECK-NEXT: "end": { CHECK-NEXT: "character": 5,