Skip to content

Commit 596a795

Browse files
committed
Fix self / static from within macros
1 parent 15a0514 commit 596a795

9 files changed

Lines changed: 282 additions & 38 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5858
- **Eloquent models always expose their primary key.** A model whose table has no migration and no schema dump no longer reports a false `Property 'id' not found` on `$model->id`. The primary key is synthesized for every Eloquent model, honouring `$primaryKey` for the column name and `$keyType` for the type (`int` by default, `string` for UUID or ULID keys).
5959
- **Laravel virtual property hover and navigation prefer backing accessors.** Hover labels for accessor and computed Eloquent properties now describe the source without echoing the backing method name, and go-to-definition on accessor/computed properties prefers the backing accessor or legacy mutator before falling back to `$appends` or other Eloquent metadata arrays. Legacy `setXAttribute` mutators are shown alongside database, cast, attribute-default, accessor, or computed-property sources when they exist. Contributed by @calebdw.
6060
- **Laravel macro callbacks registered through facades now infer `$this` as the concrete facade target.** `$this` inside callbacks such as `Request::macro('shouldReturnJson', function () { ... })` and `Context::macro(...)` now resolves to the class behind the facade instead of the surrounding service provider. Contributed by @calebdw.
61+
- **`self::` and `static::` inside macro callbacks resolve to the macro target.** In a closure passed to a `macro()` registration (Laravel `Macroable` or Carbon), `self::` and `static::` now resolve to the class the macro is registered on instead of the service provider that lexically encloses the registration, matching how the closure is bound at runtime. Carbon's static macro idiom `self::this()->...` no longer reports a false unknown-method diagnostic, and completion after `self::` inside the callback offers the target's members.
6162
- **Find References and Rename no longer match unrelated same-named methods when a call's receiver type can't be resolved.** A call like `$x->find()` used to conservatively match every `find()` method in the project when `$x`'s type couldn't be determined, drowning results in unrelated matches for common method names. Find References on an interface method now also includes every implementing class; Rename Symbol stays scoped to the single concrete implementation being renamed, so it never rewrites unrelated same-named methods elsewhere. Contributed by @sidux in https://github.com/PHPantom-dev/phpantom_lsp/pull/186.
6263
- **Go-to-implementation and type hierarchy return the same results every time.** After the workspace finished indexing, these features listed only your project's implementing classes, but a class from a dependency could slip in if you happened to have viewed it earlier in the session, so the same query gave different results depending on what you'd looked at. Results are now consistently limited to your own code, matching what the workspace index actually covers.
6364
- **Blade files with raw `<?php ... ?>` tags no longer report false syntax errors.** PHP code embedded directly in a Blade template (outside `@php`/`@endphp`) is now recognized and passed through unmodified, so string literals that happen to start with `@` (e.g. a JSON-LD `'@context'` array key) are no longer misread as Blade directives.

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ unlikely to move the needle for most users.
106106
| D15 | [Unused parameter diagnostic](todo/diagnostics.md#d15-unused-parameter-diagnostic) | Low | Low |
107107
| | **[Bug Fixes](todo/bugs.md)** | | |
108108
| B1 | [`resolve_function_name` guesses a single namespace, missing same-file multi-namespace declarations](todo/bugs.md#b1-resolve_function_name-guesses-a-single-namespace-missing-same-file-multi-namespace-declarations) | Low-Medium | Medium |
109-
| B2 | [`self::`/`static::` inside macro closures resolve to the enclosing class, not the macro target](todo/bugs.md#b2-selfstatic-inside-macro-closures-resolve-to-the-enclosing-class-not-the-macro-target) | Medium | Medium |
110109
| | **[Code Actions](todo/actions.md)** | | |
111110
| A40 | [Generate method from call](todo/actions.md#a40-generate-method-from-call) | Medium-High | Medium |
112111
| A41 | [Create class from non-existing name](todo/actions.md#a41-create-class-from-non-existing-name) | Medium | Medium |

docs/todo/bugs.md

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -40,32 +40,3 @@ Fixing this requires either threading the call expression's byte
4040
offset through to `resolve_function_name` or adding an offset-aware
4141
variant of the loader closure. Size the fix accordingly — this is not
4242
a one-line change.
43-
44-
## B2. `self::`/`static::` inside macro closures resolve to the enclosing class, not the macro target
45-
46-
Inside a closure passed to `Target::macro(...)` (Laravel `Macroable`
47-
or Carbon), `$this` correctly resolves to the macro target class via
48-
`laravel_macro_this_resolver` (`closure_this_from_static_receiver` in
49-
`src/completion/variable/closure_resolution.rs`). But `self::` and
50-
`static::` inside the same closure still resolve to the class that
51-
lexically encloses the registration (e.g. the service provider), so
52-
Carbon's static macro idiom
53-
54-
```php
55-
CarbonImmutable::macro('diffFromYear', function (int $year): string {
56-
return self::this()->diffForHumans(...);
57-
});
58-
```
59-
60-
reports a false `Method 'this' not found on class
61-
'App\Providers\DemoServiceProvider'` (`unknown_member`). At runtime
62-
both `Macroable::__call`/`__callStatic` and Carbon bind the closure
63-
with the target as scope, so `self`/`static` refer to the target and
64-
protected members like `Carbon\Traits\Mixin::this()` are accessible.
65-
66-
Subject resolution for `Self_`/`Static` needs the same "am I inside a
67-
macro registration closure?" awareness that `$this` resolution already
68-
has. The `self::this()` demo in
69-
`examples/laravel/app/Providers/DemoServiceProvider.php` was switched
70-
to the supported `$this->` form when this was discovered; restore the
71-
static idiom there once fixed.

examples/demo.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3579,6 +3579,14 @@ public function demo(): void
35793579
$router->extend('redis', function () {
35803580
$this->getDefaultDriver(); // resolves Router::getDefaultDriver()
35813581
});
3582+
3583+
// Macro-style registration: the closure is bound with the target
3584+
// class as its scope, so self:: and static:: inside it refer to
3585+
// ScaffoldingMacroTarget, not ParamClosureThisDemo.
3586+
ScaffoldingMacroTarget::macro('renderTwice', function (): string {
3587+
return self::make()->render() // resolves MacroTarget::make()
3588+
. static::make()->render(); // static:: works the same way
3589+
});
35823590
}
35833591
}
35843592

@@ -5015,6 +5023,38 @@ public function group(\Closure $callback): void {}
50155023
public function extend(string $driver, \Closure $callback): self { return $this; }
50165024
}
50175025

5026+
// ScaffoldingMacroTarget — a minimal Macroable-style class used by
5027+
// ParamClosureThisDemo. `macro()` stores the closure and `__call` binds it
5028+
// with this class as scope (like Laravel's Macroable / Carbon), so
5029+
// `self::`/`static::` inside a registered closure refer to this class.
5030+
class ScaffoldingMacroTarget
5031+
{
5032+
/** @var array<string, callable> */
5033+
private static array $macros = [];
5034+
5035+
public static function make(): static { return new static(); }
5036+
5037+
public function render(): string { return 'rendered'; }
5038+
5039+
/**
5040+
* @param-closure-this static $macro
5041+
*/
5042+
public static function macro(string $name, callable $macro): void
5043+
{
5044+
static::$macros[$name] = $macro;
5045+
}
5046+
5047+
/** @param array<int, mixed> $args */
5048+
public function __call(string $name, array $args): mixed
5049+
{
5050+
$macro = static::$macros[$name];
5051+
if ($macro instanceof \Closure) {
5052+
$macro = $macro->bindTo($this, static::class);
5053+
}
5054+
return $macro(...$args);
5055+
}
5056+
}
5057+
50185058
class ScaffoldingFirstClassCallable
50195059
{
50205060
public function dispatch(): Pen
@@ -7256,6 +7296,14 @@ function runDemoAssertions(): void
72567296
$ctExt = $ctRouter->extend('redis', function () {});
72577297
assert($ctExt instanceof ScaffoldingClosureThisRouter, 'Router::extend() must return self');
72587298

7299+
// Macro-style scope binding: self::/static:: inside a registered
7300+
// closure refer to the macro target class at runtime.
7301+
ScaffoldingMacroTarget::macro('renderTwice', function (): string {
7302+
return self::make()->render() . static::make()->render();
7303+
});
7304+
$macroTarget = new ScaffoldingMacroTarget();
7305+
assert($macroTarget->renderTwice() === 'renderedrendered', 'self::/static:: inside a macro closure must bind to ScaffoldingMacroTarget');
7306+
72597307
// ── @mixin generic substitution scaffolding ─────────────────────────
72607308
$mixinBuilder = new ScaffoldingMixinBuilder();
72617309
assert($mixinBuilder->firstOrFail() === null, 'ScaffoldingMixinBuilder::firstOrFail() must return mixed');

examples/laravel/app/Providers/DemoServiceProvider.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,11 @@ public function boot(): void
2424
Collection::mixin(new CollectionMixin());
2525

2626
// Carbon supports the same `macro()` pattern as Laravel's Macroable.
27-
// `$this` inside the closure is the CarbonImmutable instance the
28-
// macro is called on:
27+
// The closure is bound with the target as scope, so `self::`/`static::`
28+
// refer to CarbonImmutable and protected helpers like `self::this()`
29+
// (the instance the macro is called on) resolve:
2930
CarbonImmutable::macro('diffFromYear', function (int $year, bool $absolute = false): string {
30-
return $this->diffForHumans(
31+
return self::this()->diffForHumans(
3132
CarbonImmutable::create($year, 1, 1),
3233
['syntax' => \Carbon\CarbonInterface::DIFF_ABSOLUTE]
3334
);

examples/laravel/assertions.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,24 @@ class_uses_recursive(\App\Models\BlogPost::class),
320320
\App\Models\BlogPost::factory()->trashed() instanceof \Illuminate\Database\Eloquent\Factories\Factory
321321
);
322322

323+
// ─── Carbon macro closure scope binding ──────────────────────────────────────
324+
325+
// Carbon binds macro closures with the target class as scope, so `self::`
326+
// inside the closure refers to CarbonImmutable (not the class that lexically
327+
// encloses the registration) and the protected `Mixin::this()` helper — the
328+
// instance the macro is called on — is accessible.
329+
\Carbon\CarbonImmutable::macro('phpantomScopeProbe', function (): string {
330+
return self::this()->format('Y');
331+
});
332+
check(
333+
'self::this() inside a Carbon macro returns the bound instance',
334+
\Carbon\CarbonImmutable::create(2020, 1, 1)->phpantomScopeProbe() === '2020'
335+
);
336+
check(
337+
'Mixin::this() is protected static (only reachable via rebound scope)',
338+
(new ReflectionMethod(\Carbon\CarbonImmutable::class, 'this'))->isProtected()
339+
);
340+
323341
// ─── Summary ────────────────────────────────────────────────────────────────
324342

325343
echo "\n";

src/completion/resolver/mod.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,8 @@ fn resolve_target_classes_expr_inner_impl(
264264

265265
this_types
266266
}
267-
SubjectExpr::SelfKw | SubjectExpr::StaticKw => current_class
268-
.map(|cc| ResolvedType::from_class(cc.clone()))
267+
SubjectExpr::SelfKw | SubjectExpr::StaticKw => resolve_self_static_class(ctx)
268+
.map(ResolvedType::from_class)
269269
.into_iter()
270270
.collect(),
271271

@@ -308,8 +308,8 @@ fn resolve_target_classes_expr_inner_impl(
308308
// class names, so find_class_by_name / class_loader won't
309309
// find them.
310310
let owner_classes: Vec<Arc<ClassInfo>> = if is_self_or_static(class) {
311-
current_class
312-
.map(|cc| Arc::new(cc.clone()))
311+
resolve_self_static_class(ctx)
312+
.map(Arc::new)
313313
.into_iter()
314314
.collect()
315315
} else if let Some(parent_name) = resolve_class_keyword(class, current_class) {
@@ -1304,6 +1304,20 @@ fn resolve_variable_fallback(
13041304

13051305
// ── Static owner class resolution ───────────────────────────────────
13061306

1307+
/// Resolve the class that a bare `self`/`static` keyword refers to at
1308+
/// the cursor position.
1309+
///
1310+
/// Normally this is the lexically enclosing class, but inside a
1311+
/// closure whose enclosing call site declares `@param-closure-this`
1312+
/// (e.g. a Laravel `Macroable` or Carbon `macro()` registration), the
1313+
/// runtime binds the closure with the target class as its scope
1314+
/// (`Closure::bind`), so `self::` and `static::` refer to the bound
1315+
/// target rather than the class that lexically encloses the closure.
1316+
fn resolve_self_static_class(ctx: &ResolutionCtx<'_>) -> Option<ClassInfo> {
1317+
super::variable::closure_resolution::find_closure_this_override(ctx)
1318+
.or_else(|| ctx.current_class.cloned())
1319+
}
1320+
13071321
/// Resolve a static class reference (`self`, `static`, `parent`, or a
13081322
/// class name) to its `ClassInfo`.
13091323
///
@@ -1314,7 +1328,7 @@ pub(in crate::completion) fn resolve_static_owner_class(
13141328
rctx: &ResolutionCtx<'_>,
13151329
) -> Option<Arc<ClassInfo>> {
13161330
if is_self_or_static(class) {
1317-
rctx.current_class.map(|cc| Arc::new(cc.clone()))
1331+
resolve_self_static_class(rctx).map(Arc::new)
13181332
} else if let Some(resolved_name) = resolve_class_keyword(class, rctx.current_class) {
13191333
// parent — load via class_loader so we get the full parent ClassInfo
13201334
(rctx.class_loader)(&resolved_name)

tests/integration/completion_param_closure_this.rs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,124 @@ async fn test_param_closure_this_property_access() {
315315
);
316316
}
317317

318+
// ─── self:: / static:: inside a rebound closure ─────────────────────────────
319+
320+
/// Inside a closure rebound via `@param-closure-this` (e.g. a macro
321+
/// registration), the runtime binds the target class as the closure's
322+
/// scope, so `self::` refers to the macro target, not the class that
323+
/// lexically encloses the registration.
324+
#[tokio::test]
325+
async fn test_param_closure_this_self_keyword() {
326+
let backend = create_test_backend();
327+
let uri = Url::parse("file:///test/closure_self_kw.php").unwrap();
328+
329+
let src = concat!(
330+
"<?php\n",
331+
"class Macroable {\n",
332+
" public static function make(): static { return new static(); }\n",
333+
" /**\n",
334+
" * @param string $name\n",
335+
" * @param callable $macro\n",
336+
" * @param-closure-this static $macro\n",
337+
" */\n",
338+
" public static function macro(string $name, callable $macro): void {}\n",
339+
"}\n",
340+
"class App {\n",
341+
" public function run(): void {\n",
342+
" Macroable::macro('test', function () {\n",
343+
" self::\n",
344+
" });\n",
345+
" }\n",
346+
"}\n",
347+
);
348+
349+
// Line 13: ` self::` — cursor after `::`
350+
let items = complete_at(&backend, &uri, src, 13, 18).await;
351+
let names = method_names(&items);
352+
assert!(
353+
names.contains(&"make"),
354+
"Expected 'make' from the macro target (Macroable) via self::, got: {:?}",
355+
names,
356+
);
357+
}
358+
359+
/// Same as above for the `static::` keyword.
360+
#[tokio::test]
361+
async fn test_param_closure_this_static_keyword() {
362+
let backend = create_test_backend();
363+
let uri = Url::parse("file:///test/closure_static_kw.php").unwrap();
364+
365+
let src = concat!(
366+
"<?php\n",
367+
"class Macroable {\n",
368+
" public static function make(): static { return new static(); }\n",
369+
" /**\n",
370+
" * @param string $name\n",
371+
" * @param callable $macro\n",
372+
" * @param-closure-this static $macro\n",
373+
" */\n",
374+
" public static function macro(string $name, callable $macro): void {}\n",
375+
"}\n",
376+
"class App {\n",
377+
" public function run(): void {\n",
378+
" Macroable::macro('test', function () {\n",
379+
" static::\n",
380+
" });\n",
381+
" }\n",
382+
"}\n",
383+
);
384+
385+
// Line 13: ` static::` — cursor after `::`
386+
let items = complete_at(&backend, &uri, src, 13, 20).await;
387+
let names = method_names(&items);
388+
assert!(
389+
names.contains(&"make"),
390+
"Expected 'make' from the macro target (Macroable) via static::, got: {:?}",
391+
names,
392+
);
393+
}
394+
395+
/// `self::` outside the closure must still resolve to the lexically
396+
/// enclosing class, not the @param-closure-this type.
397+
#[tokio::test]
398+
async fn test_param_closure_this_self_keyword_does_not_leak() {
399+
let backend = create_test_backend();
400+
let uri = Url::parse("file:///test/closure_self_kw_no_leak.php").unwrap();
401+
402+
let src = concat!(
403+
"<?php\n",
404+
"class Macroable {\n",
405+
" public static function make(): static { return new static(); }\n",
406+
" /**\n",
407+
" * @param-closure-this static $macro\n",
408+
" */\n",
409+
" public static function macro(string $name, callable $macro): void {}\n",
410+
"}\n",
411+
"class App {\n",
412+
" public static function ownHelper(): string { return ''; }\n",
413+
" public function run(): void {\n",
414+
" Macroable::macro('test', function () {\n",
415+
" });\n",
416+
" self::\n",
417+
" }\n",
418+
"}\n",
419+
);
420+
421+
// Line 13: ` self::` — cursor OUTSIDE the closure
422+
let items = complete_at(&backend, &uri, src, 13, 14).await;
423+
let names = method_names(&items);
424+
assert!(
425+
names.contains(&"ownHelper"),
426+
"Expected 'ownHelper' from lexical class App, got: {:?}",
427+
names,
428+
);
429+
assert!(
430+
!names.contains(&"make"),
431+
"Should NOT see Macroable::make outside the closure, got: {:?}",
432+
names,
433+
);
434+
}
435+
318436
// ─── @param-closure-this with FQN type ──────────────────────────────────────
319437

320438
/// `@param-closure-this \App\Route $callback` with a leading backslash

0 commit comments

Comments
 (0)