Fix regressions

Author: AJenbo

docs/todo.md

@@ -653,66 +653,13 @@ These functions have return type semantics that don't fit into either
 
 ---
 
-### 19. Array element access after method call on variable produces no completion
-**Impact: Medium · Effort: Low-Medium**
-
-When a method returning `Pen[]` is called on a variable (not `$this`),
-array element access on the result produces no completions. In the
-`ArrayAccessDemo`:
-
-```php
-$src = new ScaffoldingArrayAccess();
-$pens = $src->fetchAll();         // Pen[] from method return
-$pens[0]->write();                // no completions (should resolve to Pen)
-
-$first = $pens[0];
-$first->color();                  // no completions (should resolve to Pen)
-```
-
-The same pattern works when the method is on `$this` (the demo worked
-before refactoring to `$src = new ...`). The variable resolution
-pipeline likely doesn't follow the chain `$src` → `ScaffoldingArrayAccess`
-→ `fetchAll()` → `Pen[]` → element type when `$src` is a local variable
-rather than `$this`.
-
----
-
-### 20. Closure and arrow-function member completion fails in namespace context
-**Impact: Medium · Effort: Low-Medium**
-
-None of the examples in `ClosureMembersDemo` produce completions.
-Closure literals (`function() { ... }`), arrow functions (`fn() => ...`),
-and chained `bindTo()` results should all resolve to the `Closure` class
-and offer members like `bindTo()` and `call()`.
-
-The feature has passing unit tests (which define a minimal inline
-`Closure` class), so the failure is specific to the `example.php`
-context where `Closure` comes from stubs and the code is inside a
-`namespace Demo { }` block. The stub resolution or namespace-qualified
-lookup for the built-in `Closure` class may not be working in this
-context.
-
-```php
-$typedClosure = function(Pen $p): string { return $p->write(); };
-$typedClosure->bindTo($this);     // no completions (should offer Closure members)
-
-$typedArrow = fn(int $x): float => $x * 1.5;
-$typedArrow->bindTo($this);       // no completions
-
-$fn = function(): void {};
-$bound = $fn->bindTo($this);
-$bound->call($this);              // no completions
-```
-
----
-
 <!-- ============================================================ -->
 <!--  TIER 4 — LOW-MEDIUM IMPACT                                  -->
 <!-- ============================================================ -->
 
 ## Low-Medium Impact
 
-### 21. Asymmetric visibility (PHP 8.4)
+### 19. Asymmetric visibility (PHP 8.4)
 **Impact: Low-Medium · Effort: Low**
 
 Separate from property hooks, PHP 8.4 allows asymmetric visibility on
@@ -742,7 +689,7 @@ is just to store the value; context-aware filtering can follow later.
 
 ---
 
-### 22. `str_contains` / `str_starts_with` / `str_ends_with` → non-empty-string narrowing
+### 20. `str_contains` / `str_starts_with` / `str_ends_with` → non-empty-string narrowing
 **Impact: Low-Medium · Effort: Low**
 
 When `str_contains($haystack, $needle)` appears in a condition and
@@ -759,7 +706,7 @@ See `StrContainingTypeSpecifyingExtension` in PHPStan.
 
 ---
 
-### 23. `count` / `sizeof` comparison → non-empty-array narrowing
+### 21. `count` / `sizeof` comparison → non-empty-array narrowing
 **Impact: Low-Medium · Effort: Low**
 
 `if (count($arr) > 0)` or `if (count($arr) >= 1)` narrows `$arr` to
@@ -777,7 +724,7 @@ branches in `TypeSpecifier::specifyTypesInCondition`.
 
 ## Low Impact
 
-### 24. Short-name collisions in `find_implementors`
+### 22. Short-name collisions in `find_implementors`
 **Impact: Low · Effort: Low**
 
 `class_implements_or_extends` matches interfaces by both short name and
@@ -793,7 +740,7 @@ before comparison.
 
 ---
 
-### 25. Fiber type resolution
+### 23. Fiber type resolution
 **Impact: Low · Effort: Low**
 
 `Generator<TKey, TValue, TSend, TReturn>` has dedicated support for
@@ -808,7 +755,7 @@ Generator extraction in `docblock/types.rs`.
 
 ---
 
-### 26. Non-empty-string propagation through string functions
+### 24. Non-empty-string propagation through string functions
 **Impact: Low · Effort: Low**
 
 PHPStan tracks `non-empty-string` through string-manipulating
@@ -826,7 +773,7 @@ See `NonEmptyStringFunctionsReturnTypeExtension` in PHPStan.
 
 ---
 
-### 27. `Closure::bind()` / `Closure::fromCallable()` return type preservation
+### 25. `Closure::bind()` / `Closure::fromCallable()` return type preservation
 **Impact: Low · Effort: Low-Medium**
 
 Variables holding closure literals, arrow functions, and first-class
@@ -842,7 +789,7 @@ See `ClosureBindDynamicReturnTypeExtension` and
 
 ---
 
-### 28. Remove deprecated text-search fallbacks
+### 26. Remove deprecated text-search fallbacks
 **Impact: Low · Effort: Medium**
 
 The go-to-definition subsystem now uses the precomputed `SymbolMap` as
@@ -874,58 +821,7 @@ would let that deprecated function be removed entirely.
 
 ---
 
-### 29. Go-to-definition jumps to itself at definition sites
-**Impact: Low · Effort: Low**
-
-Ctrl+Click on a symbol at its own definition site offers GTD and jumps
-to itself instead of suppressing the action. Known cases:
-
-1. **`define()` constant name.** `define('APP_VERSION', '1.0.0')` is
-   the definition site for `APP_VERSION`. Ctrl+Click on the constant
-   name inside the `define()` call jumps to itself.
-
-2. **Method declarations.** `public function paramTypes(GtdAlpha $item): void {}`
-   is the definition site for `paramTypes`. Ctrl+Click on the method
-   name in the declaration jumps to itself.
-
-The fix should detect that the cursor position matches (or is within)
-the stored definition offset and return `None` instead of a
-self-referential `Location`. This check could be a single guard at the
-top of the GTD handler: if the resolved `Location` points to the same
-URI and range that the cursor is already in, return `None`.
-
----
-
-### 30. Return type hint completion missing leading space before type
-**Impact: Low · Effort: Low**
-
-When completing a return type hint on a function that has no return type
-yet, the inserted text omits the space before the type name. For example,
-accepting `string` on:
-
-```php
-function typeHintDemo(User $user, string $name) { return $user; }
-```
-
-produces `:string` jammed against the closing parenthesis:
-
-```php
-function typeHintDemo(User $user, string $name):string { return $user; }
-```
-
-instead of the correct:
-
-```php
-function typeHintDemo(User $user, string $name): string { return $user; }
-```
-
-The fix should ensure the completion item's `textEdit` (or
-`additionalTextEdits`) includes the `: ` prefix with a space when
-inserting a return type after the parameter list.
-
----
-
-### 31. Non-array functions with dynamic return types
+### 27. Non-array functions with dynamic return types
 **Impact: Low · Effort: High**
 
 PHPStan also provides dynamic return type extensions for many non-array
@@ -956,22 +852,22 @@ return types (less impactful for class-based completion).
 
 ---
 
-### 32. Diagnostics
+### 28. Diagnostics
 **Impact: Low (large scope) · Effort: Very High**
 
 No error reporting (undefined methods, type mismatches, etc.).
 
 ---
 
-### 33. Code Actions
+### 29. Code Actions
 **Impact: Low · Effort: Very High**
 
 No quick fixes or refactoring suggestions. No `codeActionProvider` in
 `ServerCapabilities`, no `textDocument/codeAction` handler, and no
 `WorkspaceEdit` generation infrastructure beyond trivial `TextEdit`s for
 use-statement insertion.
 
-#### 33a. Extract Function refactoring
+#### 29a. Extract Function refactoring
 
 Select a range of statements inside a method/function and extract them into a
 new function. The LSP would need to:
@@ -1023,18 +919,14 @@ new function. The LSP would need to:
 | 16 | Partial result streaming | Medium | Medium-High |
 | 17 | Rename | Medium | Medium-High |
 | 18 | Array functions (new code paths) | Medium | High |
-| 19 | Array element access on variable method chain | Medium | Low-Medium |
-| 20 | Closure member completion in namespace context | Medium | Low-Medium |
-| 21 | Asymmetric visibility (PHP 8.4) | Low-Medium | Low |
-| 22 | `str_contains` / `str_starts_with` narrowing | Low-Medium | Low |
-| 23 | `count` / `sizeof` → non-empty-array | Low-Medium | Low |
-| 24 | Short-name collisions | Low | Low |
-| 25 | Fiber type resolution | Low | Low |
-| 26 | Non-empty-string propagation | Low | Low |
-| 27 | `Closure::bind()` preservation | Low | Low-Medium |
-| 28 | Remove deprecated text-search fallbacks | Low | Medium |
-| 29 | GTD jumps to itself at definition sites | Low | Low |
-| 30 | Return type hint missing leading space | Low | Low |
-| 31 | Non-array dynamic return types | Low | High |
-| 32 | Diagnostics | Low | Very High |
-| 33 | Code Actions / Extract Function | Low | Very High |
+| 19 | Asymmetric visibility (PHP 8.4) | Low-Medium | Low |
+| 20 | `str_contains` / `str_starts_with` narrowing | Low-Medium | Low |
+| 21 | `count` / `sizeof` → non-empty-array | Low-Medium | Low |
+| 22 | Short-name collisions | Low | Low |
+| 23 | Fiber type resolution | Low | Low |
+| 24 | Non-empty-string propagation | Low | Low |
+| 25 | `Closure::bind()` preservation | Low | Low-Medium |
+| 26 | Remove deprecated text-search fallbacks | Low | Medium |
+| 27 | Non-array dynamic return types | Low | High |
+| 28 | Diagnostics | Low | Very High |
+| 29 | Code Actions / Extract Function | Low | Very High |

src/completion/handler.rs

@@ -499,6 +499,7 @@ impl Backend {
         position: Position,
     ) -> Option<CompletionResponse> {
         let partial_lower = th_ctx.partial.to_lowercase();
+        let space_prefix = if th_ctx.needs_space_prefix { " " } else { "" };
         let mut items: Vec<CompletionItem> =
             crate::completion::type_hint_completion::PHP_NATIVE_TYPES
                 .iter()
@@ -508,7 +509,7 @@ impl Backend {
                     label: t.to_string(),
                     kind: Some(CompletionItemKind::KEYWORD),
                     detail: Some("PHP built-in type".to_string()),
-                    insert_text: Some(t.to_string()),
+                    insert_text: Some(format!("{}{}", space_prefix, t)),
                     filter_text: Some(t.to_string()),
                     sort_text: Some(format!("0_{:03}", idx)),
                     ..CompletionItem::default()
@@ -523,7 +524,26 @@ impl Backend {
             ClassNameContext::Any,
             position,
         );
-        items.extend(class_items);
+
+        // When a leading space is needed (return type after `:` with no
+        // space), prefix the insert text of each class-name item so that
+        // the result is `: ClassName` rather than `:ClassName`.
+        if th_ctx.needs_space_prefix {
+            for mut item in class_items {
+                if let Some(ref txt) = item.insert_text {
+                    item.insert_text = Some(format!(" {}", txt));
+                }
+                if let Some(CompletionTextEdit::Edit(ref te)) = item.text_edit {
+                    item.text_edit = Some(CompletionTextEdit::Edit(TextEdit {
+                        range: te.range,
+                        new_text: format!(" {}", te.new_text),
+                    }));
+                }
+                items.push(item);
+            }
+        } else {
+            items.extend(class_items);
+        }
 
         if items.is_empty() {
             // Even when empty, the caller returns early so we don't fall

src/completion/text_resolution.rs

@@ -258,7 +258,7 @@ impl Backend {
         // Detect before the call expression branch because `fn(...)`
         // ends with `)` and would otherwise be resolved as a call.
         if rhs_text.ends_with("(...)") && rhs_text.len() > 4 {
-            return Some("Closure".to_string());
+            return Some("\\Closure".to_string());
         }
 
         // RHS is a call expression — extract the return type.
@@ -282,7 +282,18 @@ impl Backend {
                 {
                     rest.contains("->") || rest.contains("::")
                 } else {
-                    true
+                    // Single-level `$var->method` (bare variable followed
+                    // by exactly one `->`) should NOT be treated as a
+                    // chain — it needs variable resolution first, which
+                    // `resolve_raw_type_from_call_chain` cannot do (it
+                    // lacks `content`/`cursor_offset`).  All other
+                    // single-arrow patterns (e.g.
+                    // `(new Foo())->method`) are genuine chains.
+                    let lhs = callee.split("->").next().unwrap_or("");
+                    let lhs = lhs.trim();
+                    let is_bare_var = lhs.starts_with('$')
+                        && lhs[1..].chars().all(|c| c.is_alphanumeric() || c == '_');
+                    !is_bare_var
                 }
             };
             let is_static_chain = !callee.contains("->") && callee.contains("::") && {
@@ -311,6 +322,38 @@ impl Backend {
                 return Self::resolve_method_return_type(owner, method_name, class_loader);
             }
 
+            // Method call on a non-`$this` variable: `$var->methodName(…)`
+            // Resolve the variable's type via assignment scanning, then
+            // look up the method on the resulting class.
+            if let Some(arrow_pos) = callee.find("->") {
+                let var_part = callee[..arrow_pos].trim();
+                let method_name = callee[arrow_pos + 2..].trim();
+                if var_part.starts_with('$')
+                    && var_part != "$this"
+                    && !method_name.is_empty()
+                    && method_name.chars().all(|c| c.is_alphanumeric() || c == '_')
+                    && let Some(var_type) = Self::extract_raw_type_from_assignment_text(
+                        var_part,
+                        content,
+                        cursor_offset,
+                        current_class,
+                        all_classes,
+                        class_loader,
+                    )
+                {
+                    let clean = crate::docblock::types::clean_type(&var_type);
+                    let lookup = short_name(&clean);
+                    let owner_class = all_classes
+                        .iter()
+                        .find(|c| c.name == lookup)
+                        .cloned()
+                        .or_else(|| class_loader(&clean));
+                    if let Some(owner) = owner_class {
+                        return Self::resolve_method_return_type(&owner, method_name, class_loader);
+                    }
+                }
+            }
+
             // Static call: `ClassName::methodName(…)`
             if let Some((class_part, method_part)) = callee.rsplit_once("::") {
                 let resolved_class = if class_part == "self" || class_part == "static" {