|
1 | | -# PHPantom — Bug Fixes |
| 1 | +# PHPantom — Bug Fixes |
| 2 | + |
| 3 | +## B3: Variable reassignment inside `try` block not tracked by analyzer |
| 4 | + |
| 5 | +When a variable is assigned one type before a `try` block and then |
| 6 | +reassigned to a different type inside the `try`, the analyzer uses the |
| 7 | +original type instead of the reassigned type for subsequent accesses |
| 8 | +within the same block. |
| 9 | + |
| 10 | +Real-world example — `MollieGateway.php`: |
| 11 | + |
| 12 | +```php |
| 13 | +$customer = $request->getCustomerOrFail(); // type: Luxplus Customer |
| 14 | +// ... |
| 15 | +try { |
| 16 | + $customer = $client->getOrCreateCustoemr($customer); // reassigned to MollieCustomer |
| 17 | + $molliePayment = $customer->createPayment($paymentData); // ← analyzer uses original type |
| 18 | +} |
| 19 | +``` |
| 20 | + |
| 21 | +Line 452 reassigns `$customer` from `Luxplus\...\Customer` to |
| 22 | +`Mollie\Api\Resources\Customer` (aliased as `MollieCustomer`). Line 453 |
| 23 | +calls `->createPayment()` which exists on `MollieCustomer` but not on |
| 24 | +the Luxplus `Customer` model. The analyzer resolves `$customer` as the |
| 25 | +original type, producing an `unknown_member` diagnostic for |
| 26 | +`createPayment` and a cascading `unresolved_member_access` for |
| 27 | +`$molliePayment->getCheckoutUrl()`. |
| 28 | + |
| 29 | +**Impact:** 2 diagnostics in the shared project (`MollieGateway:453`, |
| 30 | +`MollieGateway:462`). |
| 31 | + |
| 32 | +## B4: Relationship property access and BelongsTo return type not resolved by analyzer |
| 33 | + |
| 34 | +Eloquent relationship methods accessed as properties (without `()`) are |
| 35 | +resolved correctly in the completion engine via `LaravelModelProvider`, |
| 36 | +which synthesizes virtual properties (e.g. `translations()` returning |
| 37 | +`HasMany<T>` produces `$translations` typed as `Collection<T>`). However, |
| 38 | +the analyzer's member-access checker does not find these synthesized |
| 39 | +properties in some cross-file scenarios, reporting |
| 40 | +`unresolved_member_access`. |
| 41 | + |
| 42 | +Additionally, calling a relationship method WITH `()` (e.g. |
| 43 | +`$translation->category()`) returns a `BelongsTo` type that the LSP can |
| 44 | +resolve, but then member lookup on that `BelongsTo` fails to find |
| 45 | +methods like `associate()`. The `covariant $this` syntax in generic args |
| 46 | +(e.g. `BelongsTo<NotificationCategory, covariant $this>`) may interfere |
| 47 | +with type parsing. |
| 48 | + |
| 49 | +A separate sub-issue: `FlowService:477` accesses `$order->orderProducts` |
| 50 | +(camelCase) while the model declares the property and relationship method |
| 51 | +as `orderproducts` (all lowercase). Laravel normalises via `Str::snake()` |
| 52 | +at runtime, but the LSP does a case-sensitive property lookup. |
| 53 | + |
| 54 | +Affected diagnostics: |
| 55 | + |
| 56 | +- `NotificationCategory:52` — `$this->translations` property not resolved |
| 57 | + (HasMany relationship, no `@property` annotation) |
| 58 | +- `NotificationObject:114` — `$this->imageFile` property not resolved |
| 59 | + (HasOne relationship, no `@property` annotation) |
| 60 | +- `NotificationCategoryService:37` — `$translation->category()->associate()` |
| 61 | + — `BelongsTo` return type resolves but `associate()` not found on it |
| 62 | + |
| 63 | +`FlowService:477` (`$order->orderProducts`) is a case-sensitivity issue |
| 64 | +filed separately as B9. |
| 65 | + |
| 66 | +`FlowService:517` is a compound failure: `$reorder->order->orderproducts` |
| 67 | +is a relationship property (this bug), then `->reduce()` returns `mixed` |
| 68 | +instead of `Decimal` (generic return type inference gap), then `->add()` |
| 69 | +fails on the unresolved type. |
| 70 | + |
| 71 | +**Impact:** 4 diagnostics in the shared project |
| 72 | +(`NotificationCategory:52`, `NotificationObject:114`, |
| 73 | +`NotificationCategoryService:37`, `FlowService:517`). |
| 74 | + |
| 75 | +## B5: `$this->items` on custom Collection subclass not typed |
| 76 | + |
| 77 | +When a class extends `Collection<int, T>` via `@extends`, accessing |
| 78 | +`$this->items` should yield `array<int, T>`. Currently, `$this->items` |
| 79 | +resolves as `array` (the base `Collection`'s declared property type) |
| 80 | +without applying the generic substitution. This means iterating |
| 81 | +`$this->items` in a `foreach` or passing it to `array_any()` loses the |
| 82 | +element type. |
| 83 | + |
| 84 | +Real-world example — `PurchaseFileProductCollection.php`: |
| 85 | + |
| 86 | +```php |
| 87 | +/** |
| 88 | + * @extends Collection<int, PurchaseFileProduct> |
| 89 | + */ |
| 90 | +final class PurchaseFileProductCollection extends Collection |
| 91 | +{ |
| 92 | + public function hasIssues(): bool |
| 93 | + { |
| 94 | + return array_any($this->items, fn($item) => $item->order_amount > 0); |
| 95 | + // ^^^^^ unresolved |
| 96 | + } |
| 97 | +} |
| 98 | +``` |
| 99 | + |
| 100 | +`$this->items` should be `array<int, PurchaseFileProduct>`, so `$item` |
| 101 | +in the closure should be `PurchaseFileProduct`. Instead, `$item` is |
| 102 | +unresolved because the generic substitution is not applied to inherited |
| 103 | +properties when accessed via `$this->`. |
| 104 | + |
| 105 | +**Impact:** 2 diagnostics in the shared project |
| 106 | +(`PurchaseFileProductCollection:25` — two property accesses on `$item`). |
| 107 | + |
| 108 | +## B6: Scope methods not found on Builder in analyzer chains |
| 109 | + |
| 110 | +PHPantom's completion engine correctly injects scope methods onto |
| 111 | +`Builder<ConcreteModel>` via `try_inject_builder_scopes` in |
| 112 | +`resolve_named_type`. However, the analyzer's `check_member_on_resolved_classes` |
| 113 | +uses `resolve_class_fully_cached` which is keyed by bare FQN without |
| 114 | +generic args. A prior cache entry for `Builder` (without model-specific |
| 115 | +scopes) is returned, and the scope method is reported as not found. |
| 116 | + |
| 117 | +The analyzer does check `base_classes` first (before the cache) to avoid |
| 118 | +this, but in method chains like |
| 119 | +`ArticleCategoryTranslation::whereHas(...)->whereLanguage(...)`, the |
| 120 | +intermediate `Builder<ArticleCategoryTranslation>` type produced by |
| 121 | +`whereHas()` may not carry the scope-injected methods in `base_classes`. |
| 122 | + |
| 123 | +Affected diagnostics (5 direct + 2 cascading): |
| 124 | + |
| 125 | +Direct `unknown_member` — scope method exists on model but not found on |
| 126 | +Builder: |
| 127 | +- `ArticleRepository:69` — `whereLanguage` (scope on |
| 128 | + `ArticleCategoryTranslation`) |
| 129 | +- `ProductRepository:271` — `whereIsLuxury` (scope on `Product`) |
| 130 | +- `ProductRepository:272` — `whereIsDerma` (scope on `Product`) |
| 131 | +- `ProductRepository:273` — `whereIsProHairCare` (scope on `Product`) |
| 132 | +- `ProductRepository:369` — `whereIsLuxury` (scope on `Product`) |
| 133 | + |
| 134 | +Cascading `unresolved_member_access`: |
| 135 | +- `EventRepository:23` — `pluck` after broken |
| 136 | + `whereIsBlackFriday()->whereIsVisible()` chain |
| 137 | + |
| 138 | +Note: `EventRepository:22` reports `whereIsVisible` not found on Builder. |
| 139 | +Product has `scopeIsVisibleIn` (takes a `Country` parameter) but no |
| 140 | +`scopeWhereIsVisible` and no `is_visible` column. This may be a genuine |
| 141 | +code bug in the project rather than an LSP issue. |
| 142 | + |
| 143 | +**Impact:** 5–6 direct `unknown_member` diagnostics plus 1–2 cascading. |
| 144 | + |
| 145 | +## B7: PHPDoc `@param` generic array type not merged with native `array` hint |
| 146 | + |
| 147 | +When a method has a native type hint `array` and a PHPDoc `@param` with |
| 148 | +a generic type like `list<Request>`, PHPantom doesn't merge the PHPDoc |
| 149 | +element type with the native `array` for narrowing purposes. After an |
| 150 | +`is_array()` guard, the variable narrows to `array` but loses the `Request` |
| 151 | +element type from the docblock. |
| 152 | + |
| 153 | +Real-world example — `MobilePayConnection.php`: |
| 154 | + |
| 155 | +```php |
| 156 | +/** |
| 157 | + * @param null|list<Request>|Request $request |
| 158 | + */ |
| 159 | +protected function connect(string $uri, null|array|Request $request, ...): MobilePayResponse |
| 160 | +{ |
| 161 | + if (is_array($request)) { |
| 162 | + foreach ($request as $item) { |
| 163 | + $serializedObjects[] = $item->jsonSerialize(); |
| 164 | + // ^^^^^ unresolved |
| 165 | + } |
| 166 | + } |
| 167 | +} |
| 168 | +``` |
| 169 | + |
| 170 | +After `is_array($request)`, `$request` narrows from `null|array|Request` |
| 171 | +to `array`. The `@param` says the array case is `list<Request>`, so |
| 172 | +`$item` should be `Request`. But the LSP doesn't unify the narrowed |
| 173 | +native `array` with the docblock's `list<Request>`. |
| 174 | + |
| 175 | +**Impact:** 1 diagnostic in the shared project |
| 176 | +(`MobilePayConnection:76`). |
| 177 | + |
| 178 | +## B8: Variadic parameter element type lost in `foreach` |
| 179 | + |
| 180 | +When a method declares a variadic parameter with a union type like |
| 181 | +`HtmlString|int|string ...$placeholders`, iterating with |
| 182 | +`foreach ($placeholders as $value)` should give `$value` the element |
| 183 | +type `HtmlString|int|string`. Instead, the LSP resolves `$value` as |
| 184 | +untyped (hover returns nothing). |
| 185 | + |
| 186 | +Real-world example — `ShortTexts.php`: |
| 187 | + |
| 188 | +```php |
| 189 | +public static function get(int $id, Country $lang, HtmlString|int|string ...$placeholders): HtmlString|string |
| 190 | +{ |
| 191 | + // ... |
| 192 | + foreach ($placeholders as $value) { |
| 193 | + $isHTMLValue = $value instanceof HtmlString; |
| 194 | + if ($isHTML) { |
| 195 | + $replace[] = $isHTMLValue ? $value->toHtml() : htmlentities((string)$value); |
| 196 | + // ^^^^^^ unresolved |
| 197 | + } |
| 198 | + } |
| 199 | +} |
| 200 | +``` |
| 201 | + |
| 202 | +The variadic `...$placeholders` is internally `array<int, HtmlString|int|string>`, |
| 203 | +but the LSP doesn't propagate the element type into the `foreach` loop |
| 204 | +variable. This is a prerequisite for the `instanceof` narrowing (which |
| 205 | +would further narrow `$value` to `HtmlString` in the truthy ternary |
| 206 | +branch), but the primary failure is the missing element type. |
| 207 | + |
| 208 | +**Impact:** 1 diagnostic in the shared project (`ShortTexts:79`). |
| 209 | + |
| 210 | +## B9: Eloquent relationship property lookup is case-sensitive |
| 211 | + |
| 212 | +Laravel normalises property names via `Str::snake()` at runtime, so |
| 213 | +`$order->orderProducts` and `$order->orderproducts` both resolve to the |
| 214 | +same relationship. PHPantom's property lookup is case-sensitive, so when |
| 215 | +code uses `orderProducts` (camelCase) but the model declares the |
| 216 | +relationship method and `@property` as `orderproducts` (all lowercase), |
| 217 | +the property is not found. |
| 218 | + |
| 219 | +Real-world example — `FlowService.php`: |
| 220 | + |
| 221 | +```php |
| 222 | +// FlowService line 477: |
| 223 | +$items = $order->orderProducts->map(...); |
| 224 | +// ^^^^^^^^^^^^^^ camelCase — not found |
| 225 | + |
| 226 | +// Order model declares: |
| 227 | +public function orderproducts(): HasMany { ... } |
| 228 | +// and @property uses 'orderproducts' (lowercase) |
| 229 | +``` |
| 230 | + |
| 231 | +The fix should apply `Str::snake()`-equivalent normalisation (or |
| 232 | +case-insensitive matching) when looking up relationship-derived virtual |
| 233 | +properties on Eloquent models. |
| 234 | + |
| 235 | +**Impact:** 1 direct diagnostic (`FlowService:477`) plus 1 cascading |
| 236 | +(`FlowService:517` — compound with `Collection::reduce()` type loss). |
0 commit comments