From b001e388ccfb37f8af1f4873c63212a0155f642a Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Fri, 16 Jul 2021 13:59:37 +0700 Subject: [PATCH] [TypeDeclaration] Skip ReturnTypeFromStrictTypedCallRector on possible no return (#441) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [TypeDeclaration] Skip ReturnTypeFromStrictTypedCallRector on possible no return * Fixed 🎉 * [ci-review] Rector Rectify Co-authored-by: GitHub Action --- .../ReadWritePropertyAnalyzer.php | 2 -- .../Fixture/skip_possible_no_return.php.inc | 18 +++++++++++++++ .../ReturnTypeFromStrictTypedCallRector.php | 23 ++++++++++++++++++- .../ClassWithPublicPropertiesFactory.php | 5 ++-- src/PhpParser/Node/BetterNodeFinder.php | 15 ++++-------- 5 files changed, 47 insertions(+), 16 deletions(-) create mode 100644 rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictTypedCallRector/Fixture/skip_possible_no_return.php.inc diff --git a/packages/ReadWrite/NodeAnalyzer/ReadWritePropertyAnalyzer.php b/packages/ReadWrite/NodeAnalyzer/ReadWritePropertyAnalyzer.php index cde0b83e47d..374a65c533c 100644 --- 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/rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictTypedCallRector/Fixture/skip_possible_no_return.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictTypedCallRector/Fixture/skip_possible_no_return.php.inc new file mode 100644 index 00000000000..d36ac3ef565 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictTypedCallRector/Fixture/skip_possible_no_return.php.inc @@ -0,0 +1,18 @@ +getNumber(); + } + } + + private function getNumber(): int + { + return 100; + } +} diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictTypedCallRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictTypedCallRector.php index 421b7e2717b..d5976602277 100644 --- 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 index cf50c2eabad..cc4fe1c1d5c 100644 --- 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 index a64dcdadfc2..f3e24da7cc9 100644 --- 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 $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 $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 $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> $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 $type - * @return T|null */ private function findInstanceOfName(Node | array $nodes, string $type, string $name): ?Node {