diff --git a/src/Rules/Properties/ReadOnlyPropertyIndirectModificationRule.php b/src/Rules/Properties/ReadOnlyPropertyIndirectModificationRule.php new file mode 100644 index 00000000000..e52c8ebf6a8 --- /dev/null +++ b/src/Rules/Properties/ReadOnlyPropertyIndirectModificationRule.php @@ -0,0 +1,123 @@ + + */ +#[RegisteredRule(level: 3)] +final class ReadOnlyPropertyIndirectModificationRule implements Rule +{ + + public function __construct( + private PropertyReflectionFinder $propertyReflectionFinder, + ) + { + } + + public function getNodeType(): string + { + return PropertyAssignNode::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $propertyFetch = $node->getPropertyFetch(); + if (!$propertyFetch instanceof PropertyFetch) { + return []; + } + + return $this->checkVarChain($propertyFetch->var, $scope); + } + + /** + * @return list + */ + private function checkVarChain(Expr $expr, Scope $scope): array + { + $errors = []; + + while (true) { + if ($expr instanceof ArrayDimFetch) { + while ($expr instanceof ArrayDimFetch) { + $expr = $expr->var; + } + + if ($expr instanceof PropertyFetch && $expr->name instanceof Node\Identifier) { + $propertyType = $scope->getType($expr); + if (!(new ObjectType(ArrayAccess::class))->isSuperTypeOf($propertyType)->yes()) { + $reflections = $this->propertyReflectionFinder->findPropertyReflectionsFromNode($expr, $scope); + foreach ($reflections as $reflection) { + $nativeReflection = $reflection->getNativeReflection(); + if ($nativeReflection === null) { + continue; + } + if ($nativeReflection->isReadOnly()) { + $declaringClass = $nativeReflection->getDeclaringClass(); + $errors[] = RuleErrorBuilder::message(sprintf( + 'Readonly property %s::$%s is indirectly modified.', + $declaringClass->getDisplayName(), + $reflection->getName(), + )) + ->line($expr->name->getStartLine()) + ->identifier('property.readOnlyIndirectModification') + ->build(); + } elseif ($nativeReflection->isReadOnlyByPhpDoc()) { + if ($nativeReflection->isAllowedPrivateMutation()) { + continue; + } + $declaringClass = $nativeReflection->getDeclaringClass(); + $errors[] = RuleErrorBuilder::message(sprintf( + '@readonly property %s::$%s is indirectly modified.', + $declaringClass->getDisplayName(), + $reflection->getName(), + )) + ->line($expr->name->getStartLine()) + ->identifier('property.readOnlyByPhpDocIndirectModification') + ->build(); + } + } + } + + $expr = $expr->var; + continue; + } + + if ($expr instanceof StaticPropertyFetch) { + $expr = $expr->class instanceof Node\Name ? null : $expr->class; + if ($expr === null) { + break; + } + continue; + } + + break; + } + + if ($expr instanceof PropertyFetch) { + $expr = $expr->var; + continue; + } + + break; + } + + return $errors; + } + +} diff --git a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyIndirectModificationRuleTest.php b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyIndirectModificationRuleTest.php new file mode 100644 index 00000000000..514be8e7aa9 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyIndirectModificationRuleTest.php @@ -0,0 +1,61 @@ + + */ +class ReadOnlyPropertyIndirectModificationRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new ReadOnlyPropertyIndirectModificationRule( + new PropertyReflectionFinder(), + ); + } + + #[RequiresPhp('>= 8.1.0')] + public function testBug14481(): void + { + $this->analyse([__DIR__ . '/data/bug-14481.php'], [ + [ + 'Readonly property Bug14481\VehicleListFilterForModule::$carTypes is indirectly modified.', + 28, + ], + [ + 'Readonly property Bug14481\IndirectModificationOutsideConstructor::$items is indirectly modified.', + 47, + ], + [ + 'Readonly property Bug14481\DirectPropertyAssignThroughReadonlyArray::$items is indirectly modified.', + 65, + ], + [ + 'Readonly property Bug14481\NestedArrayDimFetch::$nested is indirectly modified.', + 86, + ], + [ + 'Readonly property Bug14481\IncrementThroughReadonlyArray::$items is indirectly modified.', + 131, + ], + [ + 'Readonly property Bug14481\DeeperChain::$wrappers is indirectly modified.', + 165, + ], + [ + '@readonly property Bug14481\ReadonlyByPhpDocClass::$items is indirectly modified.', + 195, + ], + [ + '@readonly property Bug14481\ReadonlyByPhpDocProperty::$items is indirectly modified.', + 217, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Properties/data/bug-14481.php b/tests/PHPStan/Rules/Properties/data/bug-14481.php new file mode 100644 index 00000000000..3285050017b --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-14481.php @@ -0,0 +1,219 @@ += 8.1 + +namespace Bug14481; + +class VehicleType { + /** @var array */ + public array $brand = []; +} + +class Vehicle { + public int $id = 1; + public string $name = 'test'; +} + +final readonly class VehicleListFilterForModule +{ + /** + * @var array + */ + private array $carTypes; + + /** @param array $carTypes */ + public function __construct(array $carTypes, Vehicle $vehicle, int $vehicleTyp) + { + $this->carTypes = $carTypes; + + $this + ->carTypes[$vehicleTyp] + ->brand[$vehicle->id] = $vehicle->name; + } +} + +class IndirectModificationOutsideConstructor +{ + public readonly array $items; + + /** + * @param array $items + */ + public function __construct(array $items) + { + $this->items = $items; + } + + public function doFoo(int $key): void + { + $this->items[$key]->brand[0] = 'test'; + } +} + +class DirectPropertyAssignThroughReadonlyArray +{ + public readonly array $items; + + /** + * @param array $items + */ + public function __construct(array $items) + { + $this->items = $items; + } + + public function doFoo(int $key): void + { + $this->items[$key]->brand = ['test']; + } +} + +class NestedArrayDimFetch +{ + /** + * @var array> + */ + public readonly array $nested; + + /** + * @param array> $nested + */ + public function __construct(array $nested) + { + $this->nested = $nested; + } + + public function doFoo(int $key1, int $key2): void + { + $this->nested[$key1][$key2]->brand[0] = 'test'; + } +} + +class NonReadonlyIsOk +{ + /** @var array */ + public array $items = []; + + public function doFoo(int $key): void + { + $this->items[$key]->brand[0] = 'test'; + } +} + +class ArrayAccessIsOk +{ + public readonly \ArrayObject $storage; + + public function __construct() + { + $this->storage = new \ArrayObject(); + } + + public function doFoo(): void + { + $this->storage[0]->brand[0] = 'test'; + } +} + +class IncrementThroughReadonlyArray +{ + /** @var array */ + public readonly array $items; + + /** + * @param array $items + */ + public function __construct(array $items) + { + $this->items = $items; + } + + public function doFoo(int $key): void + { + $this->items[$key]->brand[0] .= 'suffix'; + } +} + +class ReadonlyObjectPropertyIsOk +{ + public readonly VehicleType $vehicle; + + public function __construct(VehicleType $vehicle) + { + $this->vehicle = $vehicle; + } + + public function doFoo(): void + { + $this->vehicle->brand[0] = 'test'; + } +} + +class DeeperChain +{ + /** @var array */ + public readonly array $wrappers; + + /** + * @param array $wrappers + */ + public function __construct(array $wrappers) + { + $this->wrappers = $wrappers; + } + + public function doFoo(int $key): void + { + $this->wrappers[$key]->vehicle->brand[0] = 'test'; + } +} + +class Wrapper +{ + public VehicleType $vehicle; + + public function __construct(VehicleType $vehicle) + { + $this->vehicle = $vehicle; + } +} + +/** @readonly */ +class ReadonlyByPhpDocClass +{ + /** @var array */ + public array $items; + + /** + * @param array $items + */ + public function __construct(array $items) + { + $this->items = $items; + } + + public function doFoo(int $key): void + { + $this->items[$key]->brand[0] = 'test'; + } +} + +class ReadonlyByPhpDocProperty +{ + /** + * @readonly + * @var array + */ + public array $items; + + /** + * @param array $items + */ + public function __construct(array $items) + { + $this->items = $items; + } + + public function doFoo(int $key): void + { + $this->items[$key]->brand[0] = 'test'; + } +}