fix(diagnostics): flag a deleted package declaration on live edit

[Hessesian]

Summary

The debounced textDocument/didChange diagnostics path (FileChangeHandler)
builds its own semantic-diagnostics list independently of DocumentHandler's
handle_file_opened/republish_open_file_diagnostics, and never called
missing_package_diagnostic — so deleting a file's package line via a live
edit never surfaced the "Missing package declaration" warning, even though
opening (or reopening) the same file correctly did.

Root cause, confirmed not guessed: git log -S"missing_package_diagnostic"
across this file's entire history (including before its extraction from the
monolithic workspace actor in refactor(workspace): extract actor handlers (w5b)) returns zero hits. This call has never existed in this file. It's not
a regression from a recent change removing it — the missing-package
diagnostic feature, when first added, only ever wired the two
DocumentHandler call sites. Three near-identical diagnostic-list-building
blocks exist across two files, and one of them was incomplete from day one.

Fix

Extracted the closure into compute_debounced_semantic_diagnostics so this
list is independently unit-testable (it wasn't reachable as a standalone unit
before — testing it required either mocking tower_lsp::Client or going
through the full async debounce pipeline with no way to inspect what got
published), then added the missing call. The two DocumentHandler call sites
are untouched.

Test plan

• New regression test debounced_diagnostics_flag_a_deleted_package_declaration
  — real red before the fix (confirmed: panicked ... got: []), real
  green after
• cargo test — 1967 passed, 0 failed, 3 ignored
• cargo clippy --all-targets -- -D warnings — clean
• cargo fmt -- --check — clean

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

Address the diagnostic-task error handling and ensure the test covers the production publish path.

Pull request overview

Fixes live-edit diagnostics so deleting a package declaration produces the missing-package warning.

Changes:

• Extracts debounced semantic diagnostic computation.
• Adds package diagnostics to the debounced path.
• Adds regression coverage for deleted package declarations.

File summaries

File	Summary and review findings
src/workspace/file_change_handler.rs	Adds package diagnostics. Address the moderate join-error handling issue and rename pkg_diag to package_diagnostic.
src/workspace/file_change_handler_tests.rs	Adds regression coverage. Exercise the actual publish path and rename abbreviated locals (tmp, diags_before, diags_after).

Review details

Suppressed comments (5)

src/workspace/file_change_handler.rs:292

• pkg_diag is an abbreviated name for a newly introduced local and conflicts with the repository's no-abbreviations convention. Please use the full package_diagnostic name so the diagnostic's purpose is explicit.

    if let Some(pkg_diag) = missing_package_diagnostic(&text_lines, uri) {
        diagnostics.push(pkg_diag);

src/workspace/file_change_handler.rs:293

• When the index_content blocking task fails, the caller above substitutes diagnostics_text with an empty string. This unconditional check then treats the failed read as an empty Kotlin/Java file and publishes a false “Missing package declaration” warning for any path from which a package can be derived, instead of preserving the prior diagnostics or skipping this publish. Keep the join-error state distinct from a successful cache-hit result and avoid computing package diagnostics when the task failed.

    let text_lines: Vec<String> = diagnostics_text.lines().map(str::to_owned).collect();
    if let Some(pkg_diag) = missing_package_diagnostic(&text_lines, uri) {
        diagnostics.push(pkg_diag);
    }

src/workspace/file_change_handler_tests.rs:188

• tmp is a newly introduced abbreviated local name; use a descriptive name such as temporary_workspace to follow the repository's no-abbreviations convention and make the fixture setup clearer.

    let tmp = tempfile::tempdir().unwrap();

src/workspace/file_change_handler_tests.rs:207

• diags_before and the matching diags_after local use the abbreviated diags form, which is explicitly disallowed for new names by the repository's no-abbreviations convention. Rename them to diagnostics_before and diagnostics_after (and update their uses).

    let diags_before =
        super::compute_debounced_semantic_diagnostics(&indexer, &uri, with_package, 0);

src/workspace/file_change_handler_tests.rs:198

• This test constructs the handler with client: None, so the debounced task returns before compute_debounced_semantic_diagnostics is invoked (the early return is in file_change_handler.rs:136). The later direct calls test the helper, but would still pass if the production closure stopped calling it; please exercise the actual publish path with an injectable/mock client or otherwise assert the task's semantic-diagnostics output.

    let mut handler = FileChangeHandler::new(Arc::clone(&indexer), None);

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.