Skip to content

Commit 6267022

Browse files
committed
Preserve method chain type through __call fallback
1 parent 3a0f70f commit 6267022

10 files changed

Lines changed: 1427 additions & 92 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5252
- **Diagnostics now work for vendor files open in the editor.** Projects using `--prefer-source` or monorepo setups where libraries are edited from the `vendor/` directory no longer have diagnostics suppressed. Previously all vendor files were unconditionally skipped, hiding syntax errors, deprecation warnings, and all other diagnostics. The `analyze` command also accepts vendor paths as explicit overrides (e.g. `phpantom_lsp analyze vendor/some-package/src/`).
5353
- **Full-line diagnostic suppression no longer requires a code mapping.** Full-line diagnostics (from tools like PHPStan that only report line numbers) are now suppressed whenever any precise diagnostic exists on the same line, regardless of error codes. Previously suppression relied on a hand-maintained mapping between error codes from different tools, which was incomplete and allowed redundant full-line underlines to obscure the precise location of the issue.
5454

55+
- **Method chains through `__call` no longer lose the return type.** When a class defines `__call` and an unrecognized method is called (e.g. dynamic `whereColumn()` on an Eloquent Builder), the return type of `__call` is now used as a fallback. If `__call` returns `$this`, `static`, or `self`, the chain type is preserved so subsequent calls continue resolving. Previously the type was lost at the first dynamic call, breaking every link after it.
5556
- **Null narrowing from `!== null` checks in conditions.** When a null-initialized variable was guarded by `$var !== null` in an `if` or `while` condition, the variable still showed `null` in its type inside the condition's `&&` operands and inside the then-body. The `!== null` check (and `!is_null()`, bare truthy guards) now narrows away `null` both for subsequent `&&` operands and inside the corresponding body block. Chained conditions like `$a !== null && $b !== null && $a->method()` narrow all checked variables. Conditions wrapped inside ternary expressions and return statements are also handled.
5657
- **Variables assigned inside `if`/`while` conditions now resolve in the body.** `if ($admin = AdminUser::first())` and `while ($row = nextRow())` now register the assignment so the variable has a type inside the loop or branch body. Assignments wrapped in comparisons like `if (($conn = getConn()) !== null)` are also recognized.
5758
- **Fluent chains only flag the first broken link.** In a chain like `$m->callHome()->callMom()->callDad()` where `callHome` does not exist, only `callHome` is flagged. Previously every subsequent link received its own "cannot verify" warning, burying the root cause in noise. Separate statements on the same variable (`$m->callHome(); $m->callMom();`) still flag independently. Scalar member access chains (`$user->getAge()->value->deep`) flag only the first scalar break. Null-safe, static, and mixed-operator chains are all handled.

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ within the same impact tier.
2424
| # | Item | Impact | Effort |
2525
| --- | ------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ---------- |
2626
| T18 | [Method-level template parameter resolution at call sites](todo/type-inference.md#t18-method-level-template-parameter-resolution-at-call-sites) | Medium | Medium |
27-
| B21 | [Builder `__call` return type drops chain type for dynamic `where{Column}` calls](todo/bugs.md#b21-builder-__call-return-type-drops-chain-type-for-dynamic-wherecolumn-calls) | Medium | Medium |
2827
| H17 | [`missingType.iterableValue` — add `@return` with inferred element type](todo/phpstan-actions.md#h17-missingtype-iterablevalue-return-type--add-return-with-iterable-type) | Medium | High |
2928
| H10 | [`return.unusedType` — remove unused type from return union](todo/phpstan-actions.md#h10-returnunusedtype--remove-unused-type-from-return-union) | Medium | Medium |
3029
| L12 | [`App::make` / `App::makeWith` class-string return type dispatch](todo/laravel.md#l12-appmake--appmakewith-class-string-return-type-dispatch) | Medium | Low |

docs/todo/bugs.md

Lines changed: 1 addition & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1 @@
1-
# PHPantom — Bug Fixes
2-
3-
#### B21. Builder `__call` return type drops chain type for dynamic `where{Column}` calls
4-
5-
| | |
6-
|---|---|
7-
| **Impact** | Medium |
8-
| **Effort** | Medium |
9-
10-
Eloquent's `Builder::__call()` intercepts calls like `whereColumnName()`
11-
and forwards them to `where('column_name', ...)`. PHPantom correctly
12-
suppresses the "unknown member" diagnostic because `Builder` has `__call`,
13-
but the return type of the magic call is lost. This breaks every
14-
subsequent link in the chain.
15-
16-
**Reproducer:**
17-
18-
```php
19-
// SubcategoryView has scopeWhereLanguage but NOT scopeWhereSubcategoryId.
20-
// whereSubcategoryId() is a dynamic where{Column} via Builder::__call.
21-
$view = SubcategoryView::whereLanguage($lang)
22-
->whereSubcategoryId($id) // accepted (Builder has __call), but return type lost
23-
->first(); // diagnostic: "subject type could not be resolved"
24-
```
25-
26-
**Expected:** When `__call` is invoked on an Eloquent `Builder<TModel>`,
27-
the return type should be `Builder<TModel>` (i.e. `$this`), preserving
28-
the generic parameter so the chain continues resolving.
29-
30-
**Root cause:** `has_magic_method_for_access` in `unknown_members.rs`
31-
correctly detects `__call` and suppresses the diagnostic, but the
32-
call-resolution pipeline in `call_resolution.rs` does not attempt to
33-
derive a return type from `__call`. For Eloquent builders specifically,
34-
any unrecognised instance method call should return `$this` (the
35-
builder), since nearly all `Builder` methods are fluent.
36-
37-
**Where to fix:**
38-
- `src/completion/call_resolution.rs` — when resolving a method call
39-
that is not found on the class but the class has `__call`, check
40-
whether the class is an Eloquent `Builder` (or extends one). If so,
41-
return `$this` / the builder type as the call's return type instead
42-
of giving up.
43-
- Alternatively, add a fallback in `resolve_method_return_types_with_args`
44-
that checks for `__call` and uses its declared return type (or `$this`
45-
for known builder classes).
46-
47-
**Impact in shared codebase:** ~5 diagnostics (direct chain breaks after
48-
dynamic `where{Column}` calls, plus downstream cascading failures).
49-
50-
**Discovered in:** analyze-triage iteration 10.
1+
# PHPantom — Bug Fixes

src/completion/call_resolution.rs

Lines changed: 267 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ pub(super) struct MethodReturnCtx<'a> {
6161
/// appears), as opposed to the class that owns the method being called.
6262
/// Used to resolve `self`/`static`/`parent` in conditional return types.
6363
pub calling_class_name: Option<&'a str>,
64+
/// Whether the call is a static method call (`Class::method()`).
65+
///
66+
/// When `true`, the magic-method fallback checks `__callStatic`
67+
/// instead of `__call`.
68+
pub is_static: bool,
6469
}
6570

6671
/// Build a [`VarClassStringResolver`] closure from a [`ResolutionCtx`].
@@ -409,6 +414,7 @@ impl Backend {
409414
var_resolver: Some(&var_resolver),
410415
cache: ctx.resolved_class_cache,
411416
calling_class_name: ctx.current_class.map(|c| c.name.as_str()),
417+
is_static: false,
412418
};
413419
results.extend(Self::resolve_method_return_types_with_args(
414420
owner,
@@ -444,6 +450,7 @@ impl Backend {
444450
var_resolver: Some(&var_resolver),
445451
cache: ctx.resolved_class_cache,
446452
calling_class_name: ctx.current_class.map(|c| c.name.as_str()),
453+
is_static: true,
447454
};
448455
return Self::resolve_method_return_types_with_args(
449456
owner,
@@ -841,6 +848,15 @@ impl Backend {
841848
vec![]
842849
};
843850

851+
// Determine which magic method handles unknown calls for this
852+
// access kind: `__call` for instance calls, `__callStatic` for
853+
// static calls.
854+
let magic_name = if mr_ctx.is_static {
855+
"__callStatic"
856+
} else {
857+
"__call"
858+
};
859+
844860
// First check the class itself
845861
if let Some(method) = class_info.methods.iter().find(|m| m.name == method_name) {
846862
let result = resolve_method(method);
@@ -859,13 +875,263 @@ impl Backend {
859875
class_loader,
860876
mr_ctx.cache,
861877
);
878+
879+
// Look up the magic method once; used for both validation and
880+
// fallback below.
881+
let magic_method = merged
882+
.methods
883+
.iter()
884+
.find(|m| m.name.eq_ignore_ascii_case(magic_name));
885+
862886
if let Some(method) = merged.methods.iter().find(|m| m.name == method_name) {
863-
return resolve_method(method);
887+
if method.is_virtual {
888+
// ── Virtual method (from @method, @mixin, etc.) ─────
889+
// At runtime these are dispatched through __call /
890+
// __callStatic. Validate the virtual method's return
891+
// type against the magic method's native return type
892+
// the same way we validate a concrete implementation
893+
// against an interface: the virtual type can only
894+
// *narrow* the native constraint, not contradict it.
895+
if let Some(ref virtual_ret) = method.return_type {
896+
if let Some(magic) = magic_method {
897+
if let Some(ref native_ret) = magic.native_return_type {
898+
// The magic method has a native PHP type
899+
// hint. Check whether the virtual
900+
// method's declared type is a valid
901+
// narrowing of that native constraint.
902+
if is_valid_virtual_narrowing(
903+
virtual_ret,
904+
native_ret,
905+
class_info,
906+
all_classes,
907+
class_loader,
908+
) {
909+
// Valid narrowing — trust the virtual
910+
// method's declared type.
911+
let result = resolve_method(method);
912+
if !result.is_empty() {
913+
return result;
914+
}
915+
}
916+
// Invalid narrowing (lie) or the virtual
917+
// type failed to resolve. Fall through
918+
// to the magic-method fallback below,
919+
// which will use __call's own return type.
920+
} else {
921+
// Magic method has no native type hint —
922+
// trust the virtual method's declared type.
923+
let result = resolve_method(method);
924+
if !result.is_empty() {
925+
return result;
926+
}
927+
}
928+
} else {
929+
// No magic method at all — trust the virtual
930+
// method's declared type unconditionally.
931+
let result = resolve_method(method);
932+
if !result.is_empty() {
933+
return result;
934+
}
935+
}
936+
}
937+
// Virtual method with no return type (or whose type
938+
// was rejected by the validation above). Fall through
939+
// to the magic-method fallback below.
940+
} else {
941+
// ── Real method ─────────────────────────────────────
942+
// Real methods are invoked directly at runtime, never
943+
// through __call. Use whatever resolve_method
944+
// returns, even if empty.
945+
return resolve_method(method);
946+
}
947+
}
948+
949+
// ── Magic-method fallback ───────────────────────────────
950+
// Either the method was not found at all, or it was a virtual
951+
// method whose return type was absent or rejected by the
952+
// native-type validation. Use the magic method's effective
953+
// return type (docblock-overridden if available, otherwise
954+
// native). When the magic method returns `$this`/`static`/
955+
// `self`, this preserves the chain type (e.g. Builder<User>
956+
// stays Builder<User> through dynamic `where{Column}` calls).
957+
// When it returns `mixed`, no classes resolve and the caller
958+
// gets an empty vec — the same as before this fallback.
959+
if let Some(magic) = magic_method {
960+
let result = resolve_method(magic);
961+
if !result.is_empty() {
962+
return result;
963+
}
864964
}
865965

866966
vec![]
867967
}
968+
}
969+
970+
// ─── Virtual method narrowing ───────────────────────────────────────────────
971+
972+
/// Check whether a virtual method's return type is a valid narrowing of a
973+
/// magic method's (`__call` / `__callStatic`) native return type.
974+
///
975+
/// At runtime, calls to virtual methods (from `@method` tags, `@mixin`
976+
/// members, etc.) are dispatched through the magic method. The magic
977+
/// method's native PHP type hint is the runtime truth: the virtual
978+
/// method's declared type can only *narrow* it (provide a more specific
979+
/// subtype), not contradict it.
980+
///
981+
/// Returns `true` when the virtual type should be trusted, `false` when
982+
/// it should be rejected in favour of the magic method's type.
983+
///
984+
/// # Examples
985+
///
986+
/// | `__call` native | `@method` type | Result |
987+
/// |-----------------|----------------|--------|
988+
/// | `mixed` | `Frog` | ✓ (anything narrows mixed) |
989+
/// | `object` | `Frog` | ✓ (any class narrows object) |
990+
/// | `static` | `ChildClass` | ✓ if ChildClass extends the owner |
991+
/// | `Animal` | `Dog` | ✓ if Dog extends Animal |
992+
/// | `Cement` | `Frog` | ✗ (unrelated classes) |
993+
/// | `static` | `Frog` | ✗ if Frog does not extend the owner |
994+
/// | `int` | `string` | ✗ (incompatible scalars) |
995+
fn is_valid_virtual_narrowing(
996+
virtual_type: &PhpType,
997+
native_type: &PhpType,
998+
owner_class: &ClassInfo,
999+
all_classes: &[Arc<ClassInfo>],
1000+
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
1001+
) -> bool {
1002+
let native_str = native_type.to_string();
1003+
let native_lower = native_str.to_ascii_lowercase();
1004+
1005+
// `mixed` and `void` impose no constraint — any type is valid.
1006+
if matches!(native_lower.as_str(), "mixed" | "void") {
1007+
return true;
1008+
}
1009+
1010+
// `object` — any class type is a valid narrowing.
1011+
if native_lower == "object" {
1012+
// Only reject if the virtual type is a non-object scalar.
1013+
return !virtual_type.is_scalar();
1014+
}
1015+
1016+
// Self-like types (`static`, `self`, `$this`) resolve to the owner
1017+
// class at runtime. The virtual type must be the owner class itself
1018+
// or a subclass of it.
1019+
if is_self_like_type(native_type) {
1020+
return is_type_subclass_of(virtual_type, &owner_class.name, all_classes, class_loader);
1021+
}
1022+
1023+
// Both are concrete types. For scalar-to-scalar, delegate to the
1024+
// existing `should_override_type` check which handles compatible
1025+
// refinements (e.g. `string` → `class-string<T>`).
1026+
if native_type.is_scalar() {
1027+
let virtual_str = virtual_type.to_string();
1028+
return crate::docblock::should_override_type(&virtual_str, &native_str);
1029+
}
1030+
1031+
// Native is a class type — the virtual type must be the same class
1032+
// or a subclass.
1033+
is_type_subclass_of(virtual_type, &native_str, all_classes, class_loader)
1034+
}
1035+
1036+
/// Check whether `candidate_type` is the same class as `ancestor_name` or
1037+
/// a subclass of it, by walking the parent chain.
1038+
///
1039+
/// Returns `true` when:
1040+
/// - The candidate type's base name matches `ancestor_name` (case-insensitive).
1041+
/// - The candidate class's parent chain includes `ancestor_name`.
1042+
/// - The candidate class cannot be resolved (benefit of the doubt).
1043+
fn is_type_subclass_of(
1044+
candidate_type: &PhpType,
1045+
ancestor_name: &str,
1046+
all_classes: &[Arc<ClassInfo>],
1047+
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
1048+
) -> bool {
1049+
let candidate_str = candidate_type.to_string();
1050+
1051+
// Strip leading backslash and generic parameters for name comparison.
1052+
let candidate_base = candidate_str
1053+
.strip_prefix('\\')
1054+
.unwrap_or(&candidate_str)
1055+
.split('<')
1056+
.next()
1057+
.unwrap_or(&candidate_str);
1058+
let ancestor_base = ancestor_name
1059+
.strip_prefix('\\')
1060+
.unwrap_or(ancestor_name)
1061+
.split('<')
1062+
.next()
1063+
.unwrap_or(ancestor_name);
1064+
1065+
// Same class (case-insensitive, ignoring namespace prefix differences).
1066+
if candidate_base.eq_ignore_ascii_case(ancestor_base) {
1067+
return true;
1068+
}
1069+
// Also check short-name match (e.g. "Dog" vs "App\\Models\\Dog").
1070+
let candidate_short = crate::util::short_name(candidate_base);
1071+
let ancestor_short = crate::util::short_name(ancestor_base);
1072+
if candidate_short.eq_ignore_ascii_case(ancestor_short) {
1073+
return true;
1074+
}
8681075

1076+
// Try to load the candidate class and walk its parent chain.
1077+
let candidate_class = find_class_by_name(all_classes, candidate_base)
1078+
.cloned()
1079+
.or_else(|| class_loader(candidate_base));
1080+
1081+
let Some(cls) = candidate_class else {
1082+
// Cannot resolve the candidate class — give benefit of the
1083+
// doubt and trust the @method tag.
1084+
return true;
1085+
};
1086+
1087+
// Walk the parent chain up to a reasonable depth.
1088+
let mut current_parent = cls.parent_class.clone();
1089+
let mut depth = 0u32;
1090+
while let Some(ref parent_name) = current_parent {
1091+
depth += 1;
1092+
if depth > 20 {
1093+
break;
1094+
}
1095+
let parent_base = parent_name
1096+
.strip_prefix('\\')
1097+
.unwrap_or(parent_name)
1098+
.split('<')
1099+
.next()
1100+
.unwrap_or(parent_name);
1101+
if parent_base.eq_ignore_ascii_case(ancestor_base)
1102+
|| crate::util::short_name(parent_base).eq_ignore_ascii_case(ancestor_short)
1103+
{
1104+
return true;
1105+
}
1106+
// Also check implemented interfaces on the parent.
1107+
let parent_class = find_class_by_name(all_classes, parent_base)
1108+
.cloned()
1109+
.or_else(|| class_loader(parent_base));
1110+
match parent_class {
1111+
Some(p) => current_parent = p.parent_class.clone(),
1112+
None => break,
1113+
}
1114+
}
1115+
1116+
// Also check the candidate's own implemented interfaces.
1117+
for iface in &cls.interfaces {
1118+
let iface_base = iface
1119+
.strip_prefix('\\')
1120+
.unwrap_or(iface)
1121+
.split('<')
1122+
.next()
1123+
.unwrap_or(iface);
1124+
if iface_base.eq_ignore_ascii_case(ancestor_base)
1125+
|| crate::util::short_name(iface_base).eq_ignore_ascii_case(ancestor_short)
1126+
{
1127+
return true;
1128+
}
1129+
}
1130+
1131+
false
1132+
}
1133+
1134+
impl Backend {
8691135
/// Build a template substitution map for a method-level `@template` call.
8701136
///
8711137
/// Finds the method on the class (or inherited), checks for template

0 commit comments

Comments
 (0)