Skip to content

Commit 08debe5

Browse files
committed
Fix @mixin context for return types
1 parent 62bb740 commit 08debe5

4 files changed

Lines changed: 204 additions & 19 deletions

File tree

example.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -433,11 +433,11 @@ function isRegularUser(mixed $value): bool
433433

434434
// ── Static Completion (::) ──────────────────────────────────────────────────
435435

436-
User::$defaultRole; // static property
437-
User::TYPE_ADMIN; // class constant
436+
User::$defaultRole; // static property
437+
User::TYPE_ADMIN; // class constant
438438
User::findByEmail('a@b.c'); // static method
439-
User::make('Bob'); // inherited static from Model
440-
User::query(); // from @mixin Builder on Model (inherited)
439+
User::make('Bob'); // inherited static from Model
440+
User::query(); // from @mixin Builder on Model (inherited)
441441

442442

443443
// ── Enum Completion ─────────────────────────────────────────────────────────

src/completion/resolver.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2682,7 +2682,10 @@ impl Backend {
26822682
// instance, NOT the consuming class. Rewrite the return
26832683
// type to the concrete mixin class name so that resolution
26842684
// produces the mixin class rather than the consumer.
2685-
if method.return_type.as_deref() == Some("$this") {
2685+
if matches!(
2686+
method.return_type.as_deref(),
2687+
Some("$this" | "self" | "static")
2688+
) {
26862689
method.return_type = Some(mixin_class.name.clone());
26872690
}
26882691
merged.methods.push(method);

src/definition/resolve.rs

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1893,21 +1893,22 @@ impl Backend {
18931893
for fqn in &candidates {
18941894
if let Some(file_path) =
18951895
composer::resolve_class_path(&mappings, &workspace_root, fqn)
1896-
&& let Ok(target_content) = std::fs::read_to_string(&file_path) {
1897-
let short_name = fqn.rsplit('\\').next().unwrap_or(fqn);
1898-
if let Some(target_position) =
1899-
Self::find_definition_position(&target_content, short_name)
1900-
&& let Ok(target_uri) = Url::from_file_path(&file_path)
1901-
{
1902-
return Some(Location {
1903-
uri: target_uri,
1904-
range: Range {
1905-
start: target_position,
1906-
end: target_position,
1907-
},
1908-
});
1909-
}
1896+
&& let Ok(target_content) = std::fs::read_to_string(&file_path)
1897+
{
1898+
let short_name = fqn.rsplit('\\').next().unwrap_or(fqn);
1899+
if let Some(target_position) =
1900+
Self::find_definition_position(&target_content, short_name)
1901+
&& let Ok(target_uri) = Url::from_file_path(&file_path)
1902+
{
1903+
return Some(Location {
1904+
uri: target_uri,
1905+
range: Range {
1906+
start: target_position,
1907+
end: target_position,
1908+
},
1909+
});
19101910
}
1911+
}
19111912
}
19121913
}
19131914

tests/completion_mixins.rs

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2388,3 +2388,184 @@ async fn test_goto_definition_inherited_mixin_static_method() {
23882388
other => panic!("Expected Scalar location, got: {:?}", other),
23892389
}
23902390
}
2391+
2392+
// ─── Mixin return type: self / static should resolve to mixin class ────────
2393+
2394+
/// When a mixin method has `@return static` (or native return type `self`),
2395+
/// chaining on that method should resolve to the **mixin class**, not the
2396+
/// consuming class.
2397+
///
2398+
/// This is the same principle as `$this` — `@mixin` proxies calls via
2399+
/// `__call` / `__callStatic`, so self/static refer to the mixin instance.
2400+
#[tokio::test]
2401+
async fn test_completion_mixin_return_static_resolves_to_mixin_class() {
2402+
let backend = create_test_backend();
2403+
2404+
let uri = Url::parse("file:///mixin_return_static.php").unwrap();
2405+
let text = concat!(
2406+
"<?php\n", // 0
2407+
"class Builder {\n", // 1
2408+
" /** @return static */\n", // 2
2409+
" public static function query(): self { return new static(); }\n", // 3
2410+
" public function where(string $col, mixed $val): self {\n", // 4
2411+
" return $this;\n", // 5
2412+
" }\n", // 6
2413+
" public function get(): array { return []; }\n", // 7
2414+
"}\n", // 8
2415+
"/**\n", // 9
2416+
" * @mixin Builder\n", // 10
2417+
" */\n", // 11
2418+
"class Model {\n", // 12
2419+
" public function save(): bool { return true; }\n", // 13
2420+
"}\n", // 14
2421+
"class User extends Model {\n", // 15
2422+
" public function getEmail(): string { return ''; }\n", // 16
2423+
" public function test(): void {\n", // 17
2424+
" User::query()->\n", // 18
2425+
" }\n", // 19
2426+
"}\n", // 20
2427+
);
2428+
2429+
backend
2430+
.did_open(DidOpenTextDocumentParams {
2431+
text_document: TextDocumentItem {
2432+
uri: uri.clone(),
2433+
language_id: "php".to_string(),
2434+
version: 1,
2435+
text: text.to_string(),
2436+
},
2437+
})
2438+
.await;
2439+
2440+
let result = backend
2441+
.completion(CompletionParams {
2442+
text_document_position: TextDocumentPositionParams {
2443+
text_document: TextDocumentIdentifier { uri },
2444+
position: Position {
2445+
line: 18,
2446+
character: 24,
2447+
},
2448+
},
2449+
work_done_progress_params: WorkDoneProgressParams::default(),
2450+
partial_result_params: PartialResultParams::default(),
2451+
context: None,
2452+
})
2453+
.await
2454+
.unwrap();
2455+
2456+
assert!(result.is_some(), "Completion should return results");
2457+
match result.unwrap() {
2458+
CompletionResponse::Array(items) => {
2459+
let method_names: Vec<&str> = items
2460+
.iter()
2461+
.filter(|i| i.kind == Some(CompletionItemKind::METHOD))
2462+
.map(|i| i.filter_text.as_deref().unwrap())
2463+
.collect();
2464+
2465+
// `@return static` / native `self` on a mixin method should
2466+
// resolve to Builder, so we see Builder's methods.
2467+
assert!(
2468+
method_names.contains(&"where"),
2469+
"Chaining after mixin @return static should show Builder methods (where), got: {:?}",
2470+
method_names
2471+
);
2472+
assert!(
2473+
method_names.contains(&"get"),
2474+
"Chaining after mixin @return static should show Builder methods (get), got: {:?}",
2475+
method_names
2476+
);
2477+
// Consumer class methods should NOT appear — the chain resolved
2478+
// to Builder, not User or Model.
2479+
assert!(
2480+
!method_names.contains(&"save"),
2481+
"Should NOT show Model methods (save) after mixin static return, got: {:?}",
2482+
method_names
2483+
);
2484+
assert!(
2485+
!method_names.contains(&"getEmail"),
2486+
"Should NOT show User methods (getEmail) after mixin static return, got: {:?}",
2487+
method_names
2488+
);
2489+
}
2490+
_ => panic!("Expected CompletionResponse::Array"),
2491+
}
2492+
}
2493+
2494+
/// Same as above but with native `self` return type and no docblock override.
2495+
#[tokio::test]
2496+
async fn test_completion_mixin_return_self_resolves_to_mixin_class() {
2497+
let backend = create_test_backend();
2498+
2499+
let uri = Url::parse("file:///mixin_return_self.php").unwrap();
2500+
let text = concat!(
2501+
"<?php\n", // 0
2502+
"class QueryBuilder {\n", // 1
2503+
" public function where(string $col): self { return $this; }\n", // 2
2504+
" public function toSql(): string { return ''; }\n", // 3
2505+
"}\n", // 4
2506+
"/**\n", // 5
2507+
" * @mixin QueryBuilder\n", // 6
2508+
" */\n", // 7
2509+
"class Model {\n", // 8
2510+
" public function save(): bool { return true; }\n", // 9
2511+
" public function test(): void {\n", // 10
2512+
" $this->where('active')->\n", // 11
2513+
" }\n", // 12
2514+
"}\n", // 13
2515+
);
2516+
2517+
backend
2518+
.did_open(DidOpenTextDocumentParams {
2519+
text_document: TextDocumentItem {
2520+
uri: uri.clone(),
2521+
language_id: "php".to_string(),
2522+
version: 1,
2523+
text: text.to_string(),
2524+
},
2525+
})
2526+
.await;
2527+
2528+
let result = backend
2529+
.completion(CompletionParams {
2530+
text_document_position: TextDocumentPositionParams {
2531+
text_document: TextDocumentIdentifier { uri },
2532+
position: Position {
2533+
line: 11,
2534+
character: 33,
2535+
},
2536+
},
2537+
work_done_progress_params: WorkDoneProgressParams::default(),
2538+
partial_result_params: PartialResultParams::default(),
2539+
context: None,
2540+
})
2541+
.await
2542+
.unwrap();
2543+
2544+
assert!(result.is_some(), "Completion should return results");
2545+
match result.unwrap() {
2546+
CompletionResponse::Array(items) => {
2547+
let method_names: Vec<&str> = items
2548+
.iter()
2549+
.filter(|i| i.kind == Some(CompletionItemKind::METHOD))
2550+
.map(|i| i.filter_text.as_deref().unwrap())
2551+
.collect();
2552+
2553+
assert!(
2554+
method_names.contains(&"where"),
2555+
"Chaining after mixin self return should show QueryBuilder methods (where), got: {:?}",
2556+
method_names
2557+
);
2558+
assert!(
2559+
method_names.contains(&"toSql"),
2560+
"Chaining after mixin self return should show QueryBuilder methods (toSql), got: {:?}",
2561+
method_names
2562+
);
2563+
assert!(
2564+
!method_names.contains(&"save"),
2565+
"Should NOT show Model methods (save) after mixin self return, got: {:?}",
2566+
method_names
2567+
);
2568+
}
2569+
_ => panic!("Expected CompletionResponse::Array"),
2570+
}
2571+
}

0 commit comments

Comments
 (0)