Show hover at variable assignment and fix incorrect name space in hover

Author: AJenbo

src/completion/variable/raw_type_inference.rs

@@ -170,7 +170,7 @@ impl AssignmentAccumulator {
 /// candidate source for
 /// [`try_chained_array_access_with_candidates`](crate::completion::source::helpers)
 /// when resolving array access chains.
-pub(in crate::completion) fn resolve_variable_assignment_raw_type(
+pub(crate) fn resolve_variable_assignment_raw_type(
     var_name: &str,
     content: &str,
     cursor_offset: u32,

src/completion/variable/resolution.rs

@@ -784,17 +784,20 @@ fn walk_if_statement<'b>(
 
 /// Handle `foreach` statements during variable assignment walking.
 ///
-/// Only resolves the foreach value/key variable and recurses into the
-/// body when the cursor is actually inside the loop body (iteration
-/// variables are out of scope outside the loop).
+/// Resolves the foreach value/key variable and recurses into the body
+/// when the cursor is inside the loop body **or** on the foreach header
+/// (the `foreach ($expr as $val)` part).  The header check is needed so
+/// that hover on the binding variable at its definition site can resolve
+/// the iteration type.
 fn walk_foreach_statement<'b>(
     foreach: &'b Foreach<'b>,
     ctx: &VarResolutionCtx<'_>,
     results: &mut Vec<ClassInfo>,
     conditional: bool,
 ) {
     let body_span = foreach.body.span();
-    if ctx.cursor_offset >= body_span.start.offset && ctx.cursor_offset <= body_span.end.offset {
+    let header_start = foreach.foreach.span().start.offset;
+    if ctx.cursor_offset >= header_start && ctx.cursor_offset <= body_span.end.offset {
         // ── Foreach value/key type from generic iterables ──
         // When the variable we're resolving is the foreach
         // *value* variable, try to infer its type from the

src/definition/resolve.rs

@@ -165,16 +165,18 @@ impl Backend {
                 // for assignments the definition's `effective_from` is past
                 // the LHS token — the lookup would skip the definition and
                 // find an earlier one instead of recognising "at definition".
-                if let Some(def_kind) = self.lookup_var_def_kind_at(uri, name, cursor_offset) {
-                    // The cursor is on a variable at its definition site.
-                    // Try to resolve the type hint so the user can jump
-                    // from `HtmlString|string $content` to `HtmlString`.
-                    // For all definition kinds (parameter, catch, foreach,
-                    // property, assignment, etc.) the behaviour is the
-                    // same: attempt type-hint resolution and return None
-                    // when no navigable type hint exists.
-                    let _ = def_kind; // all kinds use the same path
-                    return self.resolve_type_hint_at_variable(uri, content, position, &var_name);
+                if self
+                    .lookup_var_def_kind_at(uri, name, cursor_offset)
+                    .is_some()
+                {
+                    // The cursor is on a variable at its definition site
+                    // (assignment LHS, parameter, foreach binding, catch
+                    // binding, etc.).  GTD should not trigger here — the
+                    // user is already at the definition.  Type hints next
+                    // to the variable (e.g. `Throwable` in `catch
+                    // (Throwable $it)`) are separate symbol spans that
+                    // the user can click directly.
+                    return None;
                 }
 
                 if let Some(var_def) = self.lookup_var_definition(uri, name, cursor_offset) {
@@ -199,8 +201,7 @@ impl Backend {
                 {
                     return Some(location);
                 }
-                // Already at definition — try type-hint resolution.
-                self.resolve_type_hint_at_variable(uri, content, position, &var_name)
+                None
             }
 
             SymbolKind::MemberAccess {

src/definition/variable/mod.rs

@@ -21,9 +21,10 @@
 ///   - **Array destructuring**: `[$a, $b] = …` / `list($a, $b) = …`
 ///
 /// When the cursor is already at the definition site (e.g. on a
-/// parameter), the module falls through to type-hint resolution:
-/// it extracts the type hint and jumps to the first class-like type
-/// in it (e.g. `HtmlString` in `HtmlString|string $content`).
+/// parameter or assignment LHS), GTD returns `None` — the user is
+/// already at the definition.  Type hints next to the variable
+/// (e.g. `Throwable` in `catch (Throwable $it)`) are separate
+/// symbol spans that the user can click directly.
 ///
 /// When the AST parse fails (malformed PHP, parser panic), the function
 /// returns `None` rather than falling back to text heuristics.
@@ -33,20 +34,14 @@
 /// - [`var_definition`]: AST walk that finds variable definition sites
 ///   (assignments, parameters, foreach, catch, static/global,
 ///   destructuring).
-/// - [`type_hint`]: AST walk that extracts the type hint string at a
-///   variable's definition site (parameter, property, closure/arrow
-///   function parameter).
 use tower_lsp::lsp_types::*;
 
 use crate::Backend;
-use crate::composer;
 use crate::parser::with_parsed_program;
 use crate::util::{offset_to_position, position_to_offset};
 
-mod type_hint;
 mod var_definition;
 
-use type_hint::find_type_hint_at_definition;
 use var_definition::find_variable_definition_in_program;
 
 // ═══════════════════════════════════════════════════════════════════════
@@ -60,8 +55,7 @@ pub(super) enum VarDefSearchResult {
     #[default]
     NotFound,
     /// The cursor is already sitting on the definition site (e.g. on a
-    /// parameter declaration).  The caller should fall through to
-    /// type-hint resolution.
+    /// parameter declaration).  The caller should return `None`.
     AtDefinition,
     /// Found a prior definition at the given byte offset.
     /// `offset` is the start of the `$var` token, `end_offset` is the end.
@@ -92,8 +86,8 @@ impl Backend {
     ///
     /// Returns:
     /// - `Some(Some(location))` — found a prior definition, jump there
-    /// - `Some(None)` — cursor is at a definition site (fall through to type-hint)
-    ///   OR no definition found in the AST
+    /// - `Some(None)` — cursor is at a definition site or no definition
+    ///   found in the AST
     /// - `None` — AST parse failed
     fn resolve_variable_definition_ast(
         content: &str,
@@ -118,8 +112,7 @@ impl Backend {
                 Some(None)
             }
             VarDefSearchResult::AtDefinition => {
-                // Cursor is at the definition — return Some(None) so the
-                // caller falls through to type-hint resolution.
+                // Cursor is at the definition — return Some(None).
                 Some(None)
             }
             VarDefSearchResult::FoundAt { offset, end_offset } => {
@@ -136,118 +129,6 @@ impl Backend {
             }
         }
     }
-
-    // ─── Type-Hint Resolution at Variable Definition ────────────────────
-
-    /// When the cursor is on a variable that is already at its definition
-    /// site (parameter, property, promoted property, catch variable),
-    /// extract the type hint and jump to the first class-like type in it.
-    ///
-    /// For example, given `public readonly HtmlString|string $content,`
-    /// this returns the location of the `HtmlString` class definition.
-    pub(super) fn resolve_type_hint_at_variable(
-        &self,
-        uri: &str,
-        content: &str,
-        position: Position,
-        var_name: &str,
-    ) -> Option<Location> {
-        self.resolve_type_hint_at_variable_ast(uri, content, position, var_name)
-    }
-
-    /// AST-based type-hint resolution: extract the type hint from the AST
-    /// node where the variable is defined (parameter, catch, property).
-    fn resolve_type_hint_at_variable_ast(
-        &self,
-        uri: &str,
-        content: &str,
-        position: Position,
-        var_name: &str,
-    ) -> Option<Location> {
-        let cursor_offset = position_to_offset(content, position);
-
-        let type_hint_str: Option<String> = with_parsed_program(
-            content,
-            "resolve_type_hint_at_variable_ast",
-            |program, _| find_type_hint_at_definition(program, var_name, cursor_offset),
-        );
-
-        let type_hint = type_hint_str?;
-        self.resolve_type_hint_string_to_location(uri, content, &type_hint)
-    }
-
-    /// Given a type-hint string (e.g. `HtmlString|string`, `?Foo`),
-    /// resolve it to the definition location of the first class-like type.
-    fn resolve_type_hint_string_to_location(
-        &self,
-        uri: &str,
-        content: &str,
-        type_hint: &str,
-    ) -> Option<Location> {
-        let scalars = [
-            "string", "int", "float", "bool", "array", "callable", "iterable", "object", "mixed",
-            "void", "never", "null", "false", "true", "self", "static", "parent",
-        ];
-
-        let class_name = type_hint
-            .split(['|', '&'])
-            .map(|t| t.trim_start_matches('?'))
-            .find(|t| !t.is_empty() && !scalars.contains(&t.to_lowercase().as_str()))?;
-
-        let ctx = self.file_context(uri);
-
-        let fqn = Self::resolve_to_fqn(class_name, &ctx.use_map, &ctx.namespace);
-
-        let mut candidates = vec![fqn];
-        if class_name.contains('\\') && !candidates.contains(&class_name.to_string()) {
-            candidates.push(class_name.to_string());
-        }
-
-        // Try same-file first.
-        for fqn in &candidates {
-            if let Some(location) = self.find_definition_in_ast_map(fqn, content, uri) {
-                return Some(location);
-            }
-        }
-
-        // Try Composer classmap: direct FQN → file path lookup.
-        // This covers vendor classes that haven't been loaded into ast_map
-        // yet (cold Ctrl+Click on a type hint never used in completion).
-        for fqn in &candidates {
-            if let Ok(cmap) = self.classmap.lock()
-                && let Some(file_path) = cmap.get(fqn.as_str()).cloned()
-            {
-                drop(cmap);
-                if let Some(location) = self.resolve_class_in_file(&file_path, fqn) {
-                    return Some(location);
-                }
-            }
-        }
-
-        // Try PSR-4 resolution as a last resort.
-        // PSR-4 mappings only cover user code (from composer.json).
-        // Vendor classes are resolved by the classmap above.
-        let workspace_root = self
-            .workspace_root
-            .lock()
-            .ok()
-            .and_then(|guard| guard.clone());
-
-        if let Some(workspace_root) = workspace_root
-            && let Ok(mappings) = self.psr4_mappings.lock()
-        {
-            for fqn in &candidates {
-                if let Some(file_path) =
-                    composer::resolve_class_path(&mappings, &workspace_root, fqn)
-                    && let Some(location) = self.resolve_class_in_file(&file_path, fqn)
-                {
-                    return Some(location);
-                }
-            }
-        }
-
-        None
-    }
 }
 
 // ═══════════════════════════════════════════════════════════════════════