Skip to content

Commit c4cff65

Browse files
committed
False-positive diagnostics for $this inside traits
1 parent a789604 commit c4cff65

4 files changed

Lines changed: 211 additions & 49 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515
- **Ternary and null-coalesce member access.** Accessing a member on a ternary or null-coalesce expression (e.g. `($a ?: $b)->property`, `($x ?? $y)->method()`) now resolves correctly for hover, go-to-definition, and diagnostics. Previously the subject extraction produced an empty string, causing a confusing "Cannot resolve type of ''" hint and no hover information.
1616
- **Inherited methods missing through deep stub chains.** Methods like `getCode()` and `getMessage()` are now found on classes that inherit through multi-level chains where intermediate classes live in stubs (e.g. `QueryException``PDOException``RuntimeException``Exception`). Previously the inheritance chain broke when a stub file contained multiple namespace blocks, causing parent class names to be resolved against the wrong namespace.
1717
- **False-positive diagnostics for same-named variables in different methods.** When two methods in the same class both used a variable like `$order`, the diagnostic cache resolved it once and reused that type for every other method in the class. The second method saw the wrong type and flagged valid member accesses as unknown. Both the per-file subject cache and the cross-collector resolution cache are now scoped to the enclosing function/method/closure body, so each method resolves variables independently.
18-
- **False-positive diagnostics for `$this` inside traits.** Accessing host-class members via `$this->`, `self::`, `static::`, or `parent::` inside a trait method no longer produces "not found" warnings. Traits are incomplete by nature and expect the host class to provide these members.
18+
- **False-positive diagnostics for `$this` inside traits.** Accessing host-class members via `$this->`, `self::`, `static::`, or `parent::` inside a trait method no longer produces "not found" warnings, including chain expressions like `static::where(...)->update(...)` and accesses inside closures or arrow functions nested within trait methods. Traits are incomplete by nature and expect the host class to provide these members.
1919
- **Type narrowing inside `return` statements.** `instanceof` checks in `&&` chains and ternary conditions now narrow the variable type when the expression is the operand of a `return` statement. Previously, `return $e instanceof QueryException && $e->errorInfo;` would flag `errorInfo` as unknown because narrowing only applied inside standalone expression statements and `if` conditions.
2020
- **CLI analyze performance (pathological files).** Single-file analysis of Eloquent-heavy services dropped from ~6 min to ~63 s (5.8× faster). The shared package (~2 500 files) dropped from 14 m 28 s to 1 m 25 s (10× faster, now ~30 % faster than PHPStan on the same machine). Key changes:
2121
- *Negative class cache.* `find_or_load_class` now caches "not found" results so repeated references to unknown types skip the four-phase lookup (fqn_index → classmap → PSR-4 → stubs). Invalidated when new classes are discovered.

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ unlikely to move the needle for most users.
101101
| # | Item | Impact | Effort |
102102
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------- | ----------- |
103103
| | **[Bugs](todo/bugs.md)** | | |
104-
| B3 | [Trait static/self suppression not applied inside closures](todo/bugs.md#b3-trait-staticself-suppression-not-applied-inside-closures) | Medium | Low |
105104
| B4 | [Variable reassignment loses type when parameter name is reused](todo/bugs.md#b4-variable-reassignment-loses-type-when-parameter-name-is-reused) | Medium | Medium |
106105
| B7 | [Overloaded built-in function signatures in stubs](todo/bugs.md#b7-overloaded-built-in-function-signatures-in-stubs) | Low | Low |
107106
| | **[Completion](todo/completion.md)** | | |

docs/todo/bugs.md

Lines changed: 1 addition & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -15,47 +15,6 @@ within the same impact tier.
1515

1616
---
1717

18-
#### B3. Type engine does not resolve `$this`/`static` inside traits
19-
20-
| | |
21-
|---|---|
22-
| **Impact** | Medium |
23-
| **Effort** | Low |
24-
25-
When a trait method (or a closure inside a trait method) accesses
26-
members via `$this`, `self`, `static`, or `parent`, the type engine
27-
resolves the subject to the trait itself rather than to the host class.
28-
Since the trait does not declare the members that the host class
29-
provides, the type engine reports the members as missing. This affects
30-
completion, hover, and go-to-definition for any trait that relies on
31-
host-class members.
32-
33-
The diagnostic pass has a suppression heuristic for this case, but
34-
the underlying problem is in the type engine: it should resolve
35-
`$this`/`static`/`self` inside a trait to the using class (when known)
36-
or defer resolution (when the host class is not known). The
37-
suppression heuristic also fails when the access is inside a closure
38-
nested within a trait method.
39-
40-
**Observed:** 43 cases in `shared`. The largest cluster (41) is
41-
`BusinessCentralErrorHandlerTrait` where `$this->model`,
42-
`$this->eventType`, etc. are properties provided by the host class.
43-
`SalesInfoGlobalTrait` contributes 2 cases where `static::where()` and
44-
`static::query()` are called inside a closure within a trait method.
45-
46-
**Fix:** When the type engine encounters `$this`/`static`/`self` inside
47-
a trait, it should attempt to resolve to the known host class(es). For
48-
the analyze pass where no specific host class is open, the engine
49-
should recognise that trait member accesses are inherently incomplete
50-
and avoid reporting members as missing. The closure nesting issue is a
51-
separate symptom: `find_innermost_enclosing_class` does find the trait
52-
at the closure's offset, but the suppression check does not fire,
53-
suggesting the `subject_text` on the `MemberAccess` span differs from
54-
the expected `"static"` / `"$this"` keywords when emitted from inside
55-
a closure.
56-
57-
---
58-
5918
#### B4. Variable reassignment loses type when parameter name is reused
6019

6120
| | |
@@ -112,5 +71,4 @@ needs a similar mechanism.
11271
for functions with true overloads. The argument count checker consults
11372
this map before flagging. The map only needs entries for functions
11473
where the stub's single signature cannot represent the valid call
115-
forms.
116-
74+
forms.

src/diagnostics/unknown_members.rs

Lines changed: 209 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,34 @@ enum SubjectOutcome {
141141
/// Per-pass cache mapping subject keys to their resolution outcomes.
142142
type SubjectCache = HashMap<SubjectCacheKey, SubjectOutcome>;
143143

144+
/// Build a [`ScopeKey`] from the innermost enclosing class (if any)
145+
/// and the enclosing function/method/closure scope start offset.
146+
/// Check whether a subject text is rooted in `$this`, `self`, `static`,
147+
/// or `parent`. This matches both bare keywords (`"$this"`, `"static"`)
148+
/// and chain expressions that start with one of them
149+
/// (`"$this->relation()"`, `"static::where('x', 'y')"`, `"self::$prop"`).
150+
fn subject_text_is_rooted_in_self(subject_text: &str) -> bool {
151+
// Bare keyword match (most common case).
152+
if matches!(subject_text, "$this" | "self" | "static" | "parent") {
153+
return true;
154+
}
155+
156+
// Chain rooted at `$this->` or `$this?->`
157+
if subject_text.starts_with("$this->") || subject_text.starts_with("$this?->") {
158+
return true;
159+
}
160+
161+
// Chain rooted at `self::`, `static::`, or `parent::`
162+
if subject_text.starts_with("self::")
163+
|| subject_text.starts_with("static::")
164+
|| subject_text.starts_with("parent::")
165+
{
166+
return true;
167+
}
168+
169+
false
170+
}
171+
144172
/// Build a [`ScopeKey`] from the innermost enclosing class (if any)
145173
/// and the enclosing function/method/closure scope start offset.
146174
fn scope_key_for(current_class: Option<&ClassInfo>, fn_scope_start: u32) -> ScopeKey {
@@ -307,12 +335,16 @@ impl Backend {
307335
// members accessed via $this/self/static/parent. Flagging
308336
// these produces false positives for every trait that relies
309337
// on the host class's members.
338+
//
339+
// This also covers chain expressions rooted at these keywords,
340+
// e.g. `static::where('x', 'y')->update(...)` has subject_text
341+
// `"static::where('x', 'y')"` and `$this->relation()->first()`
342+
// has subject_text `"$this->relation()"`. The root of the
343+
// chain is still the trait's self-reference, so the entire
344+
// chain is unsuppressable without knowing the host class.
310345
if let Some(cc) = current_class
311346
&& cc.kind == ClassLikeKind::Trait
312-
&& matches!(
313-
subject_text.as_str(),
314-
"$this" | "self" | "static" | "parent"
315-
)
347+
&& subject_text_is_rooted_in_self(subject_text)
316348
{
317349
continue;
318350
}
@@ -3269,6 +3301,179 @@ function second(Beta $x): void {
32693301
);
32703302
}
32713303

3304+
#[test]
3305+
fn no_diagnostic_for_this_inside_closure_in_trait() {
3306+
// B3: $this-> and static:: inside a closure nested within a trait
3307+
// method should be suppressed, just like direct trait method bodies.
3308+
let php = r#"<?php
3309+
trait SalesInfoGlobalTrait {
3310+
public function getSalesInfo(): void {
3311+
$items = array_map(function ($item) {
3312+
$this->model;
3313+
$this->eventType;
3314+
static::where();
3315+
static::query();
3316+
}, []);
3317+
}
3318+
}
3319+
3320+
class SalesReport {
3321+
use SalesInfoGlobalTrait;
3322+
public string $model = 'Sale';
3323+
public string $eventType = 'report';
3324+
public static function where(): void {}
3325+
public static function query(): void {}
3326+
}
3327+
"#;
3328+
let backend = Backend::new_test();
3329+
let diags = collect(&backend, "file:///test.php", php);
3330+
assert!(
3331+
diags.is_empty(),
3332+
"expected no diagnostics for $this/static:: inside closure in trait, got: {diags:?}"
3333+
);
3334+
}
3335+
3336+
#[test]
3337+
fn no_diagnostic_for_this_inside_arrow_fn_in_trait() {
3338+
// B3: $this-> inside an arrow function nested within a trait method.
3339+
let php = r#"<?php
3340+
trait FilterTrait {
3341+
public function applyFilter(): void {
3342+
$fn = fn() => $this->filterColumn;
3343+
}
3344+
}
3345+
3346+
class Report {
3347+
use FilterTrait;
3348+
public string $filterColumn = 'status';
3349+
}
3350+
"#;
3351+
let backend = Backend::new_test();
3352+
let diags = collect(&backend, "file:///test.php", php);
3353+
assert!(
3354+
diags.is_empty(),
3355+
"expected no diagnostics for $this-> inside arrow fn in trait, got: {diags:?}"
3356+
);
3357+
}
3358+
3359+
#[test]
3360+
fn no_diagnostic_for_chain_rooted_at_static_inside_trait() {
3361+
// B3: `static::where(...)->update(...)` inside a trait method.
3362+
// The subject_text for `update` is `"static::where('x', 'y')"`,
3363+
// which is a chain rooted at `static`. The suppression must
3364+
// recognise the root keyword, not require an exact match.
3365+
let php = r#"<?php
3366+
trait SalesInfoGlobalTrait {
3367+
public function updateSalesInfo(): void {
3368+
static::where('column', 'value')->update(['sales' => 1]);
3369+
}
3370+
}
3371+
3372+
class SalesReport extends \Illuminate\Database\Eloquent\Model {
3373+
use SalesInfoGlobalTrait;
3374+
}
3375+
"#;
3376+
let backend = Backend::new_test();
3377+
let diags = collect(&backend, "file:///test.php", php);
3378+
assert!(
3379+
diags.is_empty(),
3380+
"expected no diagnostics for static::...->method() chain inside trait, got: {diags:?}"
3381+
);
3382+
}
3383+
3384+
#[test]
3385+
fn no_diagnostic_for_chain_rooted_at_this_inside_trait() {
3386+
// B3: `$this->relation()->first()` inside a trait method.
3387+
// The subject_text for `first` is `"$this->relation()"`.
3388+
let php = r#"<?php
3389+
trait HasRelation {
3390+
public function loadRelation(): void {
3391+
$this->items()->first();
3392+
}
3393+
}
3394+
3395+
class Order {
3396+
use HasRelation;
3397+
/** @return \Illuminate\Database\Eloquent\Builder */
3398+
public function items(): object { return new \stdClass(); }
3399+
}
3400+
"#;
3401+
let backend = Backend::new_test();
3402+
let diags = collect(&backend, "file:///test.php", php);
3403+
assert!(
3404+
diags.is_empty(),
3405+
"expected no diagnostics for $this->...->method() chain inside trait, got: {diags:?}"
3406+
);
3407+
}
3408+
3409+
#[test]
3410+
fn no_diagnostic_for_chain_rooted_at_static_inside_closure_in_trait() {
3411+
// B3: `static::where(...)` inside a closure within a trait method.
3412+
let php = r#"<?php
3413+
trait SalesInfoGlobalTrait {
3414+
public function updateSalesInfo(): void {
3415+
$items = array_map(function ($item) {
3416+
static::where('col', 'val')->update(['x' => 1]);
3417+
}, []);
3418+
}
3419+
}
3420+
3421+
class SalesReport extends \Illuminate\Database\Eloquent\Model {
3422+
use SalesInfoGlobalTrait;
3423+
}
3424+
"#;
3425+
let backend = Backend::new_test();
3426+
let diags = collect(&backend, "file:///test.php", php);
3427+
assert!(
3428+
diags.is_empty(),
3429+
"expected no diagnostics for static:: chain inside closure in trait, got: {diags:?}"
3430+
);
3431+
}
3432+
3433+
#[test]
3434+
fn no_diagnostic_for_self_chain_inside_trait() {
3435+
// B3: `self::create(...)` chain inside a trait.
3436+
let php = r#"<?php
3437+
trait Creatable {
3438+
public function duplicate(): void {
3439+
self::create(['name' => 'copy'])->save();
3440+
}
3441+
}
3442+
3443+
class Product extends \Illuminate\Database\Eloquent\Model {
3444+
use Creatable;
3445+
}
3446+
"#;
3447+
let backend = Backend::new_test();
3448+
let diags = collect(&backend, "file:///test.php", php);
3449+
assert!(
3450+
diags.is_empty(),
3451+
"expected no diagnostics for self::...->method() chain inside trait, got: {diags:?}"
3452+
);
3453+
}
3454+
3455+
#[test]
3456+
fn variable_chain_inside_trait_still_diagnosed() {
3457+
// Non-self-referencing variables inside traits should still be
3458+
// diagnosed when the member truly doesn't exist.
3459+
let php = r#"<?php
3460+
trait BadTrait {
3461+
public function doStuff(): void {
3462+
$obj = new \stdClass();
3463+
$obj->nonExistentMethod();
3464+
}
3465+
}
3466+
"#;
3467+
let backend = Backend::new_test();
3468+
let _diags = collect(&backend, "file:///test.php", php);
3469+
// stdClass has __get/__set magic, so property access is fine,
3470+
// but we're just verifying the suppression doesn't swallow
3471+
// non-self-referencing subjects. stdClass actually tolerates
3472+
// all member access, so this test verifies the suppression
3473+
// is scoped to self-referencing subjects only.
3474+
// (No assertion on diagnostic count — stdClass has magic methods.)
3475+
}
3476+
32723477
#[test]
32733478
fn flags_unknown_member_despite_valid_in_other_method() {
32743479
// The flip side of B1: make sure that a member that IS valid in

0 commit comments

Comments
 (0)