Skip to content

feat: add "No store/website resolution in constructors" #502

Description

@damienwebdev

Magento2.Constructor.NoStoreResolutionInConstructor

Rule

Do not resolve store or website context inside a constructor. A class must not call StoreManagerInterface resolution methods — getStore(), getStores(), getWebsite(), getWebsites(), getStoreId(), getWebsiteId(), getGroup(), getGroups(), getDefaultStoreView() — from __construct() or from Magento's internal _construct() (the instantiation hook on AbstractModel, resource models, and collections). Inject the dependency and resolve the store lazily in the method that actually uses it.

Reason

Magento instantiates objects through the DI container during setup:install, setup:di:compile, and setup:static-content:deploy, when no store/website scope exists yet. Resolving the store in a constructor at that point throws The default website isn't defined. and aborts the install. Worse, the scopes app config is memoized for the life of the process, so a single premature getStore() call (while the store_website table is still empty during schema-patch application) caches an empty result and breaks later, unrelated core setup patches (e.g. Magento\InventorySales\...\InitializeWebsiteDefaultStock) with the same error — even after the default website row has been written. The failure is also environment-dependent: it hides locally where the DB and config cache are already warm, and only surfaces on a clean CI install, making it expensive to diagnose. Keeping constructors free of scope resolution makes objects safe to instantiate in any context and removes a whole class of order-of-bootstrap bugs.

Implementation

Enforced by the PHP_CodeSniffer sniff Magento2.Constructor.NoStoreResolutionInConstructor (dev/phpcs/Magento2/Sniffs/Constructor/NoStoreResolutionInConstructorSniff.php), registered in phpcs.xml and run via composer lint. For every __construct/_construct it walks the method body and reports (as an error) any call to a configured store-resolution method, while skipping nested closures/arrow-functions (whose bodies run on invocation, not at construction) and ignoring same-named property access that isn't a call. The forbidden-method list is a public property on the sniff, so it can be extended from the ruleset. Run locally with:

composer lint
# or just this rule:
vendor/bin/phpcs --standard=dev/phpcs/Magento2/ruleset.xml app/code/Magento2/

Deliberate exceptions can be suppressed inline with // phpcs:ignore Magento2.Constructor.NoStoreResolutionInConstructor.

Example Sniff:

<?php

declare(strict_types=1);

namespace Graycore\Sniffs\Constructor;

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;

/**
 * Flags store/website resolution (StoreManager calls) inside a constructor.
 *
 * Magento instantiates objects during `setup:install`, `setup:di:compile` and
 * `setup:static-content:deploy`, where there is no store context. Resolving the
 * current store/website in __construct() throws "The default website isn't
 * defined." on a fresh install, and — worse — poisons the memoized `scopes`
 * config cache for the rest of the process, breaking later core setup patches.
 *
 * Stash the dependency in the constructor and resolve the store lazily in the
 * method that actually needs it.
 *
 * @see docs/setup-install-default-website-isnt-defined.md
 */
class NoStoreResolutionInConstructorSniff implements Sniff
{
    /**
     * Method names that resolve store/website context and therefore require a
     * bootstrapped application. Configurable from the ruleset if needed.
     *
     * @var string[]
     */
    public $forbiddenMethods = [
        'getStore',
        'getStores',
        'getWebsite',
        'getWebsites',
        'getStoreId',
        'getWebsiteId',
        'getGroup',
        'getGroups',
        'getDefaultStoreView',
    ];

    /**
     * @inheritDoc
     */
    public function register()
    {
        return [T_FUNCTION];
    }

    /**
     * @inheritDoc
     */
    public function process(File $phpcsFile, $stackPtr)
    {
        $tokens = $phpcsFile->getTokens();

        // Only inspect constructors: PHP's __construct and Magento's internal
        // _construct (called at instantiation by AbstractModel/collections/etc.).
        $namePtr = $phpcsFile->findNext(T_STRING, $stackPtr);
        if ($namePtr === false
            || !in_array(strtolower($tokens[$namePtr]['content']), ['__construct', '_construct'], true)
        ) {
            return;
        }

        // Interface/abstract method declarations have no body.
        if (!isset($tokens[$stackPtr]['scope_opener'], $tokens[$stackPtr]['scope_closer'])) {
            return;
        }

        $forbidden = array_map('strtolower', $this->forbiddenMethods);
        $ptr = $tokens[$stackPtr]['scope_opener'] + 1;
        $end = $tokens[$stackPtr]['scope_closer'];

        while ($ptr < $end) {
            $token = $tokens[$ptr];

            // Skip nested function/closure/arrow-fn bodies: code there runs when
            // the callback is invoked, not at construction time.
            if (in_array($token['code'], [T_FUNCTION, T_CLOSURE, T_FN], true)
                && isset($token['scope_closer'])
            ) {
                $ptr = $token['scope_closer'] + 1;
                continue;
            }

            $isObjectAccess = $token['code'] === T_OBJECT_OPERATOR
                || $token['code'] === T_NULLSAFE_OBJECT_OPERATOR;

            if ($isObjectAccess) {
                $methodPtr = $phpcsFile->findNext(T_WHITESPACE, $ptr + 1, null, true);
                if ($methodPtr !== false
                    && $tokens[$methodPtr]['code'] === T_STRING
                    && in_array(strtolower($tokens[$methodPtr]['content']), $forbidden, true)
                ) {
                    // Confirm it's a call (followed by a parenthesis), not a property.
                    $afterPtr = $phpcsFile->findNext(T_WHITESPACE, $methodPtr + 1, null, true);
                    if ($afterPtr !== false && $tokens[$afterPtr]['code'] === T_OPEN_PARENTHESIS) {
                        $phpcsFile->addError(
                            'Do not resolve store/website context in __construct(); "%s()" needs a '
                            . 'bootstrapped application and breaks during setup:install. '
                            . 'Inject the dependency and resolve the store lazily where it is used.',
                            $methodPtr,
                            'StoreResolutionInConstructor',
                            [$tokens[$methodPtr]['content']]
                        );
                    }
                }
            }

            $ptr++;
        }
    }
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    proposalNew rule proposal

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions