From 3305465d1f3636183df04da9a74dc7a04fedb604 Mon Sep 17 00:00:00 2001 From: ykiko Date: Mon, 4 May 2026 19:15:07 +0800 Subject: [PATCH] feat(formatting): wire up textDocument/formatting and rangeFormatting (#441) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Wire up the existing `document_format` feature to LSP via stateless workers - Add `Format` kind to stateless worker dispatch, with a lightweight `forward_format` path in `Compiler` (no compilation/deps needed — just file path + content) - Register `textDocument/formatting` and `textDocument/rangeFormatting` handlers with `scoped_pause` - Style lookup uses `clang::format::getStyle` which walks parent directories for `.clang-format`, matching clangd's behavior ## Test plan - [x] 4 unit tests: simple format, range format, idempotent (no edits), include sort - [x] 3 integration tests: full document format (verifies applied edits match expected output), range format, already-formatted no-op - [x] Capability assertions added to `test_capabilities` - [x] All existing tests pass (554 unit, 170 integration, 2 smoke) 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit ## Release Notes * **New Features** * Added document formatting and range formatting capabilities to the LSP server * Formatting can target the entire document or a specific range of code * Server now advertises formatting support to LSP clients * **Tests** * Added comprehensive test coverage for formatting functionality --------- Co-authored-by: Claude Opus 4.6 --- src/feature/formatting.cpp | 2 +- src/server/compiler/compiler.cpp | 26 +++++++ src/server/compiler/compiler.h | 3 + src/server/protocol/worker.h | 3 + src/server/service/lsp_client.cpp | 26 +++++++ src/server/worker/stateless_worker.cpp | 17 +++++ tests/conftest.py | 6 ++ tests/data/formatting/.clang-format | 3 + tests/data/formatting/main.cpp | 1 + tests/integration/features/test_formatting.py | 74 +++++++++++++++++++ tests/integration/features/test_server.py | 2 + tests/integration/utils/client.py | 26 +++++++ tests/unit/feature/formatting_tests.cpp | 23 ++++++ 13 files changed, 211 insertions(+), 1 deletion(-) create mode 100644 tests/data/formatting/.clang-format create mode 100644 tests/data/formatting/main.cpp create mode 100644 tests/integration/features/test_formatting.py diff --git a/src/feature/formatting.cpp b/src/feature/formatting.cpp index c80370bd..0be63702 100644 --- a/src/feature/formatting.cpp +++ b/src/feature/formatting.cpp @@ -49,7 +49,7 @@ auto document_format(llvm::StringRef file, range ? tooling::Range(range->begin, range->length()) : tooling::Range(0, content.size()); auto replacements = format_content(file, content, selection); if(!replacements) { - LOG_INFO("Fail to format for {}\n{}", file, replacements.error()); + LOG_WARN("Failed to format {}: {}", file, replacements.error()); return edits; } diff --git a/src/server/compiler/compiler.cpp b/src/server/compiler/compiler.cpp index 346bf432..ded06795 100644 --- a/src/server/compiler/compiler.cpp +++ b/src/server/compiler/compiler.cpp @@ -882,6 +882,32 @@ Compiler::RawResult Compiler::forward_build(worker::BuildKind kind, co_return std::move(result.value().result_json); } +Compiler::RawResult Compiler::forward_format(Session& session, + std::optional range) { + auto path_id = session.path_id; + auto path = std::string(workspace.path_pool.resolve(path_id)); + + worker::BuildParams wp; + wp.kind = worker::BuildKind::Format; + wp.file = path; + wp.text = session.text; + + if(range) { + lsp::PositionMapper mapper(wp.text, lsp::PositionEncoding::UTF16); + auto begin = mapper.to_offset(range->start); + auto end = mapper.to_offset(range->end); + if(!begin || !end) + co_return serde_raw{"null"}; + wp.format_range = {*begin, *end}; + } + + auto result = co_await pool.send_stateless(wp); + if(!result.has_value()) { + co_return serde_raw{"null"}; + } + co_return std::move(result.value().result_json); +} + Compiler::RawResult Compiler::handle_completion(const protocol::Position& position, Session& session) { auto path_id = session.path_id; diff --git a/src/server/compiler/compiler.h b/src/server/compiler/compiler.h index 8fdbd69c..7ffd840f 100644 --- a/src/server/compiler/compiler.h +++ b/src/server/compiler/compiler.h @@ -90,6 +90,9 @@ public: const protocol::Position& position, Session& session); + /// Forward a formatting request to a stateless worker. + RawResult forward_format(Session& session, std::optional range = {}); + /// Handle completion requests. Detects preamble context (include/import) /// and serves those locally; delegates code completion to a stateless worker. RawResult handle_completion(const protocol::Position& position, Session& session); diff --git a/src/server/protocol/worker.h b/src/server/protocol/worker.h index e5a3a729..4fe9e715 100644 --- a/src/server/protocol/worker.h +++ b/src/server/protocol/worker.h @@ -64,6 +64,7 @@ enum class BuildKind : uint8_t { Index, Completion, SignatureHelp, + Format, }; /// Unified parameters for all stateless build/compilation tasks. @@ -74,6 +75,7 @@ enum class BuildKind : uint8_t { /// - Index: + pcms /// - Completion: + text, version, offset, pch, pcms /// - SignatureHelp: + text, version, offset, pch, pcms +/// - Format: + text, format_range (optional) struct BuildParams { BuildKind kind; std::string file; @@ -90,6 +92,7 @@ struct BuildParams { std::string output_path; ///< BuildPCH, BuildPCM std::string module_name; ///< BuildPCM uint32_t preamble_bound = UINT32_MAX; ///< BuildPCH + LocalSourceRange format_range; ///< Format (default = full document) }; /// Unified result for stateless build tasks. diff --git a/src/server/service/lsp_client.cpp b/src/server/service/lsp_client.cpp index d6fcf7c1..1607b60a 100644 --- a/src/server/service/lsp_client.cpp +++ b/src/server/service/lsp_client.cpp @@ -108,6 +108,8 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s caps.call_hierarchy_provider = true; caps.type_hierarchy_provider = true; caps.workspace_symbol_provider = true; + caps.document_formatting_provider = true; + caps.document_range_formatting_provider = true; protocol::SemanticTokensOptions sem_opts; { @@ -462,6 +464,30 @@ LSPClient::LSPClient(MasterServer& server, kota::ipc::JsonPeer& peer) : server(s co_return std::move(result); }); + peer.on_request( + [this](RequestContext& ctx, const protocol::DocumentFormattingParams& params) -> RawResult { + auto& srv = this->server; + auto path = uri_to_path(params.text_document.uri); + auto path_id = srv.workspace.path_pool.intern(path); + auto* session = srv.find_session(path_id); + if(!session) + co_return serde_raw{"null"}; + auto pause = srv.indexer.scoped_pause(); + co_return co_await srv.compiler.forward_format(*session); + }); + + peer.on_request([this](RequestContext& ctx, + const protocol::DocumentRangeFormattingParams& params) -> RawResult { + auto& srv = this->server; + auto path = uri_to_path(params.text_document.uri); + auto path_id = srv.workspace.path_pool.intern(path); + auto* session = srv.find_session(path_id); + if(!session) + co_return serde_raw{"null"}; + auto pause = srv.indexer.scoped_pause(); + co_return co_await srv.compiler.forward_format(*session, params.range); + }); + peer.on_request( [this, lookup_at](RequestContext& ctx, const protocol::CallHierarchyPrepareParams& params) -> RawResult { diff --git a/src/server/worker/stateless_worker.cpp b/src/server/worker/stateless_worker.cpp index 91a53704..e5910a00 100644 --- a/src/server/worker/stateless_worker.cpp +++ b/src/server/worker/stateless_worker.cpp @@ -274,6 +274,22 @@ static worker::BuildResult handle_signature_help(const worker::BuildParams& para return result; } +static worker::BuildResult handle_format(const worker::BuildParams& params) { + ScopedTimer timer; + + std::optional range; + if(params.format_range.valid()) { + range = params.format_range; + } + + auto edits = feature::document_format(params.file, params.text, range); + LOG_DEBUG("Format done: {} edits, {}ms", edits.size(), timer.ms()); + + worker::BuildResult result; + result.result_json = to_raw(edits); + return result; +} + int run_stateless_worker_mode(const std::string& worker_name, const std::string& log_dir) { logging::stderr_logger(worker_name, logging::options); if(!log_dir.empty()) { @@ -305,6 +321,7 @@ int run_stateless_worker_mode(const std::string& worker_name, const std::string& } case K::Completion: return handle_completion(params); case K::SignatureHelp: return handle_signature_help(params); + case K::Format: return handle_format(params); } return {false, "Unknown build kind"}; }); diff --git a/tests/conftest.py b/tests/conftest.py index d50097d4..05890cc1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -292,6 +292,12 @@ def _generate_test_data_cdbs(data_dir: Path) -> None: if cr_main.exists(): _write(cr_dir, [_entry(cr_dir, cr_main)]) + # formatting + fmt_dir = data_dir / "formatting" + fmt_main = fmt_dir / "main.cpp" + if fmt_main.exists(): + _write(fmt_dir, [_entry(fmt_dir, fmt_main)]) + # pch_test pt_dir = data_dir / "pch_test" if pt_dir.exists(): diff --git a/tests/data/formatting/.clang-format b/tests/data/formatting/.clang-format new file mode 100644 index 00000000..d180071c --- /dev/null +++ b/tests/data/formatting/.clang-format @@ -0,0 +1,3 @@ +BasedOnStyle: LLVM +IndentWidth: 4 +ColumnLimit: 80 diff --git a/tests/data/formatting/main.cpp b/tests/data/formatting/main.cpp new file mode 100644 index 00000000..1f764483 --- /dev/null +++ b/tests/data/formatting/main.cpp @@ -0,0 +1 @@ +int add(int a, int b) { return a + b; } diff --git a/tests/integration/features/test_formatting.py b/tests/integration/features/test_formatting.py new file mode 100644 index 00000000..57d23efe --- /dev/null +++ b/tests/integration/features/test_formatting.py @@ -0,0 +1,74 @@ +import pytest +from lsprotocol.types import Position, Range + +from tests.integration.utils.workspace import did_change + +UNFORMATTED = "int add( int a , int b ) {\nreturn a+b ;\n}\n" +FORMATTED = "int add(int a, int b) { return a + b; }\n" + + +def apply_edits(text, edits): + """Apply LSP TextEdits to a string, processing from end to start.""" + lines = text.split("\n") + for edit in sorted( + edits, key=lambda e: (e.range.start.line, e.range.start.character), reverse=True + ): + start = edit.range.start + end = edit.range.end + before = ( + "\n".join(lines[: start.line]) + + ("\n" if start.line > 0 else "") + + lines[start.line][: start.character] + ) + after = ( + lines[end.line][end.character :] + + ("\n" if end.line < len(lines) - 1 else "") + + "\n".join(lines[end.line + 1 :]) + ) + text = before + edit.new_text + after + lines = text.split("\n") + return text + + +@pytest.mark.workspace("formatting") +async def test_format_document(client, workspace): + uri, _ = await client.open_and_wait(workspace / "main.cpp") + + did_change(client, uri, 1, UNFORMATTED) + edits = await client.format_document(uri) + + assert edits is not None + assert len(edits) > 0 + result = apply_edits(UNFORMATTED, edits) + assert result == FORMATTED + + client.close(uri) + + +@pytest.mark.workspace("formatting") +async def test_format_range(client, workspace): + uri, _ = await client.open_and_wait(workspace / "main.cpp") + + did_change(client, uri, 1, UNFORMATTED) + edits = await client.format_range( + uri, + Range(start=Position(line=1, character=0), end=Position(line=2, character=0)), + ) + + assert edits is not None + assert len(edits) > 0 + + client.close(uri) + + +@pytest.mark.workspace("formatting") +async def test_format_already_formatted(client, workspace): + uri, _ = await client.open_and_wait(workspace / "main.cpp") + + did_change(client, uri, 1, FORMATTED) + edits = await client.format_document(uri) + + assert edits is not None + assert len(edits) == 0 + + client.close(uri) diff --git a/tests/integration/features/test_server.py b/tests/integration/features/test_server.py index 8b665710..b573f75e 100644 --- a/tests/integration/features/test_server.py +++ b/tests/integration/features/test_server.py @@ -34,6 +34,8 @@ async def test_capabilities(client, workspace): assert capability_enabled(caps.folding_range_provider) assert capability_enabled(caps.inlay_hint_provider) assert capability_enabled(caps.code_action_provider) + assert caps.document_formatting_provider is True + assert caps.document_range_formatting_provider is True assert caps.semantic_tokens_provider is not None diff --git a/tests/integration/utils/client.py b/tests/integration/utils/client.py index 58bcace6..8a6882b9 100644 --- a/tests/integration/utils/client.py +++ b/tests/integration/utils/client.py @@ -16,9 +16,12 @@ from lsprotocol.types import ( Diagnostic, DidCloseTextDocumentParams, DidOpenTextDocumentParams, + DocumentFormattingParams, DocumentLinkParams, + DocumentRangeFormattingParams, DocumentSymbolParams, FoldingRangeParams, + FormattingOptions, HoverParams, InlayHintParams, InitializeParams, @@ -312,6 +315,29 @@ class CliceClient(BaseLanguageClient): timeout=timeout, ) + async def format_document(self, uri: str, *, timeout: float = 30.0): + return await asyncio.wait_for( + self.text_document_formatting_async( + DocumentFormattingParams( + text_document=TextDocumentIdentifier(uri=uri), + options=FormattingOptions(tab_size=4, insert_spaces=True), + ) + ), + timeout=timeout, + ) + + async def format_range(self, uri: str, range_: Range, *, timeout: float = 30.0): + return await asyncio.wait_for( + self.text_document_range_formatting_async( + DocumentRangeFormattingParams( + text_document=TextDocumentIdentifier(uri=uri), + range=range_, + options=FormattingOptions(tab_size=4, insert_spaces=True), + ) + ), + timeout=timeout, + ) + # ── Extension protocol ─────────────────────────────────────────── async def query_context(self, uri: str, *, timeout: float = 30.0): diff --git a/tests/unit/feature/formatting_tests.cpp b/tests/unit/feature/formatting_tests.cpp index 9bd8eeb7..0a5d9a6f 100644 --- a/tests/unit/feature/formatting_tests.cpp +++ b/tests/unit/feature/formatting_tests.cpp @@ -12,6 +12,29 @@ TEST_CASE(Simple) { ASSERT_NE(edits.size(), 0U); } +TEST_CASE(RangeFormat) { + llvm::StringRef code = "int x=1;\nint y = 2 ;\nint z=3;\n"; + LocalSourceRange range; + range.begin = static_cast(code.find("int y")); + range.end = static_cast(code.find("\nint z") + 1); + auto range_edits = feature::document_format("main.cpp", code, range); + auto full_edits = feature::document_format("main.cpp", code, std::nullopt); + ASSERT_NE(range_edits.size(), 0U); + EXPECT_LE(range_edits.size(), full_edits.size()); +} + +TEST_CASE(Idempotent) { + llvm::StringRef code = "int main() {\n return 0;\n}\n"; + auto edits = feature::document_format("main.cpp", code, std::nullopt); + EXPECT_EQ(edits.size(), 0U); +} + +TEST_CASE(IncludeSort) { + llvm::StringRef code = "#include \n#include \n\nint main() {}\n"; + auto edits = feature::document_format("main.cpp", code, std::nullopt); + ASSERT_NE(edits.size(), 0U); +} + }; // TEST_SUITE(Formatting) } // namespace