From 7dcde279f9eca2c32ed0391fc6fbfafdf0eca003 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 21 Oct 2018 13:50:25 +0200 Subject: [PATCH 1/5] make use of NameResolver in Rector --- ...rrayAndArrayKeysToArrayKeyExistsRector.php | 21 ++++--------------- .../PhpParser/src/Rector/RemoveNodeRector.php | 3 +-- .../src/Rector/ConstantToStaticCallRector.php | 6 ++++-- .../Validator/ConstraintUrlOptionRector.php | 2 +- src/Builder/IdentifierRenamer.php | 18 ++++++++++++++-- src/NodeAnalyzer/NameResolver.php | 9 ++++++++ ...ositoryCallsByRepositoryPropertyRector.php | 4 +++- ...RenameClassConstantsUseToStringsRector.php | 2 +- .../Function_/FunctionToStaticCallRector.php | 2 +- .../ChangeConstantVisibilityRector.php | 6 ++++-- .../ChangeMethodVisibilityRector.php | 4 ++-- 11 files changed, 46 insertions(+), 31 deletions(-) diff --git a/packages/CodeQuality/src/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector.php b/packages/CodeQuality/src/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector.php index 5d0b273d0b66..108319af2b8c 100644 --- a/packages/CodeQuality/src/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector.php +++ b/packages/CodeQuality/src/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector.php @@ -32,7 +32,7 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if (! $this->isInArrayFunction($node)) { + if (! $this->isName($node, 'in_array')) { return null; } @@ -41,33 +41,20 @@ public function refactor(Node $node): ?Node return null; } - /** @var Name $functionName */ - $functionName = $secondArgument->name; - if ($functionName->toString() !== 'array_keys') { + if (! $this->isName($secondArgument, 'array_keys')) { return null; } + if (count($secondArgument->args) > 1) { return null; } [$key, $array] = $node->args; - $array = $array->value->args[0]; - $node->args = [$key, $array]; - $node->name = new Name('array_key_exists'); + $node->args = [$key, $array]; return $node; } - - private function isInArrayFunction(FuncCall $funcCallNode): bool - { - $funcCallName = $funcCallNode->name; - if (! $funcCallName instanceof Name) { - return false; - } - - return $funcCallName->toString() === 'in_array'; - } } diff --git a/packages/PhpParser/src/Rector/RemoveNodeRector.php b/packages/PhpParser/src/Rector/RemoveNodeRector.php index 3f6b0a5fd5f4..8e8caaa98506 100644 --- a/packages/PhpParser/src/Rector/RemoveNodeRector.php +++ b/packages/PhpParser/src/Rector/RemoveNodeRector.php @@ -69,8 +69,7 @@ public function refactor(Node $node): ?Node return null; } - $value = $node->expr->name->toString(); - if ($value !== 'false') { + if (! $this->isName($node->expr, 'false')) { return null; } diff --git a/packages/Silverstripe/src/Rector/ConstantToStaticCallRector.php b/packages/Silverstripe/src/Rector/ConstantToStaticCallRector.php index 3eee09a64aa6..7c89a2ee1341 100644 --- a/packages/Silverstripe/src/Rector/ConstantToStaticCallRector.php +++ b/packages/Silverstripe/src/Rector/ConstantToStaticCallRector.php @@ -35,12 +35,14 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if (! Strings::startsWith($node->name->toString(), 'SS_')) { + $constantName = $this->getName($node); + + if (! Strings::startsWith($constantName, 'SS_')) { return null; } $staticCallNode = new StaticCall(new FullyQualified('Environment'), 'getEnv'); - $staticCallNode->args[] = new Arg(new String_($node->name->toString())); + $staticCallNode->args[] = new Arg(new String_($constantName)); return $staticCallNode; } diff --git a/packages/Symfony/src/Rector/Validator/ConstraintUrlOptionRector.php b/packages/Symfony/src/Rector/Validator/ConstraintUrlOptionRector.php index 63f5e617b3fa..4aa723becf38 100644 --- a/packages/Symfony/src/Rector/Validator/ConstraintUrlOptionRector.php +++ b/packages/Symfony/src/Rector/Validator/ConstraintUrlOptionRector.php @@ -54,7 +54,7 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if ($node->name->toString() !== 'true') { + if (! $this->isName($node, 'true')) { return null; } diff --git a/src/Builder/IdentifierRenamer.php b/src/Builder/IdentifierRenamer.php index faaf384f101f..73adfd030db9 100644 --- a/src/Builder/IdentifierRenamer.php +++ b/src/Builder/IdentifierRenamer.php @@ -3,6 +3,7 @@ namespace Rector\Builder; use PhpParser\Node; +use PhpParser\Node\Expr; use PhpParser\Node\Expr\ClassConstFetch; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\PropertyFetch; @@ -10,6 +11,7 @@ use PhpParser\Node\Identifier; use PhpParser\Node\Stmt\ClassMethod; use Rector\Exception\NodeChanger\NodeMissingIdentifierException; +use Rector\NodeAnalyzer\NameResolver; use function Safe\sprintf; /** @@ -20,6 +22,16 @@ */ final class IdentifierRenamer { + /** + * @var NameResolver + */ + private $nameResolver; + + public function __construct(NameResolver $nameResolver) + { + $this->nameResolver = $nameResolver; + } + /** * @var string[] */ @@ -42,8 +54,10 @@ public function renameNodeWithMap(Node $node, array $renameMethodMap): void { $this->ensureNodeHasIdentifier($node); - /** @var ClassConstFetch|MethodCall|PropertyFetch|StaticCall|ClassMethod $node */ - $oldNodeMethodName = $node->name->toString(); + $oldNodeMethodName = $this->nameResolver->resolve($node); + if (! $oldNodeMethodName) { + return; + } $node->name = new Identifier($renameMethodMap[$oldNodeMethodName]); } diff --git a/src/NodeAnalyzer/NameResolver.php b/src/NodeAnalyzer/NameResolver.php index d0817c1a5fc8..d2b7747a4950 100644 --- a/src/NodeAnalyzer/NameResolver.php +++ b/src/NodeAnalyzer/NameResolver.php @@ -4,12 +4,21 @@ use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Stmt\ClassConst; use PhpParser\Node\Stmt\Property; final class NameResolver { public function resolve(Node $node): ?string { + if ($node instanceof ClassConst) { + if (! count($node->consts)) { + return null; + } + + return $this->resolve($node->consts[0]); + } + if ($node instanceof Property) { if (! count($node->props)) { return null; diff --git a/src/Rector/Architecture/RepositoryAsService/ReplaceParentRepositoryCallsByRepositoryPropertyRector.php b/src/Rector/Architecture/RepositoryAsService/ReplaceParentRepositoryCallsByRepositoryPropertyRector.php index 031bc0a4f6b9..f6819e430a40 100644 --- a/src/Rector/Architecture/RepositoryAsService/ReplaceParentRepositoryCallsByRepositoryPropertyRector.php +++ b/src/Rector/Architecture/RepositoryAsService/ReplaceParentRepositoryCallsByRepositoryPropertyRector.php @@ -95,11 +95,13 @@ public function refactor(Node $node): ?Node return null; } - $methodName = $node->name->toString(); + $methodName = $this->getName($node);; + $entityClassReflection = $this->broker->getClass($this->entityRepositoryClass); if (! $entityClassReflection->hasMethod($methodName)) { return null; } + $methodReflection = $entityClassReflection->getMethod($methodName, $node->getAttribute(Attribute::SCOPE)); if (! $methodReflection->isPublic()) { return null; diff --git a/src/Rector/Constant/RenameClassConstantsUseToStringsRector.php b/src/Rector/Constant/RenameClassConstantsUseToStringsRector.php index b5dda8d8cd5c..3629fd91afaf 100644 --- a/src/Rector/Constant/RenameClassConstantsUseToStringsRector.php +++ b/src/Rector/Constant/RenameClassConstantsUseToStringsRector.php @@ -79,7 +79,7 @@ private function getClassNameFromClassConstFetch(ClassConstFetch $classConstFetc $fqnName = $classConstFetchNode->class->getAttribute(Attribute::RESOLVED_NAME); if ($fqnName === null && $classConstFetchNode->class instanceof Variable) { - return (string) $classConstFetchNode->class->name; + return $this->getName($classConstFetchNode->class); } if ($fqnName !== null) { diff --git a/src/Rector/Function_/FunctionToStaticCallRector.php b/src/Rector/Function_/FunctionToStaticCallRector.php index 798905c8537b..59fb7735e057 100644 --- a/src/Rector/Function_/FunctionToStaticCallRector.php +++ b/src/Rector/Function_/FunctionToStaticCallRector.php @@ -59,7 +59,7 @@ public function refactor(Node $node): ?Node return null; } - $functionName = $node->name->toString(); + $functionName = $this->getName($node); if (! isset($this->functionToStaticCall[$functionName])) { return null; } diff --git a/src/Rector/Visibility/ChangeConstantVisibilityRector.php b/src/Rector/Visibility/ChangeConstantVisibilityRector.php index cddd3857305a..adf3a9e19c1b 100644 --- a/src/Rector/Visibility/ChangeConstantVisibilityRector.php +++ b/src/Rector/Visibility/ChangeConstantVisibilityRector.php @@ -89,11 +89,13 @@ public function refactor(Node $node): ?Node if (! $node->hasAttribute(Attribute::PARENT_CLASS_NAME)) { return null; } + $nodeParentClassName = $node->getAttribute(Attribute::PARENT_CLASS_NAME); if (! isset($this->constantToVisibilityByClass[$nodeParentClassName])) { return null; } - $constantName = $node->consts[0]->name->toString(); + + $constantName = $this->getName($node); if (! isset($this->constantToVisibilityByClass[$nodeParentClassName][$constantName])) { return null; } @@ -108,7 +110,7 @@ public function refactor(Node $node): ?Node private function resolveNewVisibilityForNode(ClassConst $classConstantNode): string { $nodeParentClassName = $classConstantNode->getAttribute(Attribute::PARENT_CLASS_NAME); - $constantName = $classConstantNode->consts[0]->name->toString(); + $constantName = $this->getName($classConstantNode); return $this->constantToVisibilityByClass[$nodeParentClassName][$constantName]; } diff --git a/src/Rector/Visibility/ChangeMethodVisibilityRector.php b/src/Rector/Visibility/ChangeMethodVisibilityRector.php index f8e051de7239..625e00110b72 100644 --- a/src/Rector/Visibility/ChangeMethodVisibilityRector.php +++ b/src/Rector/Visibility/ChangeMethodVisibilityRector.php @@ -101,7 +101,7 @@ public function refactor(Node $node): ?Node if (! isset($this->methodToVisibilityByClass[$nodeParentClassName])) { return null; } - $methodName = $node->name->toString(); + $methodName = $this->getName($node); if (! isset($this->methodToVisibilityByClass[$nodeParentClassName][$methodName])) { return null; } @@ -116,7 +116,7 @@ public function refactor(Node $node): ?Node private function resolveNewVisibilityForNode(ClassMethod $classMethodNode): string { - $methodName = $classMethodNode->name->toString(); + $methodName = $this->getName($classMethodNode); $nodeParentClassName = $classMethodNode->getAttribute(Attribute::PARENT_CLASS_NAME); return $this->methodToVisibilityByClass[$nodeParentClassName][$methodName]; From 2723f5ab8a3f06a1e7027a6e9f34f29e3328ae27 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 21 Oct 2018 14:00:21 +0200 Subject: [PATCH 2/5] Fix false positive on missed false constant --- packages/PhpParser/src/Rector/RemoveNodeRector.php | 2 +- .../Rector/RemoveNodeRector/Correct/correct3.php.inc | 4 +++- .../Rector/RemoveNodeRector/Correct/correct4.php.inc | 11 +++++++++++ .../Rector/RemoveNodeRector/RemoveNodeRectorTest.php | 2 ++ .../Rector/RemoveNodeRector/Wrong/wrong3.php.inc | 4 +++- .../Rector/RemoveNodeRector/Wrong/wrong4.php.inc | 11 +++++++++++ src/Builder/IdentifierRenamer.php | 11 +++++------ ...arentRepositoryCallsByRepositoryPropertyRector.php | 2 +- .../RenameClassConstantsUseToStringsRector.php | 4 ++-- .../Visibility/ChangeMethodVisibilityRector.php | 5 ++--- 10 files changed, 41 insertions(+), 15 deletions(-) create mode 100644 packages/PhpParser/tests/Rector/RemoveNodeRector/Correct/correct4.php.inc create mode 100644 packages/PhpParser/tests/Rector/RemoveNodeRector/Wrong/wrong4.php.inc diff --git a/packages/PhpParser/src/Rector/RemoveNodeRector.php b/packages/PhpParser/src/Rector/RemoveNodeRector.php index 8e8caaa98506..4991e6ac08cb 100644 --- a/packages/PhpParser/src/Rector/RemoveNodeRector.php +++ b/packages/PhpParser/src/Rector/RemoveNodeRector.php @@ -69,7 +69,7 @@ public function refactor(Node $node): ?Node return null; } - if (! $this->isName($node->expr, 'false')) { + if (! $this->isFalse($node->expr)) { return null; } diff --git a/packages/PhpParser/tests/Rector/RemoveNodeRector/Correct/correct3.php.inc b/packages/PhpParser/tests/Rector/RemoveNodeRector/Correct/correct3.php.inc index b928b7627df7..9f50b2ebd42d 100644 --- a/packages/PhpParser/tests/Rector/RemoveNodeRector/Correct/correct3.php.inc +++ b/packages/PhpParser/tests/Rector/RemoveNodeRector/Correct/correct3.php.inc @@ -1,11 +1,13 @@ nameResolver = $nameResolver; - } - /** * @var string[] */ @@ -39,6 +33,11 @@ public function __construct(NameResolver $nameResolver) ClassConstFetch::class, MethodCall::class, PropertyFetch::class, StaticCall::class, ClassMethod::class, ]; + public function __construct(NameResolver $nameResolver) + { + $this->nameResolver = $nameResolver; + } + public function renameNode(Node $node, string $newMethodName): void { $this->ensureNodeHasIdentifier($node); diff --git a/src/Rector/Architecture/RepositoryAsService/ReplaceParentRepositoryCallsByRepositoryPropertyRector.php b/src/Rector/Architecture/RepositoryAsService/ReplaceParentRepositoryCallsByRepositoryPropertyRector.php index f6819e430a40..d5b5dbc35190 100644 --- a/src/Rector/Architecture/RepositoryAsService/ReplaceParentRepositoryCallsByRepositoryPropertyRector.php +++ b/src/Rector/Architecture/RepositoryAsService/ReplaceParentRepositoryCallsByRepositoryPropertyRector.php @@ -95,7 +95,7 @@ public function refactor(Node $node): ?Node return null; } - $methodName = $this->getName($node);; + $methodName = $this->getName($node); $entityClassReflection = $this->broker->getClass($this->entityRepositoryClass); if (! $entityClassReflection->hasMethod($methodName)) { diff --git a/src/Rector/Constant/RenameClassConstantsUseToStringsRector.php b/src/Rector/Constant/RenameClassConstantsUseToStringsRector.php index 3629fd91afaf..68b11cdbf2ed 100644 --- a/src/Rector/Constant/RenameClassConstantsUseToStringsRector.php +++ b/src/Rector/Constant/RenameClassConstantsUseToStringsRector.php @@ -64,7 +64,7 @@ public function refactor(Node $node): ?Node } foreach ($oldConstantsToNewValues as $oldConstant => $newValue) { - if ((string) $node->name === $oldConstant) { + if ($this->isName($node, $oldConstant)) { return new String_($newValue); } } @@ -73,7 +73,7 @@ public function refactor(Node $node): ?Node return $node; } - private function getClassNameFromClassConstFetch(ClassConstFetch $classConstFetchNode): string + private function getClassNameFromClassConstFetch(ClassConstFetch $classConstFetchNode): ?string { /** @var FullyQualified|null $fqnName */ $fqnName = $classConstFetchNode->class->getAttribute(Attribute::RESOLVED_NAME); diff --git a/src/Rector/Visibility/ChangeMethodVisibilityRector.php b/src/Rector/Visibility/ChangeMethodVisibilityRector.php index 625e00110b72..10ecc1d54955 100644 --- a/src/Rector/Visibility/ChangeMethodVisibilityRector.php +++ b/src/Rector/Visibility/ChangeMethodVisibilityRector.php @@ -107,16 +107,15 @@ public function refactor(Node $node): ?Node } $this->visibilityModifier->removeOriginalVisibilityFromFlags($node); - $newVisibility = $this->resolveNewVisibilityForNode($node); + $newVisibility = $this->resolveNewVisibilityForNode($node, $methodName); $this->visibilityModifier->addVisibilityFlag($node, $newVisibility); return $node; } - private function resolveNewVisibilityForNode(ClassMethod $classMethodNode): string + private function resolveNewVisibilityForNode(ClassMethod $classMethodNode, string $methodName): string { - $methodName = $this->getName($classMethodNode); $nodeParentClassName = $classMethodNode->getAttribute(Attribute::PARENT_CLASS_NAME); return $this->methodToVisibilityByClass[$nodeParentClassName][$methodName]; From 6fa1d53b1cd452f37e1c8b86432550c4332d5cb7 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 21 Oct 2018 14:10:19 +0200 Subject: [PATCH 3/5] add isNull() to ConstFetchAnalyzer --- examples/MergeIsCandidateRector.php | 4 +-- .../UnnecessaryTernaryExpressionRector.php | 14 ++-------- src/NodeAnalyzer/ConstFetchAnalyzer.php | 28 +++++++++++-------- src/Rector/ConstFetchAnalyzerTrait.php | 5 ++++ 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/examples/MergeIsCandidateRector.php b/examples/MergeIsCandidateRector.php index a48b606f0607..5dec6a3e6aa8 100644 --- a/examples/MergeIsCandidateRector.php +++ b/examples/MergeIsCandidateRector.php @@ -244,11 +244,11 @@ private function createClassConstFetchFromClassName(string $className): ClassCon private function replaceReturnFalseWithReturnNull(ClassMethod $classMethod): void { $this->callbackNodeTraverser->traverseNodesWithCallable([$classMethod], function (Node $node): ?Node { - if (!$node instanceof Return_ || !$node->expr instanceof ConstFetch) { + if (! $node instanceof Return_ || ! $node->expr instanceof ConstFetch) { return null; } - if ((string) $node->expr->name === 'false') { + if ($this->isFalse($node->expr)) { return new Return_(new ConstFetch(new Name('null'))); } diff --git a/packages/CodeQuality/src/Rector/Ternary/UnnecessaryTernaryExpressionRector.php b/packages/CodeQuality/src/Rector/Ternary/UnnecessaryTernaryExpressionRector.php index b38440bd6d7e..1f1c9f0873bd 100644 --- a/packages/CodeQuality/src/Rector/Ternary/UnnecessaryTernaryExpressionRector.php +++ b/packages/CodeQuality/src/Rector/Ternary/UnnecessaryTernaryExpressionRector.php @@ -15,7 +15,6 @@ use PhpParser\Node\Expr\BinaryOp\SmallerOrEqual; use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\Ternary; -use PhpParser\Node\Identifier; use Rector\Exception\NotImplementedException; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; @@ -76,21 +75,14 @@ public function refactor(Node $node): ?Node return null; } - /** @var Identifier $ifExpressionName */ - $ifExpressionName = $ifExpression->name; - - /** @var Identifier $elseExpressionName */ - $elseExpressionName = $elseExpression->name; - - $ifValue = $ifExpressionName->toLowerString(); - $elseValue = $elseExpressionName->toLowerString(); - if (in_array('null', [$ifValue, $elseValue], true)) { + if ($this->isNull($ifExpression) || $this->isNull($elseExpression)) { return null; } + /** @var BinaryOp $binaryOperation */ $binaryOperation = $node->cond; - if ($ifValue === 'true' && $elseValue === 'false') { + if ($this->isTrue($ifExpression) && $this->isFalse($elseExpression)) { return $binaryOperation; } diff --git a/src/NodeAnalyzer/ConstFetchAnalyzer.php b/src/NodeAnalyzer/ConstFetchAnalyzer.php index 2ad9c8e29df8..d6ca6fbb6923 100644 --- a/src/NodeAnalyzer/ConstFetchAnalyzer.php +++ b/src/NodeAnalyzer/ConstFetchAnalyzer.php @@ -11,26 +11,32 @@ */ final class ConstFetchAnalyzer { - public function isFalse(Node $node): bool + public function isBool(Node $node): bool { - if (! $node instanceof ConstFetch) { - return false; - } + return $this->isTrue($node) || $this->isFalse($node); + } - return $node->name->toLowerString() === 'false'; + public function isFalse(Node $node): bool + { + return $this->isConstantWithLowercasedName($node, 'false'); } public function isTrue(Node $node): bool { - if (! $node instanceof ConstFetch) { - return false; - } + return $this->isConstantWithLowercasedName($node, 'true'); + } - return $node->name->toLowerString() === 'true'; + public function isNull(Node $node): bool + { + return $this->isConstantWithLowercasedName($node, 'null'); } - public function isBool(Node $node): bool + private function isConstantWithLowercasedName(Node $node, string $name): bool { - return $this->isTrue($node) || $this->isFalse($node); + if (! $node instanceof ConstFetch) { + return false; + } + + return $node->name->toLowerString() === $name; } } diff --git a/src/Rector/ConstFetchAnalyzerTrait.php b/src/Rector/ConstFetchAnalyzerTrait.php index 0254365ff106..87fdf106d5b7 100644 --- a/src/Rector/ConstFetchAnalyzerTrait.php +++ b/src/Rector/ConstFetchAnalyzerTrait.php @@ -38,4 +38,9 @@ public function isBool(Node $node): bool { return $this->constFetchAnalyzer->isBool($node); } + + public function isNull(Node $node): bool + { + return $this->constFetchAnalyzer->isNull($node); + } } From 79910b73c6d5cce88dd566be97d7454e959b7935 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 21 Oct 2018 14:27:50 +0200 Subject: [PATCH 4/5] make GetRequest use unique names [closes #610] --- .../Rector/HttpKernel/GetRequestRector.php | 47 +++++++++++++++++-- .../GetRequestRector/Correct/correct2.php.inc | 14 ++++++ .../GetRequestRector/GetRequestRectorTest.php | 3 +- .../GetRequestRector/Wrong/wrong2.php.inc | 14 ++++++ src/NodeAnalyzer/NameResolver.php | 35 ++++++++++---- 5 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/Correct/correct2.php.inc create mode 100644 packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/Wrong/wrong2.php.inc diff --git a/packages/Symfony/src/Rector/HttpKernel/GetRequestRector.php b/packages/Symfony/src/Rector/HttpKernel/GetRequestRector.php index 1981e43680bd..61c1b62f5ed3 100644 --- a/packages/Symfony/src/Rector/HttpKernel/GetRequestRector.php +++ b/packages/Symfony/src/Rector/HttpKernel/GetRequestRector.php @@ -16,6 +16,11 @@ final class GetRequestRector extends AbstractRector { + /** + * @var string + */ + private const REQUEST_CLASS = 'Symfony\Component\HttpFoundation\Request'; + /** * @var ControllerMethodAnalyzer */ @@ -31,6 +36,11 @@ final class GetRequestRector extends AbstractRector */ private $betterNodeFinder; + /** + * @var string + */ + private $requestVariableAndParamName; + public function __construct( ControllerMethodAnalyzer $controllerMethodAnalyzer, NodeFactory $nodeFactory, @@ -86,12 +96,16 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { + if ($node instanceof ClassMethod) { + $this->requestVariableAndParamName = $this->resolveUniqueName($node, 'request'); + } + if ($this->isGetRequestInAction($node)) { - return $this->nodeFactory->createVariable('request'); + return $this->nodeFactory->createVariable($this->requestVariableAndParamName); } if ($this->isActionWithGetRequestInBody($node)) { - $requestParam = $this->nodeFactory->createParam('request', 'Symfony\Component\HttpFoundation\Request'); + $requestParam = $this->nodeFactory->createParam($this->requestVariableAndParamName, self::REQUEST_CLASS); $node->params[] = $requestParam; @@ -137,11 +151,12 @@ private function isGetRequestInAction(Node $node): bool return false; } - if (! $this->isNames($node, ['getRequest']) && ! $this->isGetMethodCallWithRequestParameters($node)) { + if (! $this->isName($node, 'getRequest') && ! $this->isGetMethodCallWithRequestParameters($node)) { return false; } $methodNode = $node->getAttribute(Attribute::METHOD_NODE); + return $this->controllerMethodAnalyzer->isAction($methodNode); } @@ -164,4 +179,30 @@ private function isGetMethodCallWithRequestParameters(MethodCall $methodCall): b return $stringValue->value === 'request'; } + + /** + * @param ClassMethod|MethodCall $node + */ + private function resolveUniqueName(Node $node, string $name): string + { + if ($node instanceof ClassMethod) { + $candidates = $node->params; + } else { + $candidates = $node->args; + } + + $candidateNames = []; + foreach ($candidates as $candidate) { + $candidateNames[] = $this->getName($candidate); + } + + $bareName = $name; + $prefixes = ['main', 'default']; + + while (in_array($name, $candidateNames, true)) { + $name = array_shift($prefixes) . ucfirst($bareName); + } + + return $name; + } } diff --git a/packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/Correct/correct2.php.inc b/packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/Correct/correct2.php.inc new file mode 100644 index 000000000000..f941199c9fab --- /dev/null +++ b/packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/Correct/correct2.php.inc @@ -0,0 +1,14 @@ +getSomething(); + } +} diff --git a/packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/GetRequestRectorTest.php b/packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/GetRequestRectorTest.php index 6cac8f81328a..a22ebc52018e 100644 --- a/packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/GetRequestRectorTest.php +++ b/packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/GetRequestRectorTest.php @@ -20,7 +20,8 @@ public function test(string $wrong, string $fixed): void public function provideWrongToFixedFiles(): Iterator { - yield [__DIR__ . '/Wrong/wrong.php.inc', __DIR__ . '/Correct/correct.php.inc']; +// yield [__DIR__ . '/Wrong/wrong.php.inc', __DIR__ . '/Correct/correct.php.inc']; + yield [__DIR__ . '/Wrong/wrong2.php.inc', __DIR__ . '/Correct/correct2.php.inc']; } protected function provideConfig(): string diff --git a/packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/Wrong/wrong2.php.inc b/packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/Wrong/wrong2.php.inc new file mode 100644 index 000000000000..c683c1da80e1 --- /dev/null +++ b/packages/Symfony/tests/Rector/HttpKernel/GetRequestRector/Wrong/wrong2.php.inc @@ -0,0 +1,14 @@ +getRequest()->getSomething(); + } +} diff --git a/src/NodeAnalyzer/NameResolver.php b/src/NodeAnalyzer/NameResolver.php index d2b7747a4950..df68fba901f7 100644 --- a/src/NodeAnalyzer/NameResolver.php +++ b/src/NodeAnalyzer/NameResolver.php @@ -4,27 +4,46 @@ use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Param; use PhpParser\Node\Stmt\ClassConst; use PhpParser\Node\Stmt\Property; final class NameResolver { - public function resolve(Node $node): ?string + /** + * @var callable[] + */ + private $nameResolvers = []; + + public function __construct() { - if ($node instanceof ClassConst) { - if (! count($node->consts)) { + $this->nameResolvers[ClassConst::class] = function (ClassConst $classConstNode) { + if (! count($classConstNode->consts)) { return null; } - return $this->resolve($node->consts[0]); - } + return $this->resolve($classConstNode->consts[0]); + }; - if ($node instanceof Property) { - if (! count($node->props)) { + $this->nameResolvers[Property::class] = function (Property $propertyNode) { + if (! count($propertyNode->props)) { return null; } - return $this->resolve($node->props[0]); + return $this->resolve($propertyNode->props[0]); + }; + } + + public function resolve(Node $node): ?string + { + foreach ($this->nameResolvers as $type => $nameResolver) { + if (is_a($node, $type, true)) { + return $nameResolver($node); + } + } + + if ($node instanceof Param) { + return $this->resolve($node->var); } if (! property_exists($node, 'name')) { From 4cee626ee4832f72d5c129b70614ecc9309692ee Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 21 Oct 2018 14:47:09 +0200 Subject: [PATCH 5/5] Remove MethodArgumentAnalyzer --- .../src/Rector/AliasToClassRector.php | 25 ++++--- .../SpecificMethod/AssertRegExpRector.php | 6 +- ...placeCreateMethodWithoutReviewerRector.php | 19 ++---- .../Controller/RedirectToRouteRector.php | 11 +--- .../Validator/ConstraintUrlOptionRector.php | 2 +- src/NodeAnalyzer/ConstFetchAnalyzer.php | 1 - src/NodeAnalyzer/MethodArgumentAnalyzer.php | 61 ----------------- .../ArgumentDefaultValueReplacerRector.php | 1 + .../MethodArgumentAnalyzerTest.php | 66 ------------------- 9 files changed, 20 insertions(+), 172 deletions(-) delete mode 100644 src/NodeAnalyzer/MethodArgumentAnalyzer.php delete mode 100644 tests/NodeAnalyzer/MethodArgumentAnalyzerTest.php diff --git a/packages/Doctrine/src/Rector/AliasToClassRector.php b/packages/Doctrine/src/Rector/AliasToClassRector.php index 60c57074a608..300a4ceae09b 100644 --- a/packages/Doctrine/src/Rector/AliasToClassRector.php +++ b/packages/Doctrine/src/Rector/AliasToClassRector.php @@ -4,8 +4,8 @@ use PhpParser\Node; use PhpParser\Node\Expr\MethodCall; +use PhpParser\Node\Scalar\String_; use Rector\Node\NodeFactory; -use Rector\NodeAnalyzer\MethodArgumentAnalyzer; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; @@ -23,22 +23,13 @@ final class AliasToClassRector extends AbstractRector */ private $nodeFactory; - /** - * @var MethodArgumentAnalyzer - */ - private $methodArgumentAnalyzer; - /** * @param string[] $aliasesToNamespaces */ - public function __construct( - array $aliasesToNamespaces, - MethodArgumentAnalyzer $methodArgumentAnalyzer, - NodeFactory $nodeFactory - ) { + public function __construct(array $aliasesToNamespaces, NodeFactory $nodeFactory) + { $this->aliasesToNamespaces = $aliasesToNamespaces; $this->nodeFactory = $nodeFactory; - $this->methodArgumentAnalyzer = $methodArgumentAnalyzer; } public function getDefinition(): RectorDefinition @@ -75,11 +66,17 @@ public function refactor(Node $node): ?Node return null; } - if (! $this->methodArgumentAnalyzer->isMethodNthArgumentString($node, 1)) { + if (! isset($node->args[0])) { + return null; + } + + if (! $node->args[0]->value instanceof String_) { return null; } - if (! $this->isAliasWithConfiguredEntity($node->args[0]->value->value)) { + /** @var String_ $stringNode */ + $stringNode = $node->args[0]->value; + if (! $this->isAliasWithConfiguredEntity($stringNode->value)) { return null; } diff --git a/packages/PHPUnit/src/Rector/SpecificMethod/AssertRegExpRector.php b/packages/PHPUnit/src/Rector/SpecificMethod/AssertRegExpRector.php index cf7997ff4c54..846448d7e44f 100644 --- a/packages/PHPUnit/src/Rector/SpecificMethod/AssertRegExpRector.php +++ b/packages/PHPUnit/src/Rector/SpecificMethod/AssertRegExpRector.php @@ -6,7 +6,6 @@ use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\MethodCall; -use PhpParser\Node\Identifier; use PhpParser\Node\Scalar\LNumber; use Rector\Builder\IdentifierRenamer; use Rector\Rector\AbstractPHPUnitRector; @@ -102,10 +101,7 @@ private function resolveOldCondition(Node $node): int } if ($node instanceof ConstFetch) { - /** @var Identifier $constFetchName */ - $constFetchName = $node->name; - - return $constFetchName->toLowerString() === 'true' ? 1 : 0; + return $this->isTrue($node) ? 1 : 0; } } diff --git a/packages/Sylius/src/Rector/Review/ReplaceCreateMethodWithoutReviewerRector.php b/packages/Sylius/src/Rector/Review/ReplaceCreateMethodWithoutReviewerRector.php index 5d4b9c76d0df..653eb7ca9821 100644 --- a/packages/Sylius/src/Rector/Review/ReplaceCreateMethodWithoutReviewerRector.php +++ b/packages/Sylius/src/Rector/Review/ReplaceCreateMethodWithoutReviewerRector.php @@ -5,28 +5,19 @@ use PhpParser\Node; use PhpParser\Node\Expr\MethodCall; use Rector\Builder\IdentifierRenamer; -use Rector\NodeAnalyzer\MethodArgumentAnalyzer; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; final class ReplaceCreateMethodWithoutReviewerRector extends AbstractRector { - /** - * @var MethodArgumentAnalyzer - */ - private $methodArgumentAnalyzer; - /** * @var IdentifierRenamer */ private $identifierRenamer; - public function __construct( - MethodArgumentAnalyzer $methodArgumentAnalyzer, - IdentifierRenamer $identifierRenamer - ) { - $this->methodArgumentAnalyzer = $methodArgumentAnalyzer; + public function __construct(IdentifierRenamer $identifierRenamer) + { $this->identifierRenamer = $identifierRenamer; } @@ -60,15 +51,13 @@ public function refactor(Node $node): ?Node return null; } - if ($this->methodArgumentAnalyzer->hasMethodNthArgument($node, 2) - && ! $this->methodArgumentAnalyzer->isMethodNthArgumentNull($node, 2) - ) { + if (isset($node->args[1]) && ! $this->isNull($node->args[1]->value)) { return null; } $this->identifierRenamer->renameNode($node, 'createForSubject'); - if ($this->methodArgumentAnalyzer->hasMethodNthArgument($node, 2)) { + if (isset($node->args[1])) { $node->args = [array_shift($node->args)]; } diff --git a/packages/Symfony/src/Rector/Controller/RedirectToRouteRector.php b/packages/Symfony/src/Rector/Controller/RedirectToRouteRector.php index 24a7277ae050..188087408a84 100644 --- a/packages/Symfony/src/Rector/Controller/RedirectToRouteRector.php +++ b/packages/Symfony/src/Rector/Controller/RedirectToRouteRector.php @@ -5,7 +5,6 @@ use PhpParser\Node; use PhpParser\Node\Expr\MethodCall; use Rector\Node\MethodCallNodeFactory; -use Rector\NodeAnalyzer\MethodArgumentAnalyzer; use Rector\NodeTypeResolver\Node\Attribute; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; @@ -13,11 +12,6 @@ final class RedirectToRouteRector extends AbstractRector { - /** - * @var MethodArgumentAnalyzer - */ - private $methodArgumentAnalyzer; - /** * @var MethodCallNodeFactory */ @@ -29,11 +23,9 @@ final class RedirectToRouteRector extends AbstractRector private $controllerClass; public function __construct( - MethodArgumentAnalyzer $methodArgumentAnalyzer, MethodCallNodeFactory $methodCallNodeFactory, string $controllerClass = 'Symfony\Bundle\FrameworkBundle\Controller\Controller' ) { - $this->methodArgumentAnalyzer = $methodArgumentAnalyzer; $this->methodCallNodeFactory = $methodCallNodeFactory; $this->controllerClass = $controllerClass; } @@ -68,7 +60,8 @@ public function refactor(Node $node): ?Node if (! $this->isName($node, 'redirect')) { return null; } - if (! $this->methodArgumentAnalyzer->hasMethodNthArgument($node, 1)) { + + if (! isset($node->args[0])) { return null; } diff --git a/packages/Symfony/src/Rector/Validator/ConstraintUrlOptionRector.php b/packages/Symfony/src/Rector/Validator/ConstraintUrlOptionRector.php index 4aa723becf38..f8800b3258bc 100644 --- a/packages/Symfony/src/Rector/Validator/ConstraintUrlOptionRector.php +++ b/packages/Symfony/src/Rector/Validator/ConstraintUrlOptionRector.php @@ -54,7 +54,7 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if (! $this->isName($node, 'true')) { + if (! $this->isTrue($node)) { return null; } diff --git a/src/NodeAnalyzer/ConstFetchAnalyzer.php b/src/NodeAnalyzer/ConstFetchAnalyzer.php index d6ca6fbb6923..d4c2b40d7ab1 100644 --- a/src/NodeAnalyzer/ConstFetchAnalyzer.php +++ b/src/NodeAnalyzer/ConstFetchAnalyzer.php @@ -36,7 +36,6 @@ private function isConstantWithLowercasedName(Node $node, string $name): bool if (! $node instanceof ConstFetch) { return false; } - return $node->name->toLowerString() === $name; } } diff --git a/src/NodeAnalyzer/MethodArgumentAnalyzer.php b/src/NodeAnalyzer/MethodArgumentAnalyzer.php deleted file mode 100644 index 5c87cd33383b..000000000000 --- a/src/NodeAnalyzer/MethodArgumentAnalyzer.php +++ /dev/null @@ -1,61 +0,0 @@ -someMethod($argument)" - */ -final class MethodArgumentAnalyzer -{ - public function hasMethodNthArgument(Node $node, int $position): bool - { - if (! $node instanceof MethodCall) { - return false; - } - - if (count($node->args) < $position) { - return false; - } - - return true; - } - - public function isMethodNthArgumentString(Node $node, int $position): bool - { - if (! $this->hasMethodNthArgument($node, $position)) { - return false; - } - - /** @var MethodCall $methodCallNode */ - $methodCallNode = $node; - - return $methodCallNode->args[$position - 1]->value instanceof String_; - } - - public function isMethodNthArgumentNull(Node $node, int $position): bool - { - if (! $this->hasMethodNthArgument($node, $position)) { - return false; - } - - /** @var MethodCall $methodCallNode */ - $methodCallNode = $node; - - $value = $methodCallNode->args[$position - 1]->value; - if (! $value instanceof ConstFetch) { - return false; - } - - /** @var Identifier $nodeName */ - $nodeName = $value->name; - - return $nodeName->toLowerString() === 'null'; - } -} diff --git a/src/Rector/Argument/ArgumentDefaultValueReplacerRector.php b/src/Rector/Argument/ArgumentDefaultValueReplacerRector.php index ac851687406a..1c6b2dcefba0 100644 --- a/src/Rector/Argument/ArgumentDefaultValueReplacerRector.php +++ b/src/Rector/Argument/ArgumentDefaultValueReplacerRector.php @@ -163,6 +163,7 @@ private function processArgNode( private function resolveArgumentValue(Arg $argNode) { $resolvedValue = $this->constExprEvaluator->evaluateDirectly($argNode->value); + if ($resolvedValue === true) { return 'true'; } diff --git a/tests/NodeAnalyzer/MethodArgumentAnalyzerTest.php b/tests/NodeAnalyzer/MethodArgumentAnalyzerTest.php deleted file mode 100644 index ea34a6c31d2f..000000000000 --- a/tests/NodeAnalyzer/MethodArgumentAnalyzerTest.php +++ /dev/null @@ -1,66 +0,0 @@ -methodArgumentAnalyzer = $this->container->get(MethodArgumentAnalyzer::class); - - $varNode = new Variable('this'); - $this->methodCallNode = new MethodCall($varNode, 'method'); - } - - public function testHasMethodFirstArgument(): void - { - $this->assertFalse($this->methodArgumentAnalyzer->hasMethodNthArgument($this->methodCallNode, 1)); - - $this->methodCallNode->args[] = new Arg(new String_('argument')); - $this->assertTrue($this->methodArgumentAnalyzer->hasMethodNthArgument($this->methodCallNode, 1)); - } - - public function testHasMethodSecondArgument(): void - { - $this->methodCallNode->args[] = new Arg(new String_('argument')); - $this->assertFalse($this->methodArgumentAnalyzer->hasMethodNthArgument($this->methodCallNode, 2)); - - $this->methodCallNode->args[] = new Arg(new String_('argument')); - $this->assertTrue($this->methodArgumentAnalyzer->hasMethodNthArgument($this->methodCallNode, 2)); - } - - public function testIsFirstArgumentString(): void - { - $this->assertFalse($this->methodArgumentAnalyzer->isMethodNthArgumentString($this->methodCallNode, 1)); - - $this->methodCallNode->args[0] = new Arg(new String_('argument')); - $this->assertTrue($this->methodArgumentAnalyzer->isMethodNthArgumentString($this->methodCallNode, 1)); - - $this->methodCallNode->args[0] = new Arg(new BitwiseNot(new String_('argument'))); - $this->assertFalse($this->methodArgumentAnalyzer->isMethodNthArgumentString($this->methodCallNode, 1)); - } - - public function testOnNonMethodCall(): void - { - $this->assertFalse($this->methodArgumentAnalyzer->hasMethodNthArgument(new String_('string'), 1)); - $this->assertFalse($this->methodArgumentAnalyzer->hasMethodNthArgument(new String_('string'), 2)); - } -}