From 8bafaa8171ad4f5a78842ea02c506f9406f40b6e Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 9 Apr 2026 21:35:10 +0800 Subject: [PATCH] feat(document links): preserve PCH document links and add #embed support (#413) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - PCH compilation now serializes document links via `pch_links_json` in `BuildResult` and stores them in `PCHState` - Master server merges PCH document links with main-file links on `textDocument/documentLink` requests, fixing missing links for `#include` directives inside the preamble - Adds document link support for `#embed` and `__has_embed` directives ## Test plan - [x] Unit tests: `DocumentLink.Embed` and `DocumentLink.HasEmbed` added - [x] Integration tests: `test_document_links.py` verifies PCH + main merge and `#embed` links - [x] All 483 unit tests pass - [x] All 4 integration tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **New Features** * Document links now detect embeds and __has_embed directives for both quoted and angled filenames. * Document links produced during precompiled builds are cached and merged into document-link responses for more complete link sets. * **Tests** * Added integration tests for merged PCH/main links and embed/has-embed cases. * Added unit tests verifying embed handling under C++23. * **Chores** * Added test fixtures and compile command entries for document-links tests. --------- Co-authored-by: Claude Opus 4.6 (1M context) --- src/feature/document_links.cpp | 74 +++++++------ src/server/compiler.cpp | 1 + src/server/master_server.cpp | 41 +++++-- src/server/protocol.h | 1 + src/server/stateless_worker.cpp | 8 +- src/server/workspace.h | 1 + tests/conftest.py | 8 ++ tests/data/document_links/data.bin | 1 + tests/data/document_links/header_a.h | 3 + tests/data/document_links/header_b.h | 3 + tests/data/document_links/header_c.h | 3 + tests/data/document_links/main.cpp | 20 ++++ .../features/test_document_links.py | 103 ++++++++++++++++++ tests/unit/feature/document_link_tests.cpp | 38 ++++++- 14 files changed, 261 insertions(+), 44 deletions(-) create mode 100644 tests/data/document_links/data.bin create mode 100644 tests/data/document_links/header_a.h create mode 100644 tests/data/document_links/header_b.h create mode 100644 tests/data/document_links/header_c.h create mode 100644 tests/data/document_links/main.cpp create mode 100644 tests/integration/features/test_document_links.py diff --git a/src/feature/document_links.cpp b/src/feature/document_links.cpp index 8963c451..5beffb6b 100644 --- a/src/feature/document_links.cpp +++ b/src/feature/document_links.cpp @@ -23,49 +23,59 @@ auto document_links(CompilationUnitRef unit, PositionEncoding encoding) PositionMapper converter(content, encoding); auto& directives = directives_it->second; - links.reserve(directives.includes.size() + directives.has_includes.size()); + // Scan forward from offset to find a quoted/angled filename range. + auto find_filename_range = [&](std::uint32_t offset) -> std::optional { + auto tail = content.substr(offset); + auto quote_pos = tail.find_first_of("<\""); + if(quote_pos == llvm::StringRef::npos) { + return std::nullopt; + } + char open = tail[quote_pos]; + char close = open == '<' ? '>' : '"'; + auto close_pos = tail.find(close, quote_pos + 1); + if(close_pos == llvm::StringRef::npos) { + return std::nullopt; + } + return LocalSourceRange(offset + static_cast(quote_pos), + offset + static_cast(close_pos + 1)); + }; + + auto add_link_by_location = [&](clang::SourceLocation loc, llvm::StringRef target) { + auto [fid, offset] = unit.decompose_location(loc); + if(fid != interested || offset >= content.size()) { + return; + } + auto range = find_filename_range(offset); + if(!range) { + return; + } + protocol::DocumentLink link{.range = to_range(converter, *range)}; + link.target = target.str(); + links.push_back(std::move(link)); + }; for(const auto& include: directives.includes) { - auto [fid, range] = unit.decompose_range(include.filename_range); - if(fid != interested || !range.valid()) { - continue; + if(include.fid.isValid()) { + add_link_by_location(include.location, unit.file_path(include.fid)); } - - protocol::DocumentLink link{ - .range = to_range(converter, range), - }; - link.target = std::string(unit.file_path(include.fid)); - links.push_back(std::move(link)); } for(const auto& has_include: directives.has_includes) { - if(has_include.fid.isInvalid()) { - continue; + if(has_include.fid.isValid()) { + add_link_by_location(has_include.location, unit.file_path(has_include.fid)); } + } - auto [fid, offset] = unit.decompose_location(has_include.location); - if(fid != interested || offset >= content.size()) { - continue; + for(const auto& embed: directives.embeds) { + if(embed.file) { + add_link_by_location(embed.loc, embed.file->getName()); } + } - auto tail = content.substr(offset); - char open = tail.front(); - if(open != '<' && open != '"') { - continue; + for(const auto& has_embed: directives.has_embeds) { + if(has_embed.file) { + add_link_by_location(has_embed.loc, has_embed.file->getName()); } - - char close = open == '<' ? '>' : '"'; - auto close_index = tail.find(close, 1); - if(close_index == llvm::StringRef::npos) { - continue; - } - - LocalSourceRange range(offset, offset + static_cast(close_index + 1)); - protocol::DocumentLink link{ - .range = to_range(converter, range), - }; - link.target = std::string(unit.file_path(has_include.fid)); - links.push_back(std::move(link)); } return links; diff --git a/src/server/compiler.cpp b/src/server/compiler.cpp index d830d2f4..337e1ac8 100644 --- a/src/server/compiler.cpp +++ b/src/server/compiler.cpp @@ -502,6 +502,7 @@ et::task Compiler::ensure_pch(Session& session, st.bound = bound; st.hash = preamble_hash; st.deps = capture_deps_snapshot(workspace.path_pool, result.value().deps); + st.document_links_json = std::move(result.value().pch_links_json); st.building.reset(); session.pch_ref = Session::PCHRef{path_id, preamble_hash, bound}; diff --git a/src/server/master_server.cpp b/src/server/master_server.cpp index 9ee38a3e..ea342b79 100644 --- a/src/server/master_server.cpp +++ b/src/server/master_server.cpp @@ -478,15 +478,38 @@ void MasterServer::register_handlers() { co_return co_await compiler.forward_query(worker::QueryKind::DocumentSymbol, sit->second); }); - peer.on_request( - [this](RequestContext& ctx, const protocol::DocumentLinkParams& params) -> RawResult { - auto path = uri_to_path(params.text_document.uri); - auto path_id = workspace.path_pool.intern(path); - auto sit = sessions.find(path_id); - if(sit == sessions.end()) - co_return serde_raw{"null"}; - co_return co_await compiler.forward_query(worker::QueryKind::DocumentLink, sit->second); - }); + peer.on_request([this](RequestContext& ctx, + const protocol::DocumentLinkParams& params) -> RawResult { + auto path = uri_to_path(params.text_document.uri); + auto path_id = workspace.path_pool.intern(path); + auto sit = sessions.find(path_id); + if(sit == sessions.end()) + co_return serde_raw{"null"}; + auto& session = sit->second; + auto result = co_await compiler.forward_query(worker::QueryKind::DocumentLink, session); + if(!result.has_value()) + co_return serde_raw{"null"}; + // Merge document links from PCH if available. + auto& links = result.value(); + // Re-lookup session after co_await since iterators may be invalidated. + auto sit2 = sessions.find(path_id); + if(sit2 != sessions.end() && sit2->second.pch_ref) { + auto pch_it = workspace.pch_cache.find(sit2->second.pch_ref->path_id); + if(pch_it != workspace.pch_cache.end() && !pch_it->second.document_links_json.empty()) { + auto& pch_json = pch_it->second.document_links_json; + // Merge two JSON arrays. + if(!links.data.empty() && links.data != "null" && links.data.size() > 2) { + // "[a,b]" + "[c,d]" -> "[a,b,c,d]" + links.data.pop_back(); // remove trailing ']' + links.data += ','; + links.data.append(pch_json.begin() + 1, pch_json.end()); // skip '[' + } else { + links.data = pch_json; + } + } + } + co_return std::move(links); + }); peer.on_request( [this](RequestContext& ctx, const protocol::CodeActionParams& params) -> RawResult { diff --git a/src/server/protocol.h b/src/server/protocol.h index dba9dd1d..3ef5c86b 100644 --- a/src/server/protocol.h +++ b/src/server/protocol.h @@ -102,6 +102,7 @@ struct BuildResult { std::string output_path; ///< PCH or PCM path std::vector deps; std::string tu_index_data; + std::string pch_links_json; ///< Pre-serialized DocumentLink[] from PCH eventide::serde::RawValue result_json; ///< Completion/SignatureHelp result }; diff --git a/src/server/stateless_worker.cpp b/src/server/stateless_worker.cpp index bd849342..02f9f3a7 100644 --- a/src/server/stateless_worker.cpp +++ b/src/server/stateless_worker.cpp @@ -96,8 +96,13 @@ static worker::BuildResult handle_build_pch(const worker::BuildParams& params) { errors = collect_errors(unit); std::string tu_index_data; - if(success) + std::string pch_links_json; + if(success) { tu_index_data = serialize_tu_index(unit); + auto links = feature::document_links(unit); + auto raw = to_raw(links); + pch_links_json = std::move(raw.data); + } // Destroy CompilationUnit to flush PCH to disk. unit = CompilationUnit(nullptr); @@ -110,6 +115,7 @@ static worker::BuildResult handle_build_pch(const worker::BuildParams& params) { result.output_path = std::move(final_path); result.deps = pch_info.deps; result.tu_index_data = std::move(tu_index_data); + result.pch_links_json = std::move(pch_links_json); return result; } else { LOG_WARN("BuildPCH failed: file={}, {}ms, errors=[{}]", params.file, timer.ms(), errors); diff --git a/src/server/workspace.h b/src/server/workspace.h index fd8b7e6d..d5ed64c7 100644 --- a/src/server/workspace.h +++ b/src/server/workspace.h @@ -140,6 +140,7 @@ struct PCHState { std::uint32_t bound = 0; std::uint64_t hash = 0; DepsSnapshot deps; + std::string document_links_json; ///< Pre-serialized DocumentLink[] from PCH build std::shared_ptr building; }; diff --git a/tests/conftest.py b/tests/conftest.py index a04ee6ca..45fa9bdd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -231,6 +231,14 @@ def _generate_test_data_cdbs(data_dir: Path) -> None: if ic_main.exists(): _write(ic_dir, [_entry(ic_dir, ic_main, ["-I."])]) + # document_links + dl_dir = data_dir / "document_links" + dl_main = dl_dir / "main.cpp" + if dl_main.exists(): + _write( + dl_dir, [_entry(dl_dir, dl_main, [f"-I{dl_dir.as_posix()}", "-std=c++23"])] + ) + # pch_test pt_dir = data_dir / "pch_test" if pt_dir.exists(): diff --git a/tests/data/document_links/data.bin b/tests/data/document_links/data.bin new file mode 100644 index 00000000..ad471007 --- /dev/null +++ b/tests/data/document_links/data.bin @@ -0,0 +1 @@ +0123456789 \ No newline at end of file diff --git a/tests/data/document_links/header_a.h b/tests/data/document_links/header_a.h new file mode 100644 index 00000000..7fb1e2fd --- /dev/null +++ b/tests/data/document_links/header_a.h @@ -0,0 +1,3 @@ +#pragma once + +int a = 1; diff --git a/tests/data/document_links/header_b.h b/tests/data/document_links/header_b.h new file mode 100644 index 00000000..52a4206a --- /dev/null +++ b/tests/data/document_links/header_b.h @@ -0,0 +1,3 @@ +#pragma once + +int b = 2; diff --git a/tests/data/document_links/header_c.h b/tests/data/document_links/header_c.h new file mode 100644 index 00000000..8a21f615 --- /dev/null +++ b/tests/data/document_links/header_c.h @@ -0,0 +1,3 @@ +#pragma once + +int c = 3; diff --git a/tests/data/document_links/main.cpp b/tests/data/document_links/main.cpp new file mode 100644 index 00000000..e6203af6 --- /dev/null +++ b/tests/data/document_links/main.cpp @@ -0,0 +1,20 @@ +#include "header_a.h" +#include "header_b.h" +int x = 1; +#include "header_c.h" + +const char data[] = { +#embed "data.bin" +}; + +#if __has_embed("data.bin") +int has_embed_found = 1; +#endif + +#if __has_embed("no_such_file.bin") +int has_embed_not_found = 1; +#endif + +int main() { + return a + b + c; +} diff --git a/tests/integration/features/test_document_links.py b/tests/integration/features/test_document_links.py new file mode 100644 index 00000000..f75dde63 --- /dev/null +++ b/tests/integration/features/test_document_links.py @@ -0,0 +1,103 @@ +from pathlib import Path + +import pytest + + +@pytest.mark.workspace("document_links") +async def test_document_links_with_pch(client, workspace): + uri, content = await client.open_and_wait(workspace / "main.cpp") + links = await client.document_links(uri) + + assert links is not None, "document_links returned None" + + targets = sorted(Path(link.target).name for link in links) + assert targets == [ + "data.bin", + "data.bin", + "header_a.h", + "header_b.h", + "header_c.h", + ], f"Unexpected targets: {targets}" + + client.close(uri) + + +@pytest.mark.workspace("document_links") +async def test_document_links_pch_portion(client, workspace): + uri, _ = await client.open_and_wait(workspace / "main.cpp") + links = await client.document_links(uri) + + pch_links = [link for link in links if link.range.start.line < 2] + assert len(pch_links) == 2, ( + f"Expected 2 PCH links (lines 0-1), got {len(pch_links)}" + ) + + pch_targets = sorted(Path(link.target).name for link in pch_links) + assert pch_targets == ["header_a.h", "header_b.h"] + + client.close(uri) + + +@pytest.mark.workspace("document_links") +async def test_document_links_main_portion(client, workspace): + uri, _ = await client.open_and_wait(workspace / "main.cpp") + links = await client.document_links(uri) + + main_links = [link for link in links if link.range.start.line >= 2] + assert len(main_links) == 3, ( + f"Expected 3 main-file links (lines 3, 6, 9), got {len(main_links)}" + ) + + main_targets = sorted(Path(link.target).name for link in main_links) + assert main_targets == ["data.bin", "data.bin", "header_c.h"] + + client.close(uri) + + +@pytest.mark.workspace("document_links") +async def test_document_links_embed(client, workspace): + uri, _ = await client.open_and_wait(workspace / "main.cpp") + links = await client.document_links(uri) + + embed_links = [ + link + for link in links + if Path(link.target).name == "data.bin" and link.range.start.line == 6 + ] + assert len(embed_links) == 1, ( + f"Expected 1 embed link at line 6, got {len(embed_links)}" + ) + + client.close(uri) + + +@pytest.mark.workspace("document_links") +async def test_document_links_has_embed_exists(client, workspace): + uri, _ = await client.open_and_wait(workspace / "main.cpp") + links = await client.document_links(uri) + + has_embed_links = [ + link + for link in links + if Path(link.target).name == "data.bin" and link.range.start.line == 9 + ] + assert len(has_embed_links) == 1, ( + f"Expected 1 has_embed link at line 9, got {len(has_embed_links)}" + ) + + client.close(uri) + + +@pytest.mark.workspace("document_links") +async def test_document_links_has_embed_missing(client, workspace): + uri, _ = await client.open_and_wait(workspace / "main.cpp") + links = await client.document_links(uri) + + missing_links = [ + link for link in links if Path(link.target).name == "no_such_file.bin" + ] + assert len(missing_links) == 0, ( + f"Expected 0 links for non-existent file, got {len(missing_links)}" + ) + + client.close(uri) diff --git a/tests/unit/feature/document_link_tests.cpp b/tests/unit/feature/document_link_tests.cpp index cc85b3cb..85033340 100644 --- a/tests/unit/feature/document_link_tests.cpp +++ b/tests/unit/feature/document_link_tests.cpp @@ -15,9 +15,9 @@ TEST_SUITE(DocumentLink, Tester) { std::vector links; -void run(llvm::StringRef source) { +void run(llvm::StringRef source, llvm::StringRef standard = "-std=c++17") { add_files("main.cpp", source); - ASSERT_TRUE(compile()); + ASSERT_TRUE(compile(standard)); links = feature::document_links(*unit, feature::PositionEncoding::UTF8); } @@ -89,6 +89,40 @@ TEST_CASE(HasInclude) { EXPECT_LINK(1, "1", TestVFS::path("test.h")); } +TEST_CASE(Embed) { + run(R"cpp( +#[bytes.bin] +0123456789 + +#[main.cpp] +const char e[] = { +#embed @0["bytes.bin"$] +}; +)cpp", + "-std=c++23"); + + ASSERT_EQ(links.size(), 1U); + EXPECT_LINK(0, "0", TestVFS::path("bytes.bin")); +} + +TEST_CASE(HasEmbed) { + run(R"cpp( +#[data.bin] +ABCDE + +#[main.cpp] +#if __has_embed(@0["data.bin"$]) +#endif + +#if __has_embed("non_existent.bin") +#endif +)cpp", + "-std=c++23"); + + ASSERT_EQ(links.size(), 1U); + EXPECT_LINK(0, "0", TestVFS::path("data.bin")); +} + }; // TEST_SUITE(DocumentLink) } // namespace