6.x - #4124
Draft
lukeholder wants to merge 382 commits into
Draft
6.x#4124lukeholder wants to merge 382 commits into
lukeholder wants to merge 382 commits into
Conversation
…ms equivalents
craft\helpers\StringHelper is deprecated in favor of
CraftCms\Cms\Support\Str (extends Illuminate\Support\Str). Verified each
of the 4 methods actually used (23 call sites across 16 files) against
the legacy adapter's own delegation code before swapping:
- randomString($length, $extendedChars) -> Str::random($length,
$extendedChars): the legacy method's own body is `return
Str::random($length, $extendedChars);`, and CraftCms\Cms\Support\Str
overrides random() with the same two-arg signature, so this is a
confirmed 1:1 swap, not a guess.
- toTitleCase($str) -> Str::title($str): same confirmation — the legacy
method's body delegates directly to Str::title().
- UUID() -> (string)Str::uuid(): Laravel's Str::uuid() returns a
Ramsey\Uuid\UuidInterface object, not a plain string like the old
method, so every call site needed an explicit cast to keep assigning
into string-typed properties/array values.
- split($str, $delimiter) -> inlined preg_split('/\s*' . delimiter .
'\s*/', $str, -1, PREG_SPLIT_NO_EMPTY) at each of the 3 call sites:
the legacy method itself was never delegated to the new Str class (no
Str::split() exists), it's just a raw preg_split with no wrapper, so
there's nothing to route through beyond replicating that expression
directly.
…n src/
UrlHelper -> CraftCms\Cms\Support\Url (url, cpUrl, siteUrl, actionUrl,
urlWithParams): all confirmed to exist on the new class with identical
signatures, a pure rename.
MoneyHelper -> CraftCms\Cms\Support\Money (toMoney, toDecimal, toString):
verified against the legacy adapter's own implementation, which is a
straight pass-through to the new class for every method actually used
here, a pure rename.
DateTimeHelper: toDateTime() is inherited unchanged from
CraftCms\Cms\Support\DateTimeHelper (no override), so importing the new
class directly is a pure rename. secondsToInterval() and
currentUTCDateTime() are different — they exist only on the legacy
bridge class, not the new one — so their two call sites in Carts.php
were inlined directly per the legacy source's own one-line bodies:
new DateInterval("PT{$seconds}S") and now('UTC').
Also fixed a real bug this surfaced: my first pass at the DateInterval
inlining used a bare `new DateInterval(...)` with no import, which
PHPStan caught resolving to the wrong class (CraftCms\Commerce\Order\
DateInterval) since — unlike functions — unqualified class references
in PHP do NOT fall back to the global namespace. Added `use DateInterval;`
to fix it.
Not touched: FileHelper::isWritable() (the legacy version does a real
write-attempt test via fopen(); Laravel's version is just is_writable(),
a strictly weaker check — swapping would be a behavior downgrade, not a
neutral rename) and craft\helpers\Queue::push() (wraps legacy JobInterface
jobs in a LegacyJobWrapper before dispatching; its two call sites pass
SendEmail/ResaveProductVariants jobs that haven't been migrated to
Laravel ShouldQueue jobs yet, so swapping to the bare Queue facade would
break dispatching until those job classes are migrated too).
Both classes share the "Json" short name, so this is purely an import swap for the 14 files using only encode()/decode() — both are direct pass-throughs (encode) or a verified same-contract reimplementation (decode: same InvalidArgumentException-on-bad-JSON / null-on-empty behavior, confirmed against the legacy adapter's inherited Yii2 base implementation) on the new class, so no call-site text changes needed. OrdersController.php needed real changes: its ~25 fully-qualified \craft\helpers\Json::encode()/decodeIfJson() calls became \CraftCms\Cms\Support\Json::, and its 2 htmlEncode() calls (a method that only exists on the legacy adapter, inherited from Yii2's base Json class with no delegation to the new class) were inlined as Json::encode($value, JSON_UNESCAPED_UNICODE | JSON_HEX_QUOT | JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS) — the exact flag set Yii2's own htmlEncode() uses internally.
Verified all 9 methods actually used (encode, tag, a, beginTag, endTag, hiddenInput, input, hiddenLabel, namespaceInputName/namespaceInputs) before swapping. The legacy class extends \yii\helpers\Html directly with no override for encode()/input(), and the new class's __callStatic() falls back to the exact same Yii2 base class for any method it doesn't reimplement — so encode()/input() are byte-for-byte identical either way, and the 7 explicitly-reimplemented methods already have established, working call sites elsewhere in this codebase from earlier sessions. Since both classes share the "Html" short name, most files only needed their `use` statement swapped. Six files already had the new class imported as `NewHtml` (from earlier partial migration work) alongside the old bare `Html` import — those needed the bare Html:: calls renamed to NewHtml:: and the old import dropped, done via a negative-lookbehind regex to avoid double-prefixing the already-correct NewHtml:: calls (a plain find-replace would have produced NewNewHtml::). Three files used the fully-qualified \craft\helpers\Html:: form with no import; those got a proper `use` statement added and calls shortened to bare Html::.
…mfony equivalents Category A3: swaps each Yii2 framework exception (not craft\* namespace, so lower priority per CLAUDE.md, but still in scope) for its native PHP or Symfony equivalent across ~24 files. The mapping preserves the original author's own already-made distinction between "bad argument" and "bad state" at each throw site rather than re-judging every one: - yii\base\InvalidConfigException -> \RuntimeException (no perfect native equivalent — Yii2's own class just extends \Exception generically for "something about current state/config is wrong"). - yii\base\InvalidArgumentException -> native \InvalidArgumentException. Same name, but a real behavioral fix: Yii2's version actually extends \BadMethodCallException, not the native SPL class, so a catch (\InvalidArgumentException) elsewhere would never have caught it. - yii\base\Exception -> native \Exception (Yii2's own class already extends it directly with no behavior difference). - yii\base\InvalidCallException -> native \BadMethodCallException. - yii\base\ErrorException -> native \ErrorException (Yii2's own class extends it directly; constructor signature is a superset). - yii\web\HttpException / BadRequestHttpException (Webhooks.php) -> Symfony\Component\HttpKernel\Exception equivalents — same ecosystem as the Response class already used in that file, and what Laravel's own abort() throws under the hood. Required changing $exception->statusCode to $exception->getStatusCode(), since Symfony exposes it as a method, not a public property like Yii2's version. A real bug surfaced mid-sweep: unqualified native class references (new DateInterval(...), and almost RuntimeException too) do NOT fall back to the global namespace the way unqualified function calls do — PHPStan caught one resolving to CraftCms\Commerce\Order\DateInterval instead of the global class. Fixed by always using an explicit `use` import or a `\`-prefixed fully-qualified name at every throw/catch site introduced by this sweep, never a bare global class name. Left alone, with reasons (documented in updated-laravel-migration-private.md): yii\base\Event (both usages deliberately fire against a legacy class name string for third-party Event::on() listener backward compatibility), yii\mail\MailEvent (correctly paired with the legacy Mailer::EVENT_BEFORE_PREP hook, which still constructs exactly this type), yii\validators\Validator (sits inside a method explicitly marked "not yet wired up to a real validator, pending the Ruleset system" — dead code with no confirmed future type yet), yii\base\ExitException (tied to the already-flagged, not-yet-redesigned getResponse()/end() redirect flow in Payments.php), and yii\db\Expression (correctly paired with the still-legacy craft\db\Query object it's passed into — Category B2 scope, not this one).
Swaps the legacy service-locator indirection for direct container resolution across ~557 call sites in 98 files. HasServices (src/Plugin/Concerns/HasServices.php) only exists to keep src-yii2/ code working against the legacy craft\commerce\services\X wrapper classes; src/ code has no reason to route through it. The getter -> new-class mapping was derived from each legacy wrapper's own app(X::class) delegation target, not guessed. Left alone: getSettings() (framework-level Plugin/HasSettings accessor, not a service getter), the single getTransfers() site (Transfers hasn't moved to src/ yet), and ~86 event-firing chains (hasEventHandlers()/trigger()) plus two PaymentCurrencies:: convertCurrency() sites that deliberately need the legacy wrapper's inherited yii\base\Component event methods / not-yet-ported method - a first pass swapped these too and PHPStan's existing per-line ignore comments masked the breakage, so these were reverted back to Plugin::getInstance()->getX() explicitly. Also fixed a real bug this surfaced: Store/Models/Store.php's getSettings(): StoreSettings return type bare-resolves to the same-namespace Models\StoreSettings; adding a use import for the StoreSettings *service* (same short name) for an unrelated call shadowed that resolution and pointed the return type at the wrong class. Fixed by using the FQCN inline instead of importing. Verified with a full PHPStan diff (--error-format=raw) against a pre-change stash baseline, matched by (file, message) to stay immune to line-number drift from added imports - remaining differences are pre-existing type-hint gaps now attributed to the new class name instead of the legacy wrapper's, not regressions. Also ran php -l, phpstan, and fix-cs/check-cs across all touched files.
…FIG_*_KEY constants Swaps craft\commerce\services\X imports for their CraftCms\Commerce\* equivalents in the few src/ files that only reference the legacy class for a ProjectConfig CONFIG_*_KEY constant (or, for Coupons, DEFAULT_COUPON_FORMAT) - confirmed each new class already defines the identical constant. Also simplifies a few app(\Fully\Qualified\X::class) calls left over from the Category B1 pass back to bare app(X::class) now that the correct class is imported directly. craft\commerce\services\Transfers (2 sites) is deliberately left alone - Transfers hasn't been migrated to src/ yet, so no new-class target exists. src/Plugin/Concerns/HasServices.php is also left alone - its imports of the legacy service classes are intentional, not stale.
…ass_alias Swaps craft\commerce\* imports for their CraftCms\Commerce\* equivalents across 81 files, restricted to the ~52 classes confirmed runtime-identical via a literal class_alias() call in src-yii2/ (e.g. craft\commerce\elements\Order -> CraftCms\Commerce\Order\Elements\Order, craft\commerce\models\Store -> CraftCms\Commerce\Store\Models\Store) - not guessed from deprecation notices. Covers the Order element, base Purchasable/PurchasableInterface, and most Models/enums/exceptions still referenced by their legacy name. Left alone: craft\commerce\Plugin (not aliased - src-yii2's Plugin extends the new one, doesn't alias it), craft\commerce\services\* (legacy wrapper classes, some still legitimately needed - see the B1 commit), craft\commerce\web\assets\* (asset-bundle registration, already deferred), and everything else with no class_alias entry. src/Order/Adjuster/Discount.php's `use craft\commerce\adjusters\Discount as LegacyDiscount;` is deliberately excluded even though it IS aliased - LegacyDiscount::class is fired as an Event::trigger() name-string for third-party backward compatibility, and ::class resolves to the name as written, not the alias target, so renaming it would silently change which event name gets fired. Two real bugs surfaced and fixed along the way: - Several files (Inventory.php, Order.php) already had a *second*, differently-aliased import for the same target class (e.g. `Purchasable as NewPurchasable`) for a `Purchasable|NewPurchasable` union type or `instanceof` check that predates this pass. Adding a second bare import for the same class is a duplicate-type fatal (caught by php -l for type declarations) or dead code needing simplification (for instanceof/union expressions, which php -l does NOT catch - found via a full-codebase sweep for any FQCN imported under two different names). - StoreTrait.php and three model classes declared `getStore(): \craft\commerce\models\Store` inline (not via a `use` import), which PHPStan started flagging as a return-type mismatch once the *callee* (Stores::getStoreById()) was migrated to the new class name - same runtime class via class_alias, but PHPStan treats aliased classes as nominally different types. Updated all four to the new class name directly. craft\commerce\models\TransferDetail is reverted back to the legacy name in TransfersController.php specifically - it's passed into craft\commerce\elements\Transfer::addDetail(), which is entirely unmigrated legacy code still typed against the old name. Verified with a full PHPStan diff (--error-format=raw) matched by (file, message) to stay immune to line-number drift - remaining differences are pre-existing type-hint gaps in still-legacy method signatures (e.g. craft\commerce\base\Gateway), not regressions. Also ran php -l and check-cs/fix-cs across all touched files.
- src-yii2/templates/index.twig: update stale editableProductTypes() call to viewableProductTypes() (renamed pre-migration, deprecated method dropped from the new ProductTypes service) - StoresController/StoreManagementController: cast request storeId input to int before passing to strictly-typed Stores::getStoreById() - Store/Zone/Email/OrderAdjustment models: add validationData() overrides so private getter-backed attributes (name, currency, condition, to, sourceSnapshot) are actually visible to validate(), which was silently failing "required" rules and blocking saves - InventoryLocations/OrderStatuses/TaxCategories/ShippingCategories/ CatalogPricingRules/Customers/Stores: raw DB::table()->insert() calls into commerce_* pivot tables were missing dateCreated/ dateUpdated, which are NOT NULL with no DB default - Plugin.php: override cpNavIconPath() so the Commerce CP nav item gets an icon; the base implementation returns a filesystem path, but the CP's craft-icon component only resolves published icon names, so it never rendered
…rollers Same root cause as the reactive fixes already committed: Request::input() always returns raw (string) values, and strict_types=1 means PHP no longer silently coerces those into int/float/?int/?float-typed method parameters or model/DTO properties. Audited every controller under src/Http/Controllers and cast at each call site where a raw request value crossed into a strictly-typed target — IDs passed to getXById()-style service methods, model property assignments (storeId, methodId, etc.), and a couple of Localization::normalizeNumber() results assigned to float properties. No behavior changes beyond fixing the type mismatches; null-handling was preserved wherever a field was allowed to be absent/null.
… test harness Plugin.php bootstrap migration: - Group 3: migrated remaining Craft event listeners (SiteSaved/SiteDeleted, ElementSaved, DefineDeletionBlockers, ElementAuthorizing) to src/Plugin.php, removed the now-dead legacy delegator methods they fed. - Group 4: dynamic per-product-type permissions in Plugin::getPermissions(). - Group 5: CP nav ported to the NavItem builder in Plugin::getCpNavItem(). - Group 6: added craft:resave:products/variants/orders/carts console commands. - Group 7: StoreBehavior/CustomerBehavior/CustomerAddressBehavior replaced with Macroable macros on Site/User/Address (the legacy Yii2 behaviors were silently non-functional since Site/User/Address no longer extend yii\base\Component and attachBehavior() doesn't exist on them). - Group 8: GQL schema components/eager-loadable fields, element exporters, garbage collection, Twig extension, and craft.commerce Twig variables migrated to their new-system registration points. Category B2 part 3: checked all ~44 remaining stale craft\commerce\* imports in src/ individually; fixed 3 real issues (a dead PaymentIntents reference, stale StoreBehavior docblocks, and a TransferDetail class-alias type mismatch), confirmed the rest are legitimate references to not-yet-migrated legacy code. Also includes the Pest/Orchestra Testbench test harness (tests-yii2/ rename, Codeception removal, tests/Feature and tests/Unit scaffolding) and a handful of unrelated fixes (Locale::switchAppLanguage formatting locale, Stores.php schema-version guard) staged from earlier work. See updated-laravel-migration-private.md for full verification detail.
CraftCms\Cms\Plugin\Plugins::installPlugin('commerce') calls loadPlugins()
as its own first line, before Commerce has a row in the plugins table yet.
Finding nothing to register, loadPlugins() still permanently sets its
internal pluginsLoaded guard to true. Since Plugins is a container
singleton, every later loadPlugins() call for the rest of the test run
short-circuits immediately -- including the one that would otherwise pick
up Commerce moments later in the same install flow. The net effect: every
boot()-registered feature (GQL argument handlers, widgets, permissions, CP
nav, resave commands, event listeners, Macroable macros) was invisible to
the Pest suite, verified only by hand via tinker against the live app.
Root-caused via a sequence of diagnostic Pest tests (not guesswork):
getPlugin('commerce') returned null despite isPluginInstalled()/
isPluginEnabled() both true; manually calling createPlugin() in isolation
worked fine, ruling out the class/manifest as the problem.
Fix: forget the Plugins singleton and reload it right after installing
Commerce in tests/TestCase.php, so the next resolution re-scans the
now-populated plugins table and actually registers craft\commerce\Plugin
as a Laravel service provider. Same fix shape core's own
CraftCms\Cms\Plugin\Testing\InstallsPlugin trait already applies.
Added tests/Feature/PluginBootTest.php as durable regression coverage,
confirming a Group-1 boot() registration and the Group-7 Site::getStore()
macro (both method-call and magic-property syntax) now resolve correctly.
52/52 tests passing (49 existing + 3 new).
…d along the way
Inventory CP:
- selectableSites() needs an array, not a Collection
- editLocationLevels() return type didn't cover the CpScreenResponse case
- redirect to the default inventory location was missing the CP trigger prefix
Install migration:
- port a real down() (drop tables, field layouts, project config) — it was
previously just a stub, so uninstall silently left the plugin's data behind
- fix an FK identifier over MySQL's 64-char limit
Ported all tests-yii2/unit/stats/*, tests-yii2/unit/helpers/{Currency,Locale,
Localization}Test.php to Pest, and removed the now-redundant legacy DebugPanel
helper test (the DebugPanel feature itself was already removed). Added
tests/Support/OrdersFixture.php to build the order/product/customer graph the
stats tests assert against.
Real bugs surfaced while building out the stats fixture (all pre-existing,
not test-only):
- Variant::getSnapshot() included a `ruleset` validation object that
circularly referenced its own product, crashing JSON encoding on any
order line item
- LineItems.php was missing a use import for LineItemStatuses
- LineItemStatuses + 5 sibling services fired legacy Yii2 events
unconditionally, crashing the first time they actually ran
- 9 legacy condition classes narrowed defineRules() visibility against a
now-public parent, a PHP fatal
- Order::afterSave() didn't convert an already-set dateOrdered to UTC before
storage, breaking timezone-dependent date filtering
- TopPurchasables ordered by an ambiguous `sku` column
Plus SQLite compatibility for the test suite (Stat.php chart queries,
CatalogPricing's UUID/NOW() raw SQL — now extracted into src/Helpers/Sql.php),
and two test-harness gaps (missing auth.guards.craft config, Solo edition's
1-user cap blocking fixture users).
… directly instead of the shared craftcms/.github workflow Track 6.x instead of 5.x, target PHP 8.5, and split tests into Unit/Arch/Feature jobs via a shared run-tests composite action.
Ports 5.x bugfixes (up to 5.7.2) into the Laravel-migrated 6.x code: - Transfers field layout save/delete used Order::class instead of Transfer::class (src-yii2/services/Transfers.php) - PDF/cart load URLs generated from console requests returned blank (src/Pdf/Pdfs.php, src/Order/Carts.php) - Completed orders could have their recalculation mode reset to "all" (src/Order/Elements/Order.php) - Inactive carts' searchindex rows weren't purged due to delete ordering (src/Order/Carts.php) - Inventory location IDs per store weren't memoized (src/Inventory/InventoryLocations.php) New 5.x Codeception unit tests (ShippingTest, OrderRecalculationTest) added as reference material under tests-yii2/unit/, matching how the rest of the retired Yii2/Codeception suite is archived on 6.x.
CatalogPricing.php, Customers.php, Emails.php, and Carts.php referenced classes under the wrong namespace with no matching use import (CatalogPricingRule, Carts, Pdfs, PaymentCurrencies) — real bugs that would fatal at runtime if the code paths executed. ProductQuery::cleanseQueryCriteria() gated its criteria-sanitizing on `Craft::$app->controller instanceof ElementIndexesController` using Yii2 controller classes that no longer exist anywhere in Craft 6. Migrated the check to Laravel routing (request()->route()->getControllerClass()) against the new ElementIndexController/SearchController classes, matching the pattern already used in cms-6's EnsureInstalled middleware.
…Line) These no longer suppressed any actual error - PHPStan flags a bare @phpstan-ignore-next-line as unmatched once the underlying error it was written for has been fixed elsewhere during the migration. Verified each removal leaves zero errors on that line before committing; total PHPStan errors 1306 -> 1078.
…igrations/; remove dead legacy Install migration Plugin::getMigrationsPath() (yii2-adapter) resolves a plugin's migrations directory to dirname(getBasePath())/database/migrations when that directory exists - src-yii2/migrations/ was never in the scan path. These 6 files were already rewritten as genuine Laravel migrations (see b1b20f8) but landed in the wrong directory, so they've never actually run against any 6.x install: product type permission renaming, the commerce_catalogpricing_queue table, orders.customerDeleted, the subscriptions userId FK cascade fix, ordernotices.noticeType, and the allVariants->variants changedattributes fix. Verified via `php craft migrate/all --track=plugin:commerce --pretend` that all 6 are now discovered and their SQL is valid against the current schema. Also removes src-yii2/migrations/Install.php: fully superseded by database/migrations/Install.php (verified matching schema for producttypes), and per docs/extend/migrations.md, once a plugin extends CraftCms\Cms\Plugin\Plugin, only native Laravel migrations run - the old Yii2-style Install migration was already unreachable dead code.
… legacy Order alias
craft\commerce\base\Gateway implements GatewayInterface, which is itself a
class_alias to the new CraftCms\Commerce\Payment\Gateway\Contracts\GatewayInterface.
PHP's LSP compatibility check for that implements clause needs to resolve
Gateway::availableForUseWithOrder()'s craft\commerce\elements\Order parameter
type - which is itself only a class_alias to the new Order class, created
lazily on first reference.
If nothing has touched craft\commerce\elements\Order yet, resolving it happens
reentrantly, mid-compatibility-check, while PHP is already inside the include()
for Gateway.php - which reliably fails with "class ... is not available" rather
than actually resolving it. Reproduced independent of PHPStan with a plain
`class_exists('craft\commerce\base\Gateway')` after only requiring the
autoloader.
Force-resolving the Order alias as a plain top-level statement, before the
class Gateway declaration is reached, avoids the reentrant path entirely.
Commerce's Model/Record classes (ProductType, Order, LineItem, Store, OrderQuery, Email, Pdf, Transaction, Purchasable, etc.) rely on Eloquent's magic __get/__set for every column, which plain phpstan/phpstan has no way to type. That's the source of the large majority of PHPStan's property.notFound/method.notFound/staticMethod.notFound noise. Mirrors cms-6's phpstan.neon: includes larastan/larastan + nesbot/carbon's extension, and points databaseMigrationsPath at database/migrations (Install.php plus the 6 migrations moved there in the previous commit) so Larastan can infer real column types instead of requiring @Property annotations on every model. property.notFound/method.notFound/staticMethod.notFound: 511+295+100 -> 95+120+15. Total PHPStan errors: 1078 -> 429.
…rastan Same cleanup as before, for @phpstan-ignore-next-line comments that only became unmatched once Larastan resolved the underlying property/method errors they were suppressing. Total PHPStan errors: 429 -> 422.
Real fixes: - Log::error($e) passed a raw exception where Logger::error() needs a string/Arrayable/Stringable message - switched to Log::error($e->getMessage(), ['exception' => $e]) - Order\Models\Order was missing datetime casts for datePaid, dateFirstPaid, dateAuthorized, dateCreated, dateUpdated, so assigning the element's real DateTime values onto the persistence model was statically (and would eventually be a real) type mismatch; added the casts and convert via Carbon::instance() at the assignment sites - getLineItems() ran array_filter() over a strictly LineItem[]-typed array, which can never contain falsy values - dead code, removed - clearNotices()/_filterNotices() had redundant "other side is not null" checks in an exhaustive if/elseif chain, provably always true given the preceding branches - simplified Docblock/type annotations (no behavior change): - Added missing @property-read entries for the *AsCurrency accessors on Order and LineItem, and @Property for OrderNotice::$noticeType (now backed by the noticeType column from the migration moved in a previous commit) - Added @var AddressElement assertions where Elements::duplicateElement() is called through the facade, which loses its generic <T> narrowing through the @method docblock and returns plain ElementInterface Suppressions, for genuine PHPStan/Larastan blind spots (not real bugs): - User::getPrimary{Shipping,Billing}Address()/getPrimaryPaymentSource() are added via the legacy CustomerBehavior at runtime, invisible to static analysis - Several Order::trigger()/LineItem trigger() calls pass new-namespace event objects into the still-Yii2 trigger() signature - matches the "TODO: migrate event firing to Laravel" pattern already used elsewhere - Two nullsafe.neverNull findings were false positives (getCustomer() and firstWhere() are both genuinely nullable) - changing to non-null access would introduce a real null-pointer bug - Gateway/GatewayInterface class_alias-chain blind spot (same one already documented in GatewayTypes.php) hit two more call sites - Conditions::createCondition()'s InvalidArgumentException is real (verified in cms-6) but invisible to PHPStan through the facade's __callStatic dispatch - catch is not dead - CraftCms\RulesetValidation\Ruleset's template T isn't covariant, so PHPStan can't verify any ElementRules subclass against the #[Ruleset] attribute - same limitation cms-6's own AssetRules/EntryRules/UserRules have no workaround for either Total PHPStan errors: 422 -> 360.
…l addError() runtime bug phpstan.neon was missing the whole stubFiles list that cms-6's own config includes (yii/base/Component, yii/validators/Validator, yii/db/*, etc.) - these carry the accurate typed signatures for legacy Yii2 APIs (e.g. Validator::addError() accepting craft\base\ModelInterface, not just the untyped vendor source). Total PHPStan errors: 360 -> 259. Purchasable.php: - getLineItemRules()'s inline validators called $validator->addError($lineItem, $attribute, $message), routing through yii\validators\Validator::addError() which internally calls $lineItem->addError($attribute, $message) - but LineItem is a new Component/Validates class with no such method, only errors(): MessageBag. This was a real bug: any of these six validators actually firing (out of stock, invalid purchasable, qty limits, etc.) would fatal at runtime with "Call to undefined method LineItem::addError()". Switched to $lineItem->errors()->add($attribute, $message), matching the pattern already used throughout Order.php/Product.php. - Localization::normalizeNumber() was called via CraftCms\Commerce\Helpers\Localization, which only has normalizePercentage() - normalizeNumber() is a Craft core helper (craft\helpers\Localization). Fixed the import. - Added @property/@property-read docblock for storeId, basePrice, basePromotionalPrice, sku, taxCategoryId, shippingCategoryId, and the *AsCurrency accessors. - getSku() had a dead `?? ''` fallback on a non-nullable string property. Also removed two @phpstan-ignore-next-line comments (on Order's and Purchasable's #[Ruleset(...)] attributes) that the stub fix made stale. Total PHPStan errors: 259 -> 251.
…ressions Real bugs (all would fatal at runtime on the affected code paths): - loginHandler() referenced User::IMPERSONATE_KEY, a constant that doesn't exist anywhere in the codebase - Craft 6's impersonation session state moved to a dedicated CraftCms\Cms\Auth\Impersonation service. Switched to app(Impersonation::class)->isImpersonating(). This ran on every user login. - activateUserFromOrder() called $user->setScenario(Element::SCENARIO_ESSENTIALS) using scenario constants that live on ElementRules, not Element (and via craft\base\Element, itself just a class_alias to the new Element) - Craft 6's scenario API is $element->ruleset->useScenario(...), not setScenario(). This ran on every guest checkout with "create an account" enabled. - The same method used Event::once(...), which has never existed on yii\base\Event (only on(), off(), trigger(), etc.) - replaced with the standard self-removing on()/off() handler pattern. Dead code removed: two property_exists($user, 'affiliatedSiteId') guards - affiliatedSiteId is a real, always-present property on Craft 6's User now. Suppressions, for the same CustomerBehavior/CustomerAddressBehavior runtime-attached-method blind spot already documented in Order.php - not real bugs, PHPStan just can't see methods added via legacy Yii2 behaviors. Also suppressed one nullsafe.neverNull false positive (getBillingAddress()/ getShippingAddress() are genuinely nullable, matching the pattern already found in Order.php). Total PHPStan errors: 251 -> 229.
Corrected 13 @PHPStan-Ignore comments (in Order.php and Customers.php, added in the previous two commits) that incorrectly attributed User/Address methods to "the legacy CustomerBehavior/CustomerAddressBehavior, attached at runtime". Plugin.php's registerBehaviorMacros() docblock clarifies those behaviors "no longer attach to anything" post-migration - Site/User/Address extend CraftCms\Cms\Component\Component, not yii\base\Component, so attachBehavior() doesn't exist on them at all. The real mechanism is Macroable macros registered in Plugin::registerCustomerMacros()/registerCustomerAddressMacros(). Fixed the comment text to name the real mechanism instead. OrdersController.php fixes: - currentUser()?->id read a nonexistent property on the CraftUser contract (only getCraftUserId() exists) - silently wrote null into $movement->userId on every inventory fulfillment movement instead of the actual current user. - getPaymentModal() never null-checked $order after getOrderById(), then called methods on it unconditionally - added an abort_unless() guard matching the rest of the controller's pattern. - $child->order->updateOrderPaidInformation() after capture/refund could hit a null Order (Transaction::$order is genuinely nullable) - switched to nullsafe. - Simplified two provably-dead conditions (getFieldLayout() is overridden non-nullable on Order; $qty's `?? 1` already eliminates null) and made two abort_unless() truthy-checks explicit. Also added the missing Transaction::$order @property-read docblock and Order::$outstandingBalanceAsCurrency, and suppressed the remaining Site::getStore()/Gateway class_alias-chain blind spots using the corrected macro explanation. Total PHPStan errors: 229 -> 209.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://linear.app/craftcms/issue/COM-613/laravel-port