[TypeDeclaration] Skip ReturnTypeFromStrictTypedCallRector on possibl…

…e no return (#441) * [TypeDeclaration] Skip ReturnTypeFromStrictTypedCallRector on possible no return * Fixed 🎉 * [ci-review] Rector Rectify Co-authored-by: GitHub Action <[email protected]>
rectorphp · Jul 16, 2021 · b001e38 · b001e38
1 parent 40fbe10
commit b001e38
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 16 deletions.
diff --git a/packages/ReadWrite/NodeAnalyzer/ReadWritePropertyAnalyzer.php b/packages/ReadWrite/NodeAnalyzer/ReadWritePropertyAnalyzer.php
@@ -17,7 +17,6 @@
 use PhpParser\Node\Stmt\Unset_;
 use Rector\Core\Exception\ShouldNotHappenException;
 use Rector\Core\NodeManipulator\AssignManipulator;
-use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\NodeNestingScope\ParentFinder;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\ReadWrite\Guard\VariableToConstantGuard;
@@ -28,7 +27,6 @@ public function __construct(
         private VariableToConstantGuard $variableToConstantGuard,
         private AssignManipulator $assignManipulator,
         private ReadExprAnalyzer $readExprAnalyzer,
-        private BetterNodeFinder $betterNodeFinder,
         private ParentFinder $parentFinder
     ) {
     }

diff --git a/...r/ClassMethod/ReturnTypeFromStrictTypedCallRector/Fixture/skip_possible_no_return.php.inc b/...r/ClassMethod/ReturnTypeFromStrictTypedCallRector/Fixture/skip_possible_no_return.php.inc
@@ -0,0 +1,18 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictTypedCallRector\Fixture;
+
+final class SkipPossibleNoReturn
+{
+    public function getData()
+    {
+        if (mt_rand(0, 100)) {
+            return $this->getNumber();
+        }
+    }
+
+    private function getNumber(): int
+    {
+        return 100;
+    }
+}
diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictTypedCallRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictTypedCallRector.php
@@ -22,11 +22,13 @@
 use PHPStan\Type\NullType;
 use PHPStan\Type\ObjectType;
 use PHPStan\Type\UnionType;
+use PHPStan\Type\VoidType;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\Reflection\ReflectionResolver;
 use Rector\Core\ValueObject\PhpVersionFeature;
 use Rector\PHPStanStaticTypeMapper\ValueObject\TypeKind;
 use Rector\TypeDeclaration\NodeAnalyzer\TypeNodeUnwrapper;
+use Rector\TypeDeclaration\TypeInferer\ReturnTypeInferer;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -37,7 +39,8 @@ final class ReturnTypeFromStrictTypedCallRector extends AbstractRector
 {
     public function __construct(
         private TypeNodeUnwrapper $typeNodeUnwrapper,
-        private ReflectionResolver $reflectionResolver
+        private ReflectionResolver $reflectionResolver,
+        private ReturnTypeInferer $returnTypeInferer
     ) {
     }
 
@@ -95,6 +98,10 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
+        if ($this->isUnionPossibleReturnsVoid($node)) {
+            return null;
+        }
+
         /** @var Return_[] $returns */
         $returns = $this->betterNodeFinder->find((array) $node->stmts, function (Node $n) use ($node) {
             $currentFunctionLike = $this->betterNodeFinder->findParentType($n, FunctionLike::class);
@@ -136,6 +143,20 @@ public function refactor(Node $node): ?Node
         return null;
     }
 
+    private function isUnionPossibleReturnsVoid(ClassMethod | Function_ | Closure $node): bool
+    {
+        $inferReturnType = $this->returnTypeInferer->inferFunctionLike($node);
+        if ($inferReturnType instanceof UnionType) {
+            foreach ($inferReturnType->getTypes() as $type) {
+                if ($type instanceof VoidType) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
+
     private function processSingleUnionType(
         ClassMethod | Function_ | Closure $node,
         UnionType $unionType,

diff --git a/src/NodeFactory/ClassWithPublicPropertiesFactory.php b/src/NodeFactory/ClassWithPublicPropertiesFactory.php
@@ -4,8 +4,9 @@
 
 namespace Rector\Core\NodeFactory;
 
-use PhpParser\Node;
 use PhpParser\Node\NullableType;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\Namespace_;
 use Symplify\Astral\ValueObject\NodeBuilder\ClassBuilder;
 use Symplify\Astral\ValueObject\NodeBuilder\NamespaceBuilder;
 use Symplify\Astral\ValueObject\NodeBuilder\PropertyBuilder;
@@ -27,7 +28,7 @@ public function createNode(
         array $properties,
         ?string $parent = null,
         array $traits = []
-    ): Node {
+    ): Namespace_ | Class_ {
         $namespaceParts = explode('\\', ltrim($fullyQualifiedName, '\\'));
         $className = array_pop($namespaceParts);
         $namespace = implode('\\', $namespaceParts);

diff --git a/src/PhpParser/Node/BetterNodeFinder.php b/src/PhpParser/Node/BetterNodeFinder.php
@@ -45,7 +45,6 @@ public function __construct(
     /**
      * @template T of Node
      * @param class-string<T> $type
-     * @return T|null
      */
     public function findParentType(Node $node, string $type): ?Node
     {
@@ -101,7 +100,6 @@ public function findInstanceOf(Node | array $nodes, string $type): array
      * @template T of Node
      * @param class-string<T> $type
      * @param Node|Node[] $nodes
-     * @return T|null
      */
     public function findFirstInstanceOf(Node | array $nodes, string $type): ?Node
     {
@@ -182,7 +180,6 @@ public function hasInstancesOf(Node | array $nodes, array $types): bool
      * @template T of Node
      * @param class-string<T> $type
      * @param Node|Node[] $nodes
-     * @return T|null
      */
     public function findLastInstanceOf(Node | array $nodes, string $type): ?Node
     {
@@ -317,7 +314,6 @@ public function findFirstPrevious(Node $node, callable $filter): ?Node
     /**
      * @template T of Node
      * @param array<class-string<T>> $types
-     * @return T|null
      */
     public function findFirstPreviousOfTypes(Node $mainNode, array $types): ?Node
     {
@@ -373,12 +369,11 @@ public function findSameNamedExprs(Expr | Variable | Property | PropertyFetch |
             }
 
             $variables = $this->findInstancesOf($scopeNode, [Variable::class]);
-            $foundVariables = array_filter(
+
+            return array_filter(
                 $variables,
                 fn (Variable $variable) => $this->nodeNameResolver->isName($variable, $exprName)
             );
-
-            return $foundVariables;
         }
 
         if ($expr instanceof Property) {
@@ -395,20 +390,18 @@ public function findSameNamedExprs(Expr | Variable | Property | PropertyFetch |
         }
 
         $propertyFetches = $this->findInstancesOf($scopeNode, [PropertyFetch::class, StaticPropertyFetch::class]);
-        $foundProperties = array_filter(
+
+        return array_filter(
             $propertyFetches,
             fn (PropertyFetch | StaticPropertyFetch $propertyFetch) =>
                 $this->nodeNameResolver->isName($propertyFetch->name, $exprName)
         );
-
-        return $foundProperties;
     }
 
     /**
      * @template T of Node
      * @param Node|Node[] $nodes
      * @param class-string<T> $type
-     * @return T|null
      */
     private function findInstanceOfName(Node | array $nodes, string $type, string $name): ?Node
     {