From c302d0bc7a248d5ec7665229d97708fc0cc98e07 Mon Sep 17 00:00:00 2001 From: DavidLoftus <43099493+DavidLoftus@users.noreply.github.com> Date: Thu, 13 Mar 2025 18:22:20 +0100 Subject: [PATCH] Allow LSP test_file to autofill didOpen params from previous splits (#5078) Currently file tests for LSP must provide carbon source code as an escaped string within notify params, i.e. ``` [[@LSP-NOTIFY:textDocument/didOpen: "textDocument": { "uri": "file:/class.carbon", "languageId": "carbon", "text": "class A {\n fn F();\n fn G() {}\n}\n" } ]] ``` This works fine for simple, single line files but gets annoying when working with more complicated files which are necessary when testing more complicated features e.g. goto-definition ``` --- class.carbon class A { fn F(); fn G() {} } --- STDIN [[@LSP-NOTIFY:textDocument/didOpen: "textDocument": { "uri": "file:/class.carbon", "languageId": "carbon", "text": "AUTOFILL" } ]] ``` This PR extends file_test_base to be able to parse the notify/call params and inject files from the test_file's splits into the JSON input. I purposely avoid using the clangd types and manually parse the llvm::json::Value here to avoid introducing a depdendency on clangd to the generic file_test_base, but happy to change if we think that is fine. Also happy to accept other suggestions on alternative methods to achieve same result. --------- Co-authored-by: Jon Ross-Perkins --- testing/file_test/README.md | 13 ++ testing/file_test/test_file.cpp | 139 ++++++++++++++---- .../file_test/testdata/lsp_autofill.carbon | 38 +++++ .../file_test/testdata/lsp_keywords.carbon | 51 +++---- .../testdata/document_symbol/nested.carbon | 13 +- 5 files changed, 192 insertions(+), 62 deletions(-) create mode 100644 testing/file_test/testdata/lsp_autofill.carbon diff --git a/testing/file_test/README.md b/testing/file_test/README.md index 583fff343390..3846c9e25ff6 100644 --- a/testing/file_test/README.md +++ b/testing/file_test/README.md @@ -116,6 +116,19 @@ Some keywords can be inserted for content: - `id`: Assigned from ``. - `result`: Optionally assigned from ``. + Additional substitutions are made to `` in the following + cases: + + - ``` + [[@LSP-NOTIFY:textDocument/didOpen:"textDocument": { + "uri": "file:/", + "text": "FROM_FILE_SPLIT" + }]] + ``` + + The keyword `FROM_FILE_SPLIT` is substituted with the content of the + file split ``. All other properties are unchanged. + - ``` [[@TEST_NAME]] ``` diff --git a/testing/file_test/test_file.cpp b/testing/file_test/test_file.cpp index 4fb82682dc1c..e721dc8a3314 100644 --- a/testing/file_test/test_file.cpp +++ b/testing/file_test/test_file.cpp @@ -6,7 +6,11 @@ #include +#include "common/check.h" +#include "common/error.h" +#include "common/raw_string_ostream.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/JSON.h" #include "testing/base/file_helpers.h" namespace Carbon::Testing { @@ -93,9 +97,54 @@ struct SplitState { int file_index = 0; }; +// Given a `file:/` URI, returns the filename. +static auto ExtractFilePathFromUri(llvm::StringRef uri) + -> ErrorOr { + static constexpr llvm::StringRef FilePrefix = "file:/"; + if (!uri.starts_with(FilePrefix)) { + return ErrorBuilder("uri `") << uri << "` is not a file uri"; + } + return uri.drop_front(FilePrefix.size()); +} + +// When `FROM_FILE_SPLIT` is used in path `textDocument.text`, populate the +// value from the split matching the `uri`. Only used for +// `textDocument/didOpen`. +static auto AutoFillDidOpenParams(llvm::json::Object* params, + llvm::ArrayRef splits) + -> ErrorOr { + auto* text_document = params->getObject("textDocument"); + if (text_document == nullptr) { + return Success(); + } + + auto attr_it = text_document->find("text"); + if (attr_it == text_document->end() || attr_it->second != "FROM_FILE_SPLIT") { + return Success(); + } + + auto uri = text_document->getString("uri"); + if (!uri) { + return Error("missing uri in params.textDocument"); + } + + CARBON_ASSIGN_OR_RETURN(auto file_path, ExtractFilePathFromUri(*uri)); + const auto* split_it = + llvm::find_if(splits, [&](const TestFile::Split& split) { + return split.filename == file_path; + }); + if (split_it == splits.end()) { + return ErrorBuilder() << "No split found for uri: " << *uri; + } + attr_it->second = split_it->content; + return Success(); +} + // 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) -> ErrorOr { + int& lsp_call_id, + llvm::ArrayRef splits) + -> ErrorOr { llvm::StringRef content_at_keyword = llvm::StringRef(*content).substr(keyword_pos); @@ -133,31 +182,50 @@ static auto ReplaceLspKeywordAt(std::string* content, size_t keyword_pos, llvm::StringRef body = body_start.take_front(body_end); auto [method_or_id, extra_content] = body.split(":"); - // Form the JSON. - 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); - } + llvm::json::Value parsed_extra_content = nullptr; 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); + std::string extra_content_as_object = + llvm::formatv("{{{0}}", extra_content); + auto parse_result = llvm::json::parse(extra_content_as_object); + if (auto err = parse_result.takeError()) { + return ErrorBuilder() << "Error parsing extra content: " << err; + } + parsed_extra_content = std::move(*parse_result); + CARBON_CHECK(parsed_extra_content.kind() == llvm::json::Value::Object); + if (extra_content_label == "params" && + method_or_id == "textDocument/didOpen") { + CARBON_RETURN_IF_ERROR( + AutoFillDidOpenParams(parsed_extra_content.getAsObject(), splits)); } } - json += "}"; + + // Form the JSON. + RawStringOstream buffer; + llvm::json::OStream json(buffer); + + json.object([&] { + json.attribute("jsonrpc", "2.0"); + json.attribute(method_or_id_label, method_or_id); + + if (use_call_id) { + json.attribute("id", ++lsp_call_id); + } + if (parsed_extra_content != nullptr) { + if (!extra_content_label.empty()) { + json.attribute(extra_content_label, parsed_extra_content); + } else { + for (const auto& [key, value] : *parsed_extra_content.getAsObject()) { + json.attribute(key, value); + } + } + } + }); // 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(); + int content_length = buffer.size() + 2; + auto json_with_header = llvm::formatv("Content-Length: {0}\n\n{1}\n", + content_length, buffer.TakeStr()) + .str(); int keyword_len = (body_start.data() + body_end + LspEnd.size()) - keyword.data(); content->replace(keyword_pos, keyword_len, json_with_header); @@ -167,7 +235,8 @@ static auto ReplaceLspKeywordAt(std::string* content, size_t keyword_pos, // 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, + llvm::ArrayRef splits) -> ErrorOr { auto keyword = llvm::StringRef(*content).substr(keyword_pos); @@ -186,7 +255,7 @@ static auto ReplaceContentKeywordAt(std::string* content, size_t keyword_pos, } if (keyword.starts_with("[[@LSP")) { - return ReplaceLspKeywordAt(content, keyword_pos, lsp_call_id); + return ReplaceLspKeywordAt(content, keyword_pos, lsp_call_id, splits); } return ErrorBuilder() << "Unexpected use of `[[@` at `" @@ -198,7 +267,9 @@ static auto ReplaceContentKeywordAt(std::string* content, size_t keyword_pos, // TEST_NAME is the only content keyword at present, but we do validate that // other names are reserved. static auto ReplaceContentKeywords(llvm::StringRef filename, - std::string* content) -> ErrorOr { + std::string* content, + llvm::ArrayRef splits) + -> ErrorOr { static constexpr llvm::StringLiteral Prefix = "[[@"; auto keyword_pos = content->find(Prefix); @@ -227,7 +298,8 @@ 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, + splits)); keyword_pos = content->find(Prefix, keyword_end); } return Success(); @@ -237,7 +309,8 @@ static auto ReplaceContentKeywords(llvm::StringRef filename, static auto AddSplit(llvm::StringRef filename, std::string* content, llvm::SmallVector* file_splits) -> ErrorOr { - CARBON_RETURN_IF_ERROR(ReplaceContentKeywords(filename, content)); + CARBON_RETURN_IF_ERROR( + ReplaceContentKeywords(filename, content, *file_splits)); file_splits->push_back( {.filename = filename.str(), .content = std::move(*content)}); content->clear(); @@ -268,8 +341,9 @@ static auto TryConsumeSplit(llvm::StringRef line, llvm::StringRef line_trimmed, // If there's a split, all output is appended at the end of each file // before AUTOUPDATE. We may want to change that, but it's not // necessary to handle right now. - return ErrorBuilder() << "AUTOUPDATE/NOAUTOUPDATE setting must be in " - "the first file."; + return Error( + "AUTOUPDATE/NOAUTOUPDATE setting must be in " + "the first file."); } // On a file split, add the previous file, then start a new one. @@ -280,15 +354,16 @@ static auto TryConsumeSplit(llvm::StringRef line, llvm::StringRef line_trimmed, split->content.clear(); if (split->found_code_pre_split) { // For the first split, we make sure there was no content prior. - return ErrorBuilder() << "When using split files, there must be no " - "content before the first split file."; + return Error( + "When using split files, there must be no content before the first " + "split file."); } } ++split->file_index; split->filename = line_trimmed.trim(); if (split->filename.empty()) { - return ErrorBuilder() << "Missing filename for split."; + return Error("Missing filename for split."); } // The split line is added to non_check_lines for retention in autoupdate, but // is not added to the test file content. @@ -514,7 +589,7 @@ static auto TryConsumeAutoupdate(int line_index, llvm::StringRef line_trimmed, return false; } if (*found_autoupdate) { - return ErrorBuilder() << "Multiple AUTOUPDATE/NOAUTOUPDATE settings found"; + return Error("Multiple AUTOUPDATE/NOAUTOUPDATE settings found"); } *found_autoupdate = true; if (line_trimmed == Autoupdate) { diff --git a/testing/file_test/testdata/lsp_autofill.carbon b/testing/file_test/testdata/lsp_autofill.carbon new file mode 100644 index 000000000000..ddee43597793 --- /dev/null +++ b/testing/file_test/testdata/lsp_autofill.carbon @@ -0,0 +1,38 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +// 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_autofill.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_autofill.carbon + +// --- foo.carbon +class Foo { + fn foo(); + fn bar() {} +} + +// --- STDIN +[[@LSP-NOTIFY:textDocument/didOpen: + "textDocument": { + "uri": "file:/foo.carbon", + "languageId": "carbon", + "text": "FROM_FILE_SPLIT" + } +]] + +// --- AUTOUPDATE-SPLIT + +// CHECK:STDERR: --- STDIN: +// CHECK:STDERR: Content-Length: 182 +// CHECK:STDERR: +// CHECK:STDERR: {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"carbon","text":"class Foo {\n fn foo();\n fn bar() {}\n}\n\n","uri":"file:/foo.carbon"}}} +// CHECK:STDERR: +// CHECK:STDERR: +// CHECK:STDOUT: 2 args: `default_args`, `foo.carbon` +// CHECK:STDOUT: foo.carbon:1: class Foo { +// CHECK:STDOUT: foo.carbon:2: fn foo(); +// CHECK:STDOUT: foo.carbon:3: fn bar() {} +// CHECK:STDOUT: foo.carbon:4: } diff --git a/testing/file_test/testdata/lsp_keywords.carbon b/testing/file_test/testdata/lsp_keywords.carbon index 1e598e06643a..cc340a70aa8f 100644 --- a/testing/file_test/testdata/lsp_keywords.carbon +++ b/testing/file_test/testdata/lsp_keywords.carbon @@ -11,62 +11,57 @@ // --- STDIN [[@LSP:foo:]] [[@LSP:foo]] -[[@LSP:bar:content]] +[[@LSP:bar:"content": 0]] [[@LSP:baz: -multi -line +"multi": 0, +"line": 1 ]] -[[@LSP-CALL:bar:content]] +[[@LSP-CALL:bar:"content": 0]] [[@LSP-CALL:baz: -multi -line]] +"multi": 0, +"line": 1]] [[@LSP-REPLY:7]] -[[@LSP-REPLY:8:bar]] +[[@LSP-REPLY:8:"bar": 0]] [[@LSP-NOTIFY:exit]] // --- AUTOUPDATE-SPLIT // CHECK:STDERR: --- STDIN: -// CHECK:STDERR: Content-Length: 37 +// CHECK:STDERR: Content-Length: 34 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "method": "foo"} +// CHECK:STDERR: {"jsonrpc":"2.0","method":"foo"} // CHECK:STDERR: -// CHECK:STDERR: Content-Length: 37 +// CHECK:STDERR: Content-Length: 34 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "method": "foo"} +// CHECK:STDERR: {"jsonrpc":"2.0","method":"foo"} // CHECK:STDERR: // CHECK:STDERR: Content-Length: 46 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "method": "bar", content} +// CHECK:STDERR: {"jsonrpc":"2.0","method":"bar","content":0} // CHECK:STDERR: -// CHECK:STDERR: Content-Length: 50 +// CHECK:STDERR: Content-Length: 53 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "method": "baz", -// CHECK:STDERR: multi -// CHECK:STDERR: line -// CHECK:STDERR: } +// CHECK:STDERR: {"jsonrpc":"2.0","method":"baz","multi":0,"line":1} // CHECK:STDERR: -// CHECK:STDERR: Content-Length: 67 +// CHECK:STDERR: Content-Length: 64 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "method": "bar", "id": 1, "params": {content}} +// CHECK:STDERR: {"jsonrpc":"2.0","method":"bar","id":1,"params":{"content":0}} // 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: {"jsonrpc":"2.0","method":"baz","id":2,"params":{"line":1,"multi":0}} // CHECK:STDERR: -// CHECK:STDERR: Content-Length: 31 +// CHECK:STDERR: Content-Length: 28 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "id": "7"} +// CHECK:STDERR: {"jsonrpc":"2.0","id":"7"} // CHECK:STDERR: -// CHECK:STDERR: Content-Length: 48 +// CHECK:STDERR: Content-Length: 47 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "id": "8", "result": {bar}} +// CHECK:STDERR: {"jsonrpc":"2.0","id":"8","result":{"bar":0}} // CHECK:STDERR: -// CHECK:STDERR: Content-Length: 38 +// CHECK:STDERR: Content-Length: 35 // CHECK:STDERR: -// CHECK:STDERR: {"jsonrpc": "2.0", "method": "exit"} +// CHECK:STDERR: {"jsonrpc":"2.0","method":"exit"} // CHECK:STDERR: // CHECK:STDERR: // CHECK:STDOUT: 1 args: `default_args` diff --git a/toolchain/language_server/testdata/document_symbol/nested.carbon b/toolchain/language_server/testdata/document_symbol/nested.carbon index 8bc486b3094c..675602eb815c 100644 --- a/toolchain/language_server/testdata/document_symbol/nested.carbon +++ b/toolchain/language_server/testdata/document_symbol/nested.carbon @@ -8,10 +8,19 @@ // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/language_server/testdata/document_symbol/nested.carbon +// --- class.carbon +class A { + fn F(); + fn G() {} +} + // --- STDIN [[@LSP-NOTIFY:textDocument/didOpen: - "textDocument": {"uri": "file:/class.carbon", "languageId": "carbon", - "text": "class A {\n fn F();\n fn G() {}\n}\n"} + "textDocument": { + "uri": "file:/class.carbon", + "languageId": "carbon", + "text": "FROM_FILE_SPLIT" + } ]] [[@LSP-CALL:textDocument/documentSymbol: "textDocument": {"uri": "file:/class.carbon"}