Fix node invalid types discovered by PHPStan scope refresh (#1306)

This commit is contained in:
Tomas Votruba 2021-11-25 15:45:36 +03:00 committed by GitHub
parent 14f67de7e9
commit 62474db941
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 56 additions and 174 deletions

View File

@ -3,11 +3,9 @@
declare(strict_types=1);
use Rector\DependencyInjection\Rector\Class_\ActionInjectionToConstructorInjectionRector;
use Rector\DependencyInjection\Rector\Variable\ReplaceVariableByPropertyFetchRector;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
return static function (ContainerConfigurator $containerConfigurator): void {
$services = $containerConfigurator->services();
$services->set(ActionInjectionToConstructorInjectionRector::class);
$services->set(ReplaceVariableByPropertyFetchRector::class);
};

View File

@ -4,7 +4,6 @@ declare(strict_types=1);
use Rector\Core\Configuration\Option;
use Rector\DependencyInjection\Rector\Class_\ActionInjectionToConstructorInjectionRector;
use Rector\DependencyInjection\Rector\Variable\ReplaceVariableByPropertyFetchRector;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
return static function (ContainerConfigurator $containerConfigurator): void {
@ -13,5 +12,4 @@ return static function (ContainerConfigurator $containerConfigurator): void {
$services = $containerConfigurator->services();
$services->set(ActionInjectionToConstructorInjectionRector::class);
$services->set(ReplaceVariableByPropertyFetchRector::class);
};

View File

@ -4,22 +4,22 @@ declare(strict_types=1);
namespace Rector\DependencyInjection\Collector;
use PHPStan\Type\Type;
use PHPStan\Type\ObjectType;
final class VariablesToPropertyFetchCollection
{
/**
* @var Type[]
* @var array<string, ObjectType>
*/
private array $variableNameAndType = [];
public function addVariableNameAndType(string $name, Type $type): void
public function addVariableNameAndType(string $name, ObjectType $objectType): void
{
$this->variableNameAndType[$name] = $type;
$this->variableNameAndType[$name] = $objectType;
}
/**
* @return Type[]
* @return array<string, ObjectType>
*/
public function getVariableNamesAndTypes(): array
{

View File

@ -1,45 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\DependencyInjection\NodeAnalyzer;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\NodeNameResolver\NodeNameResolver;
final class ControllerClassMethodAnalyzer
{
public function __construct(
private BetterNodeFinder $betterNodeFinder,
private NodeNameResolver $nodeNameResolver
) {
}
public function isInControllerActionMethod(Variable $variable): bool
{
$class = $this->betterNodeFinder->findParentType($variable, Class_::class);
if (! $class instanceof Class_) {
return false;
}
$className = $this->nodeNameResolver->getName($class);
if (! is_string($className)) {
return false;
}
if (! \str_ends_with($className, 'Controller')) {
return false;
}
$classMethod = $this->betterNodeFinder->findParentType($variable, ClassMethod::class);
if (! $classMethod instanceof ClassMethod) {
return false;
}
// is probably in controller action
return $classMethod->isPublic();
}
}

View File

@ -5,10 +5,13 @@ declare(strict_types=1);
namespace Rector\DependencyInjection\Rector\Class_;
use PhpParser\Node;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Param;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use PHPStan\Type\ObjectType;
use Rector\Core\Exception\ShouldNotHappenException;
use Rector\Core\Rector\AbstractRector;
use Rector\DependencyInjection\Collector\VariablesToPropertyFetchCollection;
use Rector\PostRector\Collector\PropertyToAddCollector;
@ -25,7 +28,7 @@ final class ActionInjectionToConstructorInjectionRector extends AbstractRector
public function __construct(
private ServiceMapProvider $applicationServiceMapProvider,
private VariablesToPropertyFetchCollection $variablesToPropertyFetchCollection,
private PropertyToAddCollector $propertyToAddCollector
private PropertyToAddCollector $propertyToAddCollector,
) {
}
@ -48,13 +51,9 @@ CODE_SAMPLE
<<<'CODE_SAMPLE'
final class SomeController
{
/**
* @var ProductRepository
*/
private $productRepository;
public function __construct(ProductRepository $productRepository)
{
$this->productRepository = $productRepository;
public function __construct(
private ProductRepository $productRepository
) {
}
public function default()
@ -89,9 +88,40 @@ CODE_SAMPLE
$this->processClassMethod($node, $classMethod);
}
foreach ($node->getMethods() as $classMethod) {
$this->refactorVariablesToPropertyFetches($classMethod);
}
return $node;
}
private function refactorVariablesToPropertyFetches(ClassMethod $classMethod): void
{
if (! $classMethod->isPublic()) {
return;
}
$this->traverseNodesWithCallable((array) $classMethod->stmts, function (Node $node): ?PropertyFetch {
if (! $node instanceof Variable) {
return null;
}
foreach ($this->variablesToPropertyFetchCollection->getVariableNamesAndTypes() as $name => $objectType) {
if (! $this->isName($node, $name)) {
continue;
}
if (! $this->isObjectType($node, $objectType)) {
continue;
}
return $this->nodeFactory->createPropertyFetch('this', $name);
}
return null;
});
}
private function processClassMethod(Class_ $class, ClassMethod $classMethod): void
{
foreach ($classMethod->params as $key => $paramNode) {
@ -100,6 +130,9 @@ CODE_SAMPLE
}
$paramType = $this->getType($paramNode);
if (! $paramType instanceof ObjectType) {
throw new ShouldNotHappenException();
}
/** @var string $paramName */
$paramName = $this->getName($paramNode->var);

View File

@ -1,109 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\DependencyInjection\Rector\Variable;
use PhpParser\Node;
use PhpParser\Node\Expr\Variable;
use PHPStan\Type\ObjectType;
use Rector\Core\Rector\AbstractRector;
use Rector\DependencyInjection\Collector\VariablesToPropertyFetchCollection;
use Rector\DependencyInjection\NodeAnalyzer\ControllerClassMethodAnalyzer;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
/**
* @see \Rector\Tests\DependencyInjection\Rector\Class_\ActionInjectionToConstructorInjectionRector\ActionInjectionToConstructorInjectionRectorTest
*/
final class ReplaceVariableByPropertyFetchRector extends AbstractRector
{
public function __construct(
private VariablesToPropertyFetchCollection $variablesToPropertyFetchCollection,
private ControllerClassMethodAnalyzer $controllerClassMethodAnalyzer
) {
}
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Turns variable in controller action to property fetch, as follow up to action injection variable to property change.',
[
new CodeSample(
<<<'CODE_SAMPLE'
final class SomeController
{
/**
* @var ProductRepository
*/
private $productRepository;
public function __construct(ProductRepository $productRepository)
{
$this->productRepository = $productRepository;
}
public function default()
{
$products = $productRepository->fetchAll();
}
}
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
final class SomeController
{
/**
* @var ProductRepository
*/
private $productRepository;
public function __construct(ProductRepository $productRepository)
{
$this->productRepository = $productRepository;
}
public function default()
{
$products = $this->productRepository->fetchAll();
}
}
CODE_SAMPLE
),
]
);
}
/**
* @return array<class-string<Node>>
*/
public function getNodeTypes(): array
{
return [Variable::class];
}
/**
* @param Variable $node
*/
public function refactor(Node $node): ?Node
{
if (! $this->controllerClassMethodAnalyzer->isInControllerActionMethod($node)) {
return null;
}
foreach ($this->variablesToPropertyFetchCollection->getVariableNamesAndTypes() as $name => $type) {
if (! $this->isName($node, $name)) {
continue;
}
/** @var ObjectType $type */
if (! $this->isObjectType($node, $type)) {
continue;
}
return $this->nodeFactory->createPropertyFetch('this', $name);
}
return null;
}
}

View File

@ -123,7 +123,7 @@ CODE_SAMPLE
return;
}
$this->traverseNodesWithCallable($classMethod->stmts, function (Node $node): ?MethodCall {
$this->traverseNodesWithCallable($classMethod->stmts, function (Node $node): ?Arg {
if (! $node instanceof Arg) {
return null;
}
@ -139,7 +139,9 @@ CODE_SAMPLE
return null;
}
return $this->nodeFactory->createMethodCall($node->value, 'reveal');
$methodCall = $this->nodeFactory->createMethodCall($node->value, 'reveal');
$node->value = $methodCall;
return $node;
});
}

View File

@ -27,8 +27,13 @@ final class CondAndExpr
/**
* @return Expr[]
*/
public function getCondExprs(): array
public function getCondExprs(): array|null
{
// internally checked by PHPStan, cannot be empty array
if ($this->condExprs === []) {
return null;
}
return $this->condExprs;
}