Skip to content

Commit 642cc62

Browse files
committed
Resolve @mixin template parameters via property generic types
1 parent de9fd4e commit 642cc62

8 files changed

Lines changed: 512 additions & 21 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
8989
- **Single generic argument on collections bound to the wrong template parameter.** Writing `Collection<SectionTranslation>` on a class with `@template TKey of array-key` and `@template TValue` assigned the argument to `TKey` (by position), leaving `TValue` unsubstituted. When fewer generic arguments are provided than template parameters and the leading parameters have key-like bounds (`array-key`, `int`, `string`), the arguments are now right-aligned so they bind to the value parameters. Method-level template parameters whose bound parameter was not passed at the call site and defaults to `null` now resolve to `null` instead of remaining as raw template names. Together these fixes mean `Collection<SectionTranslation>::first()` correctly resolves to `SectionTranslation|null`.
9090
- **Nullable return types losing `|null` after template substitution.** When a method declared `@return TValue|null` and `TValue` was substituted with a concrete class through `@extends`, the `|null` component was silently dropped. Hover showed `AdminUser` instead of `AdminUser|null` for calls like `AdminUser::first()`. The docblock type extraction pipeline now preserves nullable unions so that `|null` survives through substitution, hover display, and variable type resolution.
9191
- **Loop-body assignments not visible inside the same loop iteration.** When a variable was initialized as `null` and reassigned later in a loop body, code earlier in the loop (reached on subsequent iterations) only saw `null`. The variable resolution walker now pre-scans the entire loop body for assignments before the positional walk, so the union of all assignments is available at every point inside the loop. Combined with `!== null` narrowing, variables like `$lastPaidEnd` correctly resolve to the assigned class type. Applies to `foreach`, `while`, `for`, and `do-while` loops.
92-
- **`@mixin` referencing a template parameter now resolves.** When a class declares `@template T` and `@mixin T`, the mixin name is substituted with the concrete type from the generic arguments. For example, a wrapper class `Subclient<EventsApi>` with `@mixin TWraps` now pulls in `EventsApi`'s methods instead of silently ignoring the mixin because no class named `TWraps` exists.
92+
- **`@mixin` referencing a template parameter now resolves.** When a class declares `@template T` and `@mixin T`, the mixin name is substituted with the concrete type from the generic arguments. For example, a wrapper class `Subclient<EventsApi>` with `@mixin TWraps` now pulls in `EventsApi`'s methods instead of silently ignoring the mixin because no class named `TWraps` exists. The initial fix only handled the ancestor-walk path (`ChildClass extends Wrapper<Concrete>`); the direct-property-access path (`/** @var Wrapper<Concrete> */ public $prop`) now also works. Additionally, mixin names that are template parameters are no longer FQN-resolved during parsing (which turned `TWraps` into `Namespace\TWraps`, breaking the template-param match).
9393
- **Eloquent `where{Property}()` methods from docblock-only `@property` tags.** Models that declare columns only via `@property` annotations (without `$casts`, `$fillable`, etc.) now correctly get `where{PropertyName}()` methods on `Builder<Model>`.
9494

9595
## [0.6.0] - 2026-03-26

src/completion/types/resolution.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
/// type alias reference.
2020
use std::sync::Arc;
2121

22-
use crate::inheritance::apply_generic_args;
22+
use crate::inheritance::{apply_generic_args, build_generic_subs};
2323
use crate::php_type::PhpType;
2424
use crate::types::*;
2525
use crate::util::{find_class_by_name, short_name};
@@ -362,6 +362,26 @@ fn resolve_named_type(
362362
};
363363
let mut result = apply_generic_args(&resolved, &generic_args);
364364

365+
// ── Template-param mixin resolution ────────────────
366+
// When a class declares `@mixin TParam` where `TParam`
367+
// is a template parameter, the mixin cannot be resolved
368+
// during `resolve_class_fully` because the concrete type
369+
// is not yet known. Now that generic args are concrete,
370+
// resolve those mixins and merge their members.
371+
if cls.mixins.iter().any(|m| cls.template_params.contains(m)) {
372+
let subs = build_generic_subs(&cls, &generic_args);
373+
if !subs.is_empty() {
374+
let mixin_members = virtual_members::phpdoc::resolve_template_param_mixins(
375+
&cls,
376+
&subs,
377+
class_loader,
378+
);
379+
if !mixin_members.is_empty() {
380+
virtual_members::merge_virtual_members(&mut result, mixin_members);
381+
}
382+
}
383+
}
384+
365385
// ── Eloquent Builder scope injection ───────────────
366386
laravel::try_inject_builder_scopes(
367387
&mut result,

src/completion/variable/rhs_resolution.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,8 +469,29 @@ fn resolve_rhs_instantiation(
469469
type_arg_strings.iter().map(|s| s.as_str()).collect();
470470
let resolved =
471471
crate::virtual_members::resolve_class_fully(cls, ctx.class_loader);
472-
let substituted =
472+
let mut substituted =
473473
crate::inheritance::apply_generic_args(&resolved, &type_args);
474+
475+
// ── Template-param mixin resolution ────────────────
476+
// When a class declares `@mixin TParam` where `TParam`
477+
// is a template parameter, the mixin cannot be resolved
478+
// during `resolve_class_fully` because the concrete type
479+
// is not yet known. Now that generic args are concrete,
480+
// resolve those mixins and merge their members.
481+
if cls.mixins.iter().any(|m| cls.template_params.contains(m)) {
482+
let generic_subs = crate::inheritance::build_generic_subs(cls, &type_args);
483+
if !generic_subs.is_empty() {
484+
let mixin_members = crate::virtual_members::phpdoc::resolve_template_param_mixins(
485+
cls,
486+
&generic_subs,
487+
ctx.class_loader,
488+
);
489+
if !mixin_members.is_empty() {
490+
crate::virtual_members::merge_virtual_members(&mut substituted, mixin_members);
491+
}
492+
}
493+
}
494+
474495
return vec![ResolvedType::from_class(substituted)];
475496
}
476497
}

src/inheritance.rs

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -999,25 +999,17 @@ pub(crate) fn apply_substitution<'a>(
999999
}
10001000
}
10011001

1002-
/// Apply explicit generic type arguments to a class's members.
1003-
///
1004-
/// When a type hint includes generic parameters (e.g. `Collection<int, User>`),
1005-
/// this function maps them to the class's `@template` parameters and rewrites
1006-
/// all method return types, method parameter types, and property type hints
1007-
/// with the concrete types.
1002+
/// Build a substitution map from a class's template parameters and
1003+
/// concrete type arguments.
10081004
///
1009-
/// If the class has no `template_params` or no `type_args` are provided,
1010-
/// returns a clone of the class unchanged.
1011-
///
1012-
/// # Example
1005+
/// Handles right-alignment when fewer arguments than template parameters
1006+
/// are provided (see [`apply_generic_args`] for details on the heuristic).
10131007
///
1014-
/// Given a `Collection` class with `@template TKey` and `@template TValue`,
1015-
/// calling `apply_generic_args(&collection_class, &["int", "User"])` will
1016-
/// substitute every occurrence of `TKey` with `int` and `TValue` with `User`
1017-
/// in the class's methods and properties.
1018-
pub(crate) fn apply_generic_args(class: &ClassInfo, type_args: &[&str]) -> ClassInfo {
1008+
/// Returns an empty map when no substitutions can be made (e.g. when
1009+
/// `template_params` or `type_args` is empty).
1010+
pub(crate) fn build_generic_subs(class: &ClassInfo, type_args: &[&str]) -> HashMap<String, PhpType> {
10191011
if class.template_params.is_empty() || type_args.is_empty() {
1020-
return class.clone();
1012+
return HashMap::new();
10211013
}
10221014

10231015
// When fewer type arguments are provided than template parameters,
@@ -1053,6 +1045,28 @@ pub(crate) fn apply_generic_args(class: &ClassInfo, type_args: &[&str]) -> Class
10531045
}
10541046
}
10551047

1048+
subs
1049+
}
1050+
1051+
/// Apply explicit generic type arguments to a class's members.
1052+
///
1053+
/// When a type hint includes generic parameters (e.g. `Collection<int, User>`),
1054+
/// this function maps them to the class's `@template` parameters and rewrites
1055+
/// all method return types, method parameter types, and property type hints
1056+
/// with the concrete types.
1057+
///
1058+
/// If the class has no `template_params` or no `type_args` are provided,
1059+
/// returns a clone of the class unchanged.
1060+
///
1061+
/// # Example
1062+
///
1063+
/// Given a `Collection` class with `@template TKey` and `@template TValue`,
1064+
/// calling `apply_generic_args(&collection_class, &["int", "User"])` will
1065+
/// substitute every occurrence of `TKey` with `int` and `TValue` with `User`
1066+
/// in the class's methods and properties.
1067+
pub(crate) fn apply_generic_args(class: &ClassInfo, type_args: &[&str]) -> ClassInfo {
1068+
let subs = build_generic_subs(class, type_args);
1069+
10561070
if subs.is_empty() {
10571071
return class.clone();
10581072
}

src/parser/ast_update.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -633,11 +633,21 @@ impl Backend {
633633
}
634634
}
635635

636-
// Resolve mixin names to fully-qualified names
636+
// Resolve mixin names to fully-qualified names.
637+
// Skip names that match a template parameter — these are
638+
// not class names but placeholders that will be substituted
639+
// with concrete types when the generic class is instantiated
640+
// (e.g. `@template TWraps` + `@mixin TWraps`).
637641
class.mixins = class
638642
.mixins
639643
.iter()
640-
.map(|m| Self::resolve_name(m, use_map, namespace))
644+
.map(|m| {
645+
if class.template_params.contains(m) {
646+
m.clone()
647+
} else {
648+
Self::resolve_name(m, use_map, namespace)
649+
}
650+
})
641651
.collect();
642652

643653
// Resolve custom collection class name to FQN

src/virtual_members/phpdoc.rs

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,83 @@ fn collect_mixin_members(
605605
}
606606
}
607607

608+
/// Resolve `@mixin` tags that name a template parameter, using concrete
609+
/// generic arguments provided at a call site.
610+
///
611+
/// During [`PHPDocProvider::provide`], mixin names that are template
612+
/// parameters (e.g. `@mixin TWraps`) cannot be resolved because the
613+
/// concrete type arguments are not yet known — they are applied later
614+
/// by [`apply_generic_args`](crate::inheritance::apply_generic_args).
615+
/// This function fills that gap: after generic substitution has been
616+
/// performed, call it with the **original** (unsubstituted) class and
617+
/// the substitution map to collect members from the now-concrete mixin
618+
/// classes.
619+
///
620+
/// Only mixins whose names match a template parameter are processed;
621+
/// non-template mixins were already resolved during `provide`.
622+
///
623+
/// The returned [`VirtualMembers`](super::VirtualMembers) should be
624+
/// merged into the substituted class via
625+
/// [`merge_virtual_members`](super::merge_virtual_members).
626+
pub fn resolve_template_param_mixins(
627+
original_class: &ClassInfo,
628+
template_subs: &HashMap<String, PhpType>,
629+
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
630+
) -> super::VirtualMembers {
631+
if template_subs.is_empty() || original_class.mixins.is_empty() {
632+
return super::VirtualMembers {
633+
methods: Vec::new(),
634+
properties: Vec::new(),
635+
constants: Vec::new(),
636+
};
637+
}
638+
639+
// Only process mixins whose name is a template parameter — the
640+
// rest were already resolved during `PHPDocProvider::provide`.
641+
let template_mixins: Vec<String> = original_class
642+
.mixins
643+
.iter()
644+
.filter(|m| original_class.template_params.contains(m))
645+
.cloned()
646+
.collect();
647+
648+
if template_mixins.is_empty() {
649+
return super::VirtualMembers {
650+
methods: Vec::new(),
651+
properties: Vec::new(),
652+
constants: Vec::new(),
653+
};
654+
}
655+
656+
let dedup = MixinDedup {
657+
methods: HashSet::new(),
658+
properties: HashSet::new(),
659+
constants: HashSet::new(),
660+
};
661+
662+
let mut collector = MixinCollector {
663+
methods: Vec::new(),
664+
properties: Vec::new(),
665+
constants: Vec::new(),
666+
dedup,
667+
};
668+
669+
collect_mixin_members(
670+
&template_mixins,
671+
&original_class.mixin_generics,
672+
class_loader,
673+
&mut collector,
674+
template_subs,
675+
0,
676+
);
677+
678+
super::VirtualMembers {
679+
methods: collector.methods,
680+
properties: collector.properties,
681+
constants: collector.constants,
682+
}
683+
}
684+
608685
/// Build a substitution map for mixin generic resolution by zipping the
609686
/// parent class's `@template` parameters with the type arguments provided
610687
/// by the child's `@extends` / `@implements` generics.

tests/integration/completion_generics.rs

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7389,3 +7389,118 @@ async fn test_function_template_param_resolves_to_bound_inside_body() {
73897389
_ => panic!("Expected CompletionResponse::Array"),
73907390
}
73917391
}
7392+
7393+
/// When a class declares `@template TWraps` and `@mixin TWraps`, and a
7394+
/// property is typed as `Wrapper<ConcreteApi>`, the mixin should resolve
7395+
/// `TWraps` → `ConcreteApi` and expose `ConcreteApi`'s methods on the
7396+
/// property. This is the "direct property access" scenario — no child
7397+
/// class extends the wrapper; the concrete type comes entirely from the
7398+
/// `@var` generic annotation on the property.
7399+
///
7400+
/// Reproduces the Klaviyo SDK pattern:
7401+
/// `/** @var Subclient<EventsApi> */ public $Events;`
7402+
#[tokio::test]
7403+
async fn test_mixin_template_param_resolved_via_property_generic_type() {
7404+
let backend = create_test_backend();
7405+
7406+
let wrapper_uri = Url::parse("file:///Subclient.php").unwrap();
7407+
let wrapper_text = r#"<?php
7408+
/**
7409+
* @template TWraps of object
7410+
* @mixin TWraps
7411+
*/
7412+
class Subclient {
7413+
public function getApiInstance(): object {}
7414+
}
7415+
"#;
7416+
7417+
let api_uri = Url::parse("file:///EventsApi.php").unwrap();
7418+
let api_text = r#"<?php
7419+
class EventsApi {
7420+
public function createEvent(array $body): array {}
7421+
public function getEvents(string $filter): array {}
7422+
public function deleteEvent(string $id): void {}
7423+
}
7424+
"#;
7425+
7426+
let consumer_uri = Url::parse("file:///KlaviyoClient.php").unwrap();
7427+
let consumer_text = r#"<?php
7428+
class KlaviyoClient {
7429+
/** @var Subclient<EventsApi> */
7430+
public $Events;
7431+
7432+
function test() {
7433+
$this->Events->
7434+
}
7435+
}
7436+
"#;
7437+
7438+
// Open all files
7439+
for (uri, text) in [
7440+
(&wrapper_uri, wrapper_text),
7441+
(&api_uri, api_text),
7442+
(&consumer_uri, consumer_text),
7443+
] {
7444+
backend
7445+
.did_open(DidOpenTextDocumentParams {
7446+
text_document: TextDocumentItem {
7447+
uri: uri.clone(),
7448+
language_id: "php".to_string(),
7449+
version: 1,
7450+
text: text.to_string(),
7451+
},
7452+
})
7453+
.await;
7454+
}
7455+
7456+
let completion_params = CompletionParams {
7457+
text_document_position: TextDocumentPositionParams {
7458+
text_document: TextDocumentIdentifier {
7459+
uri: consumer_uri.clone(),
7460+
},
7461+
position: Position {
7462+
line: 6,
7463+
character: 24,
7464+
},
7465+
},
7466+
work_done_progress_params: WorkDoneProgressParams::default(),
7467+
partial_result_params: PartialResultParams::default(),
7468+
context: None,
7469+
};
7470+
7471+
let result = backend.completion(completion_params).await.unwrap();
7472+
assert!(result.is_some(), "Completion should return results");
7473+
7474+
match result.unwrap() {
7475+
CompletionResponse::Array(items) => {
7476+
let method_names: Vec<&str> = items
7477+
.iter()
7478+
.filter(|i| i.kind == Some(CompletionItemKind::METHOD))
7479+
.map(|i| i.filter_text.as_deref().unwrap_or(&i.label))
7480+
.collect();
7481+
7482+
assert!(
7483+
method_names.contains(&"createEvent"),
7484+
"Mixin template param via @var Subclient<EventsApi> should expose 'createEvent', got: {:?}",
7485+
method_names
7486+
);
7487+
assert!(
7488+
method_names.contains(&"getEvents"),
7489+
"Mixin template param via @var Subclient<EventsApi> should expose 'getEvents', got: {:?}",
7490+
method_names
7491+
);
7492+
assert!(
7493+
method_names.contains(&"deleteEvent"),
7494+
"Mixin template param via @var Subclient<EventsApi> should expose 'deleteEvent', got: {:?}",
7495+
method_names
7496+
);
7497+
// The wrapper's own method should also be present.
7498+
assert!(
7499+
method_names.contains(&"getApiInstance"),
7500+
"Wrapper's own method 'getApiInstance' should still be present, got: {:?}",
7501+
method_names
7502+
);
7503+
}
7504+
_ => panic!("Expected CompletionResponse::Array"),
7505+
}
7506+
}

0 commit comments

Comments
 (0)