Hover stands down at every declaration site, not just most of them

Author: AJenbo

docs/CHANGELOG.md

@@ -126,6 +126,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Hover stands down at every declaration site, not just most of them.** A class, interface, namespace, method, property, class constant, and enum case already answered nothing on their own declaration, since the signature and docblock are already on screen. A global function's own name, a top-level `const`, and a constructor-promoted parameter did not follow that rule: hovering `function helper()` repeated its signature, hovering `const LIMIT = 5;` repeated its value, and a promoted parameter (`__construct(public string $sku)`) hovered as a local variable rather than the property it actually declares. All three now stand down the same way the rest do.
 - **Hover no longer depends on indexing timing.** A hover, go-to-definition, or any other request that arrived before the file's first parse published its symbol map answered null (or fell back to poorer resolution), so the same request could succeed in one session and fail in the next depending on how far background indexing had gotten. A request for a file with no symbol map now parses it on the spot from the content it already fetched, so the first answer matches every later one. Closes #343.
 - **A raw Blade echo is read from its own opening brace.** `{!! $html !!}` was only recognised when written as `{{!! $html !!}}`, a spelling Blade does not use, so the expression inside a real raw echo was masked as HTML: a variable declared in a `<?php ?>` block and displayed through `{!! … !!}` was reported unused, and completion, hover, and go-to-definition inside the echo answered nothing. The raw echo now compiles the way Blade compiles it, to a plain `echo` with no `e()` escape around it, and each echo form only closes at its own terminator, so `!!}` no longer ends an escaped `{{ … }}` early. Closes #370.
 - **An echo opener with no terminator no longer swallows the rest of the template.** A `{{` with no `}}` anywhere after it, or a `{!!` with no `!!}` — `<script>if (a) {!!b}</script>` was enough — put the rest of the file into PHP mode: every later line was read as code instead of markup and the whole template stopped parsing. Such an opener is now closed at the end of its own line, so at most that line degrades and everything after it works as usual. An echo that is still being typed, or that spans lines with its terminator further down, stays open exactly as before, so completion inside a half-written echo keeps working.

docs/todo/bugs.md

@@ -45,31 +45,4 @@ No outstanding items.
 
 ## Miscellaneous
 
-### B178. Three declaration sites still hover
-
-**Impact: Low · Complexity: Low**
-
-Hover stands down at a definition site, because the signature is on the
-line under the cursor and the docblock is on the lines above it, so the
-popup would only repeat what is already on screen. Class, interface,
-namespace, method, property, class constant, and enum case declarations
-all follow that rule. Three do not:
-
-- a global `function helper(): int` at its own name answers with its
-  docblock and signature;
-- a global `const LIMIT = 5;` (and `define()`) at its own name answers
-  with its value;
-- a constructor-promoted parameter (`__construct(public string $sku)`)
-  answers as a local parameter, which is also the wrong reading of it:
-  the name declares a property, not a variable.
-
-All three go through `hover_from_symbol` in `src/hover/mod.rs`: the
-first two land in the `FunctionCall` / `ConstantReference` arms, which
-never check the `is_definition` flag their spans already carry, and the
-third lands in the `Variable` arm, where `VarDefKind::Parameter` is on
-the allow-list and no caller distinguishes a promoted parameter
-(`is_promoted_property_param` in `src/definition/resolve.rs` is the
-existing test for one).
-
-Decide the rule once and apply it to all three rather than case by
-case: a hover on a *declaration* is either useful everywhere or nowhere.
+No outstanding items.

src/hover/mod.rs

@@ -147,7 +147,7 @@ impl Backend {
                 // bindings, hover is useful to show the resolved type and
                 // any docblock descriptions.
                 if let Some(def_kind) = self.lookup_var_def_kind_at(uri, name, cursor_offset)
-                    && !matches!(
+                    && (!matches!(
                         def_kind,
                         VarDefKind::Assignment
                             | VarDefKind::CompoundAssignment
@@ -156,8 +156,13 @@ impl Backend {
                             | VarDefKind::Catch
                             | VarDefKind::ArrayDestructuring
                             | VarDefKind::ListDestructuring
-                    )
+                    ) || (def_kind == VarDefKind::Parameter
+                        && self.is_promoted_property_param(uri, symbol.start)))
                 {
+                    // A constructor-promoted parameter declares a property,
+                    // not a local variable, so it follows the same
+                    // "already visible on screen" rule as any other
+                    // property declaration.
                     return None;
                 }
                 self.hover_variable(name, uri, content, cursor_offset, current_class, &ctx)
@@ -403,9 +408,17 @@ impl Backend {
 
             SymbolKind::FunctionCall {
                 name,
+                is_definition,
                 is_docblock_reference,
-                ..
             } => {
+                // The user is already at the function's own declaration —
+                // showing hover here would just repeat the signature and
+                // docblock that are already on screen, same as a class or
+                // member declaration.
+                if *is_definition {
+                    return None;
+                }
+
                 // An unqualified `@see name()` describes the documented
                 // class's own member first, as phpDocumentor reads it.
                 if *is_docblock_reference
@@ -483,7 +496,18 @@ impl Backend {
                 }
             }
 
-            SymbolKind::ConstantReference { name, .. } => {
+            SymbolKind::ConstantReference {
+                name,
+                is_definition,
+            } => {
+                // The user is already at the `const NAME = ...;` declaration
+                // — the value is on the same line, so showing hover here
+                // would just repeat it, same as a class or member
+                // declaration.
+                if *is_definition {
+                    return None;
+                }
+
                 // The name is resolved against the file the same way the
                 // type engine resolves it, so a namespaced constant is
                 // found through whichever spelling the reference used.

tests/integration/hover.rs

@@ -1542,6 +1542,41 @@ enum Status: string {
     }
 }
 
+/// A global function's own name, a top-level constant's own name, and a
+/// constructor-promoted parameter's own name all declare something whose
+/// full signature/value is already on screen — same rule as
+/// [`hover_member_declaration_sites_return_none`], applied to the three
+/// declaration sites that used to slip through it.
+#[test]
+fn hover_global_declaration_sites_return_none() {
+    let backend = create_test_backend();
+    let uri = "file:///test.php";
+    let content = r#"<?php
+/** Adds a shipping surcharge. */
+function helper(): int {
+    return 1;
+}
+
+const LIMIT = 5;
+
+class Product {
+    public function __construct(public string $sku) {}
+}
+"#;
+
+    for (what, line, character) in [
+        ("function", 2u32, 10u32),
+        ("top-level const", 6, 7),
+        ("promoted property parameter", 9, 48),
+    ] {
+        let hover = hover_at(&backend, uri, content, line, character);
+        assert!(
+            hover.is_none(),
+            "should not show hover on the {what} declaration site, got: {hover:?}"
+        );
+    }
+}
+
 /// A request can arrive before the file has ever been parsed: an editor
 /// fires hover the instant it opens a file, and a raw LSP client can ask
 /// about a file it never opened at all, ahead of background indexing.