Skip to content

fix: return definition links and honor includeDeclaration - #168

Open
lvliangxiong wants to merge 3 commits into
coder3101:mainfrom
lvliangxiong:codex/improve-definition-links-and-references
Open

lvliangxiong wants to merge 3 commits into
coder3101:mainfrom
lvliangxiong:codex/improve-definition-links-and-references

Conversation

@lvliangxiong

@lvliangxiong lvliangxiong commented Oct 7, 2026 •

Copy link
Copy Markdown

Protols currently returns only Location for definition requests, omitting the source range needed to identify a complete import path or qualified type. Definition resolution also depends on which segment of a qualified type contains the cursor. Separately, reference requests always include declarations even when includeDeclaration is false, and resolving each reference repeatedly clones the entire workspace model, making queries unresponsive in larger workspaces.

This change supplies the navigation information directly from Protols' parsed metamodel:

  • Read textDocument.definition.linkSupport during initialization. Return LocationLink[] only when it is true, otherwise preserve the existing Location / Location[] response format.
  • Populate originSelectionRange from the complete type reference or import path, and populate the target declaration and name ranges separately. Import paths exclude their quotes, keywords, and semicolons, including public/weak imports.
  • Resolve the complete qualified type for definition navigation, including an optional leading dot. Every segment of example.api.Outer.Header targets Header; rename continues to use its existing segment-sensitive resolution.
  • Honor ReferenceParams.context.includeDeclaration, allowing clients to distinguish one actual use from the declaration plus that use.
  • Borrow cached documents during exact/suffix lookups and resolve only references with a matching final type name. Candidate references still go through scope/package resolution, preserving correctness for shadowed and same-named types.

Validation on macOS arm64 with Rust 1.95.0:

  • cargo fmt --all -- --check

  • cargo clippy --locked --all-targets -- -D warnings

  • cargo test --locked --verbose: 126 passed, including 8 new tests for capability negotiation, legacy responses, complete type/import ranges, RPCs, multiple definitions, and reference filtering. Updated the expected definition and import-hover snapshots.

  • cargo build --locked and cargo build --locked --release

  • Stdio JSON-RPC smoke test against the built server using temporary proto files: initialize with linkSupport true, false, and omitted; query every character of a qualified type and import path; verify response shape/ranges and includeDeclaration true/false with one and two actual uses.

  • Binary --help / --version, plus protoc and clang-format checks using sample/simple.proto.

  • Large-workspace regression check with 309 existing proto files plus a test document: the previous reference query timed out after 45 seconds; the updated query returned the same two expected references in approximately 10 ms on the same local machine.

@lvliangxiong

lvliangxiong commented Oct 7, 2026 •

Copy link
Copy Markdown
Author

中文说明:这个 PR 完善 Protols 语言服务器的定义跳转和引用查询,使返回范围更准确,并遵守 LSP 的能力协商和请求参数。

  1. 返回完整的链接范围。 从已有的语法解析结果中提取完整导入路径和类型名,填入 originSelectionRange。例如 api/common.proto 和 example.api.Outer.Header 都作为一个整体;导入路径不包含引号、关键字和分号。

  2. 统一完整类型名的跳转目标。 点击 example.api.Outer.Header 中的任意一段,都会定位到 Header 的定义,同时支持以 . 开头的完整限定名。重命名继续按当前分段处理。

  3. 正确处理引用查询参数。 遵守 includeDeclaration:为 false 时只返回实际使用位置,为 true 时包含定义位置。这样客户端能够准确判断实际引用数量。

  4. 兼容不同客户端。 初始化时读取 textDocument.definition.linkSupport;支持时返回 LocationLink[],否则继续返回 Location 或 Location[]。目标声明的完整范围与名称范围分别提供。

  5. 减少大工作区的查询开销。 查找定义时直接读取缓存文档,只复制匹配结果;查询引用时先排除名称不匹配的类型,再对候选项做完整的作用域和包名解析,避免为每个字段反复复制整个工作区。补充了同名类型、跨包引用和嵌套类型遮蔽的回归测试。

验证:本地 126 项测试、严格 Clippy 检查、格式检查、debug/release 构建和实际 LSP 请求验证均已通过,覆盖能力开关、完整路径与类型范围,以及不同引用数量。

在包含 309 个现有 proto 文件及一个测试文件的本地工作区中,原引用查询超过 45 秒超时;优化后同一查询约 10 毫秒返回两个预期引用。

Comment thread src/state/navigation_tests.rs Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests should be at their own modules. such as reference, definition etc

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! Addressed in be1190a. The definition and import navigation tests now live in definition.rs, and the reference query tests live in resolve.rs, alongside their implementations. I removed navigation_tests.rs; only shared fixtures and helpers remain in test_helpers.rs.

All 126 tests, formatting, build, and strict Clippy checks pass locally with Rust 1.99.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants