From 34ceb6bbbee49118424b2ea1bf4d05665c50c8d6 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 5 Feb 2025 15:39:27 -0800 Subject: [PATCH] Add @LSP-CALL to file_test (#4896) Adds @LSP-CALL and refactors LSP keyword handling to implicitly handle a little more of the LSP structure. This is coming out of textDocument call handling, where this at least reduces some boilerplate of `"params": {...}`. --- testing/file_test/README.md | 20 ++++ testing/file_test/file_test_base.cpp | 109 ++++++++++-------- .../{lsp_calls.carbon => lsp_keywords.carbon} | 38 ++++-- .../testdata/initialize.carbon | 6 +- .../testdata/notify_parse_error.carbon | 2 +- .../testdata/unexpected_reply.carbon | 2 +- .../testdata/unsupported_call.carbon | 6 +- 7 files changed, 112 insertions(+), 71 deletions(-) rename testing/file_test/testdata/{lsp_calls.carbon => lsp_keywords.carbon} (54%) diff --git a/testing/file_test/README.md b/testing/file_test/README.md index 5e806572698e..9fe588726e56 100644 --- a/testing/file_test/README.md +++ b/testing/file_test/README.md @@ -88,6 +88,7 @@ Some keywords can be inserted for content: - ``` [[@LSP::]] + [[@LSP-CALL::]] [[@LSP-NOTIFY::]] [[@LSP-REPLY::]] ``` @@ -96,6 +97,25 @@ Some keywords can be inserted for content: the `Content-Length` header. The `:` is optional, and may be omitted. + The difference between the replacements mirrors LSP calling conventions: + + - `LSP` does a minimal JSON format, and is primarily intended for testing + errors. + - `method`: Assigned from ``. + - `LSP-CALL` maps to + [Request Message](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#requestMessage). + - `method`: Assigned from ``. + - `id`: Automatically incremented across calls. + - `params`: Optionally assigned from ``. + - `LSP-NOTIFY` maps to + [Notification Message](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#notificationMessage). + - `method`: Assigned from ``. + - `params`: Optionally assigned from ``. + - `LSP-REPLY` maps to + [Response Message](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#responseMessage). + - `id`: Assigned from ``. + - `result`: Optionally assigned from ``. + - ``` [[@TEST_NAME]] ``` diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index 99d254bb45d6..c4d9e566ab7c 100644 --- a/testing/file_test/file_test_base.cpp +++ b/testing/file_test/file_test_base.cpp @@ -517,62 +517,81 @@ struct SplitState { int file_index = 0; }; -// Reformats `[[@LSP:` and `[[LSP-NOTIFY:` as an LSP call with headers. For -// notifications, `lsp_call_id` is null. +// Reformats `[[@LSP:` and similar keyword as an LSP call with headers. static auto ReplaceLspKeywordAt(std::string* content, size_t keyword_pos, - int* lsp_call_id, llvm::StringLiteral keyword, - llvm::StringLiteral method_or_id_label) - -> ErrorOr { - auto method_or_id_start = keyword_pos + keyword.size(); + int& lsp_call_id) -> ErrorOr { + llvm::StringRef content_at_keyword = + llvm::StringRef(*content).substr(keyword_pos); + + auto [keyword, body_start] = content_at_keyword.split(":"); + if (body_start.empty()) { + return ErrorBuilder() << "Missing `:` for `" + << content_at_keyword.take_front(10) << "`"; + } + + // Whether the first param is a method or id. + llvm::StringRef method_or_id_label = "method"; + // Whether to attach the `lsp_call_id`. + bool use_call_id = false; + // The JSON label for extra content. + llvm::StringRef extra_content_label; + if (keyword == "[[@LSP-CALL") { + use_call_id = true; + extra_content_label = "params"; + } else if (keyword == "[[@LSP-NOTIFY") { + extra_content_label = "params"; + } else if (keyword == "[[@LSP-REPLY") { + method_or_id_label = "id"; + extra_content_label = "result"; + } else if (keyword != "[[@LSP") { + return ErrorBuilder() << "Unrecognized @LSP keyword at `" + << keyword.take_front(10) << "`"; + } static constexpr llvm::StringLiteral LspEnd = "]]"; - auto keyword_end = content->find("]]", method_or_id_start); - if (keyword_end == std::string::npos) { + auto body_end = body_start.find(LspEnd); + if (body_end == std::string::npos) { return ErrorBuilder() << "Missing `" << LspEnd << "` after `" << keyword << "`"; } - - auto method_or_id_end = content->find(":", method_or_id_start); - auto extra_content_start = method_or_id_end + 1; - if (method_or_id_end == std::string::npos || method_or_id_end > keyword_end) { - method_or_id_end = keyword_end; - extra_content_start = keyword_end; - } - auto method_or_id = - llvm::StringRef(*content).slice(method_or_id_start, method_or_id_end); - - auto extra_content = - llvm::StringRef(*content).slice(extra_content_start, keyword_end); - std::string extra_content_sep; - if (!extra_content.empty()) { - extra_content_sep = ","; - if (!extra_content.starts_with("\n")) { - extra_content_sep += " "; - } - } + llvm::StringRef body = body_start.take_front(body_end); + auto [method_or_id, extra_content] = body.split(":"); // Form the JSON. - std::string json = R"({"jsonrpc": "2.0", )"; - if (lsp_call_id) { - json += llvm::formatv(R"("id": "{0}", )", ++(*lsp_call_id)); + std::string json = llvm::formatv(R"({{"jsonrpc": "2.0", "{0}": "{1}")", + method_or_id_label, method_or_id); + if (use_call_id) { + // Omit quotes on the ID because we know it's an integer. + json += llvm::formatv(R"(, "id": {0})", ++lsp_call_id); } - json += llvm::formatv(R"("{0}": "{1}"{2}{3}})", method_or_id_label, - method_or_id, extra_content_sep, extra_content); + if (!extra_content.empty()) { + json += ","; + if (extra_content_label.empty()) { + if (!extra_content.starts_with("\n")) { + json += " "; + } + json += extra_content; + } else { + json += llvm::formatv(R"( "{0}": {{{1}})", extra_content_label, + extra_content); + } + } + json += "}"; // Add the Content-Length header. The `2` accounts for extra newlines. auto json_with_header = llvm::formatv("Content-Length: {0}\n\n{1}\n", json.size() + 2, json) .str(); - // Insert the content. - content->replace(keyword_pos, keyword_end + 2 - keyword_pos, - json_with_header); + int keyword_len = + (body_start.data() + body_end + LspEnd.size()) - keyword.data(); + content->replace(keyword_pos, keyword_len, json_with_header); return keyword_pos + json_with_header.size(); } // Replaces the keyword at the given position. Returns the position to start a // find for the next keyword. static auto ReplaceContentKeywordAt(std::string* content, size_t keyword_pos, - llvm::StringRef test_name, int* lsp_call_id) + llvm::StringRef test_name, int& lsp_call_id) -> ErrorOr { auto keyword = llvm::StringRef(*content).substr(keyword_pos); @@ -590,20 +609,8 @@ static auto ReplaceContentKeywordAt(std::string* content, size_t keyword_pos, return keyword_pos + test_name.size(); } - static constexpr llvm::StringLiteral Lsp = "[[@LSP:"; - if (keyword.starts_with(Lsp)) { - return ReplaceLspKeywordAt(content, keyword_pos, lsp_call_id, Lsp, - "method"); - } - static constexpr llvm::StringLiteral LspNotify = "[[@LSP-NOTIFY:"; - if (keyword.starts_with(LspNotify)) { - return ReplaceLspKeywordAt(content, keyword_pos, - /*lsp_call_id=*/nullptr, LspNotify, "method"); - } - static constexpr llvm::StringLiteral LspReply = "[[@LSP-REPLY:"; - if (keyword.starts_with(LspReply)) { - return ReplaceLspKeywordAt(content, keyword_pos, - /*lsp_call_id=*/nullptr, LspReply, "id"); + if (keyword.starts_with("[[@LSP")) { + return ReplaceLspKeywordAt(content, keyword_pos, lsp_call_id); } return ErrorBuilder() << "Unexpected use of `[[@` at `" @@ -644,7 +651,7 @@ static auto ReplaceContentKeywords(llvm::StringRef filename, while (keyword_pos != std::string::npos) { CARBON_ASSIGN_OR_RETURN( auto keyword_end, - ReplaceContentKeywordAt(content, keyword_pos, test_name, &lsp_call_id)); + ReplaceContentKeywordAt(content, keyword_pos, test_name, lsp_call_id)); keyword_pos = content->find(Prefix, keyword_end); } return Success(); diff --git a/testing/file_test/testdata/lsp_calls.carbon b/testing/file_test/testdata/lsp_keywords.carbon similarity index 54% rename from testing/file_test/testdata/lsp_calls.carbon rename to testing/file_test/testdata/lsp_keywords.carbon index 8edcd223e0e9..1e598e06643a 100644 --- a/testing/file_test/testdata/lsp_calls.carbon +++ b/testing/file_test/testdata/lsp_keywords.carbon @@ -4,9 +4,9 @@ // AUTOUPDATE // TIP: To test this file alone, run: -// TIP: bazel test //testing/file_test:file_test_base_test --test_arg=--file_tests=testing/file_test/testdata/lsp_calls.carbon +// TIP: bazel test //testing/file_test:file_test_base_test --test_arg=--file_tests=testing/file_test/testdata/lsp_keywords.carbon // TIP: To dump output, run: -// TIP: bazel run //testing/file_test:file_test_base_test -- --dump_output --file_tests=testing/file_test/testdata/lsp_calls.carbon +// TIP: bazel run //testing/file_test:file_test_base_test -- --dump_output --file_tests=testing/file_test/testdata/lsp_keywords.carbon // --- STDIN [[@LSP:foo:]] @@ -16,6 +16,10 @@ multi line ]] +[[@LSP-CALL:bar:content]] +[[@LSP-CALL:baz: +multi +line]] [[@LSP-REPLY:7]] [[@LSP-REPLY:8:bar]] [[@LSP-NOTIFY:exit]] @@ -23,32 +27,42 @@ line // --- AUTOUPDATE-SPLIT // CHECK:STDERR: --- STDIN: -// CHECK:STDERR: Content-Length: 48 +// CHECK:STDERR: Content-Length: 37 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "id": "1", "method": "foo"} +// CHECK:STDERR: {"jsonrpc": "2.0", "method": "foo"} // CHECK:STDERR: -// CHECK:STDERR: Content-Length: 48 +// CHECK:STDERR: Content-Length: 37 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "id": "2", "method": "foo"} +// CHECK:STDERR: {"jsonrpc": "2.0", "method": "foo"} // CHECK:STDERR: -// CHECK:STDERR: Content-Length: 57 +// CHECK:STDERR: Content-Length: 46 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "id": "3", "method": "bar", content} +// CHECK:STDERR: {"jsonrpc": "2.0", "method": "bar", content} // CHECK:STDERR: -// CHECK:STDERR: Content-Length: 61 +// CHECK:STDERR: Content-Length: 50 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "id": "4", "method": "baz", +// CHECK:STDERR: {"jsonrpc": "2.0", "method": "baz", // CHECK:STDERR: multi // CHECK:STDERR: line // CHECK:STDERR: } // CHECK:STDERR: +// CHECK:STDERR: Content-Length: 67 +// CHECK:STDERR: +// CHECK:STDERR: {"jsonrpc": "2.0", "method": "bar", "id": 1, "params": {content}} +// CHECK:STDERR: +// CHECK:STDERR: Content-Length: 71 +// CHECK:STDERR: +// CHECK:STDERR: {"jsonrpc": "2.0", "method": "baz", "id": 2, "params": { +// CHECK:STDERR: multi +// CHECK:STDERR: line}} +// CHECK:STDERR: // CHECK:STDERR: Content-Length: 31 // CHECK:STDERR: // CHECK:STDERR: {"jsonrpc": "2.0", "id": "7"} // CHECK:STDERR: -// CHECK:STDERR: Content-Length: 36 +// CHECK:STDERR: Content-Length: 48 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "id": "8", bar} +// CHECK:STDERR: {"jsonrpc": "2.0", "id": "8", "result": {bar}} // CHECK:STDERR: // CHECK:STDERR: Content-Length: 38 // CHECK:STDERR: diff --git a/toolchain/language_server/testdata/initialize.carbon b/toolchain/language_server/testdata/initialize.carbon index d344234b59c9..8d0a23ca5639 100644 --- a/toolchain/language_server/testdata/initialize.carbon +++ b/toolchain/language_server/testdata/initialize.carbon @@ -9,15 +9,15 @@ // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/language_server/testdata/initialize.carbon // --- STDIN -[[@LSP:initialize]] +[[@LSP-CALL:initialize]] [[@LSP-NOTIFY:exit]] // --- AUTOUPDATE-SPLIT -// CHECK:STDOUT: Content-Length: 148{{\r}} +// CHECK:STDOUT: Content-Length: 146{{\r}} // CHECK:STDOUT: {{\r}} // CHECK:STDOUT: { -// CHECK:STDOUT: "id": "1", +// CHECK:STDOUT: "id": 1, // CHECK:STDOUT: "jsonrpc": "2.0", // CHECK:STDOUT: "result": { // CHECK:STDOUT: "capabilities": { diff --git a/toolchain/language_server/testdata/notify_parse_error.carbon b/toolchain/language_server/testdata/notify_parse_error.carbon index 36081bff54eb..7d8f2f27f383 100644 --- a/toolchain/language_server/testdata/notify_parse_error.carbon +++ b/toolchain/language_server/testdata/notify_parse_error.carbon @@ -15,6 +15,6 @@ // --- AUTOUPDATE-SPLIT -// CHECK:STDERR: warning: -32602: in call to `textDocument/didOpen`, JSON parse failed: expected object [LanguageServerNotificationParseError] +// CHECK:STDERR: warning: -32602: in call to `textDocument/didOpen`, JSON parse failed: missing value at (root).textDocument [LanguageServerNotificationParseError] // CHECK:STDERR: // CHECK:STDOUT: diff --git a/toolchain/language_server/testdata/unexpected_reply.carbon b/toolchain/language_server/testdata/unexpected_reply.carbon index 69755c66a45a..143d2c35fdcd 100644 --- a/toolchain/language_server/testdata/unexpected_reply.carbon +++ b/toolchain/language_server/testdata/unexpected_reply.carbon @@ -18,6 +18,6 @@ // CHECK:STDERR: warning: unexpected reply to request ID "1": null [LanguageServerUnexpectedReply] // CHECK:STDERR: -// CHECK:STDERR: warning: unexpected reply to request ID "2": 3 [LanguageServerUnexpectedReply] +// CHECK:STDERR: warning: unexpected reply to request ID "2": {"result":3} [LanguageServerUnexpectedReply] // CHECK:STDERR: // CHECK:STDOUT: diff --git a/toolchain/language_server/testdata/unsupported_call.carbon b/toolchain/language_server/testdata/unsupported_call.carbon index d7417f246a5c..b2df38a8e463 100644 --- a/toolchain/language_server/testdata/unsupported_call.carbon +++ b/toolchain/language_server/testdata/unsupported_call.carbon @@ -10,18 +10,18 @@ // --- STDIN -[[@LSP:unknown-call]] +[[@LSP-CALL:unknown-call]] [[@LSP-NOTIFY:exit]] // --- AUTOUPDATE-SPLIT -// CHECK:STDOUT: Content-Length: 122{{\r}} +// CHECK:STDOUT: Content-Length: 120{{\r}} // CHECK:STDOUT: {{\r}} // CHECK:STDOUT: { // CHECK:STDOUT: "error": { // CHECK:STDOUT: "code": -32601, // CHECK:STDOUT: "message": "unsupported call `unknown-call`" // CHECK:STDOUT: }, -// CHECK:STDOUT: "id": "1", +// CHECK:STDOUT: "id": 1, // CHECK:STDOUT: "jsonrpc": "2.0" // CHECK:STDOUT: }