#2 handles immutability with multiple inheritance, handles inherited immutability

Author: svnldwg

src/Helper/BackwardsIterator.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Svnldwg\PHPStan\Helper;
+
+class BackwardsIterator
+{
+    /**
+     * @param mixed[] $array
+     *
+     * @return \Generator<mixed>
+     */
+    public static function iterateBackwards(array $array): \Generator
+    {
+        for (end($array); key($array) !== null; prev($array)) {
+            yield current($array);
+        }
+    }
+}

src/Helper/Converter.php

@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Svnldwg\PHPStan\Helper;
+
+use PhpParser\Node;
+
+class Converter
+{
+    /**
+     * @param array<Node\Stmt\Property> $properties
+     *
+     * @return string[]
+     */
+    public static function propertyStringNames(array $properties)
+    {
+        return array_map(static function (Node\Stmt\Property $property): string {
+            return (string)reset($property->props)->name;
+        }, $properties);
+    }
+}

src/Rules/ImmutableObjectRule.php

@@ -11,6 +11,8 @@
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleErrorBuilder;
 use Svnldwg\PHPStan\Helper\AnnotationParser;
+use Svnldwg\PHPStan\Helper\BackwardsIterator;
+use Svnldwg\PHPStan\Helper\Converter;
 use Svnldwg\PHPStan\Helper\NodeParser;
 
 /**
@@ -46,21 +48,28 @@ public function processNode(Node $node, Scope $scope): array
             return [];
         }
 
-        $immutableProperties = $this->getParentClasses($scope);
+        [$immutableProperties, $hasImmutableParent] = $this->getInheritedImmutableProperties($scope);
 
         $nodes = $this->parser->parseFile($scope->getFile());
+        $hasImmutableClassAnnotation = AnnotationParser::classHasAnnotation(self::WHITELISTED_ANNOTATIONS, $nodes);
 
-        if (!AnnotationParser::classHasAnnotation(self::WHITELISTED_ANNOTATIONS, $nodes)) {
-            $immutableProperties = array_merge(
-                $immutableProperties,
-                AnnotationParser::propertiesWithWhitelistedAnnotations(self::WHITELISTED_ANNOTATIONS, $nodes)
-            );
-
-            if (empty($immutableProperties)) {
-                return [];
+        if (!empty($immutableProperties)) {
+            $classNode = NodeParser::getClassNode($nodes);
+            if ($classNode !== null) {
+                $classProperties = NodeParser::getClassProperties($classNode);
+                $classPropertyNames = Converter::propertyStringNames($classProperties);
+                $immutableProperties = array_merge($immutableProperties, $classPropertyNames);
             }
         }
 
+        if (empty($immutableProperties)) {
+            $immutableProperties = AnnotationParser::propertiesWithWhitelistedAnnotations(self::WHITELISTED_ANNOTATIONS, $nodes);
+        }
+
+        if (!$hasImmutableParent && !$hasImmutableClassAnnotation && empty($immutableProperties)) {
+            return [];
+        }
+
         while ($node->var instanceof Node\Expr\ArrayDimFetch) {
             $node = $node->var;
         }
@@ -112,17 +121,23 @@ public function processNode(Node $node, Scope $scope): array
         ];
     }
 
-    private function getParentClasses(Scope $scope): array
+    /**
+     * @param Scope $scope
+     *
+     * @return array<string[]>
+     */
+    private function getInheritedImmutableProperties(Scope $scope): array
     {
         if ($scope->getClassReflection() === null) {
             return [];
         }
 
         $immutableParentProperties = [];
 
-        // TODO: consider multiple layers of inheritance (parent 1 is not declared immutable, but parent 2 is, so properties of parent 1 need to inherit immutability
-
-        foreach ($scope->getClassReflection()->getParents() as $parent) {
+        $parents = $scope->getClassReflection()->getParents();
+        $parentsTopDown = BackwardsIterator::iterateBackwards($parents);
+        $hasImmutableParent = false;
+        foreach ($parentsTopDown as $parent) {
             $fileName = $parent->getFileName();
             if (!$fileName) {
                 continue;
@@ -134,15 +149,15 @@ private function getParentClasses(Scope $scope): array
                 continue;
             }
 
-            if (AnnotationParser::classHasAnnotation(self::WHITELISTED_ANNOTATIONS, $nodes)) {
-                $immutableParentProperties += array_map(static function (Node\Stmt\Property $property): string {
-                    return (string)reset($property->props)->name;
-                }, NodeParser::getNonPrivateProperties($classNode));
+            if ($hasImmutableParent || AnnotationParser::classHasAnnotation(self::WHITELISTED_ANNOTATIONS, $nodes)) {
+                $hasImmutableParent = true;
+
+                $immutableParentProperties += Converter::propertyStringNames(NodeParser::getNonPrivateProperties($classNode));
             }
 
-            // @TODO: detect non private parent properties annotated as immutable
+            // @TODO: detect non private parent properties annotated as immutable (instead of whole class)
         }
 
-        return $immutableParentProperties;
+        return [$immutableParentProperties, $hasImmutableParent];
     }
 }

test/Fixture/ImmutableObjectRule/Failure/MutationInChildClass.php renamed to test/Fixture/ImmutableObjectRule/Failure/Inheritance/Basic/MutationInChildClass.php

@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure;
+namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure\Inheritance\Basic;
 
 /**
  * @immutable

test/Fixture/ImmutableObjectRule/Failure/MutationInChildClassChild.php renamed to test/Fixture/ImmutableObjectRule/Failure/Inheritance/Basic/MutationInChildClassChild.php

@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure;
+namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure\Inheritance\Basic;
 
 class MutationInChildClassChild extends MutationInChildClass
 {

test/Fixture/ImmutableObjectRule/Failure/Inheritance/Deep/ChildClass.php

@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure\Inheritance\Deep;
+
+class ChildClass extends ParentClass
+{
+    public function mutate(): void
+    {
+        $this->parentValue = 10;
+    }
+}

test/Fixture/ImmutableObjectRule/Failure/Inheritance/Deep/GrandParentClass.php

@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure\Inheritance\Deep;
+
+/**
+ * @immutable
+ */
+class GrandParentClass
+{
+}

test/Fixture/ImmutableObjectRule/Failure/Inheritance/Deep/ParentClass.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure\Inheritance\Deep;
+
+class ParentClass extends GrandParentClass
+{
+    protected $parentValue;
+}

test/Fixture/ImmutableObjectRule/Failure/Inheritance/MutateChildPropertyWithImmutableParent/ChildPropertyMutation.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure\Inheritance\MutateChildPropertyWithImmutableParent;
+
+class ChildPropertyMutation extends ImmutableParent
+{
+    private $childProperty;
+
+    public function setChildProperty()
+    {
+        $this->childProperty = true;
+    }
+}

test/Fixture/ImmutableObjectRule/Failure/Inheritance/MutateChildPropertyWithImmutableParent/ChildPropertyMutation2.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure\Inheritance\MutateChildPropertyWithImmutableParent;
+
+class ChildPropertyMutation2 extends ImmutableParentWithProperty
+{
+    private $immu;
+
+    public function set()
+    {
+        $this->immu = 'mutated';
+    }
+}