Merge pull request #986 from acoulton/bug-remove-protected-method-from-final-class

bug: `protected` method in final class -> `private` is not a BC break

Author: Ocramius

src/DetectChanges/BCBreak/ClassBased/MethodRemoved.php

@@ -44,9 +44,8 @@ public function __invoke(ReflectionClass $fromClass, ReflectionClass $toClass):
     /** @return array<string, ReflectionMethod> */
     private function accessibleMethods(ReflectionClass $class): array
     {
-        $methods = Vec\filter($class->getMethods(), function (ReflectionMethod $method): bool {
-            return ($method->isPublic()
-                || $method->isProtected())
+        $methods = Vec\filter($class->getMethods(), function (ReflectionMethod $method) use ($class): bool {
+            return $this->isAccessibleMethod($method, $class)
                 && ! $this->isInternalDocComment($method->getDocComment());
         });
 
@@ -58,6 +57,15 @@ private function accessibleMethods(ReflectionClass $class): array
         );
     }
 
+    private function isAccessibleMethod(ReflectionMethod $method, ReflectionClass $class): bool
+    {
+        if ($method->isPublic()) {
+            return true;
+        }
+
+        return $method->isProtected() && ! $class->isFinal();
+    }
+
     private function isInternalDocComment(string|null $comment): bool
     {
         return $comment !== null

test/unit/DetectChanges/BCBreak/ClassBased/MethodRemovedTest.php

@@ -13,6 +13,7 @@
 use Roave\BetterReflection\Reflection\ReflectionClass;
 use Roave\BetterReflection\Reflector\DefaultReflector;
 use Roave\BetterReflection\SourceLocator\Type\SingleFileSourceLocator;
+use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;
 
 use function array_map;
 use function iterator_to_array;
@@ -60,6 +61,48 @@ public static function classesToBeTested(): array
                     '[BC] REMOVED: Method RoaveTestAsset\ClassWithMethodsBeingRemoved#removedProtectedMethod() was removed',
                 ],
             ],
+            'final class with protected changing to private' => [
+                (new DefaultReflector(new StringSourceLocator(
+                    <<<'PHP'
+                    <?php
+                    final class FinalClassWithProtectedMethod {
+                        protected function foo(string $bar): void {}
+                    }
+                    PHP,
+                    $locator,
+                )))->reflectClass('FinalClassWithProtectedMethod'),
+                (new DefaultReflector(new StringSourceLocator(
+                    <<<'PHP'
+                    <?php
+                    final class FinalClassWithProtectedMethod {
+                        private function foo(string $bar): void {}
+                    }
+                    PHP,
+                    $locator,
+                )))->reflectClass('FinalClassWithProtectedMethod'),
+                [],
+            ],
+            'final class with public changing to private' => [
+                (new DefaultReflector(new StringSourceLocator(
+                    <<<'PHP'
+                    <?php
+                    final class FinalClassWithPublicMethod {
+                        public function foo(string $bar): void {}
+                    }
+                    PHP,
+                    $locator,
+                )))->reflectClass('FinalClassWithPublicMethod'),
+                (new DefaultReflector(new StringSourceLocator(
+                    <<<'PHP'
+                    <?php
+                    final class FinalClassWithPublicMethod {
+                        private function foo(string $bar): void {}
+                    }
+                    PHP,
+                    $locator,
+                )))->reflectClass('FinalClassWithPublicMethod'),
+                ['[BC] REMOVED: Method FinalClassWithPublicMethod#foo() was removed'],
+            ],
         ];
     }
 }