Skip to content

Commit 108c175

Browse files
committed
Fix instanceof narrowing with unresolvable target class
1 parent 7230d35 commit 108c175

18 files changed

Lines changed: 558 additions & 326 deletions

docs/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- **`instanceof` narrowing with unresolvable target class.** When the `instanceof` target class cannot be loaded (e.g. it lives inside a phar archive), the variable's type is now treated as unknown instead of keeping the un-narrowed base type. This eliminates false-positive "unknown member" diagnostics for members that only exist on the narrowed subclass. Applies to all narrowing contexts: if-bodies, ternary expressions, `assert()` statements, and inline `&&` chains.
1213
- **`DB::select()` return type.** `DB::select()`, `DB::selectFromWriteConnection()`, and `DB::selectResultSets()` now return `array<int, stdClass>` instead of bare `array`, and `DB::selectOne()` returns `?stdClass` instead of `mixed`. Property access on query results (e.g. `$result[0]->column_name`) no longer produces false-positive diagnostics. The same fix applies to the underlying `Illuminate\Database\Connection` class.
1314
- **Redis `Connection` method resolution.** `Illuminate\Redis\Connections\Connection` now has `@mixin \Redis` applied automatically, so Redis commands like `del()`, `get()`, `set()`, etc. resolve through the phpredis stubs instead of producing false-positive diagnostics.
1415

@@ -57,6 +58,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5758

5859
### Changed
5960

61+
- **Diagnostics.** Unified the diagnostic type-resolution pipeline with the completion and hover pipeline. Variable resolution now produces the same result in all three contexts, eliminating a class of false positives where diagnostics disagreed with completion about a variable's type.
6062
- **`@phpstan-ignore` is never the preferred quickfix.** The "Ignore PHPStan error" code action now explicitly sets `is_preferred: false`. Previously it used `None`, which some editors treated as absent, causing the suppress-with-comment action to be applied on the keyboard shortcut (e.g. Ctrl+. then Enter) when no other quickfix set `is_preferred`.
6163

6264
- **Generate PHPDoc infers `@return` from the function body.** Typing `/**` above a function that returns `array` now produces `@return list<string>` (or whatever the body actually returns) instead of the previous `@return array<mixed>`. The same return-statement scanning used by the `missingType.return` and `missingType.iterableValue` code actions is now shared with docblock generation.

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ within the same impact tier.
2323

2424
| # | Item | Impact | Effort |
2525
| --- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ---------- |
26-
| B10 | [`instanceof` ternary narrowing fails when target class is in a phar](todo/bugs.md#b10-instanceof-ternary-narrowing-fails-when-target-class-is-in-a-phar) | Low | Low-Medium |
2726
| B12 | [`Collection::reduce()` generic return type not inferred](todo/bugs.md#b12-collectionreduce-generic-return-type-not-inferred) | Low | Medium |
2827
| B13 | [Array shape tracking from keyed literal assignments in loops](todo/bugs.md#b13-array-shape-tracking-from-keyed-literal-assignments-in-loops) | Low | 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 |

docs/todo/bugs.md

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,5 @@
11
# PHPantom — Bug Fixes
22

3-
## B10. `instanceof` ternary narrowing fails when target class is in a phar
4-
**Impact: Low · Effort: Low-Medium**
5-
6-
Pattern:
7-
```php
8-
$types = $argType instanceof UnionType ? $argType->getTypes() : [$argType];
9-
```
10-
11-
The type engine is unified: completion and diagnostics both use
12-
`resolve_variable_types``walk_statements_for_assignments`
13-
`try_apply_ternary_instanceof_narrowing`. A local-class test
14-
(assignment RHS context, interface parameter, `instanceof` subclass
15-
in ternary condition) passes with zero diagnostics.
16-
17-
The failure in the shared project is specific to PHPStan phar classes.
18-
`PHPStan\Type\UnionType` lives inside `phpstan.phar` and may not be
19-
loadable at resolution time, so `apply_instanceof_inclusion` silently
20-
fails (cannot resolve the class name to a `ClassInfo`), and the
21-
variable keeps its un-narrowed type (`Type`).
22-
23-
**Observed in:** `DecimalDivThrowTypeExtension:54``$argType` is
24-
typed as `PHPStan\Type\Type`, `getTypes()` exists on `UnionType` but
25-
not on the `Type` interface. The `instanceof UnionType` check in the
26-
ternary condition should narrow the type in the then-branch.
27-
28-
**Root cause:** The `instanceof` target class (`UnionType`) cannot be
29-
loaded from the phar, so `apply_instanceof_inclusion` has no
30-
`ClassInfo` to narrow to. The narrowing architecture itself is correct
31-
and unified across completion and diagnostics. The fix is either
32-
phar class indexing or suppressing diagnostics when the `instanceof`
33-
target class is unresolvable (since the developer clearly expects
34-
narrowing to occur).
35-
36-
---
37-
383
## B12. `Collection::reduce()` generic return type not inferred
394
**Impact: Low · Effort: Medium**
405

@@ -101,5 +66,4 @@ unresolvable because the shape is lost.
10166

10267
**Depends on:** T19 (structured type representation) or at minimum
10368
a basic array shape inference that preserves `array{key: Type}` from
104-
literal array constructors and propagates it through foreach.
105-
69+
literal array constructors and propagates it through foreach.

src/completion/call_resolution.rs

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,11 @@ impl Backend {
144144
.into_iter()
145145
.collect()
146146
} else {
147-
super::resolver::resolve_target_classes(&subject_text, crate::AccessKind::Arrow, rctx)
147+
ResolvedType::into_arced_classes(super::resolver::resolve_target_classes(
148+
&subject_text,
149+
crate::AccessKind::Arrow,
150+
rctx,
151+
))
148152
};
149153

150154
for owner in &owner_classes {
@@ -399,8 +403,9 @@ impl Backend {
399403
let method_name = method.as_str();
400404

401405
// Resolve the base expression to class(es).
402-
let lhs_classes: Vec<Arc<ClassInfo>> =
403-
super::resolver::resolve_target_classes_expr(base, AccessKind::Arrow, ctx);
406+
let lhs_classes: Vec<Arc<ClassInfo>> = ResolvedType::into_arced_classes(
407+
super::resolver::resolve_target_classes_expr(base, AccessKind::Arrow, ctx),
408+
);
404409

405410
let mut results = Vec::new();
406411
for owner in &lhs_classes {
@@ -432,9 +437,13 @@ impl Backend {
432437

433438
let owner_class = if class.starts_with('$') {
434439
// Variable holding a class-string (e.g. `$cls::make()`).
435-
super::resolver::resolve_target_classes(class, AccessKind::DoubleColon, ctx)
436-
.into_iter()
437-
.next()
440+
ResolvedType::into_arced_classes(super::resolver::resolve_target_classes(
441+
class,
442+
AccessKind::DoubleColon,
443+
ctx,
444+
))
445+
.into_iter()
446+
.next()
438447
} else {
439448
super::resolver::resolve_static_owner_class(class, ctx)
440449
};
@@ -659,8 +668,9 @@ impl Backend {
659668
// 4. Resolve the variable's type and check for __invoke().
660669
// When $f holds an object with an __invoke() method,
661670
// $f() should return __invoke()'s return type.
662-
let var_classes =
663-
super::resolver::resolve_target_classes(var_name, AccessKind::Arrow, ctx);
671+
let var_classes = ResolvedType::into_arced_classes(
672+
super::resolver::resolve_target_classes(var_name, AccessKind::Arrow, ctx),
673+
);
664674
for owner in &var_classes {
665675
if let Some(invoke) = owner.methods.iter().find(|m| m.name == "__invoke")
666676
&& let Some(ref ret) = invoke.return_type
@@ -699,8 +709,9 @@ impl Backend {
699709
// from a function name) ───────────────────────────────
700710
_ => {
701711
// Resolve the callee expression to class(es).
702-
let callee_classes =
703-
super::resolver::resolve_target_classes_expr(callee, AccessKind::Arrow, ctx);
712+
let callee_classes = ResolvedType::into_arced_classes(
713+
super::resolver::resolve_target_classes_expr(callee, AccessKind::Arrow, ctx),
714+
);
704715

705716
// When the callee resolves to an object with __invoke(),
706717
// the call returns __invoke()'s return type, not the
@@ -1308,7 +1319,13 @@ impl Backend {
13081319
ctx,
13091320
);
13101321
if let Some(first) = classes.first() {
1311-
return Some(first.name.clone());
1322+
return Some(
1323+
first
1324+
.class_info
1325+
.as_ref()
1326+
.map(|ci| ci.name.clone())
1327+
.unwrap_or_else(|| first.type_string.to_string()),
1328+
);
13121329
}
13131330
}
13141331

@@ -1402,8 +1419,9 @@ impl Backend {
14021419
.unwrap_or(&call_body[..pos]);
14031420
let method_name = &call_body[pos + 2..];
14041421

1405-
let lhs_classes =
1406-
super::resolver::resolve_target_classes(lhs, AccessKind::Arrow, ctx);
1422+
let lhs_classes = ResolvedType::into_arced_classes(
1423+
super::resolver::resolve_target_classes(lhs, AccessKind::Arrow, ctx),
1424+
);
14071425
for cls in &lhs_classes {
14081426
if let Some(rt) = crate::inheritance::resolve_method_return_type(
14091427
cls,
@@ -1447,8 +1465,9 @@ impl Backend {
14471465
.unwrap_or(&arg_text[..pos]);
14481466
let prop_name = &arg_text[pos + 2..];
14491467
if !prop_name.is_empty() && prop_name.chars().all(|c| c.is_alphanumeric() || c == '_') {
1450-
let lhs_classes =
1451-
super::resolver::resolve_target_classes(lhs, AccessKind::Arrow, ctx);
1468+
let lhs_classes = ResolvedType::into_arced_classes(
1469+
super::resolver::resolve_target_classes(lhs, AccessKind::Arrow, ctx),
1470+
);
14521471
for cls in &lhs_classes {
14531472
if let Some(rt) =
14541473
crate::inheritance::resolve_property_type_hint(cls, prop_name, class_loader)

src/completion/handler.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ use crate::completion::class_completion::{
4141
use crate::completion::named_args::{NamedArgContext, parse_existing_args};
4242
use crate::docblock::types::PHPDOC_TYPE_KEYWORDS;
4343
use crate::symbol_map::SymbolKind;
44-
use crate::types::ClassInfo;
44+
use crate::types::{ClassInfo, ResolvedType};
4545
use crate::types::{CompletionTarget, FileContext};
4646
use crate::util::{find_class_at_offset, position_to_byte_offset, position_to_offset};
4747

@@ -911,7 +911,7 @@ impl Backend {
911911
}
912912
}
913913

914-
resolved
914+
ResolvedType::into_arced_classes(resolved)
915915
};
916916
if candidates.is_empty() {
917917
return vec![];

0 commit comments

Comments
 (0)