#4 detect immutable parent properties

svnldwg · Nov 5, 2020 · 3a695a7 · 3a695a7
1 parent 35cf80c
commit 3a695a7
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 8 deletions.
diff --git a/src/Helper/AnnotationParser.php b/src/Helper/AnnotationParser.php
@@ -22,7 +22,7 @@ public static function classHasAnnotation(array $annotations, array $nodes): boo
             return false;
         }
 
-        return self::isWhitelisted($classNode, $annotations);
+        return self::hasNodeImmutableAnnotation($classNode, $annotations);
     }
 
     /**
@@ -42,7 +42,7 @@ public static function propertiesWithWhitelistedAnnotations(array $annotations,
 
         $classProperties = NodeParser::getClassProperties($classNode);
         foreach ($classProperties as $property) {
-            $whitelisted = self::isWhitelisted($property, $annotations);
+            $whitelisted = self::hasNodeImmutableAnnotation($property, $annotations);
             if ($whitelisted) {
                 foreach ($property->props as $prop) {
                     $whitelistedProperties[] = (string)$prop->name;
@@ -79,7 +79,7 @@ public static function getAnnotations(Node $node): array
      *
      * @return bool
      */
-    private static function isWhitelisted(Node $node, array $whitelistAnnotations): bool
+    public static function hasNodeImmutableAnnotation(Node $node, array $whitelistAnnotations): bool
     {
         $nodeAnnotations = self::getAnnotations($node);
 

diff --git a/src/Rules/ImmutableObjectRule.php b/src/Rules/ImmutableObjectRule.php
@@ -59,6 +59,10 @@ public function processNode(Node $node, Scope $scope): array
             return [];
         }
 
+        if ($scope->getFunctionName() === '__construct') {
+            return [];
+        }
+
         $this->detectImmutableProperties($scope);
 
         $isImmutable = $this->isImmutable;
@@ -89,10 +93,6 @@ public function processNode(Node $node, Scope $scope): array
             return [];
         }
 
-        if ($scope->getFunctionName() === '__construct') {
-            return [];
-        }
-
         $propertyName = $node->var->name;
         if ($propertyName instanceof Node\Identifier) {
             $propertyName = (string)$propertyName;
@@ -154,9 +154,16 @@ private function getInheritedImmutableProperties(Scope $scope): array
                 $hasImmutableParent = true;
 
                 $immutableParentProperties += Converter::propertyStringNames(NodeParser::getNonPrivateProperties($classNode));
+
+                continue;
             }
 
-            // @TODO: detect non private parent properties annotated as immutable (instead of whole class)
+            $nonPrivateParentProperties = NodeParser::getNonPrivateProperties($classNode);
+            foreach ($nonPrivateParentProperties as $property) {
+                if (AnnotationParser::hasNodeImmutableAnnotation($property, self::WHITELISTED_ANNOTATIONS)) {
+                    $immutableParentProperties[] = Converter::propertyToString($property);
+                }
+            }
         }
 
         return ['properties' => $immutableParentProperties, 'hasImmutableParent' => $hasImmutableParent];

diff --git a/test/Fixture/ImmutableObjectRule/Failure/Inheritance/ImmutableParentProperty/ChildClass.php b/test/Fixture/ImmutableObjectRule/Failure/Inheritance/ImmutableParentProperty/ChildClass.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure\Inheritance\ImmutableParentProperty;
+
+class ChildClass extends ParentClass
+{
+    public function set(): void
+    {
+        $this->foo = 10; // declared immutable in parent class
+    }
+}
diff --git a/test/Fixture/ImmutableObjectRule/Failure/Inheritance/ImmutableParentProperty/ParentClass.php b/test/Fixture/ImmutableObjectRule/Failure/Inheritance/ImmutableParentProperty/ParentClass.php
@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Svnldwg\PHPStan\Test\Fixture\ImmutableObjectRule\Failure\Inheritance\ImmutableParentProperty;
+
+class ParentClass
+{
+    /** @psalm-immutable */
+    protected $foo;
+}
diff --git a/test/Integration/Rules/ImmutableObjectRuleTest.php b/test/Integration/Rules/ImmutableObjectRuleTest.php
@@ -109,6 +109,13 @@ public function provideCasesWhereAnalysisShouldFail(): iterable
                     9,
                 ],
             ],
+            'immutable-parent-property' => [
+                __DIR__ . '/../../Fixture/ImmutableObjectRule/Failure/Inheritance/ImmutableParentProperty/ChildClass.php',
+                [
+                    'Property is declared immutable, but class property "foo" is modified in method "set"',
+                    11,
+                ],
+            ],
         ];
 
         foreach ($paths as $description => [$path, $error]) {