diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/Fixture/private_method.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/Fixture/private_method.php.inc new file mode 100644 index 00000000000..08d11295a4d --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/Fixture/private_method.php.inc @@ -0,0 +1,31 @@ + +----- + diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/Fixture/private_method_list_doc.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/Fixture/private_method_list_doc.php.inc new file mode 100644 index 00000000000..9d7008871f3 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/Fixture/private_method_list_doc.php.inc @@ -0,0 +1,36 @@ + +----- + + */ + private function run(): array + { + $values = []; + $values[] = 'item'; + + return $values; + } +} + +?> diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/Fixture/skip_protected_method.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/Fixture/skip_protected_method.php.inc new file mode 100644 index 00000000000..2d5d869c493 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/Fixture/skip_protected_method.php.inc @@ -0,0 +1,13 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/config/configured_rule.php b/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/config/configured_rule.php new file mode 100644 index 00000000000..8f073f529c6 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector/config/configured_rule.php @@ -0,0 +1,9 @@ +withRules([PrivateMethodReturnTypeFromStrictNewArrayRector::class]); diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector/Fixture/skip_private_method.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector/Fixture/skip_private_method.php.inc new file mode 100644 index 00000000000..cfdbbace9c5 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector/Fixture/skip_private_method.php.inc @@ -0,0 +1,13 @@ +getClassName()); $instanceof = new Instanceof_($expr, $fullyQualified); diff --git a/rules/Naming/Matcher/VariableAndCallAssignMatcher.php b/rules/Naming/Matcher/VariableAndCallAssignMatcher.php index fe2bb23901e..effa3f85e9a 100644 --- a/rules/Naming/Matcher/VariableAndCallAssignMatcher.php +++ b/rules/Naming/Matcher/VariableAndCallAssignMatcher.php @@ -41,7 +41,10 @@ public function match(Assign $assign, ClassMethod|Closure|Function_ $functionLik $isVariableFoundInCallArgs = (bool) $this->betterNodeFinder->findFirst( $call->isFirstClassCallable() ? [] : $call->getArgs(), - fn (Node $subNode): bool => $subNode instanceof Variable && $this->nodeNameResolver->isName($subNode, $variableName) + fn (Node $subNode): bool => $subNode instanceof Variable && $this->nodeNameResolver->isName( + $subNode, + $variableName + ) ); if ($isVariableFoundInCallArgs) { diff --git a/rules/Privatization/NodeManipulator/VisibilityManipulator.php b/rules/Privatization/NodeManipulator/VisibilityManipulator.php index 1ce24b21298..455583f46a8 100644 --- a/rules/Privatization/NodeManipulator/VisibilityManipulator.php +++ b/rules/Privatization/NodeManipulator/VisibilityManipulator.php @@ -188,10 +188,8 @@ private function removeVisibility(ClassMethod|Property|ClassConst|Param $node): /** * @api */ - private function addVisibilityFlag( - Class_|ClassMethod|Property|ClassConst|Param $node, - int $visibility - ): void { + private function addVisibilityFlag(Class_|ClassMethod|Property|ClassConst|Param $node, int $visibility): void + { $node->flags |= $visibility; } diff --git a/rules/TypeDeclaration/NodeAnalyzer/StrictReturnNewArrayResolver.php b/rules/TypeDeclaration/NodeAnalyzer/StrictReturnNewArrayResolver.php new file mode 100644 index 00000000000..7ad68b09308 --- /dev/null +++ b/rules/TypeDeclaration/NodeAnalyzer/StrictReturnNewArrayResolver.php @@ -0,0 +1,218 @@ +stmts; + if ($stmts === null) { + return null; + } + + $variables = $this->matchArrayAssignedVariable($stmts); + if ($variables === []) { + return null; + } + + $returns = $this->betterNodeFinder->findReturnsScoped($node); + if (! $this->returnAnalyzer->hasOnlyReturnWithExpr($node, $returns)) { + return null; + } + + $variables = $this->matchVariableNotOverriddenByNonArray($node, $variables); + if ($variables === []) { + return null; + } + + if (count($returns) > 1) { + $returnType = $this->returnTypeInferer->inferFunctionLike($node); + return $this->processAddArrayReturnType($node, $returnType); + } + + $onlyReturn = $returns[0]; + if (! $onlyReturn->expr instanceof Variable) { + return null; + } + + if (! $this->nodeComparator->isNodeEqual($onlyReturn->expr, $variables)) { + return null; + } + + $returnType = $this->nodeTypeResolver->getNativeType($onlyReturn->expr); + return $this->processAddArrayReturnType($node, $returnType); + } + + private function processAddArrayReturnType( + ClassMethod|Function_ $node, + Type $returnType + ): ClassMethod|Function_|null { + if (! $returnType->isArray()->yes()) { + return null; + } + + // always returns array + $node->returnType = new Identifier('array'); + + // add more precise array type if suitable + if ($this->shouldAddReturnArrayDocType($returnType)) { + $this->changeReturnType($node, $returnType); + } + + return $node; + } + + private function changeReturnType(ClassMethod|Function_ $node, Type $arrayType): void + { + $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); + + // skip already filled type, on purpose + if (! $phpDocInfo->getReturnType() instanceof MixedType) { + return; + } + + // can handle only exactly 1-type array + if ($arrayType instanceof ConstantArrayType && count($arrayType->getValueTypes()) !== 1) { + return; + } + + $itemType = $arrayType->getIterableValueType(); + if ($itemType instanceof IntersectionType) { + $narrowArrayType = $arrayType; + } else { + $narrowArrayType = new ArrayType(new MixedType(), $itemType); + } + + if ($arrayType->isList()->yes()) { + $narrowArrayType = TypeCombinator::intersect($narrowArrayType, new AccessoryArrayListType()); + } + + $this->phpDocTypeChanger->changeReturnType($node, $phpDocInfo, $narrowArrayType); + } + + /** + * @param Variable[] $variables + * @return Variable[] + */ + private function matchVariableNotOverriddenByNonArray(ClassMethod|Function_ $functionLike, array $variables): array + { + // is variable overridden? + /** @var Assign[] $assigns */ + $assigns = $this->betterNodeFinder->findInstancesOfInFunctionLikeScoped($functionLike, Assign::class); + foreach ($assigns as $assign) { + if (! $assign->var instanceof Variable) { + continue; + } + + foreach ($variables as $key => $variable) { + if (! $this->nodeNameResolver->areNamesEqual($assign->var, $variable)) { + continue; + } + + if ($assign->expr instanceof Array_) { + continue; + } + + $nativeType = $this->nodeTypeResolver->getNativeType($assign->expr); + if (! $nativeType->isArray()->yes()) { + unset($variables[$key]); + } + } + } + + return $variables; + } + + /** + * @param Stmt[] $stmts + * @return Variable[] + */ + private function matchArrayAssignedVariable(array $stmts): array + { + $variables = []; + foreach ($stmts as $stmt) { + if (! $stmt instanceof Expression) { + continue; + } + + if (! $stmt->expr instanceof Assign) { + continue; + } + + $assign = $stmt->expr; + if (! $assign->var instanceof Variable) { + continue; + } + + $nativeType = $this->nodeTypeResolver->getNativeType($assign->expr); + if ($nativeType->isArray()->yes()) { + $variables[] = $assign->var; + } + } + + return $variables; + } + + private function shouldAddReturnArrayDocType(Type $arrayType): bool + { + if ($arrayType instanceof ConstantArrayType) { + if ($arrayType->getIterableValueType() instanceof NeverType) { + return false; + } + + // handle only simple arrays + if (! $arrayType->getIterableKeyType()->isInteger()->yes()) { + return false; + } + } + + return true; + } +} diff --git a/rules/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector.php b/rules/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector.php new file mode 100644 index 00000000000..4bd2d2d680e --- /dev/null +++ b/rules/TypeDeclaration/Rector/ClassMethod/PrivateMethodReturnTypeFromStrictNewArrayRector.php @@ -0,0 +1,94 @@ +> + */ + public function getNodeTypes(): array + { + return [ClassMethod::class]; + } + + /** + * @param ClassMethod $node + */ + public function refactor(Node $node): ?Node + { + if ($this->shouldSkip($node)) { + return null; + } + + return $this->strictReturnNewArrayResolver->resolve($node); + } + + public function provideMinPhpVersion(): int + { + return PhpVersion::PHP_70; + } + + private function shouldSkip(ClassMethod $classMethod): bool + { + if (! $classMethod->isPrivate()) { + return true; + } + + return $classMethod->returnType instanceof Node; + } +} diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector.php index d0372b24e59..6da5b7df9d3 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector.php @@ -5,31 +5,12 @@ namespace Rector\TypeDeclaration\Rector\ClassMethod; use PhpParser\Node; -use PhpParser\Node\Expr\Array_; -use PhpParser\Node\Expr\Assign; -use PhpParser\Node\Expr\Closure; -use PhpParser\Node\Expr\Variable; -use PhpParser\Node\Identifier; -use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\Function_; use PHPStan\Analyser\Scope; -use PHPStan\Type\Accessory\AccessoryArrayListType; -use PHPStan\Type\ArrayType; -use PHPStan\Type\Constant\ConstantArrayType; -use PHPStan\Type\IntersectionType; -use PHPStan\Type\MixedType; -use PHPStan\Type\NeverType; -use PHPStan\Type\Type; -use PHPStan\Type\TypeCombinator; -use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; -use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger; -use Rector\PhpParser\Node\BetterNodeFinder; use Rector\PHPStan\ScopeFetcher; use Rector\Rector\AbstractRector; -use Rector\TypeDeclaration\NodeAnalyzer\ReturnAnalyzer; -use Rector\TypeDeclaration\TypeInferer\ReturnTypeInferer; +use Rector\TypeDeclaration\NodeAnalyzer\StrictReturnNewArrayResolver; use Rector\ValueObject\PhpVersion; use Rector\VendorLocker\NodeVendorLocker\ClassMethodReturnTypeOverrideGuard; use Rector\VersionBonding\Contract\MinPhpVersionInterface; @@ -42,12 +23,8 @@ final class ReturnTypeFromStrictNewArrayRector extends AbstractRector implements MinPhpVersionInterface { public function __construct( - private readonly PhpDocTypeChanger $phpDocTypeChanger, private readonly ClassMethodReturnTypeOverrideGuard $classMethodReturnTypeOverrideGuard, - private readonly ReturnTypeInferer $returnTypeInferer, - private readonly PhpDocInfoFactory $phpDocInfoFactory, - private readonly BetterNodeFinder $betterNodeFinder, - private readonly ReturnAnalyzer $returnAnalyzer + private readonly StrictReturnNewArrayResolver $strictReturnNewArrayResolver ) { } @@ -101,43 +78,7 @@ public function refactor(Node $node): ?Node return null; } - // 1. is variable instantiated with array - $stmts = $node->stmts; - if ($stmts === null) { - return null; - } - - $variables = $this->matchArrayAssignedVariable($stmts); - if ($variables === []) { - return null; - } - - $returns = $this->betterNodeFinder->findReturnsScoped($node); - if (! $this->returnAnalyzer->hasOnlyReturnWithExpr($node, $returns)) { - return null; - } - - $variables = $this->matchVariableNotOverriddenByNonArray($node, $variables); - if ($variables === []) { - return null; - } - - if (count($returns) > 1) { - $returnType = $this->returnTypeInferer->inferFunctionLike($node); - return $this->processAddArrayReturnType($node, $returnType); - } - - $onlyReturn = $returns[0]; - if (! $onlyReturn->expr instanceof Variable) { - return null; - } - - if (! $this->nodeComparator->isNodeEqual($onlyReturn->expr, $variables)) { - return null; - } - - $returnType = $this->nodeTypeResolver->getNativeType($onlyReturn->expr); - return $this->processAddArrayReturnType($node, $returnType); + return $this->strictReturnNewArrayResolver->resolve($node); } public function provideMinPhpVersion(): int @@ -145,143 +86,20 @@ public function provideMinPhpVersion(): int return PhpVersion::PHP_70; } - private function processAddArrayReturnType( - ClassMethod|Function_|Closure $node, - Type $returnType - ): ClassMethod|Function_|Closure|null { - if (! $returnType->isArray()->yes()) { - return null; - } - - // always returns array - $node->returnType = new Identifier('array'); - - // add more precise array type if suitable - if ($this->shouldAddReturnArrayDocType($returnType)) { - $this->changeReturnType($node, $returnType); - } - - return $node; - } - - private function shouldSkip(ClassMethod|Function_|Closure $node, Scope $scope): bool + private function shouldSkip(ClassMethod|Function_ $node, Scope $scope): bool { if ($node->returnType instanceof Node) { return true; } + // private methods are handled by PrivateMethodReturnTypeFromStrictNewArrayRector + if ($node instanceof ClassMethod && $node->isPrivate()) { + return true; + } + return $node instanceof ClassMethod && $this->classMethodReturnTypeOverrideGuard->shouldSkipClassMethod( $node, $scope ); } - - private function changeReturnType(ClassMethod|Function_|Closure $node, Type $arrayType): void - { - $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); - - // skip already filled type, on purpose - if (! $phpDocInfo->getReturnType() instanceof MixedType) { - return; - } - - // can handle only exactly 1-type array - if ($arrayType instanceof ConstantArrayType && count($arrayType->getValueTypes()) !== 1) { - return; - } - - $itemType = $arrayType->getIterableValueType(); - if ($itemType instanceof IntersectionType) { - $narrowArrayType = $arrayType; - } else { - $narrowArrayType = new ArrayType(new MixedType(), $itemType); - } - - if ($arrayType->isList()->yes()) { - $narrowArrayType = TypeCombinator::intersect($narrowArrayType, new AccessoryArrayListType()); - } - - $this->phpDocTypeChanger->changeReturnType($node, $phpDocInfo, $narrowArrayType); - } - - /** - * @param Variable[] $variables - * @return Variable[] - */ - private function matchVariableNotOverriddenByNonArray( - ClassMethod|Function_ $functionLike, - array $variables - ): array { - // is variable overridden? - /** @var Assign[] $assigns */ - $assigns = $this->betterNodeFinder->findInstancesOfInFunctionLikeScoped($functionLike, Assign::class); - foreach ($assigns as $assign) { - if (! $assign->var instanceof Variable) { - continue; - } - - foreach ($variables as $key => $variable) { - if (! $this->nodeNameResolver->areNamesEqual($assign->var, $variable)) { - continue; - } - - if ($assign->expr instanceof Array_) { - continue; - } - - $nativeType = $this->nodeTypeResolver->getNativeType($assign->expr); - if (! $nativeType->isArray()->yes()) { - unset($variables[$key]); - } - } - } - - return $variables; - } - - /** - * @param Stmt[] $stmts - * @return Variable[] - */ - private function matchArrayAssignedVariable(array $stmts): array - { - $variables = []; - foreach ($stmts as $stmt) { - if (! $stmt instanceof Expression) { - continue; - } - - if (! $stmt->expr instanceof Assign) { - continue; - } - - $assign = $stmt->expr; - if (! $assign->var instanceof Variable) { - continue; - } - - $nativeType = $this->nodeTypeResolver->getNativeType($assign->expr); - if ($nativeType->isArray()->yes()) { - $variables[] = $assign->var; - } - } - - return $variables; - } - - private function shouldAddReturnArrayDocType(Type $arrayType): bool - { - if ($arrayType instanceof ConstantArrayType) { - if ($arrayType->getIterableValueType() instanceof NeverType) { - return false; - } - - // handle only simple arrays - if (! $arrayType->getIterableKeyType()->isInteger()->yes()) { - return false; - } - } - - return true; - } } diff --git a/src/Config/Level/TypeDeclarationLevel.php b/src/Config/Level/TypeDeclarationLevel.php index e9af567a9bd..f76e8b39fa2 100644 --- a/src/Config/Level/TypeDeclarationLevel.php +++ b/src/Config/Level/TypeDeclarationLevel.php @@ -38,6 +38,7 @@ use Rector\TypeDeclaration\Rector\ClassMethod\ObjectParamTypeByMethodCallTypeRector; use Rector\TypeDeclaration\Rector\ClassMethod\ParamTypeByMethodCallTypeRector; use Rector\TypeDeclaration\Rector\ClassMethod\ParamTypeByParentCallTypeRector; +use Rector\TypeDeclaration\Rector\ClassMethod\PrivateMethodReturnTypeFromStrictNewArrayRector; use Rector\TypeDeclaration\Rector\ClassMethod\ReturnNeverTypeRector; use Rector\TypeDeclaration\Rector\ClassMethod\ReturnNullableTypeRector; use Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromGetRepositoryDocblockRector; @@ -98,6 +99,8 @@ final class TypeDeclarationLevel AddArrowFunctionReturnTypeRector::class, BoolReturnTypeFromBooleanConstReturnsRector::class, + // private methods first, as safest - no external contract + PrivateMethodReturnTypeFromStrictNewArrayRector::class, ReturnTypeFromStrictNewArrayRector::class, // scalar and array from constant diff --git a/src/PhpParser/Node/BetterNodeFinder.php b/src/PhpParser/Node/BetterNodeFinder.php index 413b0fd58fc..899e8e198e5 100644 --- a/src/PhpParser/Node/BetterNodeFinder.php +++ b/src/PhpParser/Node/BetterNodeFinder.php @@ -135,7 +135,10 @@ public function find(Node|array $nodes, callable $filter): array public function findFirstNonAnonymousClass(array $nodes): ?Node { // skip anonymous classes - return $this->findFirst($nodes, fn (Node $node): bool => $node instanceof Class_ && ! $this->classAnalyzer->isAnonymousClass($node)); + return $this->findFirst( + $nodes, + fn (Node $node): bool => $node instanceof Class_ && ! $this->classAnalyzer->isAnonymousClass($node) + ); } /** @@ -298,6 +301,9 @@ private function findInstanceOfName(Node|array $nodes, string $type, string $nam { Assert::isAOf($type, Node::class); - return $this->nodeFinder->findFirst($nodes, fn (Node $node): bool => $node instanceof $type && $this->nodeNameResolver->isName($node, $name)); + return $this->nodeFinder->findFirst( + $nodes, + fn (Node $node): bool => $node instanceof $type && $this->nodeNameResolver->isName($node, $name) + ); } }