From 73c15ecd256ad3a5b64347ec17d7c4d20cc80394 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Tue, 28 Jan 2020 16:34:34 +0100 Subject: [PATCH 1/2] misc --- .../Rector/If_/ExplicitBoolCompareRector.php | 3 ++- .../NodeTypeResolver/src/NodeTypeResolver.php | 22 ++++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/CodeQuality/src/Rector/If_/ExplicitBoolCompareRector.php b/packages/CodeQuality/src/Rector/If_/ExplicitBoolCompareRector.php index d2eb18edfed8..a7852b18b5c8 100644 --- a/packages/CodeQuality/src/Rector/If_/ExplicitBoolCompareRector.php +++ b/packages/CodeQuality/src/Rector/If_/ExplicitBoolCompareRector.php @@ -12,6 +12,7 @@ use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\BinaryOp\NotIdentical; use PhpParser\Node\Expr\BooleanNot; +use PhpParser\Node\Expr\Cast\Bool_; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\Ternary; use PhpParser\Node\Scalar\DNumber; @@ -90,7 +91,7 @@ public function refactor(Node $node): ?Node $isNegated = false; } - if ($conditionNode instanceof Expr\Cast\Bool_) { + if ($conditionNode instanceof Bool_) { return null; } diff --git a/packages/NodeTypeResolver/src/NodeTypeResolver.php b/packages/NodeTypeResolver/src/NodeTypeResolver.php index f51c206c293a..457359f6cd0d 100644 --- a/packages/NodeTypeResolver/src/NodeTypeResolver.php +++ b/packages/NodeTypeResolver/src/NodeTypeResolver.php @@ -199,15 +199,8 @@ public function isNullableType(Node $node): bool public function getStaticType(Node $node): Type { if ($this->isArrayExpr($node)) { - /** @var Scope $scope */ - $scope = $node->getAttribute(AttributeKey::SCOPE); /** @var Expr $node */ - $arrayType = $scope->getType($node); - if ($arrayType instanceof ArrayType) { - return $arrayType; - } - - return new ArrayType(new MixedType(), new MixedType()); + return $this->resolveArrayType($node); } if ($node instanceof Arg) { @@ -488,4 +481,17 @@ private function isArrayExpr(Node $node): bool { return $node instanceof Expr && $this->arrayTypeAnalyzer->isArrayType($node); } + + private function resolveArrayType(Expr $expr): ArrayType + { + /** @var Scope $scope */ + $scope = $expr->getAttribute(AttributeKey::SCOPE); + + $arrayType = $scope->getType($expr); + if ($arrayType instanceof ArrayType) { + return $arrayType; + } + + return new ArrayType(new MixedType(), new MixedType()); + } } From 5afed89800d75268c8a3ed870c613f013f5aa1b0 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Tue, 28 Jan 2020 17:01:22 +0100 Subject: [PATCH 2/2] decouple FunctionToServiceMap --- packages/Laravel/config/config.yaml | 10 + packages/Laravel/src/FunctionToServiceMap.php | 174 ++++++++++++++ ...erFunctionToConstructorInjectionRector.php | 219 +++++------------- .../ValueObject/ArrayFunctionToMethodCall.php | 72 ++++++ .../src/ValueObject/FunctionToMethodCall.php | 68 ++++++ .../NodeTypeResolver/src/NodeTypeResolver.php | 10 +- .../src/ValueObject/ServiceMap/ServiceMap.php | 18 -- phpstan.neon | 2 + 8 files changed, 386 insertions(+), 187 deletions(-) create mode 100644 packages/Laravel/config/config.yaml create mode 100644 packages/Laravel/src/FunctionToServiceMap.php create mode 100644 packages/Laravel/src/ValueObject/ArrayFunctionToMethodCall.php create mode 100644 packages/Laravel/src/ValueObject/FunctionToMethodCall.php diff --git a/packages/Laravel/config/config.yaml b/packages/Laravel/config/config.yaml new file mode 100644 index 000000000000..249d8bb1b979 --- /dev/null +++ b/packages/Laravel/config/config.yaml @@ -0,0 +1,10 @@ +services: + _defaults: + public: true + autowire: true + + Rector\Laravel\: + resource: '../src' + exclude: + - '../src/Rector/**/*Rector.php' + - '../src/ValueObject/*' diff --git a/packages/Laravel/src/FunctionToServiceMap.php b/packages/Laravel/src/FunctionToServiceMap.php new file mode 100644 index 000000000000..a690894eb7fb --- /dev/null +++ b/packages/Laravel/src/FunctionToServiceMap.php @@ -0,0 +1,174 @@ +initFunctionToMethodCalls(); + $this->initArrayFunctionToMethodCalls(); + } + + /** + * @return FunctionToMethodCall|ArrayFunctionToMethodCall|null + */ + public function findByFunction(string $functionName): ?object + { + foreach ($this->functionToMethodCalls as $functionToMethodCall) { + if ($functionToMethodCall->getFunction() !== $functionName) { + continue; + } + + return $functionToMethodCall; + } + + foreach ($this->arrayFunctionToMethodCalls as $arrayFunctionToMethodCall) { + if ($arrayFunctionToMethodCall->getFunction() !== $functionName) { + continue; + } + + return $arrayFunctionToMethodCall; + } + + return null; + } + + private function initArrayFunctionToMethodCalls(): void + { + $this->arrayFunctionToMethodCalls[] = new ArrayFunctionToMethodCall( + 'config', + 'Illuminate\Contracts\Config\Repository', + 'configRepository', + 'set', + 'get' + ); + + $this->arrayFunctionToMethodCalls[] = new ArrayFunctionToMethodCall( + 'session', + 'Illuminate\Session\SessionManager', + 'sessionManager', + 'put', + 'get' + ); + } + + private function initFunctionToMethodCalls(): void + { + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'auth', 'Illuminate\Contracts\Auth\Guard', 'guard' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'policy', 'Illuminate\Contracts\Auth\Access\Gate', 'policy', 'getPolicyFor' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'cookie', 'Illuminate\Contracts\Cookie\Factory', 'cookieFactory', 'make' + ); + + // router + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'put', 'Illuminate\Routing\Router', 'router', 'put' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'get', 'Illuminate\Routing\Router', 'router', 'get' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'post', 'Illuminate\Routing\Router', 'router', 'post' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'patch', 'Illuminate\Routing\Router', 'router', 'patch' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'delete', 'Illuminate\Routing\Router', 'router', 'delete' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'resource', 'Illuminate\Routing\Router', 'router', 'resource' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'response', 'Illuminate\Contracts\Routing\ResponseFactory', 'responseFactory', 'make' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'info', 'Illuminate\Log\Writer', 'logWriter', 'info' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'view', 'Illuminate\Contracts\View\Factory', 'viewFactory', 'make' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'bcrypt', 'Illuminate\Hashing\BcryptHasher', 'bcryptHasher', 'make' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'redirect', 'Illuminate\Routing\Redirector', 'redirector', 'back' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'broadcast', 'Illuminate\Contracts\Broadcasting\Factory', 'broadcastFactory', 'event' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'event', 'Illuminate\Events\Dispatcher', 'eventDispatcher', 'fire' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'dispatch', 'Illuminate\Events\Dispatcher', 'eventDispatcher', 'dispatch' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'route', 'Illuminate\Routing\UrlGenerator', 'urlGenerator', 'route' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'asset', 'Illuminate\Routing\UrlGenerator', 'urlGenerator', 'asset' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'url', 'Illuminate\Contracts\Routing\UrlGenerator', 'urlGenerator', 'to' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'action', 'Illuminate\Routing\UrlGenerator', 'urlGenerator', 'action' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'trans', 'Illuminate\Translation\Translator', 'translator', 'trans' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'trans_choice', 'Illuminate\Translation\Translator', 'translator', 'transChoice' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'logger', 'Illuminate\Log\Writer', 'logWriter', 'debug' + ); + + $this->functionToMethodCalls[] = new FunctionToMethodCall( + 'back', 'Illuminate\Routing\Redirector', 'redirector', 'back', 'back' + ); + } +} diff --git a/packages/Laravel/src/Rector/FuncCall/HelperFunctionToConstructorInjectionRector.php b/packages/Laravel/src/Rector/FuncCall/HelperFunctionToConstructorInjectionRector.php index cdb5f4355335..2d05e4085d28 100644 --- a/packages/Laravel/src/Rector/FuncCall/HelperFunctionToConstructorInjectionRector.php +++ b/packages/Laravel/src/Rector/FuncCall/HelperFunctionToConstructorInjectionRector.php @@ -9,6 +9,9 @@ use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Stmt\Class_; use Rector\Exception\ShouldNotHappenException; +use Rector\Laravel\FunctionToServiceMap; +use Rector\Laravel\ValueObject\ArrayFunctionToMethodCall; +use Rector\Laravel\ValueObject\FunctionToMethodCall; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\PHPStan\Type\FullyQualifiedObjectType; use Rector\Rector\AbstractRector; @@ -24,152 +27,14 @@ final class HelperFunctionToConstructorInjectionRector extends AbstractRector { /** - * @var string[][] + * @var FunctionToServiceMap */ - private $functionToService = [ - // set/get - 'config' => [ - 'type' => 'Illuminate\Contracts\Config\Repository', - 'property' => 'configRepository', - 'array_method' => 'set', - 'non_array_method' => 'get', - ], - 'session' => [ - 'type' => 'Illuminate\Session\SessionManager', - 'property' => 'sessionManager', - 'array_method' => 'put', - 'non_array_method' => 'get', - ], + private $functionToServiceMap; - // methods if args/property fetch - 'policy' => [ - 'type' => 'Illuminate\Contracts\Auth\Access\Gate', - 'property' => 'policy', - 'method_if_args' => 'getPolicyFor', - ], - 'cookie' => [ - 'type' => 'Illuminate\Contracts\Cookie\Factory', - 'property' => 'cookieFactory', - 'method_if_args' => 'make', - ], - // router - 'put' => [ - 'type' => 'Illuminate\Routing\Router', - 'property' => 'router', - 'method_if_args' => 'put', - ], - 'get' => [ - 'type' => 'Illuminate\Routing\Router', - 'property' => 'router', - 'method_if_args' => 'get', - ], - 'post' => [ - 'type' => 'Illuminate\Routing\Router', - 'property' => 'router', - 'method_if_args' => 'post', - ], - 'patch' => [ - 'type' => 'Illuminate\Routing\Router', - 'property' => 'router', - 'method_if_args' => 'patch', - ], - 'delete' => [ - 'type' => 'Illuminate\Routing\Router', - 'property' => 'router', - 'method_if_args' => 'delete', - ], - 'resource' => [ - 'type' => 'Illuminate\Routing\Router', - 'property' => 'router', - 'method_if_args' => 'resource', - ], - 'response' => [ - 'type' => 'Illuminate\Contracts\Routing\ResponseFactory', - 'property' => 'responseFactory', - 'method_if_args' => 'make', - ], - - 'info' => [ - 'type' => 'Illuminate\Log\Writer', - 'property' => 'logWriter', - 'method_if_args' => 'info', - ], - 'view' => [ - 'type' => 'Illuminate\Contracts\View\Factory', - 'property' => 'viewFactory', - 'method_if_args' => 'make', - ], - 'bcrypt' => [ - 'type' => 'Illuminate\Hashing\BcryptHasher', - 'property' => 'bcryptHasher', - 'method_if_args' => 'make', - ], - 'redirect' => [ - 'type' => 'Illuminate\Routing\Redirector', - 'property' => 'redirector', - 'method_if_args' => 'back', - ], - 'back' => [ - 'type' => 'Illuminate\Routing\Redirector', - 'property' => 'redirector', - 'method_if_args' => 'back', - 'method_if_no_args' => 'back', - ], - 'broadcast' => [ - 'type' => 'Illuminate\Contracts\Broadcasting\Factory', - 'property' => 'broadcastFactory', - 'method_if_args' => 'event', - ], - 'event' => [ - 'type' => 'Illuminate\Events\Dispatcher', - 'property' => 'eventDispatcher', - 'method_if_args' => 'fire', - ], - 'dispatch' => [ - 'type' => 'Illuminate\Events\Dispatcher', - 'property' => 'eventDispatcher', - 'method_if_args' => 'dispatch', - ], - 'route' => [ - 'type' => 'Illuminate\Routing\UrlGenerator', - 'property' => 'urlGenerator', - 'method_if_args' => 'route', - ], - 'auth' => [ - 'type' => 'Illuminate\Contracts\Auth\Guard', - 'property' => 'guard', - ], - 'asset' => [ - 'type' => 'Illuminate\Routing\UrlGenerator', - 'property' => 'urlGenerator', - 'method_if_args' => 'asset', - ], - 'url' => [ - 'type' => 'Illuminate\Contracts\Routing\UrlGenerator', - 'property' => 'urlGenerator', - 'method_if_args' => 'to', - ], - 'action' => [ - 'type' => 'Illuminate\Routing\UrlGenerator', - 'property' => 'urlGenerator', - 'method_if_args' => 'action', - ], - 'trans' => [ - 'type' => 'Illuminate\Translation\Translator', - 'property' => 'translator', - 'method_if_args' => 'trans', - ], - 'trans_choice' => [ - 'type' => 'Illuminate\Translation\Translator', - 'property' => 'translator', - 'method_if_args' => 'transChoice', - ], - 'logger' => [ - 'type' => 'Illuminate\Log\Writer', - 'property' => 'logWriter', - 'method_if_args' => 'debug', - ], - ]; + public function __construct(FunctionToServiceMap $functionToServiceMap) + { + $this->functionToServiceMap = $functionToServiceMap; + } public function getDefinition(): RectorDefinition { @@ -230,38 +95,45 @@ public function refactor(Node $node): ?Node /** @var Class_ $classNode */ $classNode = $node->getAttribute(AttributeKey::CLASS_NODE); - foreach ($this->functionToService as $function => $service) { - if (! $this->isName($node, $function)) { - continue; - } + $functionName = $this->getName($node); + if ($functionName === null) { + return null; + } - $this->addPropertyToClass($classNode, new FullyQualifiedObjectType($service['type']), $service['property']); - $propertyFetchNode = $this->createPropertyFetch('this', $service['property']); + $functionChange = $this->functionToServiceMap->findByFunction($functionName); + if ($functionChange === null) { + return null; + } - if (count($node->args) === 0) { - if (isset($service['method_if_no_args'])) { - return new MethodCall($propertyFetchNode, $service['method_if_no_args']); - } + $objectType = new FullyQualifiedObjectType($functionChange->getClass()); + $this->addPropertyToClass($classNode, $objectType, $functionChange->getProperty()); - return $propertyFetchNode; - } + $propertyFetchNode = $this->createPropertyFetch('this', $functionChange->getProperty()); - if (isset($service['method_if_args']) && count($node->args) >= 1) { - return new MethodCall($propertyFetchNode, $service['method_if_args'], $node->args); + if (count($node->args) === 0) { + if ($functionChange instanceof FunctionToMethodCall && $functionChange->getMethodIfNoArgs()) { + return new MethodCall($propertyFetchNode, $functionChange->getMethodIfNoArgs()); } - if (isset($service['array_method']) && $this->isArrayType($node->args[0]->value)) { - return new MethodCall($propertyFetchNode, $service['array_method'], $node->args); - } + return $propertyFetchNode; + } + + if ($this->isFunctionToMethodCallWithArgs($node, $functionChange)) { + /** @var FunctionToMethodCall $functionChange */ + return new MethodCall($propertyFetchNode, $functionChange->getMethodIfArgs(), $node->args); + } - if (isset($service['non_array_method']) && ! $this->isArrayType($node->args[0]->value)) { - return new MethodCall($propertyFetchNode, $service['non_array_method'], $node->args); + if ($functionChange instanceof ArrayFunctionToMethodCall) { + if ($functionChange->getArrayMethod() && $this->isArrayType($node->args[0]->value)) { + return new MethodCall($propertyFetchNode, $functionChange->getArrayMethod(), $node->args); } - throw new ShouldNotHappenException(); + if ($functionChange->getNonArrayMethod() && ! $this->isArrayType($node->args[0]->value)) { + return new MethodCall($propertyFetchNode, $functionChange->getNonArrayMethod(), $node->args); + } } - return null; + throw new ShouldNotHappenException(); } private function shouldSkipFuncCall(FuncCall $funcCall): bool @@ -277,6 +149,23 @@ private function shouldSkipFuncCall(FuncCall $funcCall): bool if ($classMethod === null) { return true; } + return $classMethod->isStatic(); } + + /** + * @param FunctionToMethodCall|ArrayFunctionToMethodCall $functionChange + */ + private function isFunctionToMethodCallWithArgs(Node $node, $functionChange): bool + { + if (! $functionChange instanceof FunctionToMethodCall) { + return false; + } + + if ($functionChange->getMethodIfArgs() === null) { + return false; + } + + return count($node->args) >= 1; + } } diff --git a/packages/Laravel/src/ValueObject/ArrayFunctionToMethodCall.php b/packages/Laravel/src/ValueObject/ArrayFunctionToMethodCall.php new file mode 100644 index 000000000000..f79e1b717dc5 --- /dev/null +++ b/packages/Laravel/src/ValueObject/ArrayFunctionToMethodCall.php @@ -0,0 +1,72 @@ +function = $function; + $this->class = $class; + $this->property = $property; + $this->arrayMethod = $arrayMethod; + $this->nonArrayMethod = $nonArrayMethod; + } + + public function getFunction(): string + { + return $this->function; + } + + public function getClass(): string + { + return $this->class; + } + + public function getProperty(): string + { + return $this->property; + } + + public function getArrayMethod(): string + { + return $this->arrayMethod; + } + + public function getNonArrayMethod(): string + { + return $this->nonArrayMethod; + } +} diff --git a/packages/Laravel/src/ValueObject/FunctionToMethodCall.php b/packages/Laravel/src/ValueObject/FunctionToMethodCall.php new file mode 100644 index 000000000000..980c68877e82 --- /dev/null +++ b/packages/Laravel/src/ValueObject/FunctionToMethodCall.php @@ -0,0 +1,68 @@ +function = $function; + $this->class = $class; + $this->property = $property; + $this->methodIfArgs = $methodIfArgs; + $this->methodIfNoArgs = $methodIfNoArgs; + } + + public function getFunction(): string + { + return $this->function; + } + + public function getClass(): string + { + return $this->class; + } + + public function getProperty(): string + { + return $this->property; + } + + public function getMethodIfNoArgs(): ?string + { + return $this->methodIfNoArgs; + } + + public function getMethodIfArgs(): ?string + { + return $this->methodIfArgs; + } +} diff --git a/packages/NodeTypeResolver/src/NodeTypeResolver.php b/packages/NodeTypeResolver/src/NodeTypeResolver.php index 457359f6cd0d..e708a851a28a 100644 --- a/packages/NodeTypeResolver/src/NodeTypeResolver.php +++ b/packages/NodeTypeResolver/src/NodeTypeResolver.php @@ -484,12 +484,14 @@ private function isArrayExpr(Node $node): bool private function resolveArrayType(Expr $expr): ArrayType { - /** @var Scope $scope */ + /** @var Scope|null $scope */ $scope = $expr->getAttribute(AttributeKey::SCOPE); - $arrayType = $scope->getType($expr); - if ($arrayType instanceof ArrayType) { - return $arrayType; + if ($scope instanceof Scope) { + $arrayType = $scope->getType($expr); + if ($arrayType instanceof ArrayType) { + return $arrayType; + } } return new ArrayType(new MixedType(), new MixedType()); diff --git a/packages/Symfony/src/ValueObject/ServiceMap/ServiceMap.php b/packages/Symfony/src/ValueObject/ServiceMap/ServiceMap.php index 85df1aef69c2..169b8f5c7a1b 100644 --- a/packages/Symfony/src/ValueObject/ServiceMap/ServiceMap.php +++ b/packages/Symfony/src/ValueObject/ServiceMap/ServiceMap.php @@ -4,11 +4,8 @@ namespace Rector\Symfony\ValueObject\ServiceMap; -use PhpParser\Node\Expr; -use PHPStan\Analyser\Scope; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; -use PHPStan\Type\TypeUtils; use Rector\Symfony\ValueObject\ServiceDefinition; final class ServiceMap @@ -26,14 +23,6 @@ public function __construct(array $services) $this->services = $services; } - /** - * @return ServiceDefinition[] - */ - public function getServices(): array - { - return $this->services; - } - public function hasService(string $id): bool { return isset($this->services[$id]); @@ -88,11 +77,4 @@ public function getServicesByTag(string $tagName): array return $servicesWithTag; } - - public function getServiceIdFromNode(Expr $expr, Scope $scope): ?string - { - $strings = TypeUtils::getConstantStrings($scope->getType($expr)); - - return count($strings) === 1 ? $strings[0]->getValue() : null; - } } diff --git a/phpstan.neon b/phpstan.neon index a046f7a21b10..1f7f31781824 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -231,3 +231,5 @@ parameters: - '#Parameter \#1 \$name of method Rector\\Rector\\AbstractRector\:\:getShortName\(\) expects PhpParser\\Node\\Identifier\|PhpParser\\Node\\Name\|string, PhpParser\\Node\\Identifier\|null given#' - '#Parameter \#1 \$entityClass of method Rector\\CakePHPToSymfony\\Rector\\NodeFactory\\DoctrineNodeFactory\:\:createConstructorWithGetRepositoryAssign\(\) expects string, string\|null given#' - '#Parameter \#1 \$node of method PHPStan\\Analyser\\Scope\:\:getType\(\) expects PhpParser\\Node\\Expr, PhpParser\\Node given#' + + - '#Parameter \#2 \$name of class PhpParser\\Node\\Expr\\MethodCall constructor expects PhpParser\\Node\\Expr\|PhpParser\\Node\\Identifier\|string, string\|null given#'