Skip to content

Commit 319c4f5

Browse files
committed
Fix 4 bugs detected by PHPactor's tests
1 parent 2bf56f0 commit 319c4f5

16 files changed

Lines changed: 224 additions & 139 deletions

docs/CHANGELOG.md

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

1414
### Fixed
1515

16+
- **`@phpstan-type` aliases in foreach.** Type aliases defined via `@phpstan-type` or `@psalm-type` now resolve correctly when the aliased type is iterated in a `foreach` loop, destructured with `list()`/`[]`, or used as a foreach key type.
17+
- **Mixed `->` then `::` accessor chains.** Expressions like `$obj->prop::$staticProp` and `$obj->method()::staticMethod()` now resolve through the full chain instead of losing the instance prefix.
18+
- **Inline `(new Foo)->method()` chaining.** Parenthesized `new` expressions used as the root of a method chain now resolve for completion. Previously only assigned `new` expressions (`$x = new Foo()`) worked.
19+
- **Literal string conditional return types.** Conditional return types that check against a literal string value (`$param is "foo" ? A : B`) now resolve the correct branch when the call-site argument is a matching string literal.
1620
- **Inherited `@method` and `@property` tags.** Virtual members declared via `@method` or `@property` on a parent class now appear on child classes. Previously only the declaring class itself surfaced these members.
1721
- **Class constant and enum case assignment resolution.** Assigning from a class constant (`$x = Foo::SOME_CONST`) or enum case (`$x = Status::Active`) now resolves the variable's type correctly for subsequent completion.
1822
- **Sequential assert narrowing.** Multiple `assert($x instanceof A); assert($x instanceof B);` statements now accumulate, showing members from both types. Previously only the last assertion's narrowing applied.

docs/todo/bugs.md

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -138,33 +138,6 @@ error message.
138138

139139
---
140140

141-
## 10. Mixed arrow then static accessor chaining not resolved
142-
**Impact: Low · Effort: Low**
143-
144-
Chaining `$obj->prop::$staticProp` or `$obj->method()::staticMethod()`
145-
is not resolved. The subject extractor does not handle a transition from
146-
`->` to `::` within the same chain.
147-
148-
**Discovered via:** fixture conversion (completion/static_prop_after_arrow).
149-
150-
---
151-
152-
## 12. Inline `(new Foo)->method()` chaining not resolved
153-
**Impact: Medium · Effort: Low-Medium**
154-
155-
Parenthesized `new` expressions used as the start of a chain are not
156-
resolved for completion:
157-
158-
```php
159-
(new Foo())->method()-><cursor>
160-
```
161-
162-
The parenthesized `new` expression is handled for simple variable
163-
assignment (`$x = (new Foo())`), but not when it appears inline as the
164-
root of a chain that feeds into completion or further resolution.
165-
166-
---
167-
168141
## 13. Evict transiently-loaded files from ast_map after GTI and Find References
169142
**Impact: Low · Effort: Low**
170143

docs/todo/testing.md

Lines changed: 7 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
# PHPantom — Ignored Fixture Tasks
22

3-
There are **228 fixture tests** in `tests/fixtures/`. Of these, **183
4-
pass** and **45 are ignored** because they exercise features or bug
3+
There are **228 fixture tests** in `tests/fixtures/`. Of these, **188
4+
pass** and **40 are ignored** because they exercise features or bug
55
fixes that are not yet implemented. Each ignored fixture has a
66
`// ignore:` comment explaining what is missing.
77

8-
This document groups the 45 ignored fixtures by the underlying work
8+
This document groups the 40 ignored fixtures by the underlying work
99
needed to un-ignore them. Tasks are ordered by the number of fixtures
1010
they unblock (descending), then by estimated effort. Once a task is
1111
complete, remove the `// ignore:` line from each fixture, verify the
@@ -218,29 +218,6 @@ name is a variable, resolve the variable's type. If it is
218218

219219
---
220220

221-
## 10. Mixed `->` then `::` accessor chaining (2 fixtures)
222-
223-
**Ref:** [bugs.md §10](bugs.md#10-mixed-arrow-then-static-accessor-chaining-not-resolved)
224-
**Impact: Low · Effort: Low**
225-
226-
`$obj->prop::$staticProp` and `$obj->method()::staticMethod()` are not
227-
resolved. The subject extractor does not handle a transition from `->`
228-
to `::` within the same chain.
229-
230-
**Fixtures:**
231-
232-
- [ ] `completion/static_prop_after_arrow.fixture``$obj->prop::$staticProp` chain
233-
- [ ] `member_access/static_property_instance.fixture` — same pattern in member_access context
234-
235-
**Implementation notes:**
236-
237-
In subject extraction (or the AST-based chain walker), when processing
238-
a chain segment that switches from instance (`->`) to static (`::`)
239-
access, resolve the instance segment first, then use its result type as
240-
the class for the static access.
241-
242-
---
243-
244221
## 11. `class-string<T>` on interface method not inherited (1 fixture)
245222

246223
**Ref:** [type-inference.md §25](type-inference.md#25-class-stringt-on-interface-method-not-inherited)
@@ -303,21 +280,6 @@ class-string argument at the call site.
303280

304281
---
305282

306-
## 19. Inline `(new Foo)->method()` chaining (1 fixture)
307-
308-
**Ref:** [bugs.md §12](bugs.md#12-inline-new-foo-method-chaining-not-resolved)
309-
**Impact: Medium · Effort: Low-Medium**
310-
311-
Parenthesized `new` expressions used inline as the root of a method
312-
chain do not resolve for completion. `$x = (new Foo())` works, but
313-
`(new Foo())->method()->` does not.
314-
315-
**Fixture:**
316-
317-
- [ ] `member_access/new_no_parenthesis.fixture``(new Foo)->bar()->` resolves
318-
319-
---
320-
321283
## 20. Elseif chain narrowing with `is_*()` (1 fixture)
322284

323285
**Ref:** [type-inference.md §3](type-inference.md#3-parse-and-resolve-param-is-t--a--b-return-types) (related)
@@ -350,36 +312,6 @@ existing `array_pop`/`array_shift` handling.
350312

351313
---
352314

353-
## 22. Literal string conditional return type (1 fixture)
354-
355-
**Ref:** [type-inference.md §24](type-inference.md#24-literal-string-conditional-return-type)
356-
**Impact: Low · Effort: Low-Medium**
357-
358-
Conditional return types using literal string comparison
359-
(`$param is "foo"`) are not resolved. Only class/interface type
360-
conditions work today.
361-
362-
**Fixture:**
363-
364-
- [ ] `type/conditional_return_type_string.fixture` — literal string conditional resolves correct branch
365-
366-
---
367-
368-
## 23. `@phpstan-type` alias in foreach context (1 fixture)
369-
370-
**Ref:** [type-inference.md §29](type-inference.md#29-phpstan-type-alias-in-foreach-context)
371-
**Impact: Low · Effort: Low**
372-
373-
When a method's return type uses a `@phpstan-type` alias and the result
374-
is iterated in a `foreach`, the alias is not resolved before extracting
375-
the foreach value type.
376-
377-
**Fixture:**
378-
379-
- [ ] `type/phpstan_type_alias.fixture` — type alias resolves for foreach iteration
380-
381-
---
382-
383315
## 24. Variable scope isolation in closures (1 fixture)
384316

385317
**Impact: Low · Effort: Low-Medium**
@@ -435,17 +367,13 @@ argument to the right side and returns the result.
435367

436368
## Summary by effort
437369

438-
Quick wins (Low effort, 1 fixture each):
439-
440-
| Task | Fixture |
441-
|---|---|
442-
| §23 `@phpstan-type` in foreach | `type/phpstan_type_alias` |
443-
444-
Moderate wins (Low-Medium effort, multiple fixtures):
370+
Moderate wins (Low-Medium effort, few fixtures):
445371

446372
| Task | Fixtures |
447373
|---|---|
448-
| §10 Mixed `->` then `::` chaining | 2 |
374+
| §24 Variable scope isolation in closures | 1 |
375+
| §25 Pass-by-reference parameter type inference | 1 |
376+
| §26 Pipe operator (PHP 8.5) | 1 |
449377

450378
Biggest unlocks (Medium effort, many fixtures):
451379

docs/todo/type-inference.md

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -681,19 +681,6 @@ property_narrowing_negated, combination/property_instanceof).
681681

682682
---
683683

684-
685-
686-
## 24. Literal string conditional return type
687-
**Impact: Low · Effort: Low-Medium**
688-
689-
Conditional return types using literal string comparison
690-
(`$param is "foo"`) are not resolved. Only class/interface type checks
691-
work in the conditional return type parser today.
692-
693-
**Discovered via:** fixture conversion (conditional_return_type_string).
694-
695-
---
696-
697684
## 25. `class-string<T>` on interface method not inherited
698685
**Impact: Medium · Effort: Medium**
699686

@@ -732,17 +719,6 @@ an object type.
732719

733720
---
734721

735-
## 29. `@phpstan-type` alias in foreach context
736-
**Impact: Low · Effort: Low**
737-
738-
When a method's return type uses a `@phpstan-type` alias and the result
739-
is iterated in a `foreach`, the alias is not resolved before the
740-
foreach value type is extracted.
741-
742-
**Discovered via:** fixture conversion (type/phpstan_type_alias).
743-
744-
---
745-
746722
## 30. Invoked closure/arrow function return type
747723
**Impact: Low · Effort: Low-Medium**
748724

example.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,20 @@ public function demo(): void
6868
}
6969

7070

71+
// ── Mixed Accessor Chaining ─────────────────────────────────────────────────
72+
73+
class MixedAccessorDemo
74+
{
75+
public function demo(): void
76+
{
77+
$foobar = new StaticPropHolder();
78+
$foobar->holder::$shared; // $obj->prop::$static chain
79+
80+
// Inline (new Foo)->method() chaining
81+
(new Pen())->write(); // resolves Pen then write()
82+
}
83+
}
84+
7185
// ── Method & Property Chaining ──────────────────────────────────────────────
7286

7387
class ChainingDemo
@@ -303,6 +317,11 @@ public function demo(): void
303317

304318
$appPen = app(Pen::class); // conditional on standalone function
305319
$appPen->write();
320+
321+
// Literal string conditional return type
322+
$mapper = new TreeMapperImpl();
323+
$result = $mapper->map('foo', 'bar');
324+
$result->write(); // "foo" → Pen (literal string match)
306325
}
307326
}
308327

@@ -544,6 +563,7 @@ public function demo(): void
544563
/**
545564
* @phpstan-type UserData array{name: string, email: string, age: int}
546565
* @phpstan-type StatusInfo array{code: int, label: string}
566+
* @phpstan-type UserList array<int, Profile>
547567
*/
548568
class TypeAliasDemo
549569
{
@@ -554,6 +574,11 @@ public function demo(): void
554574

555575
$status = $this->getStatus();
556576
$status['label']; // StatusInfo alias → array shape keys
577+
578+
// Type alias resolves through foreach iteration
579+
foreach ($this->getUsers() as $user) {
580+
$user->getDisplayName(); // UserList → array<int, Profile> → Profile
581+
}
557582
}
558583

559584
/** @return UserData */
@@ -567,6 +592,12 @@ public function getStatus(): array
567592
{
568593
return ['code' => 200, 'label' => 'OK'];
569594
}
595+
596+
/** @return UserList */
597+
public function getUsers(): array
598+
{
599+
return [];
600+
}
570601
}
571602

572603
/**
@@ -1931,6 +1962,28 @@ class ImplementMethodsDemo extends ScaffoldingAbstractShape implements Scaffoldi
19311962
// ═══════════════════════════════════════════════════════════════════════════
19321963
// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
19331964
// ┃ SCAFFOLDING — Supporting definitions below this line. ┃
1965+
1966+
// StaticPropHolder — used by MixedAccessorDemo
1967+
class StaticPropHolder
1968+
{
1969+
public static string $shared = 'hello';
1970+
1971+
/** @var self */
1972+
public self $holder;
1973+
}
1974+
1975+
// TreeMapperImpl — used by ConditionalReturnDemo (literal string conditional)
1976+
class TreeMapperImpl
1977+
{
1978+
/**
1979+
* @return ($signature is "foo" ? Pen : Marker)
1980+
*/
1981+
public function map(string $signature, mixed $source): Pen|Marker
1982+
{
1983+
return new Pen();
1984+
}
1985+
}
1986+
19341987
// ┃ Everything below exists to support the demos above. ┃
19351988
// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
19361989
//

src/completion/types/conditional.rs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,30 @@ pub(crate) fn resolve_conditional_with_text_args(
158158
// Can't statically determine; fall through to else.
159159
resolve_conditional_with_text_args(else_type, params, text_args, var_resolver)
160160
}
161+
ParamCondition::LiteralString(expected) => {
162+
// Check if the argument is a quoted string literal
163+
// matching the expected value (e.g. `'foo'` or `"foo"`).
164+
if let Some(arg) = arg_text {
165+
let trimmed = arg.trim();
166+
let arg_value = if (trimmed.starts_with('\'') && trimmed.ends_with('\''))
167+
|| (trimmed.starts_with('"') && trimmed.ends_with('"'))
168+
{
169+
Some(&trimmed[1..trimmed.len() - 1])
170+
} else {
171+
None
172+
};
173+
if arg_value == Some(expected.as_str()) {
174+
return resolve_conditional_with_text_args(
175+
then_type,
176+
params,
177+
text_args,
178+
var_resolver,
179+
);
180+
}
181+
}
182+
// Argument doesn't match the literal → else branch.
183+
resolve_conditional_with_text_args(else_type, params, text_args, var_resolver)
184+
}
161185
}
162186
}
163187
}
@@ -356,6 +380,43 @@ pub(crate) fn resolve_conditional_with_args<'b>(
356380
// arbitrary expression; fall through to else.
357381
resolve_conditional_with_args(else_type, params, argument_list, var_resolver)
358382
}
383+
ParamCondition::LiteralString(expected) => {
384+
// Check if the argument is a string literal matching
385+
// the expected value.
386+
let matches = match arg_expr {
387+
Some(Expression::Literal(Literal::String(lit_str))) => {
388+
// `value` is the unquoted content; fall back
389+
// to stripping quotes from `raw`.
390+
let inner = lit_str.value.map(|v| v.to_string()).unwrap_or_else(|| {
391+
let raw = lit_str.raw;
392+
raw.strip_prefix('\'')
393+
.and_then(|s| s.strip_suffix('\''))
394+
.or_else(|| {
395+
raw.strip_prefix('"').and_then(|s| s.strip_suffix('"'))
396+
})
397+
.unwrap_or(raw)
398+
.to_string()
399+
});
400+
inner == *expected
401+
}
402+
_ => false,
403+
};
404+
if matches {
405+
resolve_conditional_with_args(
406+
then_type,
407+
params,
408+
argument_list,
409+
var_resolver,
410+
)
411+
} else {
412+
resolve_conditional_with_args(
413+
else_type,
414+
params,
415+
argument_list,
416+
var_resolver,
417+
)
418+
}
419+
}
359420
}
360421
}
361422
}

0 commit comments

Comments
 (0)