Skip to content

fix: --check diagnoses every file twice - #3499

Open
Rathoz wants to merge 1 commit into
LuaLS:masterfrom
Rathoz:fix/check-double-diagnosis
Open

Rathoz wants to merge 1 commit into
LuaLS:masterfrom
Rathoz:fix/check-double-diagnosis

Conversation

@Rathoz

@Rathoz Rathoz commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

--check runs the full diagnostics twice on every file.

Cause

The check worker calls files.open(uri) and then diag.doDiagnostic(uri, true). Opening the file fires the open event, and its handler in script/provider/diagnostic.lua already calls m.doDiagnostic(uri). The CLI disables await, so that diagnosis finishes inside files.open, and the explicit call then repeats it.

Fix

Drop the explicit doDiagnostic call and the now unused require.

Testing

  • Small workspace: identical JSON output before and after.
  • Liquipedia/Lua-Modules (~1,800 files): 92s → 49s single threaded, 86s → 55s with --num_threads=2. Same problems reported, apart from a few in two files whose results already vary between runs on 3.19.2.

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings November 11, 2026 11:00

@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.

中文:该改动从 check_worker.lua 中移除了 diag.doDiagnostic(uri, true) 调用,但代码中没有任何证据表明诊断仍在其他地方运行;如果诊断没有在 files.open(uri) 触发的流程中隐式发生,那么 --check 可能不再输出任何诊断结果,这将是一个功能回归。建议确认诊断是否由其他代码路径(例如文件打开事件或后续的处理循环)触发,并补充一个测试来验证 --check 仍能报告诊断信息。
English: This change removes the diag.doDiagnostic(uri, true) call from check_worker.lua, but there is no evidence in the diff that diagnostics are still executed elsewhere. If diagnostics are not implicitly triggered by files.open(uri), --check may no longer emit any diagnostics — a functional regression. Please verify that diagnostics are triggered by another code path (e.g., file-open event or a later processing loop), and add a test confirming --check still reports diagnostics.

中文:changelog 条目写的是“不再对每个文件诊断两次”,但 diff 只展示了删除诊断调用,没有展示新增/恢复的调用位置。这使得审查者无法判断修复方式的正确性。建议在同一 PR 中同时展示新增的诊断触发点,或在 PR 描述中说明诊断现在在哪里运行。
English: The changelog states "--check no longer diagnoses every file twice", but the diff only shows a removed diagnostic call and no added/restored call site. This makes it impossible for reviewers to validate the fix. Please also include the diagnostic trigger point that now handles this, or document in the PR description where diagnostics are now run.

中文:综合评审:该改动意图明确(修复重复诊断问题),但范围很小且缺少配套证据。主要风险是误删了唯一的诊断触发点导致 --check 失效。需要确认诊断仍被正确触发,并最好用一个回归测试锁定行为。
English: Overall review: the intent is clear (fix duplicate diagnostics), but the change is minimal and lacks supporting evidence. The main risk is accidentally removing the only diagnostic trigger, breaking --check. Please confirm diagnostics are still triggered correctly and ideally lock in the behavior with a regression test.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are reported for the CLI --check diagnostic cleanup.

No open issues 🎉

What changed in this PR

中文:此 PR 消除 CLI --check 对每个文件的重复诊断,改为仅依赖打开文件事件触发诊断,减少不必要的工作。

English: This PR removes duplicate per-file diagnostics from CLI --check, relying on the file-open event to trigger diagnostics and avoiding redundant work.

Changes:

  • 中文:删除显式诊断调用及不再使用的导入。
    English: Remove the explicit diagnostic call and its unused import.
  • 中文:在更新日志中记录修复。
    English: Document the fix in the changelog.
File Description
script/​cli/​check_worker.lua 中文:移除重复诊断调用及相关导入。
English: Remove the redundant diagnostic call and import.
changelog.md 中文:添加 --check 重复诊断修复说明。
English: Add a release note for the duplicate-diagnostics fix.

Review effort: Balanced

`files.open` already diagnoses the file through the `open` handler in
provider/diagnostic.lua, so the explicit `doDiagnostic` call repeated it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Rathoz
Rathoz force-pushed the fix/check-double-diagnosis branch from b86fee5 to 605f357 Compare November 11, 2026 11:35
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