Skip to content

Commit 40a02ca

Browse files
AJenboAJenbo
committed
Address GTD edge cases
1 parent 6bcf1ba commit 40a02ca

3 files changed

Lines changed: 213 additions & 70 deletions

File tree

‎docs/todo.md‎

Lines changed: 59 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,43 @@ target.
416416

417417
---
418418

419-
### 10. `BackedEnum::from()` / `::tryFrom()` return type refinement
419+
### 10. Suppress GTD on parameter variable names and class declaration names
420+
**Impact: Medium · Effort: Low**
421+
422+
Go-to-definition fires on parameter variable names (`$supplier`, `$country`)
423+
and promoted property variable names when they have a class type hint,
424+
navigating to the type hint's class. Only the type hint itself (e.g.
425+
`Supplier`, `Country`) should be clickable. The variable name should
426+
return no result because the type hint is already visible right next to it
427+
and is separately navigable.
428+
429+
Similarly, class declaration names (e.g. `ExcelImportProvider` in
430+
`final class ExcelImportProvider {}`) should not offer GTD since the user
431+
is already at the definition site. The symbol-map path already returns
432+
`None` for `ClassDeclaration`, but the text-based fallback may still
433+
resolve the name to itself in some edge cases.
434+
435+
**Root cause:** `extract_from_method` and `extract_from_function` in
436+
`symbol_map.rs` do not emit a `SymbolSpan` for parameter variables (only
437+
a `VarDefSite`). Without a span, the symbol-map lookup returns `None` and
438+
the request falls through to the text-based path, which calls
439+
`resolve_type_hint_at_variable` and navigates to the class. Closures and
440+
arrow functions already emit the span correctly.
441+
442+
**Fix:** Add `SymbolSpan { kind: SymbolKind::Variable }` for each
443+
parameter in `extract_from_method` and `extract_from_function` (matching
444+
what closures and arrow functions already do). The existing
445+
`resolve_from_symbol` logic for `VarDefKind::Parameter` already returns
446+
`None`, so no changes are needed in the resolution path. Update the two
447+
tests in `definition_variables.rs` that currently expect parameter-at-
448+
definition to navigate to the type hint class
449+
(`test_goto_definition_parameter_at_definition_jumps_to_type_hint` and
450+
`test_goto_definition_variable_at_definition_jumps_to_type_hint`) to
451+
expect `None` instead.
452+
453+
---
454+
455+
### 11. `BackedEnum::from()` / `::tryFrom()` return type refinement
420456
**Impact: Medium · Effort: Low**
421457

422458
When calling `MyEnum::from($value)` or `MyEnum::tryFrom($value)`,
@@ -433,14 +469,14 @@ See `BackedEnumFromMethodDynamicReturnTypeExtension` in PHPStan.
433469

434470
---
435471

436-
### 11. Document Symbols (`textDocument/documentSymbol`)
472+
### 12. Document Symbols (`textDocument/documentSymbol`)
437473
**Impact: Medium · Effort: Low**
438474

439475
No outline view. Editors can't show a file's class/method/property structure.
440476

441477
---
442478

443-
### 12. Workspace Symbols (`workspace/symbol`)
479+
### 13. Workspace Symbols (`workspace/symbol`)
444480
**Impact: Medium · Effort: Low-Medium**
445481

446482
Can't search for classes/functions across the project. The `ast_map`
@@ -452,7 +488,7 @@ LSP `Location`s.
452488

453489
---
454490

455-
### 13. No go-to-definition for built-in (stub) functions and constants
491+
### 14. No go-to-definition for built-in (stub) functions and constants
456492
**Impact: Medium · Effort: Medium**
457493

458494
Clicking on a built-in function name like `array_map`, `strlen`, or
@@ -474,7 +510,7 @@ limitation.
474510

475511
---
476512

477-
### 14. Property hooks (PHP 8.4)
513+
### 15. Property hooks (PHP 8.4)
478514
**Impact: Medium · Effort: Medium**
479515

480516
PHP 8.4 introduced property hooks (`get` / `set`):
@@ -509,7 +545,7 @@ scopes, and parse the set-visibility modifier into a new
509545

510546
---
511547

512-
### 15. Narrow types of `&$var` parameters after function calls
548+
### 16. Narrow types of `&$var` parameters after function calls
513549
**Impact: Medium · Effort: Medium**
514550

515551
When a function takes a parameter by reference, the variable's type
@@ -538,7 +574,7 @@ extension) or use a built-in map for known functions.
538574

539575
---
540576

541-
### 16. SPL iterator generic stubs
577+
### 17. SPL iterator generic stubs
542578
**Impact: Medium · Effort: Medium**
543579

544580
PHPStan's `iterable.stub` provides full `@template TKey` /
@@ -559,7 +595,7 @@ certainly missing these generic annotations. We should either:
559595

560596
---
561597

562-
### 17. Partial result streaming via `$/progress`
598+
### 18. Partial result streaming via `$/progress`
563599
**Impact: Medium · Effort: Medium-High**
564600

565601
The LSP spec (3.17) allows requests that return arrays — such as
@@ -639,7 +675,7 @@ developer arrive before vendor matches, even within a single phase.
639675

640676
---
641677

642-
### 18. Rename (`textDocument/rename`)
678+
### 19. Rename (`textDocument/rename`)
643679
**Impact: Medium · Effort: Medium-High**
644680

645681
No rename refactoring support. Rename builds on find-references (§8) —
@@ -654,7 +690,7 @@ position without text scanning.
654690

655691
---
656692

657-
### 19. Array functions needing new code paths
693+
### 20. Array functions needing new code paths
658694
**Impact: Medium · Effort: High**
659695

660696
These functions have return type semantics that don't fit into either
@@ -693,7 +729,7 @@ These functions have return type semantics that don't fit into either
693729

694730
## Low-Medium Impact
695731

696-
### 20. Asymmetric visibility (PHP 8.4)
732+
### 21. Asymmetric visibility (PHP 8.4)
697733
**Impact: Low-Medium · Effort: Low**
698734

699735
Separate from property hooks, PHP 8.4 allows asymmetric visibility on
@@ -723,7 +759,7 @@ is just to store the value; context-aware filtering can follow later.
723759

724760
---
725761

726-
### 21. `str_contains` / `str_starts_with` / `str_ends_with` → non-empty-string narrowing
762+
### 22. `str_contains` / `str_starts_with` / `str_ends_with` → non-empty-string narrowing
727763
**Impact: Low-Medium · Effort: Low**
728764

729765
When `str_contains($haystack, $needle)` appears in a condition and
@@ -740,7 +776,7 @@ See `StrContainingTypeSpecifyingExtension` in PHPStan.
740776

741777
---
742778

743-
### 22. `count` / `sizeof` comparison → non-empty-array narrowing
779+
### 23. `count` / `sizeof` comparison → non-empty-array narrowing
744780
**Impact: Low-Medium · Effort: Low**
745781

746782
`if (count($arr) > 0)` or `if (count($arr) >= 1)` narrows `$arr` to
@@ -758,7 +794,7 @@ branches in `TypeSpecifier::specifyTypesInCondition`.
758794

759795
## Low Impact
760796

761-
### 23. Short-name collisions in `find_implementors`
797+
### 24. Short-name collisions in `find_implementors`
762798
**Impact: Low · Effort: Low**
763799

764800
`class_implements_or_extends` matches interfaces by both short name and
@@ -774,7 +810,7 @@ before comparison.
774810

775811
---
776812

777-
### 24. Fiber type resolution
813+
### 25. Fiber type resolution
778814
**Impact: Low · Effort: Low**
779815

780816
`Generator<TKey, TValue, TSend, TReturn>` has dedicated support for
@@ -789,7 +825,7 @@ Generator extraction in `docblock/types.rs`.
789825

790826
---
791827

792-
### 25. Non-empty-string propagation through string functions
828+
### 26. Non-empty-string propagation through string functions
793829
**Impact: Low · Effort: Low**
794830

795831
PHPStan tracks `non-empty-string` through string-manipulating
@@ -807,7 +843,7 @@ See `NonEmptyStringFunctionsReturnTypeExtension` in PHPStan.
807843

808844
---
809845

810-
### 26. `Closure::bind()` / `Closure::fromCallable()` return type preservation
846+
### 27. `Closure::bind()` / `Closure::fromCallable()` return type preservation
811847
**Impact: Low · Effort: Low-Medium**
812848

813849
Variables holding closure literals, arrow functions, and first-class
@@ -823,7 +859,7 @@ See `ClosureBindDynamicReturnTypeExtension` and
823859

824860
---
825861

826-
### 27. Remove deprecated text-search fallbacks
862+
### 28. Remove deprecated text-search fallbacks
827863
**Impact: Low · Effort: Medium**
828864

829865
The go-to-definition subsystem now uses the precomputed `SymbolMap` as
@@ -855,7 +891,7 @@ would let that deprecated function be removed entirely.
855891

856892
---
857893

858-
### 28. Non-array functions with dynamic return types
894+
### 29. Non-array functions with dynamic return types
859895
**Impact: Low · Effort: High**
860896

861897
PHPStan also provides dynamic return type extensions for many non-array
@@ -886,7 +922,7 @@ return types (less impactful for class-based completion).
886922

887923
---
888924

889-
### 29. Language construct signature help and hover
925+
### 30. Language construct signature help and hover
890926
**Impact: Low · Effort: Low**
891927

892928
PHP language constructs that use parentheses (`unset()`, `isset()`, `empty()`,
@@ -903,22 +939,22 @@ need a similar hardcoded lookup.
903939

904940
---
905941

906-
### 30. Diagnostics
942+
### 31. Diagnostics
907943
**Impact: Low (large scope) · Effort: Very High**
908944

909945
No error reporting (undefined methods, type mismatches, etc.).
910946

911947
---
912948

913-
### 31. Code Actions
949+
### 32. Code Actions
914950
**Impact: Low · Effort: Very High**
915951

916952
No quick fixes or refactoring suggestions. No `codeActionProvider` in
917953
`ServerCapabilities`, no `textDocument/codeAction` handler, and no
918954
`WorkspaceEdit` generation infrastructure beyond trivial `TextEdit`s for
919955
use-statement insertion.
920956

921-
#### 30a. Extract Function refactoring
957+
#### 32a. Extract Function refactoring
922958

923959
Select a range of statements inside a method/function and extract them into a
924960
new function. The LSP would need to:
@@ -943,43 +979,3 @@ new function. The LSP would need to:
943979
| Simple code actions (add use stmt, implement interface) | Builds the code action + `WorkspaceEdit` plumbing |
944980

945981
---
946-
947-
<!-- ============================================================ -->
948-
<!-- SUMMARY TABLE -->
949-
<!-- ============================================================ -->
950-
951-
## Summary
952-
953-
| # | Item | Impact | Effort |
954-
|---|---|---|---|
955-
| 1 | Named-arg completion: symbol-map migration | Critical | Low |
956-
| 2 | Completion member access: symbol-map primary path | Critical | Medium |
957-
| 3 | Pipe operator (PHP 8.5) | High | Low |
958-
| 4 | Function-level `@template` generic resolution | High | Medium |
959-
| 5 | Conditional return type syntax | High | Medium |
960-
| 6 | Composer environment warnings | High | Medium |
961-
| 7 | Find References | High | Medium-High |
962-
| 8 | File system watching | Medium-High | Medium |
963-
| 9 | Reverse jump: impl → interface | Medium | Low |
964-
| 10 | `BackedEnum::from()` refinement | Medium | Low |
965-
| 11 | Document Symbols | Medium | Low |
966-
| 12 | Workspace Symbols | Medium | Low-Medium |
967-
| 13 | Built-in stub go-to-definition | Medium | Medium |
968-
| 14 | Property hooks (PHP 8.4) | Medium | Medium |
969-
| 15 | Parameter out types (by-reference) | Medium | Medium |
970-
| 16 | SPL iterator generic stubs | Medium | Medium |
971-
| 17 | Partial result streaming | Medium | Medium-High |
972-
| 18 | Rename | Medium | Medium-High |
973-
| 19 | Array functions (new code paths) | Medium | High |
974-
| 20 | Asymmetric visibility (PHP 8.4) | Low-Medium | Low |
975-
| 21 | `str_contains` / `str_starts_with` narrowing | Low-Medium | Low |
976-
| 22 | `count` / `sizeof` → non-empty-array | Low-Medium | Low |
977-
| 23 | Short-name collisions | Low | Low |
978-
| 24 | Fiber type resolution | Low | Low |
979-
| 25 | Non-empty-string propagation | Low | Low |
980-
| 26 | `Closure::bind()` preservation | Low | Low-Medium |
981-
| 27 | Remove deprecated text-search fallbacks | Low | Medium |
982-
| 28 | Non-array dynamic return types | Low | High |
983-
| 29 | Language construct signature help / hover | Low | Low |
984-
| 30 | Diagnostics | Low | Very High |
985-
| 31 | Code Actions / Extract Function | Low | Very High |

‎src/definition/member.rs‎

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -915,15 +915,49 @@ impl Backend {
915915
.iter()
916916
.find(|m| m.name == member_name && !m.is_static)?;
917917

918-
// Extract the model name from the scope method's return type.
918+
// Extract the model name from a Builder-typed return type.
919+
//
919920
// The return type is typically
920921
// `\Illuminate\Database\Eloquent\Builder<App\Models\User>`.
921-
let ret = scope_method.return_type.as_deref()?;
922-
let model_name = crate::docblock::types::parse_generic_args(ret)
923-
.1
924-
.into_iter()
925-
.next()
926-
.map(|s| s.strip_prefix('\\').unwrap_or(s).to_string())?;
922+
// We specifically look for return types whose base type is
923+
// the Eloquent Builder (with or without leading backslash)
924+
// and extract the first generic arg as the model name.
925+
let extract_model_from_builder_ret = |ret: &str| -> Option<String> {
926+
let (base, args) = crate::docblock::types::parse_generic_args(ret);
927+
if args.is_empty() {
928+
return None;
929+
}
930+
// Check that the base type is the Eloquent Builder.
931+
let base_clean = base.strip_prefix('\\').unwrap_or(base);
932+
if base_clean != ELOQUENT_BUILDER_FQN && base_clean != "Builder" {
933+
return None;
934+
}
935+
args.into_iter()
936+
.next()
937+
.map(|s| s.strip_prefix('\\').unwrap_or(s).to_string())
938+
};
939+
940+
// When a scope declares a bare `Builder` return type (without
941+
// generic args like `<Model>`), the extraction above fails.
942+
// In that case, scan all other instance methods on the
943+
// resolved candidate for a Builder-typed return that carries
944+
// the model name. All scope methods on the same
945+
// Builder<Model> instance share the same model, so any match
946+
// is valid.
947+
let model_name = scope_method
948+
.return_type
949+
.as_deref()
950+
.and_then(&extract_model_from_builder_ret)
951+
.or_else(|| {
952+
resolved_candidate.methods.iter().find_map(|m| {
953+
if m.is_static {
954+
return None;
955+
}
956+
m.return_type
957+
.as_deref()
958+
.and_then(&extract_model_from_builder_ret)
959+
})
960+
})?;
927961

928962
// Load the model and verify it extends Eloquent Model.
929963
let model = class_loader(&model_name)?;

0 commit comments

Comments
 (0)