From dae869b054903db47b226fb8fd92cc830b7b2ee8 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 10 Aug 2026 21:00:58 +0200 Subject: [PATCH] [deprecation] Deprecate AddReturnDocblockForDimFetchArrayFromAssignsRector, as the guessed array shape is vague and unreliable The rule guesses the array shape from conditional assigns inside a method. Any later assign, loop or dynamic key can widen the real type, so the added @return docblock is often too narrow. Add the docblock manually instead. --- phpstan.neon | 6 +- ...kForDimFetchArrayFromAssignsRectorTest.php | 28 ---- .../Fixture/conditional_assign.php.inc | 42 ----- .../multiple_conditional_assign.php.inc | 50 ------ .../Fixture/some_item.php.inc | 36 ----- .../config/configured_rule.php | 9 -- ...blockForDimFetchArrayFromAssignsRector.php | 144 +----------------- .../Level/TypeDeclarationDocblocksLevel.php | 4 - 8 files changed, 9 insertions(+), 310 deletions(-) delete mode 100644 rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/AddReturnDocblockForDimFetchArrayFromAssignsRectorTest.php delete mode 100644 rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/conditional_assign.php.inc delete mode 100644 rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/multiple_conditional_assign.php.inc delete mode 100644 rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/some_item.php.inc delete mode 100644 rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/config/configured_rule.php diff --git a/phpstan.neon b/phpstan.neon index 8a1aff66b00..7ea282e9f07 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -465,11 +465,7 @@ parameters: - '#Class "Rector\\CodingStyle\\Rector\\Enum_\\EnumCaseToPascalCaseRector" is missing @see annotation with test case class reference#' - '#Class "Rector\\CodeQuality\\Rector\\Concat\\JoinStringConcatRector" is missing @see annotation with test case class reference#' - '#Class "Rector\\CodeQuality\\Rector\\Switch_\\SwitchTrueToIfRector" is missing @see annotation with test case class reference#' - - # false positive - - - identifier: phpstanApi.varTagAssumption - path: rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector.php + - '#Class "Rector\\TypeDeclarationDocblocks\\Rector\\ClassMethod\\AddReturnDocblockForDimFetchArrayFromAssignsRector" is missing @see annotation with test case class reference#' # @todo fix in phpstan-rules - diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/AddReturnDocblockForDimFetchArrayFromAssignsRectorTest.php b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/AddReturnDocblockForDimFetchArrayFromAssignsRectorTest.php deleted file mode 100644 index e45ffa6d107..00000000000 --- a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/AddReturnDocblockForDimFetchArrayFromAssignsRectorTest.php +++ /dev/null @@ -1,28 +0,0 @@ -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 deleted file mode 100644 index 1a031ce2a9f..00000000000 --- a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/conditional_assign.php.inc +++ /dev/null @@ -1,42 +0,0 @@ - ------ - - */ - 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 deleted file mode 100644 index c5929d0b80f..00000000000 --- a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/multiple_conditional_assign.php.inc +++ /dev/null @@ -1,50 +0,0 @@ - ------ - - */ - 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 deleted file mode 100644 index 3edcb1b1f73..00000000000 --- a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/Fixture/some_item.php.inc +++ /dev/null @@ -1,36 +0,0 @@ - ------ - - */ - 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 deleted file mode 100644 index b54cbe0ed37..00000000000 --- a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector/config/configured_rule.php +++ /dev/null @@ -1,9 +0,0 @@ -withRules([AddReturnDocblockForDimFetchArrayFromAssignsRector::class]); diff --git a/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector.php b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector.php index d2cbf4d2cd5..000a9b97d00 100644 --- a/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector.php +++ b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForDimFetchArrayFromAssignsRector.php @@ -5,38 +5,18 @@ namespace Rector\TypeDeclarationDocblocks\Rector\ClassMethod; use PhpParser\Node; -use PhpParser\Node\Expr\Array_; -use PhpParser\Node\Expr\Assign; -use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\Node\Stmt\Expression; -use PhpParser\Node\Stmt\Return_; -use PHPStan\Type\Constant\ConstantArrayType; -use PHPStan\Type\Type; -use PHPStan\Type\UnionType; -use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; -use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; +use Rector\Exception\ShouldNotHappenException; use Rector\Rector\AbstractRector; -use Rector\TypeDeclarationDocblocks\NodeFinder\ReturnNodeFinder; -use Rector\TypeDeclarationDocblocks\TagNodeAnalyzer\UsefulArrayTagNodeAnalyzer; -use Rector\TypeDeclarationDocblocks\TypeResolver\ConstantArrayTypeGeneralizer; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see \Rector\Tests\TypeDeclarationDocblocks\Rector\ClassMethod\AddReturnDocblockForDimFetchArrayFromAssignsRector\AddReturnDocblockForDimFetchArrayFromAssignsRectorTest + * @deprecated This rule is deprecated, as the array shape is guessed from conditional assigns. The result is vague and unreliable, as any later assign can widen the type. Add the @return docblock manually instead. */ -final class AddReturnDocblockForDimFetchArrayFromAssignsRector extends AbstractRector +final class AddReturnDocblockForDimFetchArrayFromAssignsRector extends AbstractRector implements DeprecatedInterface { - public function __construct( - private readonly PhpDocInfoFactory $phpDocInfoFactory, - private readonly UsefulArrayTagNodeAnalyzer $usefulArrayTagNodeAnalyzer, - private readonly ReturnNodeFinder $returnNodeFinder, - private readonly ConstantArrayTypeGeneralizer $constantArrayTypeGeneralizer, - private readonly PhpDocTypeChanger $phpDocTypeChanger, - ) { - } - public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( @@ -101,117 +81,9 @@ public function getNodeTypes(): array */ 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) { - return array_all( - $returnedExprType->getTypes(), - fn (Type $unionedType): bool => $unionedType instanceof ConstantArrayType - ); - } - - return $returnedExprType instanceof ConstantArrayType; + throw new ShouldNotHappenException(sprintf( + '"%s" rule is deprecated, as the array shape guessed from conditional assigns is vague and unreliable. Add the @return docblock manually instead', + self::class + )); } } diff --git a/src/Config/Level/TypeDeclarationDocblocksLevel.php b/src/Config/Level/TypeDeclarationDocblocksLevel.php index 08d540cb241..49a5c45d39b 100644 --- a/src/Config/Level/TypeDeclarationDocblocksLevel.php +++ b/src/Config/Level/TypeDeclarationDocblocksLevel.php @@ -18,7 +18,6 @@ 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; @@ -58,8 +57,5 @@ final class TypeDeclarationDocblocksLevel // return DocblockGetterReturnArrayFromPropertyDocblockVarRector::class, NarrowArrayCollectionUnionReturnDocblockRector::class, - - // run latter after other rules, as more generic - AddReturnDocblockForDimFetchArrayFromAssignsRector::class, ]; }