Sitelet https://github.com/craftcms/commerce/pull/4124
Skip to content

6.x - #4124

Draft
lukeholder wants to merge 382 commits into
5.xfrom
6.x
Draft

6.x#4124
lukeholder wants to merge 382 commits into
5.xfrom
6.x

Conversation

@lukeholder

@lukeholder lukeholder commented Sep 23, 2025 •

Copy link
Copy Markdown
Member

@lukeholder
lukeholder requested a review from a team as a code owner September 23, 2025 07:42
@lukeholder
lukeholder marked this pull request as draft September 23, 2025 07:42
…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.
@lukeholder
lukeholder removed the request for review from a team August 19, 2026 10:07
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants