-
Notifications
You must be signed in to change notification settings - Fork 10
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Include and document ObjectInstantiationSniff as experimental rule (see
#17)
- Loading branch information
1 parent
600e3ca
commit 2a3fb96
Showing
3 changed files
with
213 additions
and
0 deletions.
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Extdn\Samples\Classes; | ||
|
||
class ObjectInstantiation | ||
{ | ||
public function instantiateSomeObjects() | ||
{ | ||
new \DateTime('yesterday'); | ||
|
||
new stdclass; | ||
|
||
$className = 'stdclass'; | ||
|
||
new $className; | ||
|
||
$anonymous = new class implements \Countable { | ||
public function count() | ||
{ | ||
return 0; | ||
} | ||
}; | ||
|
||
$e = new Exception; | ||
|
||
throw $e; | ||
|
||
throw new Exception; | ||
} | ||
} |
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
} | ||
``` |