diff --git a/Extdn/Samples/Classes/ObjectInstantiation.php b/Extdn/Samples/Classes/ObjectInstantiation.php new file mode 100644 index 0000000..93ee266 --- /dev/null +++ b/Extdn/Samples/Classes/ObjectInstantiation.php @@ -0,0 +1,32 @@ + *.php + + */Test/* + 1 + diff --git a/MEQP2/Sniffs/Classes/ObjectInstantiationSniff.md b/MEQP2/Sniffs/Classes/ObjectInstantiationSniff.md new file mode 100644 index 0000000..093947c --- /dev/null +++ b/MEQP2/Sniffs/Classes/ObjectInstantiationSniff.md @@ -0,0 +1,177 @@ +# Rule: Keyword `new` should not be used +## Background + +The `new` keyword should not be used to instantiate new classes. Constructor DI should be used instead: +Inject a factory (e.g. `ProductInterfaceFactory`) which can be generated automatically, Magento then takes care of +which actual class to instantiate. + + +**This is an experimental rule, only applied with lowest severity treshold of 1.** + +## Reasoning +Magento uses constructor dependency injection with autowiring. It automatically generates interceptors that extend the +original class to add plugins. + +It is encouraged to use these mechanisms instead of instantiating objects directly with `new`. This way, Magento can transparently +replace the concrete class with an interceptor or alternative implementations based on DI preference configuration. + +This flexibility with plugins and preferences is lost if classes are instantiated directly. + +### Exceptions + +- Instantiating exceptions with `new` is allowed, there is no need for alternative implementations or plugins in exceptions +- The rule does not apply to unit tests, where automatic constructor DI is not present + +### Dispute + +**Classes from PHP core or non-Magento libraries:** While it is possible to generate factories for them too, it is +considered overkill by many. + +Compare + +```php +new DateTime('yesterday') +``` +vs +```php +$this->dateTimeFactory->create(['time' => 'yesterday']) +``` + +**Data transport objects:** Data transport objects (DTOs) are often used in events to allow changing values in observers: + +`new \Magento\Framework\DataObject(['key' => 'value])` + +Using factories seems unnecessary too here, and Magento does not do it in the core either. + +As an alternative, since `DataObject` is not more than a glorified array with magic getters and setters, an `stdclass` object could be used instead, circumventing +the `new` rule: `(object)['key' => 'value']` + +## How it works +Any usage of `new` that is not preceded by `throw` is considered a violation + +## How to fix + +Given code like this: + +```php +namespace Lotr; + +use Lotr\Ring; + +class Sauron +{ + public function forgeTheOneRing() + { + $ring = new Ring('Ash nazg durbatulûk, ash nazg gimbatul, ash nazg thrakatulûk, agh burzum-ishi krimpatul'); + $this->putOnFinger($ring); + } +} +``` +you would introduce constructor dependency injection with an autogenerated factory like this: + +```php +namespace Lotr; + +use Lotr\RingFactory; + +class Sauron +{ + /** + * @var RingFactory + */ + private $ringFactory; + + public function __construct(RingFactory $ringFactory) + { + $this->ringFactory = $ringFactory; + } + + public function forgeTheOneRing() + { + $ring = $this->ringFactory->create( + [ + 'inscription' => 'Ash nazg durbatulûk, ash nazg gimbatul, ash nazg thrakatulûk, agh burzum-ishi krimpatul' + ] + ); + $this->putOnFinger($ring); + } +} +``` + +Note that the argument for **create** is always an array where the keys have to equal the constructor argument names for +the class to be instantiated. So the example above assumes a constructor signature for `Ring` like: + +```php +public function __construct(string $inscription); +``` + +If there is an **interface** for `Ring`, e.g. `RingInterface` it is recommended to use a generated factory for the interface, so that the +actual implementation may be exchanged: + +```php +public function __construct(RingInterfaceFactory $ringFactory) +``` + +### Special case: Custom factory + +Sometimes, the automatically generated factories do not quite serve our needs and more specific creation methods than +the generic `create` are helpful. You can **create your own factory implementation**, Magento will only generate a +factory class if it cannot find an existing one. + +Let's say, we want specific factory methods for different rings, we could implement our own factory class like this: + +```php +namespace Lotr; + +class RingFactory +{ + public function createElvenRing() { ... } + public function createDwarfRing() { ... } + public function createHumanRing() { ... } + public function createTheOneRing() { ... } +} +``` + +In this factory, we still won't use the `new` keyword, but instead the object manager can be used (this is an exception to the rule "Never use the object manager directly"!) + +```php +namespace Lotr; + +use Lotr\Ring; + +class RingFactory +{ + /** + * @var \Magento\Framework\ObjectManagerInterface + */ + private $objectManager; + + public function __construct(\Magento\Framework\ObjectManagerInterface $objectManager) + { + $this->objectManager = $objectManager; + } + + public function createTheOneRing(): Ring + { + return $this->objectManager->create( + Ring::class, + [ + 'inscription' => 'Ash nazg durbatulûk, ash nazg gimbatul, ash nazg thrakatulûk, agh burzum-ishi krimpatul' + ] + ); + } +} +``` + +The signature of `ObjectManager::create()` is similar to the `create()` method of the automatically generated factories, +but additionally needs a class or interface name as first argument. + +With such a custom factory, Sauron can forge the ring and let the "ring factory" decide about the inscription: + +```php + public function forgeTheOneRing() + { + $ring = $this->ringFactory->createTheOneRing(); + $this->putOnFinger($ring); + } +``` \ No newline at end of file