From 5732429771c86bb00f442ae00de37ef55066c3ae Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 15 Jul 2023 00:23:45 +0700 Subject: [PATCH 1/8] [TypeDeclaration] Null stmts check early on ReturnTypeFromStrictParamRector --- .../ReturnTypeFromStrictParamRector.php | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php index 07f9e25942f..ea2efdfdee1 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php @@ -15,6 +15,7 @@ use PhpParser\Node\Name; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Param; +use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; use PhpParser\Node\Stmt\Return_; @@ -90,11 +91,16 @@ public function provideMinPhpVersion(): int */ public function refactorWithScope(Node $node, Scope $scope): ?Node { + $stmts = $node->stmts; + if ($stmts === null) { + return null; + } + if ($this->shouldSkipNode($node)) { return null; } - $return = $this->findCurrentScopeReturn($node); + $return = $this->findCurrentScopeReturn($stmts); if ($return === null || $return->expr === null) { return null; } @@ -121,15 +127,14 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node return null; } - private function findCurrentScopeReturn(ClassMethod|Function_ $node): ?Return_ + /** + * @param Stmt[] $stmts + */ + private function findCurrentScopeReturn(array $stmts): ?Return_ { $return = null; - if ($node->stmts === null) { - return null; - } - - $this->traverseNodesWithCallable($node->stmts, function (Node $node) use (&$return): ?int { + $this->traverseNodesWithCallable($stmts, function (Node $node) use (&$return): ?int { if (! $node instanceof Return_) { return null; } From ad643427dc556c8143c783994efd6fb6eae0b82b Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 15 Jul 2023 00:26:27 +0700 Subject: [PATCH 2/8] [TypeDeclaration] Null stmts check early on ReturnTypeFromStrictParamRector --- .../ReturnTypeFromStrictParamRector.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php index ea2efdfdee1..3de6c1cff0b 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php @@ -111,7 +111,7 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node continue; } - if ($this->shouldSkipParam($param, $node)) { + if ($this->shouldSkipParam($param, $stmts)) { continue; } @@ -157,17 +157,15 @@ private function findCurrentScopeReturn(array $stmts): ?Return_ return $return; } - private function shouldSkipParam(Param $param, ClassMethod|Function_ $functionLike): bool + /** + * @param Stmt[] $stmts + */ + private function shouldSkipParam(Param $param, array $stmts): bool { $paramName = $this->getName($param); - $isParamModified = false; - if ($functionLike->stmts === null) { - return true; - } - - $this->traverseNodesWithCallable($functionLike->stmts, function (Node $node) use ( + $this->traverseNodesWithCallable($stmts, function (Node $node) use ( $paramName, &$isParamModified ): int|null { From 8c880ec9febdcd36a1a11b822e5d62ccd0765b27 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 15 Jul 2023 00:27:13 +0700 Subject: [PATCH 3/8] [TypeDeclaration] Null stmts check early on ReturnTypeFromStrictParamRector --- .../ReturnTypeFromStrictParamRector.php | 32 ++++++------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php index 3de6c1cff0b..68287f7479d 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php @@ -6,14 +6,10 @@ use PhpParser\Node; use PhpParser\Node\Expr; -use PhpParser\Node\Expr\ArrayDimFetch; -use PhpParser\Node\Expr\Closure; -use PhpParser\Node\Expr\FuncCall; +use PhpParser\Node\Expr\Assign; +use PhpParser\Node\Expr\AssignRef; use PhpParser\Node\Expr\Variable; use PhpParser\Node\FunctionLike; -use PhpParser\Node\Identifier; -use PhpParser\Node\Name; -use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Param; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\ClassMethod; @@ -22,10 +18,8 @@ use PhpParser\NodeTraverser; use PHPStan\Analyser\Scope; use PHPStan\Type\MixedType; -use PHPStan\Type\ObjectType; use PHPStan\Type\TypeCombinator; use PHPStan\Type\UnionType; -use Rector\Core\Rector\AbstractRector; use Rector\Core\Rector\AbstractScopeAwareRector; use Rector\Core\ValueObject\PhpVersionFeature; use Rector\TypeDeclaration\TypeInferer\ReturnTypeInferer; @@ -101,13 +95,13 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node } $return = $this->findCurrentScopeReturn($stmts); - if ($return === null || $return->expr === null) { + if (! $return instanceof Return_ || ! $return->expr instanceof Expr) { return null; } $returnName = $this->getName($return->expr); foreach ($node->getParams() as $param) { - if (!$param->type instanceof Node) { + if (! $param->type instanceof Node) { continue; } @@ -134,7 +128,7 @@ private function findCurrentScopeReturn(array $stmts): ?Return_ { $return = null; - $this->traverseNodesWithCallable($stmts, function (Node $node) use (&$return): ?int { + $this->traverseNodesWithCallable($stmts, static function (Node $node) use (&$return): ?int { if (! $node instanceof Return_) { return null; } @@ -169,14 +163,12 @@ private function shouldSkipParam(Param $param, array $stmts): bool $paramName, &$isParamModified ): int|null { - if ($node instanceof Expr\AssignRef) { - if ($this->isName($node->expr, $paramName)) { - $isParamModified = true; - return NodeTraverser::STOP_TRAVERSAL; - } + if ($node instanceof AssignRef && $this->isName($node->expr, $paramName)) { + $isParamModified = true; + return NodeTraverser::STOP_TRAVERSAL; } - if (! $node instanceof Expr\Assign) { + if (! $node instanceof Assign) { return null; } @@ -217,10 +209,6 @@ private function shouldSkipNode(ClassMethod|Function_ $node): bool } $returnType = TypeCombinator::removeNull($returnType); - if ($returnType instanceof UnionType) { - return true; - } - - return false; + return $returnType instanceof UnionType; } } From 4fe8a3f6d7117e2c8b3029614d68f60a91792457 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Fri, 14 Jul 2023 17:28:55 +0000 Subject: [PATCH 4/8] [ci-review] Rector Rectify --- .../TypeAnalyzer/ReturnStrictTypeAnalyzer.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/rules/TypeDeclaration/TypeAnalyzer/ReturnStrictTypeAnalyzer.php b/rules/TypeDeclaration/TypeAnalyzer/ReturnStrictTypeAnalyzer.php index 88576faf398..193b8e1bf3b 100644 --- a/rules/TypeDeclaration/TypeAnalyzer/ReturnStrictTypeAnalyzer.php +++ b/rules/TypeDeclaration/TypeAnalyzer/ReturnStrictTypeAnalyzer.php @@ -4,6 +4,10 @@ namespace Rector\TypeDeclaration\TypeAnalyzer; +use PhpParser\Node\Expr\Array_; +use PhpParser\Node\Scalar\String_; +use PhpParser\Node\Scalar\LNumber; +use PhpParser\Node\Scalar\DNumber; use PhpParser\Node; use PhpParser\Node\Expr; use PhpParser\Node\Expr\FuncCall; @@ -51,10 +55,10 @@ public function collectStrictReturnTypes(array $returns, Scope $scope): array $containsStrictCall = true; $returnNode = $this->resolveMethodCallReturnNode($returnedExpr); } elseif ( - $returnedExpr instanceof Expr\Array_ - || $returnedExpr instanceof Node\Scalar\String_ - || $returnedExpr instanceof Node\Scalar\LNumber - || $returnedExpr instanceof Node\Scalar\DNumber + $returnedExpr instanceof Array_ + || $returnedExpr instanceof String_ + || $returnedExpr instanceof LNumber + || $returnedExpr instanceof DNumber ) { $returnNode = $this->resolveLiteralReturnNode($returnedExpr, $scope); } else { @@ -97,7 +101,7 @@ public function resolveMethodCallReturnNode(MethodCall | StaticCall | FuncCall $ return $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode($returnType, TypeKind::RETURN); } - private function resolveLiteralReturnNode(Expr\Array_|Scalar $returnedExpr, Scope $scope): ?Node + private function resolveLiteralReturnNode(Array_|Scalar $returnedExpr, Scope $scope): ?Node { $returnType = $scope->getType($returnedExpr); return $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode($returnType, TypeKind::RETURN); From c69c8e64f13a5ca222bf2ac3afcdae1ceeceb1df Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 15 Jul 2023 00:32:28 +0700 Subject: [PATCH 5/8] remove unnecessary assign when possible --- .../Rector/ClassMethod/ReturnTypeFromStrictParamRector.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php index 68287f7479d..151c25055a5 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php @@ -85,8 +85,7 @@ public function provideMinPhpVersion(): int */ public function refactorWithScope(Node $node, Scope $scope): ?Node { - $stmts = $node->stmts; - if ($stmts === null) { + if ($node->stmts === null) { return null; } @@ -94,12 +93,14 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node return null; } - $return = $this->findCurrentScopeReturn($stmts); + $return = $this->findCurrentScopeReturn($node->stmts); if (! $return instanceof Return_ || ! $return->expr instanceof Expr) { return null; } $returnName = $this->getName($return->expr); + $stmts = $node->stmts; + foreach ($node->getParams() as $param) { if (! $param->type instanceof Node) { continue; From 3026074875b8fcecbcd77af1208642e34da15863 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 15 Jul 2023 00:35:45 +0700 Subject: [PATCH 6/8] Revert "remove unnecessary assign when possible" This reverts commit c69c8e64f13a5ca222bf2ac3afcdae1ceeceb1df. --- .../Rector/ClassMethod/ReturnTypeFromStrictParamRector.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php index 151c25055a5..68287f7479d 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php @@ -85,7 +85,8 @@ public function provideMinPhpVersion(): int */ public function refactorWithScope(Node $node, Scope $scope): ?Node { - if ($node->stmts === null) { + $stmts = $node->stmts; + if ($stmts === null) { return null; } @@ -93,14 +94,12 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node return null; } - $return = $this->findCurrentScopeReturn($node->stmts); + $return = $this->findCurrentScopeReturn($stmts); if (! $return instanceof Return_ || ! $return->expr instanceof Expr) { return null; } $returnName = $this->getName($return->expr); - $stmts = $node->stmts; - foreach ($node->getParams() as $param) { if (! $param->type instanceof Node) { continue; From d3ecc82767b9db32ff0e7589efb312feaa8f3188 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 15 Jul 2023 00:41:16 +0700 Subject: [PATCH 7/8] Fixture cache --- .../Fixture/fixture.php.inc | 36 ++++++++++++----- .../Fixture/fixture2.php.inc | 40 +++++++++++++------ 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/rules-tests/DeadCode/Rector/Cast/RecastingRemovalRector/Fixture/fixture.php.inc b/rules-tests/DeadCode/Rector/Cast/RecastingRemovalRector/Fixture/fixture.php.inc index 1d72a80a08e..bf21b3bb489 100644 --- a/rules-tests/DeadCode/Rector/Cast/RecastingRemovalRector/Fixture/fixture.php.inc +++ b/rules-tests/DeadCode/Rector/Cast/RecastingRemovalRector/Fixture/fixture.php.inc @@ -1,23 +1,39 @@ ----- diff --git a/rules-tests/DeadCode/Rector/Cast/RecastingRemovalRector/Fixture/fixture2.php.inc b/rules-tests/DeadCode/Rector/Cast/RecastingRemovalRector/Fixture/fixture2.php.inc index aad418cf537..5018c9cfb2b 100644 --- a/rules-tests/DeadCode/Rector/Cast/RecastingRemovalRector/Fixture/fixture2.php.inc +++ b/rules-tests/DeadCode/Rector/Cast/RecastingRemovalRector/Fixture/fixture2.php.inc @@ -1,23 +1,39 @@ ----- From f09936739140c3737e41ecfeed536ba7a2f710ae Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 15 Jul 2023 00:41:46 +0700 Subject: [PATCH 8/8] Revert "Revert "remove unnecessary assign when possible"" This reverts commit 3026074875b8fcecbcd77af1208642e34da15863. --- .../Rector/ClassMethod/ReturnTypeFromStrictParamRector.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php index 68287f7479d..151c25055a5 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php @@ -85,8 +85,7 @@ public function provideMinPhpVersion(): int */ public function refactorWithScope(Node $node, Scope $scope): ?Node { - $stmts = $node->stmts; - if ($stmts === null) { + if ($node->stmts === null) { return null; } @@ -94,12 +93,14 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node return null; } - $return = $this->findCurrentScopeReturn($stmts); + $return = $this->findCurrentScopeReturn($node->stmts); if (! $return instanceof Return_ || ! $return->expr instanceof Expr) { return null; } $returnName = $this->getName($return->expr); + $stmts = $node->stmts; + foreach ($node->getParams() as $param) { if (! $param->type instanceof Node) { continue;