Bug fixes

Author: AJenbo

docs/CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Change visibility.** The code action no longer appears when the cursor is inside a method body. It now only triggers on the method signature (modifiers, name, parameters, return type).
 - **Update docblock.** The code action no longer appears when the cursor is inside a function or method body. It now only triggers on the signature or the preceding docblock.
 - **Update docblock.** No longer suggests adding redundant `@param` tags when the docblock has no `@param` tags and all parameters already have sufficient native type hints. This matches the generate-docblock behaviour, which intentionally omits `@param` for fully-typed non-templated parameters.
+- **PHPStan diagnostics.** PHPStan cache pruning after deduplication now unconditionally writes the pruned set back, fixing a theoretically possible stale-entry reappearance when pruning changed diagnostics without changing the count.
 
 ### Added
 
@@ -54,6 +55,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Hover.** Hovering over unresolved function calls, unknown constants, or unresolvable `self`/`static`/`parent`/`$this` keywords no longer shows a bare placeholder. If the symbol cannot be found, no hover is shown.
+- **Add @throws.** The code action no longer double-indents the closing `*/` when inserting a `@throws` tag into an existing multi-line docblock.
+- **PHPStan stale-diagnostic clearing.** The `@throws`-based staleness check now scopes to the enclosing function's docblock instead of searching the entire file. A `@throws` tag on a different function no longer causes an unrelated diagnostic to be incorrectly cleared.
 - **Closure and arrow function variable scope.** Variable name completion now correctly respects PHP scoping rules for anonymous functions and arrow functions. Parameters of a closure are visible inside its body, `use`-captured variables appear alongside them, and `$this` is available when the closure is defined in an instance method. Outer method locals that were not captured do not leak in. Arrow function parameters are now visible inside the arrow body while the enclosing scope's variables remain accessible, matching PHP's implicit capture behaviour.
 - **Namespace alias completion.** Typing a class name through a namespace alias (e.g. `OA\Re` with `use OpenApi\Attributes as OA`) now correctly suggests classes under the aliased namespace such as `OA\Response` and `OA\RequestBody`. Previously only unrelated classes matched because the alias was not expanded before prefix matching.
 - **Virtual property merging.** Native type hints are now considered when determining virtual property specificity. Previously only docblock types were compared, causing properties with native PHP type declarations (e.g., `public string $name`) to be incorrectly overridden by less specific virtual properties.

docs/todo.md

@@ -23,10 +23,6 @@ within the same impact tier.
 
 | #   | Item                                                                                                              | Impact | Effort |
 | --- | ----------------------------------------------------------------------------------------------------------------- | ------ | ------ |
-| B13 | [Stop showing dummy symbols in hover](todo/bugs.md#b13--hover-shows-dummy-symbols)                                | Medium | Low    |
-| B14 | [Add @throws action inserts misaligned docblock](todo/bugs.md#b14--add-throws-action-inserts-misaligned-docblock) | Medium | Low    |
-| B15 | [Completion after `->()` should not insert parentheses](todo/bugs.md#b15--completion-after---should-not-insert-parentheses) | Medium | Low    |
-| B16 | [PHPStan stale-diagnostic clearing is overly aggressive](todo/bugs.md#b16--phpstan-stale-diagnostic-clearing-is-overly-aggressive) | Medium | Medium |
 |     | **Release 0.6.0**                                                                                                 |        |        |
 
 ## Sprint 4 — Refactoring toolkit
@@ -163,7 +159,6 @@ unlikely to move the needle for most users.
 | X6  | Disk cache (evaluate later)                                                                                                                                  | Medium     | High        |
 |     | **[Bug Fixes](todo/bugs.md)**                                                                                                                                |            |             |
 | B11 | [Diagnostic deduplication drops distinct diagnostics on same range](todo/bugs.md#b11--diagnostic-deduplication-drops-distinct-diagnostics-on-the-same-range) | Medium     | Low         |
-| B12 | [PHPStan cache pruning uses length-only comparison](todo/bugs.md#b12--phpstan-cache-pruning-uses-length-only-comparison)                                     | Low        | Low         |
 | B13 | [Argument count diagnostic flags too many arguments by default](todo/bugs.md#b13-argument-count-diagnostic-flags-too-many-arguments-by-default)              | High       | Low         |
 |     | **[Inline Completion](todo/inline-completion.md)**                                                                                                           |            |             |
 | N1  | Template engine (type-aware snippets)                                                                                                                        | Medium     | High        |

docs/todo/bugs.md

@@ -15,123 +15,4 @@ within the same impact tier.
 
 ---
 
-## B11 — Diagnostic deduplication drops distinct diagnostics on the same range
-
-| Impact | Effort |
-| ------ | ------ |
-| Medium | Low    |
-
-`deduplicate_diagnostics` in `src/diagnostics/mod.rs` calls
-`dedup_by(|a, b| a.range == b.range)` after sorting by range. This
-removes **all** diagnostics that share the exact same span, regardless
-of their diagnostic code, message, or severity. If two genuinely
-different native diagnostics land on the same range (e.g. an
-`argument_count` error and an `unknown_member` warning on the same
-expression), the second one is silently dropped.
-
-**Fix:** Change the dedup key from `a.range == b.range` to
-`a.range == b.range && a.code == b.code`. This preserves distinct
-diagnostic codes on the same span while still collapsing true
-duplicates produced by different analysis phases.
-
----
-
-## B12 — PHPStan cache pruning uses length-only comparison
-
-| Impact | Effort |
-| ------ | ------ |
-| Low    | Low    |
-
-In `publish_diagnostics_for_file` (`src/diagnostics/mod.rs`), the
-PHPStan cache pruning step only updates the cache when
-`pruned.len() != cached.len()`. If deduplication replaces one PHPStan
-diagnostic with a different one at the same count (same number of
-entries but different content), the cache is not updated. On the next
-Phase 1 merge the stale entry would reappear.
-
-In practice this is unlikely because pruning only ever removes entries
-(never replaces them), but the check is technically incorrect.
-
-**Fix:** Replace the length comparison with a content comparison, or
-unconditionally write the pruned set back into the cache (the extra
-write is negligible).
-
----
-
-## B13 — Hover shows dummy symbols
-
-| Impact | Effort |
-| ------ | ------ |
-| Medium | Low    |
-
-When hovering over certain constructs the hover popup displays
-internal dummy/placeholder symbols instead of filtering them out.
-These symbols are not meaningful to the user and clutter the hover
-output.
-
-**Fix:** Filter out dummy symbols before building the hover response
-so only real, user-relevant information is shown.
-
----
-
-## B14 — Add `@throws` action inserts misaligned docblock
-
-| Impact | Effort |
-| ------ | ------ |
-| Medium | Low    |
-
-When the "Add `@throws`" code action creates a new docblock (no
-existing docblock is present), the inserted block is not aligned with
-the function/method it annotates. The opening `/**` and closing `*/`
-use incorrect indentation, producing code that is visually broken and
-fails style checks.
-
-**Fix:** Detect the indentation of the target function/method line and
-use it as the base indentation for every line of the generated
-docblock (`/**`, ` * @throws …`, ` */`).
-
----
-
-## B15 — Completion after `->|()` should not insert parentheses
-
-| Impact | Effort |
-| ------ | ------ |
-| Medium | Low    |
-
-When completing a method call where the cursor is immediately before
-existing parentheses (e.g. `$obj->|()` with the cursor at `|`), the
-completion item still inserts its own parentheses, producing
-`$obj->method()()`. The completion engine should detect that
-parentheses already follow the cursor and suppress the snippet
-suffix in that case.
-
-**Fix:** Before attaching the `()` (or `($1)`) snippet suffix to a
-callable completion item, check whether the character immediately
-after the completion range is `(`. If so, emit a plain text insert
-without parentheses.
-
----
-
-## B16 — PHPStan stale-diagnostic clearing is overly aggressive
-
-| Impact | Effort |
-| ------ | ------ |
-| Medium | Medium |
-
-`is_stale_phpstan_diagnostic()` in `src/diagnostics/mod.rs` sometimes
-clears diagnostics that are still valid. For example, adding a
-`@throws` tag for one exception may cause an unrelated diagnostic on
-a nearby line to be treated as stale if the simple substring check
-matches text that happens to appear elsewhere in the file. The
-heuristic-based approach (checking whether a short name appears after
-`@throws` anywhere in the file content) has false positives.
-
-**Fix:** Audit each branch of `is_stale_phpstan_diagnostic()` and
-tighten the checks:
-
-1. Scope `content_has_throws_tag` to the docblock enclosing the
-   diagnostic line rather than searching the entire file.
-2. For `@phpstan-ignore` coverage, verify that the ignore comment is
-   on the exact diagnostic line (or the line immediately before it),
-   not just any line in the file.
-3. Add regression tests for the false-positive scenarios.
+No outstanding items.

src/code_actions/phpstan/add_throws.rs

@@ -439,7 +439,6 @@ fn insert_throws_into_existing_docblock(
         insert_text.push_str(&format!("{} *\n", indent));
     }
     insert_text.push_str(&format!("{} * @throws {}\n", indent, short_name));
-    insert_text.push_str(indent);
 
     // Find where the `*/` line starts (including leading whitespace).
     let close_line_start = doc[..close_pos].rfind('\n').map(|p| p + 1).unwrap_or(0);
@@ -666,6 +665,54 @@ mod tests {
         );
     }
 
+    #[test]
+    fn multiline_insert_does_not_double_indent_closing_tag() {
+        // Simulate applying the edit: insert text at the `*/` line start.
+        // The insert must NOT include trailing indent that would double up
+        // with the existing `     */` line.
+        let php = "<?php\nclass Foo {\n    /**\n     * Summary.\n     */\n    public function bar(): void {\n        throw new \\RuntimeException();\n    }\n}\n";
+        let info = find_enclosing_docblock(php, 6).unwrap();
+        let edit = build_throws_edit(php, &info, "RuntimeException");
+
+        // Apply the edit to the source text.
+        let start = byte_offset_to_lsp(php, 0); // unused, we apply manually
+        let _ = start;
+        let insert_offset = {
+            let mut off = 0usize;
+            for (i, line) in php.lines().enumerate() {
+                if i == edit.range.start.line as usize {
+                    off += edit.range.start.character as usize;
+                    break;
+                }
+                off += line.len() + 1;
+            }
+            off
+        };
+        let end_offset = {
+            let mut off = 0usize;
+            for (i, line) in php.lines().enumerate() {
+                if i == edit.range.end.line as usize {
+                    off += edit.range.end.character as usize;
+                    break;
+                }
+                off += line.len() + 1;
+            }
+            off
+        };
+        let mut result = String::new();
+        result.push_str(&php[..insert_offset]);
+        result.push_str(&edit.new_text);
+        result.push_str(&php[end_offset..]);
+
+        // The `*/` line should have exactly 4 spaces of indent (matching `/**`).
+        let close_line = result.lines().find(|l| l.trim() == "*/").unwrap();
+        assert_eq!(
+            close_line, "     */",
+            "closing */ should be aligned with the docblock (5 chars: 4 spaces + space before */).\nFull result:\n{}",
+            result
+        );
+    }
+
     #[test]
     fn inserts_throws_into_single_line_docblock() {
         let php = "<?php\nclass Foo {\n    /** Summary. */\n    public function bar(): void {\n        throw new \\RuntimeException();\n    }\n}\n";
@@ -698,6 +745,12 @@ mod tests {
             "should contain @throws: {:?}",
             edit.new_text
         );
+        // Every line of the new docblock should start with the same indent
+        // as the method signature (4 spaces).
+        assert_eq!(
+            edit.new_text, "    /**\n     * @throws RuntimeException\n     */\n",
+            "new docblock should be aligned with the method"
+        );
     }
 
     // ── Docblock with existing @throws ──────────────────────────────
@@ -729,6 +782,70 @@ mod tests {
         );
     }
 
+    #[test]
+    fn inserts_throws_after_return_aligned() {
+        // Reproduce the exact scenario: existing docblock with @return,
+        // insert @throws.  The `*/` must not get double-indented.
+        let php = concat!(
+            "<?php\nclass Foo {\n",
+            "    /**\n",
+            "     * @return Response\n",
+            "     */\n",
+            "    public function clientside(): Response {\n",
+            "        throw new \\RuntimeException();\n",
+            "    }\n",
+            "}\n",
+        );
+        let info = find_enclosing_docblock(php, 6).unwrap();
+        let edit = build_throws_edit(php, &info, "RuntimeException");
+
+        // Apply the edit to the source text.
+        let insert_offset = {
+            let mut off = 0usize;
+            for (i, line) in php.lines().enumerate() {
+                if i == edit.range.start.line as usize {
+                    off += edit.range.start.character as usize;
+                    break;
+                }
+                off += line.len() + 1;
+            }
+            off
+        };
+        let end_offset = {
+            let mut off = 0usize;
+            for (i, line) in php.lines().enumerate() {
+                if i == edit.range.end.line as usize {
+                    off += edit.range.end.character as usize;
+                    break;
+                }
+                off += line.len() + 1;
+            }
+            off
+        };
+        let mut result = String::new();
+        result.push_str(&php[..insert_offset]);
+        result.push_str(&edit.new_text);
+        result.push_str(&php[end_offset..]);
+
+        let expected = concat!(
+            "<?php\nclass Foo {\n",
+            "    /**\n",
+            "     * @return Response\n",
+            "     *\n",
+            "     * @throws RuntimeException\n",
+            "     */\n",
+            "    public function clientside(): Response {\n",
+            "        throw new \\RuntimeException();\n",
+            "    }\n",
+            "}\n",
+        );
+        assert_eq!(
+            result, expected,
+            "inserted @throws must not double-indent the closing */.\nGot:\n{}",
+            result
+        );
+    }
+
     // ── Standalone function ─────────────────────────────────────────
 
     #[test]

src/code_actions/phpstan/mod.rs

@@ -46,4 +46,4 @@ impl Backend {
         // ── Remove invalid/unused @throws ───────────────────────────
         self.collect_remove_throws_actions(uri, content, params, out);
     }
-}
+}

src/code_actions/update_docblock.rs

@@ -123,13 +123,15 @@ impl Backend {
         let function_loader = self.function_loader(&ctx);
 
         // Determine if anything needs updating.
-        let needs_update = check_needs_update(&info, content, &class_loader, Some(&function_loader));
+        let needs_update =
+            check_needs_update(&info, content, &class_loader, Some(&function_loader));
         if !needs_update {
             return;
         }
 
         // Build the replacement docblock.
-        let new_docblock = build_updated_docblock(&info, content, &class_loader, Some(&function_loader));
+        let new_docblock =
+            build_updated_docblock(&info, content, &class_loader, Some(&function_loader));
         if new_docblock == info.docblock_text {
             return;
         }
@@ -712,7 +714,14 @@ fn check_needs_update(
     }
 
     // Check for missing @throws tags.
-    let uncaught = throws_analysis::find_uncaught_throw_types_with_context(content, info.docblock_position, Some(&ThrowsContext { class_loader, function_loader }));
+    let uncaught = throws_analysis::find_uncaught_throw_types_with_context(
+        content,
+        info.docblock_position,
+        Some(&ThrowsContext {
+            class_loader,
+            function_loader,
+        }),
+    );
     let existing_lower: Vec<String> = info
         .doc_throws
         .iter()
@@ -944,7 +953,14 @@ fn build_updated_docblock(
     }
 
     // Add missing @throws tags.
-    let uncaught = throws_analysis::find_uncaught_throw_types_with_context(content, info.docblock_position, Some(&ThrowsContext { class_loader, function_loader }));
+    let uncaught = throws_analysis::find_uncaught_throw_types_with_context(
+        content,
+        info.docblock_position,
+        Some(&ThrowsContext {
+            class_loader,
+            function_loader,
+        }),
+    );
     let existing_throws_lower: Vec<String> = info
         .doc_throws
         .iter()
@@ -1884,7 +1900,12 @@ class Foo {
             "Should find function info when cursor is inside the docblock"
         );
         let cl = no_class_loader();
-        assert!(check_needs_update(&info.unwrap(), php, &cl, no_function_loader()));
+        assert!(check_needs_update(
+            &info.unwrap(),
+            php,
+            &cl,
+            no_function_loader()
+        ));
     }
 
     #[test]