Skip to content

fix: include see references when renaming types - #3497

Open
sogav21 wants to merge 2 commits into
LuaLS:masterfrom
sogav21:codex/issue-3470-rename-see-references
Open

sogav21 wants to merge 2 commits into
LuaLS:masterfrom
sogav21:codex/issue-3470-rename-see-references

Conversation

@sogav21

@sogav21 sogav21 commented Nov 11, 2026

Copy link
Copy Markdown

Renaming a LuaDoc class now updates its resolved @see references. Rename can also start from an @see reference to a class, alias or enum. References are matched to the type declaration so same-named globals and types in other workspace scopes keep their names.

Fixes #3470.

Validation

  • The new regression reproduces the missing edit on the pinned baseline.
  • The full Windows test suite passed, including the additional rename and workspace-scope cases.
  • JSON-RPC prepareRename and rename requests passed from both the declaration and @see.

Developed with AI assistance.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

中文:在 ofDocTypeName 中,declarations[doc] = true 存储的是 doc 源节点(例如 doc.class),而 getSeeType 返回的是 target.class / target.alias / target.enum(即节点上的名称字段)。随后用 declarations[target.parent] 判断,恰好依赖 target.parent 等于最初的 doc 节点。这个隐式耦合很脆弱:一旦 getSeeType 的返回方式改变(例如直接返回 doc.class 的归属节点),重命名会静默失效。建议让 getSeeType 返回声明节点本身,或让 declarations 直接以名称节点为键,使逻辑显式一致。
English: In ofDocTypeName, declarations[doc] = true stores the declaration doc source node (e.g. doc.class), while getSeeType returns target.class / target.alias / target.enum (the name field on the node). The check declarations[target.parent] then relies on target.parent being exactly the original doc node. This implicit coupling is fragile: if getSeeType changes its return shape (e.g. returns the owning node directly), renaming will silently stop working. Recommend making getSeeType return the declaration node itself, or keying declarations by the name node, so the logic is explicitly consistent.

中文:getSeeType 对每个 doc.see.name 都会调用 wssymbol(source[1], guide.getUri(source)),然后遍历所有 workspace symbol。在后续对所有文件遍历 doc.see.name 的过程中,若 oldname 常见(例如短类名),这会产生大量重复的 workspace 级符号扫描,可能造成重命名性能明显下降。建议在一次重命名中缓存 oldname 的符号查找结果(或按 uri 缓存),并在 ofDocTypeName 中将结果复用。
English: getSeeType calls wssymbol(source[1], guide.getUri(source)) and iterates all workspace symbols for every doc.see.name. During the later pass over all files' doc.see.name nodes, if oldname is common (e.g. a short class name), this causes many repeated workspace-wide symbol scans and can significantly degrade rename performance. Recommend caching the symbol lookup for oldname (or per-uri) within a single rename and reusing it in ofDocTypeName.

中文:ofDocTypeName 现在会遍历 files.eachFile(uri) 的所有文件,并用 text:find(oldname, 1, true) 做纯文本预过滤。这是一个合理优化,但需要注意 getText/getState 可能触发大量文件加载;对大项目而言,重命名一个类型名可能拉入整个工作区文件,造成明显延迟和内存峰值。建议至少在测试或负载场景中验证,或考虑基于索引/引用搜索来定位 doc.see.name 引用,而不是全量遍历。
English: ofDocTypeName now iterates every file via files.eachFile(uri) and pre-filters with text:find(oldname, 1, true). This is a reasonable optimization, but getText/getState may trigger loading many files; in large projects renaming a type name could pull in the entire workspace, causing noticeable latency and memory spikes. Suggest at least validating this under test/load scenarios, or locating doc.see.name references via index/reference search instead of a full crawl.

中文:测试中通过 files.getText(edit.uri) 和 assert(editScript:find('OldName', ...)) 反推编辑位置,默认每个编辑文件里只有一处 OldName。在 seeScript 中同时存在 ---@see OldName 和赋值 OldName = 1 时,find 返回第一处,若不匹配会得到误导性断言失败;且该测试没有验证 OldName = 1 / print(OldName) 未被错误重命名。建议按预期文本直接计算偏移或对同一文件中的多处出现进行精确匹配,并显式断言非类型引用未被修改。
English: The tests derive edit positions via files.getText(edit.uri) and assert(editScript:find('OldName', ...)), assuming only one occurrence of OldName per edited file. When seeScript contains both ---@see OldName and the assignment OldName = 1, find returns the first occurrence, and a mismatch produces misleading assertion failures; the test also does not verify that OldName = 1 / print(OldName) were left untouched. Suggest computing offsets against expected text directly or matching specific occurrences, and explicitly asserting non-type references are not renamed.

中文:整体改动方向正确,补充了 doc.see.name 的 accept 支持并覆盖了 class、alias、enum 的声明前后位置与跨文件场景。建议在合并前处理上述 declarations 与 getSeeType 的隐式耦合以及全工作区遍历的性能问题。
English: The overall direction is correct: it adds doc.see.name to the accept list and covers declarations before/after the class, alias, and enum cases plus cross-file scenarios. Recommend addressing the implicit coupling between declarations and getSeeType and the full-workspace traversal performance concern before merging.

@sogav21

sogav21 commented Nov 11, 2026

Copy link
Copy Markdown
Author

The repeated symbol enumeration was measurable, so d6a1160 resolves the name through the existing type index and its scope-aware declaration cache. In a local fixture with 3,000 extra globals and 512 @see references, both rename entry points now produce the same 513 edits without enumerating all globals. A repeat-reference regression was added and the full Windows suite and JSON-RPC checks passed.

The declaration/name-node parent relation follows the parser's existing AST representation. The tests check complete output text for same-named globals and exact edit counts/ranges across workspace scopes. files.getText reads the existing file text, and the remaining matching-file traversal follows the reference search used elsewhere in this module.

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.

@see annotation is not renamed on refactor

1 participant