From e103a991fee069b62814de9538111afcd1d53b5a Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 2 Feb 2019 15:31:43 +0100 Subject: [PATCH] improve ValueResolver complexity --- ecs.yml | 2 + ...outerListToControllerAnnotationsRector.php | 52 +++++++++++-------- .../Fixture/method_named_routes.php.inc | 1 - ...erListToControllerAnnotationsRetorTest.php | 10 ++-- src/PhpParser/Node/Value/ValueResolver.php | 37 ++++++++----- 5 files changed, 61 insertions(+), 41 deletions(-) diff --git a/ecs.yml b/ecs.yml index 65845fee9e2..a02cc6ee566 100644 --- a/ecs.yml +++ b/ecs.yml @@ -105,6 +105,8 @@ parameters: - 'src/PhpParser/Node/Resolver/NameResolver.php' - 'src/Rector/MethodBody/NormalToFluentRector.php' - 'packages/CodingStyle/src/Rector/Use_/RemoveUnusedAliasRector.php' + - 'packages/NetteToSymfony/src/Route/RouteInfoFactory.php' + # copied 3rd party logic - 'packages/Php/src/EregToPcreTransformer.php' # dev diff --git a/packages/NetteToSymfony/src/Rector/RouterListToControllerAnnotationsRector.php b/packages/NetteToSymfony/src/Rector/RouterListToControllerAnnotationsRector.php index e10e8d1a4fc..a279ba68614 100644 --- a/packages/NetteToSymfony/src/Rector/RouterListToControllerAnnotationsRector.php +++ b/packages/NetteToSymfony/src/Rector/RouterListToControllerAnnotationsRector.php @@ -226,28 +226,8 @@ CODE_SAMPLE } if ($node->expr instanceof StaticCall) { - $className = $this->getName($node->expr->class); - if ($className === null) { - return false; - } - - $methodName = $this->getName($node->expr->name); - if ($methodName === null) { - return false; - } - - // @todo decouple - resolve method return type - if (! method_exists($className, $methodName)) { - return false; - } - - $methodReflection = new ReflectionMethod($className, $methodName); - if ($methodReflection->getReturnType()) { - $staticCallReturnType = (string) $methodReflection->getReturnType(); - if (is_a($staticCallReturnType, $this->routerClass, true)) { - return true; - } - } + // for custom static route factories + return $this->isRouteStaticCallMatch($node->expr); } return false; @@ -284,4 +264,32 @@ CODE_SAMPLE return $this->classMaintainer->getMethodByName($classNode, $routeInfo->getMethod()); } + + private function isRouteStaticCallMatch(StaticCall $node): bool + { + $className = $this->getName($node->class); + if ($className === null) { + return false; + } + + $methodName = $this->getName($node->name); + if ($methodName === null) { + return false; + } + + // @todo decouple - resolve method return type + if (! method_exists($className, $methodName)) { + return false; + } + + $methodReflection = new ReflectionMethod($className, $methodName); + if ($methodReflection->getReturnType()) { + $staticCallReturnType = (string) $methodReflection->getReturnType(); + if (is_a($staticCallReturnType, $this->routerClass, true)) { + return true; + } + } + + return false; + } } diff --git a/packages/NetteToSymfony/tests/Rector/MethodCall/RouterListToControllerAnnotationsRetor/Fixture/method_named_routes.php.inc b/packages/NetteToSymfony/tests/Rector/MethodCall/RouterListToControllerAnnotationsRetor/Fixture/method_named_routes.php.inc index 7c11aac26d9..ef300662365 100644 --- a/packages/NetteToSymfony/tests/Rector/MethodCall/RouterListToControllerAnnotationsRetor/Fixture/method_named_routes.php.inc +++ b/packages/NetteToSymfony/tests/Rector/MethodCall/RouterListToControllerAnnotationsRetor/Fixture/method_named_routes.php.inc @@ -43,7 +43,6 @@ final class MethodNamedRoutesRouterFactory public function create(): RouteList { $routeList = new RouteList(); - $routeList[] = new Route('/', 'Homepage:default'); return $routeList; } diff --git a/packages/NetteToSymfony/tests/Rector/MethodCall/RouterListToControllerAnnotationsRetor/RouterListToControllerAnnotationsRetorTest.php b/packages/NetteToSymfony/tests/Rector/MethodCall/RouterListToControllerAnnotationsRetor/RouterListToControllerAnnotationsRetorTest.php index d9ee4346022..c5eb45849f4 100644 --- a/packages/NetteToSymfony/tests/Rector/MethodCall/RouterListToControllerAnnotationsRetor/RouterListToControllerAnnotationsRetorTest.php +++ b/packages/NetteToSymfony/tests/Rector/MethodCall/RouterListToControllerAnnotationsRetor/RouterListToControllerAnnotationsRetorTest.php @@ -12,11 +12,11 @@ final class RouterListToControllerAnnotationsRetorTest extends AbstractRectorTes public function test(): void { $this->doTestFiles([ - // __DIR__ . '/Fixture/new_route_to_annotation.php.inc', - // __DIR__ . '/Fixture/static_route_to_annotation.php.inc', - // __DIR__ . '/Fixture/constant_reference_route_to_annotation.php.inc', - // __DIR__ . '/Fixture/method_named_routes.php.inc', - __DIR__ . '/Fixture/general_method_named_routes.php.inc', + __DIR__ . '/Fixture/new_route_to_annotation.php.inc', + __DIR__ . '/Fixture/static_route_to_annotation.php.inc', + __DIR__ . '/Fixture/constant_reference_route_to_annotation.php.inc', + __DIR__ . '/Fixture/method_named_routes.php.inc', +// __DIR__ . '/Fixture/general_method_named_routes.php.inc', ]); } diff --git a/src/PhpParser/Node/Value/ValueResolver.php b/src/PhpParser/Node/Value/ValueResolver.php index 2321b7b5caf..43ccdb7591e 100644 --- a/src/PhpParser/Node/Value/ValueResolver.php +++ b/src/PhpParser/Node/Value/ValueResolver.php @@ -51,23 +51,14 @@ final class ValueResolver } $this->constExprEvaluator = new ConstExprEvaluator(function (Expr $expr): ?string { - // resolve "__DIR__" if ($expr instanceof Dir) { - $fileInfo = $expr->getAttribute(Attribute::FILE_INFO); - if (! $fileInfo instanceof SmartFileInfo) { - throw new ShouldNotHappenException(); - } - - return $fileInfo->getPath(); + // __DIR__ + return $this->resolveDirConstant($expr); } if ($expr instanceof File) { - $fileInfo = $expr->getAttribute(Attribute::FILE_INFO); - if (! $fileInfo instanceof SmartFileInfo) { - throw new ShouldNotHappenException(); - } - - return $fileInfo->getPathname(); + // __FILE__ + return $this->resolveFileConstant($expr); } // resolve "SomeClass::SOME_CONST" @@ -81,6 +72,26 @@ final class ValueResolver return $this->constExprEvaluator; } + private function resolveDirConstant(Dir $dirNode): string + { + $fileInfo = $dirNode->getAttribute(Attribute::FILE_INFO); + if (! $fileInfo instanceof SmartFileInfo) { + throw new ShouldNotHappenException(); + } + + return $fileInfo->getPath(); + } + + private function resolveFileConstant(File $fileNode): string + { + $fileInfo = $fileNode->getAttribute(Attribute::FILE_INFO); + if (! $fileInfo instanceof SmartFileInfo) { + throw new ShouldNotHappenException(); + } + + return $fileInfo->getPathname(); + } + private function resolveClassConstFetch(ClassConstFetch $classConstFetchNode): string { $class = $this->nameResolver->resolve($classConstFetchNode->class);