Skip to content

Commit 7502692

Browse files
committed
Add more todo
1 parent 2b926d2 commit 7502692

1 file changed

Lines changed: 118 additions & 0 deletions

File tree

docs/todo.md

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,124 @@ target.
199199

200200
---
201201

202+
#### 41. `find_class_file_content` short-name collision breaks go-to-definition for inherited members
203+
204+
When a child class and its parent share the same short name (e.g.
205+
`App\Console\Kernel extends Illuminate\Foundation\Console\Kernel`),
206+
go-to-definition on an inherited member like `$this->load()` fails.
207+
Completion works correctly because the resolution pipeline uses
208+
`find_or_load_class`, which is namespace-aware. But the definition
209+
handler calls `find_class_file_content(declaring_class.name, ...)` with
210+
just the short name `"Kernel"`. That function checks the current file
211+
first, finds the child's `Kernel` (same short name), and returns the
212+
wrong file. Then `find_member_position` cannot find the inherited method
213+
in the child file and returns `None`.
214+
215+
This commonly surfaces with aliased imports that exist precisely because
216+
the names collide:
217+
218+
```php
219+
namespace App\Console;
220+
use Illuminate\Foundation\Console\Kernel as ConsoleKernel;
221+
222+
class Kernel extends ConsoleKernel {
223+
protected function commands(): void {
224+
$this->load(); // go-to-definition fails, completion works
225+
}
226+
}
227+
```
228+
229+
The alias itself is resolved correctly (the use-map stores
230+
`ConsoleKernel` to the FQN, and `resolve_parent_class_names` applies
231+
it). The problem is purely in `find_class_file_content` in
232+
`definition/member.rs`, which matches by short name without considering
233+
the file's namespace.
234+
235+
**Fix:** make `find_class_file_content` namespace-aware, mirroring the
236+
logic in `find_class_in_ast_map` (which already cross-checks
237+
`namespace_map`). The function could accept the FQN instead of the short
238+
name, or it could take an optional expected namespace and skip files
239+
whose namespace does not match.
240+
241+
---
242+
243+
#### 43. Go-to-definition on foreach variable jumps to previous loop instead of recognising current definition site
244+
245+
When the same variable name is used in consecutive `foreach` loops,
246+
go-to-definition on the `as $var` in the second loop incorrectly jumps
247+
to the first loop:
248+
249+
```php
250+
foreach ($a as $b) {
251+
}
252+
foreach ($c as $b) { // go-to-definition on $b jumps to previous loop
253+
}
254+
```
255+
256+
The cursor on `$b` in the second `foreach` is already at a definition
257+
site (the `as` clause assigns `$b`), so there should be no jump at all.
258+
259+
The bug is in `resolve_variable_definition` in `definition/variable.rs`.
260+
The function skips the cursor line and scans backwards, assuming that if
261+
the cursor is at a definition site there will be nothing earlier to find.
262+
That assumption breaks when the same variable is redefined in a later
263+
scope. The backwards scan finds the previous `as $b` and returns it.
264+
265+
**Fix:** before scanning backwards, check whether the cursor line itself
266+
defines the variable (using `line_defines_variable`). If it does, return
267+
`None` immediately so the caller falls through to type-hint resolution
268+
(or returns nothing for constructs like `foreach` that have no navigable
269+
type hint).
270+
271+
---
272+
273+
#### 42. Array element access (`$var[0]->`) fails when variable type comes from an assignment
274+
275+
When a variable holds an array returned from a method call, `$var[0]->`
276+
offers no completion and go-to-definition on the subsequent method call
277+
fails. For example:
278+
279+
```php
280+
$attributes = (new \ReflectionClass(static::class))
281+
->getAttributes(UseFactory::class);
282+
283+
$attributes[0]->newInstance(); // no completion, no definition
284+
```
285+
286+
`ReflectionClass::getAttributes()` returns `ReflectionAttribute<T>[]`,
287+
so `$attributes[0]` should resolve to `ReflectionAttribute`. Two
288+
independent gaps prevent this:
289+
290+
1. **AST path (`resolve_rhs_array_access` in `variable_resolution.rs`)**
291+
only checks docblock annotations (`@var` / `@param`) for the base
292+
variable's iterable type. It does not fall back to assignment-based
293+
type inference, so `$attributes[0]` gets nothing when there is no
294+
explicit `@var` annotation.
295+
296+
2. **Text path (`resolve_chained_array_access` via
297+
`extract_raw_type_from_assignment_text`)** delegates to
298+
`resolve_raw_type_from_call_chain`, which splits the callee at
299+
`rfind("->")`. For multi-line chains this leaves trailing
300+
whitespace/newlines in the LHS (e.g.
301+
`"(new \\ReflectionClass(...))\n "`), causing
302+
`extract_new_expression_class` to fail its `ends_with(')')` check.
303+
Even in the single-line case the text path works only when every
304+
intermediate step resolves correctly; any gap in the chain aborts.
305+
306+
**Fix (AST path):** after the docblock lookup returns nothing, fall back
307+
to resolving the base variable via the normal assignment scanner
308+
(`resolve_variable_types` or an equivalent), extract the resulting
309+
`ClassInfo`'s iterable element type (e.g. from its `@return` annotation
310+
or generic parameter), and use that as the element type.
311+
312+
**Fix (text path):** trim whitespace from the LHS in
313+
`resolve_raw_type_from_call_chain` before passing it to
314+
`resolve_lhs_to_class`, and similarly trim in `resolve_lhs_to_class`
315+
before the `starts_with('(') && ends_with(')')` check in
316+
`extract_new_expression_class`.
317+
318+
---
319+
202320
### Remaining by user need
203321

204322
#### 27. No completion inside string interpolation

0 commit comments

Comments
 (0)