Improve reflection for Variadic analyzer (#310)

rectorphp · Jun 27, 2021 · 4f4dce9 · 4f4dce9
1 parent 20ad16b
commit 4f4dce9
Show file tree

Hide file tree

Showing 15 changed files with 171 additions and 109 deletions.
diff --git a/packages/BetterPhpDocParser/ValueObject/Type/SpacingAwareArrayTypeNode.php b/packages/BetterPhpDocParser/ValueObject/Type/SpacingAwareArrayTypeNode.php
@@ -39,7 +39,8 @@ public function __toString(): string
 
     private function isGenericArrayCandidate(TypeNode $typeNode): bool
     {
-        if (! $this->getAttribute(ArrayTypeMapper::HAS_GENERIC_TYPE_PARENT)) {
+        $hasGenericTypeParent = (bool) $this->getAttribute(ArrayTypeMapper::HAS_GENERIC_TYPE_PARENT);
+        if (! $hasGenericTypeParent) {
             return false;
         }
 

diff --git a/phpstan.neon b/phpstan.neon
@@ -513,3 +513,13 @@ parameters:
             message: '#Cannot return include_once/require_once#'
             path: rules/Renaming/Rector/Name/RenameClassRector.php
 
+        # required assigns - parent looping
+        -
+            message: '#foreach\(\.\.\.\), while\(\), for\(\) or if\(\.\.\.\) cannot contains a complex expression\. Extract it to a new variable assign on line before#'
+            paths:
+                - packages/NodeNestingScope/FlowOfControlLocator.php
+                - src/PhpParser/Node/BetterNodeFinder.php
+                - rules/Php70/Rector/FuncCall/MultiDirnameRector.php
+                - src/Application/FileProcessor.php
+                - rules/CodingStyle/Rector/Assign/ManualJsonStringToJsonEncodeArrayRector.php
+                - rules/CodeQuality/Rector/Return_/SimplifyUselessVariableRector.php
diff --git a/rules/CodeQuality/Rector/Foreach_/UnusedForeachValueToArrayKeysRector.php b/rules/CodeQuality/Rector/Foreach_/UnusedForeachValueToArrayKeysRector.php
@@ -93,7 +93,8 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        if (is_a($this->getStaticType($node->expr), ObjectType::class)) {
+        $exprType = $this->getStaticType($node->expr);
+        if ($exprType instanceof ObjectType) {
             return null;
         }
 

diff --git a/rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php b/rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php
@@ -128,9 +128,12 @@ private function shouldSkip(If_ $if): bool
         if (! $this->valueResolver->isFalse($returnedExpr)) {
             return ! $this->valueResolver->isTrueOrFalse($nextNode->expr);
         }
-        if (! \str_contains($this->print($if->cond), '!=')) {
+
+        $condString = $this->print($if->cond);
+        if (! \str_contains($condString, '!=')) {
             return ! $this->valueResolver->isTrueOrFalse($nextNode->expr);
         }
+
         return true;
     }
 

diff --git a/rules/DowngradePhp72/Rector/Class_/DowngradeParameterTypeWideningRector.php b/rules/DowngradePhp72/Rector/Class_/DowngradeParameterTypeWideningRector.php
@@ -124,7 +124,8 @@ public function refactor(Node $node): ?Node
             }
 
             // refactor here
-            if ($this->refactorClassMethod($classMethod, $classReflection, $ancestors, $interfaces) !== null) {
+            $changedClassMethod = $this->refactorClassMethod($classMethod, $classReflection, $ancestors, $interfaces);
+            if ($changedClassMethod !== null) {
                 $hasChanged = true;
             }
         }

diff --git a/rules/Php71/Rector/FuncCall/RemoveExtraParametersRector.php b/rules/Php71/Rector/FuncCall/RemoveExtraParametersRector.php
@@ -13,8 +13,8 @@
 use PHPStan\Reflection\MethodReflection;
 use PHPStan\Reflection\Php\PhpMethodReflection;
 use PHPStan\Reflection\Type\UnionTypeMethodReflection;
+use Rector\Core\NodeAnalyzer\VariadicAnalyzer;
 use Rector\Core\PHPStan\Reflection\CallReflectionResolver;
-use Rector\Core\PHPStan\Reflection\VariadicAnalyzer;
 use Rector\Core\Rector\AbstractRector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -88,44 +88,30 @@ public function refactor(Node $node): ?Node
         return $node;
     }
 
-    /**
-     * @param FuncCall|MethodCall|StaticCall $node
-     */
-    private function shouldSkip(Node $node): bool
+    private function shouldSkip(FuncCall | MethodCall | StaticCall $call): bool
     {
-        if ($node->args === []) {
+        if ($call->args === []) {
             return true;
         }
 
-        if ($node instanceof StaticCall) {
-            if (! $node->class instanceof Name) {
+        if ($call instanceof StaticCall) {
+            if (! $call->class instanceof Name) {
                 return true;
             }
 
-            if ($this->isName($node->class, 'parent')) {
+            if ($this->isName($call->class, 'parent')) {
                 return true;
             }
         }
 
-        $functionReflection = $this->callReflectionResolver->resolveCall($node);
-        if ($functionReflection === null) {
-            return true;
-        }
-
-        if ($functionReflection->getVariants() === []) {
-            return true;
-        }
-
-        return $this->variadicAnalyzer->hasVariadicParameters($functionReflection);
+        return $this->variadicAnalyzer->hasVariadicParameters($call);
     }
 
-    /**
-     * @param MethodReflection|FunctionReflection $reflection
-     */
-    private function resolveMaximumAllowedParameterCount(object $reflection): int
-    {
+    private function resolveMaximumAllowedParameterCount(
+        MethodReflection | FunctionReflection $functionLikeReflection
+    ): int {
         $parameterCounts = [0];
-        foreach ($reflection->getVariants() as $parametersAcceptor) {
+        foreach ($functionLikeReflection->getVariants() as $parametersAcceptor) {
             $parameterCounts[] = count($parametersAcceptor->getParameters());
         }
 

diff --git a/rules/Php74/Rector/FuncCall/ArrayKeyExistsOnPropertyRector.php b/rules/Php74/Rector/FuncCall/ArrayKeyExistsOnPropertyRector.php
@@ -66,7 +66,8 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        if (! $this->getStaticType($node->args[1]->value) instanceof ObjectType) {
+        $firstArgStaticType = $this->getStaticType($node->args[1]->value);
+        if (! $firstArgStaticType instanceof ObjectType) {
             return null;
         }
 

diff --git a/rules/Php74/Rector/FuncCall/MbStrrposEncodingArgumentPositionRector.php b/rules/Php74/Rector/FuncCall/MbStrrposEncodingArgumentPositionRector.php
@@ -53,7 +53,8 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        if ($this->getStaticType($node->args[2]->value) instanceof IntegerType) {
+        $secondArgType = $this->getStaticType($node->args[2]->value);
+        if ($secondArgType instanceof IntegerType) {
             return null;
         }
 

diff --git a/rules/PhpSpecToPHPUnit/Rector/ClassMethod/PhpSpecMethodToPHPUnitMethodRector.php b/rules/PhpSpecToPHPUnit/Rector/ClassMethod/PhpSpecMethodToPHPUnitMethodRector.php
@@ -68,12 +68,14 @@ private function processTestMethod(ClassMethod $classMethod): void
         // reorder instantiation + expected exception
         $previousStmt = null;
         foreach ((array) $classMethod->stmts as $key => $stmt) {
-            if ($previousStmt &&
-                \str_contains($this->print($stmt), 'duringInstantiation') &&
-                \str_contains($this->print($previousStmt), 'beConstructedThrough')
-            ) {
-                $classMethod->stmts[$key - 1] = $stmt;
-                $classMethod->stmts[$key] = $previousStmt;
+            $printedStmt = $this->print($stmt);
+
+            if ($previousStmt && \str_contains($printedStmt, 'duringInstantiation')) {
+                $printedPreviousStmt = $this->print($previousStmt);
+                if (\str_contains($printedPreviousStmt, 'beConstructedThrough')) {
+                    $classMethod->stmts[$key - 1] = $stmt;
+                    $classMethod->stmts[$key] = $previousStmt;
+                }
             }
 
             $previousStmt = $stmt;

diff --git a/rules/Restoration/Rector/Namespace_/CompleteImportForPartialAnnotationRector.php b/rules/Restoration/Rector/Namespace_/CompleteImportForPartialAnnotationRector.php
@@ -87,9 +87,11 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
+        $printedClass = $this->print($class);
+
         foreach ($this->useImportsToRestore as $useImportToRestore) {
             $annotationToSeek = '#\*\s+\@' . $useImportToRestore->getAlias() . '#';
-            if (! Strings::match($this->print($class), $annotationToSeek)) {
+            if (! Strings::match($printedClass, $annotationToSeek)) {
                 continue;
             }
 

diff --git a/src/NodeAnalyzer/VariadicAnalyzer.php b/src/NodeAnalyzer/VariadicAnalyzer.php
@@ -0,0 +1,70 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Core\NodeAnalyzer;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\FuncCall;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\StaticCall;
+use PhpParser\Node\Stmt\Function_;
+use PHPStan\Reflection\FunctionReflection;
+use PHPStan\Reflection\MethodReflection;
+use PHPStan\Reflection\Php\PhpFunctionReflection;
+use Rector\Core\PhpParser\AstResolver;
+use Rector\Core\PhpParser\Node\BetterNodeFinder;
+use Rector\Core\Reflection\ReflectionResolver;
+use Rector\NodeNameResolver\NodeNameResolver;
+
+final class VariadicAnalyzer
+{
+    public function __construct(
+        private BetterNodeFinder $betterNodeFinder,
+        private NodeNameResolver $nodeNameResolver,
+        private AstResolver $astResolver,
+        private ReflectionResolver $reflectionResolver
+    ) {
+    }
+
+    public function hasVariadicParameters(FuncCall | StaticCall | MethodCall $call): bool
+    {
+        $functionLikeReflection = $this->reflectionResolver->resolveFunctionLikeReflectionFromCall($call);
+        if ($functionLikeReflection === null) {
+            return false;
+        }
+
+        if ($this->hasVariadicVariant($functionLikeReflection)) {
+            return true;
+        }
+
+        if ($functionLikeReflection instanceof PhpFunctionReflection) {
+            $function = $this->astResolver->resolveFunctionFromFunctionReflection($functionLikeReflection);
+            if (! $function instanceof Function_) {
+                return false;
+            }
+
+            return (bool) $this->betterNodeFinder->findFirst($function->stmts, function (Node $node): bool {
+                if (! $node instanceof FuncCall) {
+                    return false;
+                }
+
+                return $this->nodeNameResolver->isNames($node, ['func_get_args', 'func_num_args', 'func_get_arg']);
+            });
+        }
+
+        return false;
+    }
+
+    private function hasVariadicVariant(MethodReflection | FunctionReflection $functionLikeReflection): bool
+    {
+        foreach ($functionLikeReflection->getVariants() as $parametersAcceptor) {
+            // can be any number of arguments → nothing to limit here
+            if ($parametersAcceptor->isVariadic()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}
diff --git a/src/PHPStan/Reflection/VariadicAnalyzer.php b/src/PHPStan/Reflection/VariadicAnalyzer.php
diff --git a/src/PhpParser/AstResolver.php b/src/PhpParser/AstResolver.php
@@ -214,10 +214,18 @@ private function resolveClassFromClassReflection(ClassReflection $classReflectio
             return $this->classesByName[$classReflection->getName()];
         }
 
-        /** @var string $fileName */
         $fileName = $classReflection->getFileName();
 
-        $nodes = $this->parser->parse($this->smartFileSystem->readFile($fileName));
+        // probably internal class
+        if ($fileName === false) {
+            // avoid parsing falsy-file again
+            $this->classesByName[$classReflection->getName()] = null;
+            return null;
+        }
+
+        $fileContent = $this->smartFileSystem->readFile($fileName);
+
+        $nodes = $this->parser->parse($fileContent);
         if ($nodes === null) {
             // avoid parsing falsy-file again
             $this->classesByName[$classReflection->getName()] = null;

diff --git a/src/Reflection/ReflectionResolver.php b/src/Reflection/ReflectionResolver.php
@@ -4,10 +4,13 @@
 
 namespace Rector\Core\Reflection;
 
+use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\New_;
 use PhpParser\Node\Expr\StaticCall;
+use PhpParser\Node\Name\FullyQualified;
 use PhpParser\Node\Stmt\ClassMethod;
+use PHPStan\Reflection\FunctionReflection;
 use PHPStan\Reflection\MethodReflection;
 use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\Type\TypeUtils;
@@ -96,6 +99,20 @@ public function resolveMethodReflectionFromMethodCall(MethodCall $methodCall): ?
         return $this->resolveMethodReflection($callerType->getClassName(), $methodName);
     }
 
+    public function resolveFunctionLikeReflectionFromCall(
+        MethodCall | StaticCall | FuncCall $call
+    ): MethodReflection | FunctionReflection | null {
+        if ($call instanceof MethodCall) {
+            return $this->resolveMethodReflectionFromMethodCall($call);
+        }
+
+        if ($call instanceof StaticCall) {
+            return $this->resolveMethodReflectionFromStaticCall($call);
+        }
+
+        return $this->resolveFunctionReflectionFromFuncCall($call);
+    }
+
     public function resolveMethodReflectionFromClassMethod(ClassMethod $classMethod): ?MethodReflection
     {
         $class = $classMethod->getAttribute(AttributeKey::CLASS_NAME);
@@ -116,4 +133,19 @@ public function resolveMethodReflectionFromNew(New_ $new): ?MethodReflection
 
         return $this->resolveMethodReflection($newClassType->getClassName(), MethodName::CONSTRUCT);
     }
+
+    private function resolveFunctionReflectionFromFuncCall(FuncCall $funcCall): ?FunctionReflection
+    {
+        $functionName = $this->nodeNameResolver->getName($funcCall);
+        if ($functionName === null) {
+            return null;
+        }
+
+        $functionNameFullyQualified = new FullyQualified($functionName);
+        if (! $this->reflectionProvider->hasFunction($functionNameFullyQualified, null)) {
+            return null;
+        }
+
+        return $this->reflectionProvider->getFunction($functionNameFullyQualified, null);
+    }
 }