diff --git a/phpstan.neon b/phpstan.neon index d619aa2887d..3f0fa0994e7 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -464,3 +464,8 @@ parameters: - path: src/PhpParser/Node/CustomNode/FileWithoutNamespace.php identifier: symplify.forbiddenExtendOfNonAbstractClass + + # false positive + - + identifier: phpstanApi.varTagAssumption + path: rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector.php diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/AddReturnDocblockForDimFetchArrayFromAssignsRectorTest.php b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/AddReturnDocblockForDimFetchArrayFromAssignsRectorTest.php new file mode 100644 index 00000000000..e45ffa6d107 --- /dev/null +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/AddReturnDocblockForDimFetchArrayFromAssignsRectorTest.php @@ -0,0 +1,28 @@ +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/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/conditional_assign.php.inc b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/conditional_assign.php.inc new file mode 100644 index 00000000000..1a031ce2a9f --- /dev/null +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/conditional_assign.php.inc @@ -0,0 +1,42 @@ + +----- + + */ + public function toArray(): array + { + $items = []; + + if (mt_rand(0, 1)) { + $items['key'] = 100; + } + + return $items; + } +} + +?> diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/multiple_conditional_assign.php.inc b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/multiple_conditional_assign.php.inc new file mode 100644 index 00000000000..c5929d0b80f --- /dev/null +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/multiple_conditional_assign.php.inc @@ -0,0 +1,50 @@ + +----- + + */ + public function toArray(): array + { + $items = []; + + if (mt_rand(0, 1)) { + $items['key'] = 'some_string'; + } + + if (mt_rand(0, 1)) { + $items['another_key'] = 500; + } + + return $items; + } +} + +?> diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/some_item.php.inc b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/some_item.php.inc new file mode 100644 index 00000000000..3edcb1b1f73 --- /dev/null +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/some_item.php.inc @@ -0,0 +1,36 @@ + +----- + + */ + public function toArray(): array + { + $items = []; + $items['key'] = 100; + + return $items; + } +} + +?> diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/config/configured_rule.php b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/config/configured_rule.php new file mode 100644 index 00000000000..b54cbe0ed37 --- /dev/null +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/config/configured_rule.php @@ -0,0 +1,9 @@ +withRules([AddReturnDocblockForDimFetchArrayFromAssignsRector::class]); diff --git a/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector.php b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector.php new file mode 100644 index 00000000000..3698f2bb697 --- /dev/null +++ b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector.php @@ -0,0 +1,220 @@ + + */ + public function toArray() + { + $items = []; + + if (mt_rand(0, 1)) { + $items['key'] = 'value'; + } + + if (mt_rand(0, 1)) { + $items['another_key'] = 'another_value'; + } + + return $items; + } +} +CODE_SAMPLE + ), + + ] + ); + } + + public function getNodeTypes(): array + { + return [ClassMethod::class]; + } + + /** + * @param ClassMethod $node + */ + public function refactor(Node $node): ?ClassMethod + { + if ($node->stmts === null) { + return null; + } + + $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); + + if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) { + return null; + } + + $soleReturn = $this->returnNodeFinder->findOnlyReturnWithExpr($node); + if (! $soleReturn instanceof Return_) { + return null; + } + + // only variable + if (! $soleReturn->expr instanceof Variable) { + return null; + } + + // @todo check type here + $returnedExprType = $this->getType($soleReturn->expr); + + if (! $this->isConstantArrayType($returnedExprType)) { + return null; + } + + // find stmts with $item = []; + $returnedVariableName = $this->getName($soleReturn->expr); + if (! is_string($returnedVariableName)) { + return null; + } + + if (! $this->isVariableInstantiated($node, $returnedVariableName)) { + return null; + } + + if ($returnedExprType->getReferencedClasses() !== []) { + // better handled by shared-interface/class rule, to avoid turning objects to mixed + return null; + } + + // conditional assign + $genericUnionedTypeNodes = []; + + if ($returnedExprType instanceof UnionType) { + foreach ($returnedExprType->getTypes() as $unionedType) { + if ($unionedType instanceof ConstantArrayType) { + // skip empty array + if ($unionedType->getKeyTypes() === [] && $unionedType->getValueTypes() === []) { + continue; + } + + $genericUnionedTypeNode = $this->constantArrayTypeGeneralizer->generalize($unionedType); + $genericUnionedTypeNodes[] = $genericUnionedTypeNode; + } + } + } else { + /** @var ConstantArrayType $returnedExprType */ + $genericTypeNode = $this->constantArrayTypeGeneralizer->generalize($returnedExprType); + $this->phpDocTypeChanger->changeReturnTypeNode($node, $phpDocInfo, $genericTypeNode); + + return $node; + } + + // @todo handle multiple type nodes + $this->phpDocTypeChanger->changeReturnTypeNode($node, $phpDocInfo, $genericUnionedTypeNodes[0]); + + return $node; + } + + private function isVariableInstantiated(ClassMethod $classMethod, string $returnedVariableName): bool + { + foreach ((array) $classMethod->stmts as $stmt) { + if (! $stmt instanceof Expression) { + continue; + } + + if (! $stmt->expr instanceof Assign) { + continue; + } + + $assign = $stmt->expr; + if (! $assign->var instanceof Variable) { + continue; + } + + if (! $this->isName($assign->var, $returnedVariableName)) { + continue; + } + + // must be array assignment + if (! $assign->expr instanceof Array_) { + continue; + } + + return true; + } + + return false; + } + + private function isConstantArrayType(Type $returnedExprType): bool + { + if ($returnedExprType instanceof UnionType) { + foreach ($returnedExprType->getTypes() as $unionedType) { + if (! $unionedType instanceof ConstantArrayType) { + return false; + } + } + + return true; + } + + return $returnedExprType instanceof ConstantArrayType; + } +} diff --git a/src/Config/Level/TypeDeclarationDocblocksLevel.php b/src/Config/Level/TypeDeclarationDocblocksLevel.php index bcc9caa97bb..bd1d0bbee59 100644 --- a/src/Config/Level/TypeDeclarationDocblocksLevel.php +++ b/src/Config/Level/TypeDeclarationDocblocksLevel.php @@ -19,6 +19,7 @@ use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\AddParamArrayDocblockFromDimFetchAccessRector; use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\AddReturnDocblockForArrayDimAssignedObjectRector; use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\AddReturnDocblockForCommonObjectDenominatorRector; +use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\AddReturnDocblockForDimFetchArrayFromAssignsRector; use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\AddReturnDocblockForJsonArrayRector; use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\DocblockGetterReturnArrayFromPropertyDocblockVarRector; use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\DocblockReturnArrayFromDirectArrayInstanceRector; @@ -59,5 +60,8 @@ final class TypeDeclarationDocblocksLevel // return DocblockGetterReturnArrayFromPropertyDocblockVarRector::class, + + // run latter after other rules, as more generic + AddReturnDocblockForDimFetchArrayFromAssignsRector::class, ]; }