magento / magento/magento-coding-standard
feat: add "No store/website resolution in constructors"
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 375
- Forks
- 165
- PR merge metrics
- No merged PRs in 30d
Description
`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:
```bash
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
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++;
}
}
}
```
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with dev/phpcs/Magento2/Sniffs/Constructor/NoStoreResolutionInConstructorSniff.php and its registration in phpcs.xml. Run composer lint, or vendor/bin/phpcs --standard=dev/phpcs/Magento2/ruleset.xml app/code/Magento2/, and verify that listed store-resolution calls in constructors are reported while nested closures and same-named property access are ignored.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100