Refactor signature help to use AST for callable variable extraction

Author: AJenbo

docs/todo/bugs.md

@@ -22,3 +22,20 @@ handle this automatically.
 `tests/psalm_assertions/method_call.php` (out of scope until
 upstream stubs land).
 
+
+## B17. Double-`$` prefix in assignment dependency tracking
+
+`src/completion/variable/forward_walk.rs` L5972-5977
+(`collect_expr_assignment_deps`) and L5985-5987
+(`collect_rhs_variables`) use `format!("${}", dv.name)` to
+construct variable names. Since `dv.name` already includes the
+`$` prefix (e.g. `"$fn"`), this produces `"$$fn"`. The bug is
+latent because both the LHS key and the RHS set values are
+consistently double-prefixed, so dependency tracking still
+works. However, any code that compares these names against
+normally-prefixed variable names (single `$`) would fail to
+match.
+
+**Fix:** Remove the `format!("${}", ...)` wrapper and use
+`dv.name.to_string()` directly in both functions.
+

docs/todo/refactor.md

@@ -214,19 +214,6 @@ Each item must include:
 
 ## Outstanding items
 
-### R2. Mini expression resolver in `call_resolution.rs` 🔴
-
-`src/completion/call_resolution.rs` L2528 has `resolve_expression_to_type`, a
-separate expression-to-`PhpType` resolver for conditional return type argument
-matching. Handles `SubjectExpr::PropertyChain` and `SubjectExpr::Variable`
-through different code paths than the forward walker's `resolve_rhs_expression`.
-Fewer cases, different path, will diverge as features are added.
-
-**Action:** Route through the shared `resolve_rhs_expression` /
-`resolve_expression_type` pipeline.
-
-**Files:** `src/completion/call_resolution.rs`
-
 ### R3. Backward `@var` scanner in `docblock/tags.rs` 🟠
 
 `src/docblock/tags.rs` L520–620 (`find_var_raw_type_in_source`) and L1000–1100
@@ -243,17 +230,6 @@ unnecessary.
 **Files:** `src/docblock/tags.rs`, `src/completion/call_resolution.rs`,
 `src/completion/array_shape.rs`
 
-### R4. Backward callable scan in `signature_help.rs` 🟠
-
-`src/signature_help.rs` L438–470 (`extract_callable_target_from_variable`) uses
-raw `rfind` backward text scan to find `$fn = someTarget(...)` assignments,
-bypassing the forward walker which already tracks variable assignments.
-
-**Action:** Use the forward walker's assignment tracking instead of raw text
-scanning.
-
-**Files:** `src/signature_help.rs`
-
 ---
 
 ## Intentional overlap (reference, not actionable)

src/completion/call_resolution.rs

@@ -2512,110 +2512,17 @@ impl Backend {
 
 /// Resolve an arbitrary expression to a [`PhpType`].
 ///
-/// Unlike [`super::resolver::resolve_target_classes`] (which returns
-/// class-bearing `ResolvedType` values and drops scalars), this function
-/// returns the *type* of the expression — including scalars like `string`,
-/// `int`, `bool`, etc.
-///
-/// Strategy:
-/// 1. Parse the expression with [`SubjectExpr::parse`].
-/// 2. For property chains (`$var->prop`), resolve the base to classes
-///    and look up the property's type hint directly.
-/// 3. For method calls (`$var->method()`), resolve the base and look
-///    up the method's return type.
-/// 4. For everything else, fall back to [`super::resolver::resolve_target_classes`]
-///    which handles variables, `$this`, `new …`, etc.
+/// Delegates to [`super::resolver::resolve_target_classes`] which
+/// handles all expression patterns (variables, property chains,
+/// method calls, static accesses, etc.) and preserves scalar types
+/// through the `type_string` field of [`ResolvedType`].
 fn resolve_expression_to_type(text: &str, ctx: &ResolutionCtx<'_>) -> Option<PhpType> {
-    let expr = SubjectExpr::parse(text);
-
-    match &expr {
-        // $var->prop or $this->prop — resolve base to classes, then
-        // look up the property's type hint (which preserves scalars).
-        SubjectExpr::PropertyChain { base, property } => {
-            let base_text = base.to_subject_text();
-            let base_classes = ResolvedType::into_arced_classes(
-                super::resolver::resolve_target_classes(&base_text, crate::AccessKind::Arrow, ctx),
-            );
-            for cls in &base_classes {
-                if let Some(hint) =
-                    crate::inheritance::resolve_property_type_hint(cls, property, ctx.class_loader)
-                {
-                    return Some(hint);
-                }
-            }
-            None
-        }
-
-        // $var->method() or $this->helper->getText() — resolve the
-        // full call expression to return-type classes first; if that
-        // yields nothing class-like, resolve the raw return type
-        // (which preserves scalars).
-        SubjectExpr::CallExpr { callee, args_text } => {
-            // Try class-bearing resolution first (handles non-scalar returns).
-            // Use the _with_hint variant to capture the raw return type
-            // (which preserves generic parameters like `ASTArtifactList<ASTClass>`).
-            // Without this, generic args are lost and downstream template
-            // substitution resolves to unsubstituted template params.
-            let mut return_hint: Option<PhpType> = None;
-            let classes = Backend::resolve_call_return_types_expr_with_hint(
-                callee,
-                args_text,
-                ctx,
-                Some(&mut return_hint),
-            );
-            if let Some(first) = classes.first() {
-                // Prefer the return type hint (preserves generics) over
-                // the bare class FQN.  Fall back to FQN when no hint
-                // was captured or when the hint is a raw template name.
-                if let Some(ref hint) = return_hint
-                    && !hint.is_untyped()
-                    && !hint.is_mixed()
-                {
-                    return Some(hint.clone());
-                }
-                return Some(PhpType::Named(first.fqn().to_string()));
-            }
-
-            // Fall back to raw return type for scalar returns.
-            if let SubjectExpr::MethodCall { base, method } = callee.as_ref() {
-                let base_text = base.to_subject_text();
-                let base_classes =
-                    ResolvedType::into_arced_classes(super::resolver::resolve_target_classes(
-                        &base_text,
-                        crate::AccessKind::Arrow,
-                        ctx,
-                    ));
-                for cls in &base_classes {
-                    if let Some(rt) = crate::inheritance::resolve_method_return_type(
-                        cls,
-                        method,
-                        ctx.class_loader,
-                    ) {
-                        return Some(rt);
-                    }
-                }
-            }
-            if let SubjectExpr::StaticMethodCall { class, method } = callee.as_ref()
-                && let Some(owner) = super::resolver::resolve_static_owner_class(class, ctx)
-                && let Some(rt) =
-                    crate::inheritance::resolve_method_return_type(&owner, method, ctx.class_loader)
-            {
-                return Some(rt);
-            }
-            None
-        }
-
-        // Everything else ($var, $this, ClassName, new X, etc.) —
-        // use the standard resolver which returns class-bearing types.
-        _ => {
-            let resolved =
-                super::resolver::resolve_target_classes(text, crate::types::AccessKind::Arrow, ctx);
-            if let Some(first) = resolved.first() {
-                return Some(first.type_string.clone());
-            }
-            None
-        }
+    let results =
+        super::resolver::resolve_target_classes(text, crate::types::AccessKind::Arrow, ctx);
+    if let Some(first) = results.first() {
+        return Some(first.type_string.clone());
     }
+    None
 }
 
 /// Resolve a `ClassName::Member` expression to a type.