[no squash] Use DI container to inject ContentFetchers class#2910
[no squash] Use DI container to inject ContentFetchers class#2910jtojnar wants to merge 10 commits intoRSS-Bridge:masterfrom
Conversation
lib/Container.php
Outdated
There was a problem hiding this comment.
We do not need the PSR-11 interface for now – it will become important once we want to allow library consumers to use their own DI container.
|
It's a good pr. Won't merge because of too much introduced complexity and enterprisey logic. |
|
I removed the PSR 11 interface and constructors of individual bridges (they can get the |
|
Sorry too much complexity. Don't want that container. If were to use a container it's not gonna do autowiring. Also I want to preserve |
This is just a very stupid DI container that creates requested objects and passes them dependencies based on typehints of their constructor methods. In the future, we may want to implement PSR-11 so that consumers of the library can re-use their own dependency container instead of relying on this one. https://www.php-fig.org/psr/psr-11/ Use singleton for now, until we can use DI everywhere.
|
Can’t really say I like it but here it is without a container: #2939 |
In the future, we will inject it a Cache instance.
So that the lines do not overflow once we apply the refactoring.
They need to be regular methods to have access fetcher property we are going to add next.
So that bridges do not need to inject it themselves and we do not need to use global functions.
So that we do not need to use global functions.
```
$ composer require --dev rector/rector
$ vendor/bin/rector --version
Rector 0.13.9
$ echo > rector.php '<?php
declare(strict_types=1);
namespace Rules {
use PhpParser\Node;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\Stmt\Class_;
use PHPStan\Type\ObjectType;
use Rector\Core\Exception\ShouldNotHappenException;
use Rector\Core\NodeManipulator\ClassDependencyManipulator;
use Rector\Core\Rector\AbstractRector;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\PostRector\Collector\PropertyToAddCollector;
use Rector\PostRector\ValueObject\PropertyMetadata;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
// Rector seems to re-import the file for some reason.
if (!class_exists(RenameContentsFunctions::class)) {
class RenameContentsFunctions extends AbstractRector
{
private const FUNCTIONS_TO_RENAME = [
"getContents",
"getSimpleHTMLDOM",
"getSimpleHTMLDOMCached",
];
private const FETCHER_PROPERTY_NAME = "fetcher";
public function __construct(
private readonly ClassDependencyManipulator $classDependencyManipulator,
private readonly PropertyToAddCollector $propertyToAddCollector,
) {
}
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition("Updates references to contents functions.", []);
}
/**
* @return array<class-string<Node>>
*/
public function getNodeTypes(): array
{
return [FuncCall::class];
}
/**
* @param FuncCall $node
*/
public function refactor(Node $node): ?Node
{
foreach (self::FUNCTIONS_TO_RENAME as $function) {
if (! $this->isName($node, $function)) {
continue;
}
// not to refactor here
$isVirtual = (bool) $node->name->getAttribute(AttributeKey::VIRTUAL_NODE);
if ($isVirtual) {
continue;
}
// $this->addFetcherInjection($node);
$fetcher = new PropertyFetch(new Variable("this"), new Identifier(self::FETCHER_PROPERTY_NAME));
return new MethodCall($fetcher, $function, $node->args);
}
return null;
}
private function addFetcherInjection(Node $node): void
{
$class = $this->betterNodeFinder->findParentType($node, Class_::class);
if (! $class instanceof Class_) {
throw new ShouldNotHappenException();
}
$propertyMetadata = new PropertyMetadata(self::FETCHER_PROPERTY_NAME, new ObjectType("Fetcher"), Class_::MODIFIER_PRIVATE);
$this->propertyToAddCollector->addPropertyToClass($class, $propertyMetadata);
$this->classDependencyManipulator->addConstructorDependency($class, $propertyMetadata);
}
}
}
}
namespace {
use Rector\Config\RectorConfig;
return static function (RectorConfig $rectorConfig): void {
$rectorConfig->paths([
__DIR__ . "/actions",
__DIR__ . "/bridges",
__DIR__ . "/caches",
__DIR__ . "/formats",
__DIR__ . "/lib",
]);
$rectorConfig->rule(Rules\RenameContentsFunctions::class);
};
}'
$ vendor/bin/rector
```
Rector tends to squash function call arguments to a single line but since there is a lot of arguments, it violates line length limit of our coding style. PHP CodeSniffer should be wise enough to split the arguments one-per-line but unfortunately it does not so let’s restore the original format manually.
Motivation
Cache,Logger) in a constructor and the container will pass them automatically upon instantiation.$_SERVERsuperglobal and having to hustle with it in tests, they will just takeRequestInfoobject as a dependency and in tests, we will pass a custom one through the container.CacheInterfaceimplementation for library consumers: As simple as$dic->add(CacheInterface::class, fn () => new MyCustomCache());What this PR does
This is in preparation to allow us injecting
Cacheinstance, commits to be reviewed independently:lib/contents.phpinto a newContentsFetcherclass so that it can receiveCache.ContentsFetcherinto theBridgeAbstractvia a constructor and madeBridgeFactoryuse theContainerto create bridges.contentsFetcher.RSS-Bridge should be fully functional at each of the commits.