Merge pull request #253 from Roave/fix/#202-removed-protected-propert…

…ies-of-final-classes-are-not-bc-breaks Fix #202 - `PropertyRemoved` should only operate on `public` properties for `final` classes
Roave · Jun 22, 2020 · 15611fe · 15611fe
2 parents 8a3d4a3 + de333c4
commit 15611fe
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 2 deletions.
diff --git a/src/DetectChanges/BCBreak/ClassBased/PropertyRemoved.php b/src/DetectChanges/BCBreak/ClassBased/PropertyRemoved.php
@@ -45,9 +45,11 @@ public function __invoke(ReflectionClass $fromClass, ReflectionClass $toClass):
     /** @return ReflectionProperty[] */
     private function accessibleProperties(ReflectionClass $class): array
     {
-        return array_filter($class->getProperties(), function (ReflectionProperty $property): bool {
+        $classIsOpen = ! $class->isFinal();
+
+        return array_filter($class->getProperties(), function (ReflectionProperty $property) use ($classIsOpen): bool {
             return ($property->isPublic()
-                || $property->isProtected())
+                || ($classIsOpen && $property->isProtected()))
                 && ! $this->isInternalDocComment($property->getDocComment());
         });
     }

diff --git a/test/unit/DetectChanges/BCBreak/ClassBased/PropertyRemovedTest.php b/test/unit/DetectChanges/BCBreak/ClassBased/PropertyRemovedTest.php
@@ -11,6 +11,7 @@
 use Roave\BetterReflection\Reflection\ReflectionClass;
 use Roave\BetterReflection\Reflector\ClassReflector;
 use Roave\BetterReflection\SourceLocator\Type\SingleFileSourceLocator;
+use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;
 
 use function array_map;
 use function iterator_to_array;
@@ -64,6 +65,41 @@ public function classesToBeTested(): array
                     '[BC] REMOVED: Property RoaveTestAsset\ClassWithPropertiesBeingRemoved#$nameCaseChangeProtectedProperty was removed',
                 ],
             ],
+            'Decreased property visibility / removed properties in a final class - only `public` properties affect BC' => [
+                (new ClassReflector(new StringSourceLocator(
+                    <<<'PHP'
+<?php
+
+final class FinalClass
+{
+    public $decreasedVisibilityPublicProperty;
+    public $removedPublicProperty;
+    protected $decreasedVisibilityProtectedProperty;
+    protected $removedProtectedProperty;
+    private $privateProperty;
+}
+PHP
+                    ,
+                    $locator
+                )))->reflect('FinalClass'),
+                (new ClassReflector(new StringSourceLocator(
+                    <<<'PHP'
+<?php
+
+final class FinalClass
+{
+    protected $decreasedVisibilityPublicProperty;
+    private $decreasedVisibilityProtectedProperty;
+}
+PHP
+                    ,
+                    $locator
+                )))->reflect('FinalClass'),
+                [
+                    '[BC] REMOVED: Property FinalClass#$decreasedVisibilityPublicProperty was removed',
+                    '[BC] REMOVED: Property FinalClass#$removedPublicProperty was removed',
+                ],
+            ],
         ];
     }
 }