Merge pull request #1472 from kukulich/asymetric-visibility

Asymmetric visibility fixes around implicit `public`, `private` and `protected` visibility
Roave · Dec 30, 2024 · 54c3dfb · 54c3dfb
2 parents 3aaa022 + b3dc8b1
commit 54c3dfb
Show file tree

Hide file tree

Showing 8 changed files with 144 additions and 38 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -213,10 +213,14 @@
       <code><![CDATA[isAbstract]]></code>
       <code><![CDATA[isFinal]]></code>
       <code><![CDATA[isPrivate]]></code>
+      <code><![CDATA[isPrivate]]></code>
       <code><![CDATA[isPrivateSet]]></code>
       <code><![CDATA[isProtected]]></code>
+      <code><![CDATA[isProtected]]></code>
       <code><![CDATA[isProtectedSet]]></code>
       <code><![CDATA[isPublic]]></code>
+      <code><![CDATA[isPublic]]></code>
+      <code><![CDATA[isPublicSet]]></code>
       <code><![CDATA[isReadonly]]></code>
       <code><![CDATA[isStatic]]></code>
       <code><![CDATA[traitExists]]></code>

diff --git a/src/Reflection/ReflectionProperty.php b/src/Reflection/ReflectionProperty.php
@@ -666,16 +666,32 @@ private function assertObject(mixed $object): object
     /** @return int-mask-of<ReflectionPropertyAdapter::IS_*> */
     private function computeModifiers(PropertyNode $node): int
     {
-        $modifiers  = $node->isReadonly() ? ReflectionPropertyAdapter::IS_READONLY : 0;
+        $modifiers  = $node->isReadonly() ? CoreReflectionProperty::IS_READONLY : 0;
         $modifiers += $node->isStatic() ? CoreReflectionProperty::IS_STATIC : 0;
         $modifiers += $node->isPrivate() ? CoreReflectionProperty::IS_PRIVATE : 0;
-        $modifiers += $node->isPrivateSet() ? ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY : 0;
+        $modifiers += ! $node->isPrivate() && $node->isPrivateSet() ? ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY : 0;
         $modifiers += $node->isProtected() ? CoreReflectionProperty::IS_PROTECTED : 0;
-        $modifiers += $node->isProtectedSet() ? ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY : 0;
+        $modifiers += ! $node->isProtected() && $node->isProtectedSet() ? ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY : 0;
         $modifiers += $node->isPublic() ? CoreReflectionProperty::IS_PUBLIC : 0;
         $modifiers += $node->isFinal() ? ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY : 0;
         $modifiers += $node->isAbstract() ? ReflectionPropertyAdapter::IS_ABSTRACT_COMPATIBILITY : 0;
 
+        if (
+            ! ($modifiers & ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY)
+            && ($modifiers & ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY)
+        ) {
+            $modifiers += ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY;
+        }
+
+        if (
+            ! ($modifiers & (ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY))
+            && ! $node->isPublicSet()
+            && $node->isPublic()
+            && ($modifiers & CoreReflectionProperty::IS_READONLY)
+        ) {
+            $modifiers += ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY;
+        }
+
         /** @phpstan-ignore return.type */
         return $modifiers;
     }

diff --git a/...unit/Fixture/AsymetricVisibilityClass.php → ...nit/Fixture/AsymmetricVisibilityClass.php b/...unit/Fixture/AsymetricVisibilityClass.php → ...nit/Fixture/AsymmetricVisibilityClass.php
@@ -2,7 +2,7 @@
 
 namespace Roave\BetterReflectionTest\Fixture;
 
-class AsymetricVisibilityClass
+class AsymmetricVisibilityClass
 {
     public public(set) string $publicPublicSet = 'string';
     public protected(set) string $publicProtectedSet = 'string';
@@ -22,4 +22,3 @@ public function __construct(
     {
     }
 }
-
diff --git a/...ixture/AsymetricVisibilityClassExport.txt → ...xture/AsymmetricVisibilityClassExport.txt b/...ixture/AsymetricVisibilityClassExport.txt → ...xture/AsymmetricVisibilityClassExport.txt
@@ -1,5 +1,5 @@
-Class [ <user> class Roave\BetterReflectionTest\Fixture\AsymetricVisibilityClass ] {
-  @@ %s/Fixture/AsymetricVisibilityClass.php 5-24
+Class [ <user> class Roave\BetterReflectionTest\Fixture\AsymmetricVisibilityClass ] {
+  @@ %s/Fixture/AsymmetricVisibilityClass.php 5-24
 
   - Constants [0] {
   }
@@ -27,7 +27,7 @@ Class [ <user> class Roave\BetterReflectionTest\Fixture\AsymetricVisibilityClass
 
   - Methods [1] {
     Method [ <user, ctor> public method __construct ] {
-      @@ %s/Fixture/AsymetricVisibilityClass.php 14 - 23
+      @@ %s/Fixture/AsymmetricVisibilityClass.php 14 - 23
 
       - Parameters [6] {
         Parameter #0 [ <required> string $promotedPublicPublicSet ]

diff --git a/test/unit/Fixture/AsymmetricVisibilityImplicitFinal.php b/test/unit/Fixture/AsymmetricVisibilityImplicitFinal.php
@@ -0,0 +1,10 @@
+<?php
+
+namespace Roave\BetterReflectionTest\Fixture;
+
+class AsymmetricVisibilityImplicitFinal
+{
+    public private(set) bool $publicPrivateSetIsFinal = true;
+    protected private(set) bool $protectedPrivateSetIsFinal = true;
+    private private(set) bool $privatePrivateSetIsNotFinal = true;
+}
diff --git a/test/unit/Fixture/AsymmetricVisibilityImplicitProtectedSet.php b/test/unit/Fixture/AsymmetricVisibilityImplicitProtectedSet.php
@@ -0,0 +1,15 @@
+<?php
+
+namespace Roave\BetterReflectionTest\Fixture;
+
+class AsymmetricVisibilityImplicitProtectedSet
+{
+    public public(set) readonly bool $publicPublicSet;
+    public protected(set) readonly bool $publicProtectedSet;
+    public private(set) readonly bool $publicPrivateSet;
+
+    protected readonly bool $protected;
+    private readonly bool $private;
+
+    public readonly bool $publicImplicitProtectedSet;
+}
diff --git a/test/unit/Reflection/ReflectionClassTest.php b/test/unit/Reflection/ReflectionClassTest.php
@@ -761,28 +761,28 @@ public function testGetPropertiesWithFilter(int $filter, int $count): void
     }
 
     /** @return list<array{0: int-mask-of<ReflectionPropertyAdapter::IS_*>, 1: int}> */
-    public static function getPropertiesWithAsymetricVisibilityFilterDataProvider(): array
+    public static function getPropertiesWithAsymmetricVisibilityFilterDataProvider(): array
     {
         return [
             [CoreReflectionProperty::IS_PUBLIC, 6],
             [CoreReflectionProperty::IS_PROTECTED, 4],
             [CoreReflectionProperty::IS_PRIVATE, 2],
-            [ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY, 4],
-            [ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY, 6],
+            [ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY, 2],
+            [ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY, 4],
             [
                 CoreReflectionProperty::IS_PUBLIC |
                 ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY,
-                10,
+                8,
             ],
         ];
     }
 
     /** @param int-mask-of<CoreReflectionProperty::IS_*> $filter */
-    #[DataProvider('getPropertiesWithAsymetricVisibilityFilterDataProvider')]
-    public function testGetPropertiesWithAsymetricVisibilityFilter(int $filter, int $count): void
+    #[DataProvider('getPropertiesWithAsymmetricVisibilityFilterDataProvider')]
+    public function testGetPropertiesWithAsymmetricVisibilityFilter(int $filter, int $count): void
     {
-        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymetricVisibilityClass.php', $this->astLocator));
-        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymetricVisibilityClass');
+        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymmetricVisibilityClass.php', $this->astLocator));
+        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymmetricVisibilityClass');
 
         self::assertCount($count, $classInfo->getProperties($filter));
         self::assertCount($count, $classInfo->getImmediateProperties($filter));
@@ -2308,13 +2308,13 @@ public function testToString(): void
         );
     }
 
-    public function testToStringWithAsymetricVisibility(): void
+    public function testToStringWithAsymmetricVisibility(): void
     {
-        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymetricVisibilityClass.php', $this->astLocator));
-        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymetricVisibilityClass');
+        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymmetricVisibilityClass.php', $this->astLocator));
+        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymmetricVisibilityClass');
 
         self::assertStringMatchesFormat(
-            file_get_contents(__DIR__ . '/../Fixture/AsymetricVisibilityClassExport.txt'),
+            file_get_contents(__DIR__ . '/../Fixture/AsymmetricVisibilityClassExport.txt'),
             $classInfo->__toString(),
         );
     }

diff --git a/test/unit/Reflection/ReflectionPropertyTest.php b/test/unit/Reflection/ReflectionPropertyTest.php
@@ -148,6 +148,24 @@ public function testVisibilityMethods(): void
         self::assertFalse($onlyPublicProp->isReadOnly());
     }
 
+    public function isImplicitPublic(): void
+    {
+        $php = <<<'PHP'
+            <?php
+
+            class Foo
+            {
+                $boo = 'boo';
+            }
+        PHP;
+
+        $classReflection    = (new DefaultReflector(new StringSourceLocator($php, $this->astLocator)))->reflectClass('Foo');
+        $propertyReflection = $classReflection->getProperty('boo');
+
+        self::assertTrue($propertyReflection->isPublic());
+        self::assertSame(CoreReflectionProperty::IS_PUBLIC, $propertyReflection->getModifiers());
+    }
+
     public function testIsStatic(): void
     {
         $classInfo = $this->reflector->reflectClass(ExampleClass::class);
@@ -254,7 +272,7 @@ public static function modifierProvider(): array
             ['protectedProperty', CoreReflectionProperty::IS_PROTECTED],
             ['privateProperty', CoreReflectionProperty::IS_PRIVATE],
             ['publicStaticProperty', CoreReflectionProperty::IS_PUBLIC | CoreReflectionProperty::IS_STATIC],
-            ['readOnlyProperty', CoreReflectionProperty::IS_PUBLIC | ReflectionPropertyAdapter::IS_READONLY],
+            ['readOnlyProperty', CoreReflectionProperty::IS_PUBLIC | ReflectionPropertyAdapter::IS_READONLY | ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY],
             ['finalPublicProperty', CoreReflectionProperty::IS_PUBLIC | ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY],
         ];
     }
@@ -897,39 +915,39 @@ public function testWithImplementingClass(): void
     }
 
     /** @return list<array{0: non-empty-string, 1: int-mask-of<ReflectionPropertyAdapter::IS_*>}> */
-    public static function asymetricVisibilityModifierProvider(): array
+    public static function asymmetricVisibilityModifierProvider(): array
     {
         return [
             ['publicPublicSet', CoreReflectionProperty::IS_PUBLIC],
             ['publicProtectedSet', CoreReflectionProperty::IS_PUBLIC | ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY],
-            ['publicPrivateSet', CoreReflectionProperty::IS_PUBLIC | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY],
-            ['protectedProtectedSet', CoreReflectionProperty::IS_PROTECTED | ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY],
-            ['protectedPrivateSet', CoreReflectionProperty::IS_PROTECTED | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY],
-            ['privatePrivateSet', CoreReflectionProperty::IS_PRIVATE | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY],
+            ['publicPrivateSet', CoreReflectionProperty::IS_PUBLIC | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY | ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY],
+            ['protectedProtectedSet', CoreReflectionProperty::IS_PROTECTED],
+            ['protectedPrivateSet', CoreReflectionProperty::IS_PROTECTED | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY | ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY],
+            ['privatePrivateSet', CoreReflectionProperty::IS_PRIVATE],
             ['promotedPublicPublicSet', CoreReflectionProperty::IS_PUBLIC],
             ['promotedPublicProtectedSet', CoreReflectionProperty::IS_PUBLIC | ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY],
-            ['promotedPublicPrivateSet', CoreReflectionProperty::IS_PUBLIC | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY],
-            ['promotedProtectedProtectedSet', CoreReflectionProperty::IS_PROTECTED | ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY],
-            ['promotedProtectedPrivateSet', CoreReflectionProperty::IS_PROTECTED | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY],
-            ['promotedPrivatePrivateSet', CoreReflectionProperty::IS_PRIVATE | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY],
+            ['promotedPublicPrivateSet', CoreReflectionProperty::IS_PUBLIC | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY | ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY],
+            ['promotedProtectedProtectedSet', CoreReflectionProperty::IS_PROTECTED],
+            ['promotedProtectedPrivateSet', CoreReflectionProperty::IS_PROTECTED | ReflectionPropertyAdapter::IS_PRIVATE_SET_COMPATIBILITY | ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY],
+            ['promotedPrivatePrivateSet', CoreReflectionProperty::IS_PRIVATE],
         ];
     }
 
     /** @param non-empty-string $propertyName */
-    #[DataProvider('asymetricVisibilityModifierProvider')]
-    public function testGetAsymetricVisibilityMethods(string $propertyName, int $expectedModifier): void
+    #[DataProvider('asymmetricVisibilityModifierProvider')]
+    public function testGetAsymmetricVisibilityMethods(string $propertyName, int $expectedModifier): void
     {
-        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymetricVisibilityClass.php', $this->astLocator));
-        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymetricVisibilityClass');
+        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymmetricVisibilityClass.php', $this->astLocator));
+        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymmetricVisibilityClass');
         $property  = $classInfo->getProperty($propertyName);
 
         self::assertSame($expectedModifier, $property->getModifiers());
     }
 
     public function testIsProtectedSet(): void
     {
-        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymetricVisibilityClass.php', $this->astLocator));
-        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymetricVisibilityClass');
+        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymmetricVisibilityClass.php', $this->astLocator));
+        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymmetricVisibilityClass');
 
         $publicPublicSetProperty    = $classInfo->getProperty('publicPublicSet');
         $publicProtectedSetProperty = $classInfo->getProperty('publicProtectedSet');
@@ -940,8 +958,8 @@ public function testIsProtectedSet(): void
 
     public function testIsPrivateSet(): void
     {
-        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymetricVisibilityClass.php', $this->astLocator));
-        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymetricVisibilityClass');
+        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymmetricVisibilityClass.php', $this->astLocator));
+        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymmetricVisibilityClass');
 
         $protectedProtectedSet = $classInfo->getProperty('protectedProtectedSet');
         $protectedPrivateSet   = $classInfo->getProperty('protectedPrivateSet');
@@ -950,6 +968,50 @@ public function testIsPrivateSet(): void
         self::assertTrue($protectedPrivateSet->isPrivateSet());
     }
 
+    /** @return list<array{0: non-empty-string, 1: bool}> */
+    public static function asymmetricVisibilityImplicitFinalProvider(): array
+    {
+        return [
+            ['publicPrivateSetIsFinal', true],
+            ['protectedPrivateSetIsFinal', true],
+            ['privatePrivateSetIsNotFinal', false],
+        ];
+    }
+
+    /** @param non-empty-string $propertyName */
+    #[DataProvider('asymmetricVisibilityImplicitFinalProvider')]
+    public function testAsymmetricVisibilityImplicitFinal(string $propertyName, bool $isFinal): void
+    {
+        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymmetricVisibilityImplicitFinal.php', $this->astLocator));
+        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymmetricVisibilityImplicitFinal');
+        $property  = $classInfo->getProperty($propertyName);
+
+        self::assertSame($isFinal, $property->isFinal());
+    }
+
+    /** @return list<array{0: non-empty-string, 1: bool}> */
+    public static function asymmetricVisibilityImplicitProtectedSetProvider(): array
+    {
+        return [
+            ['publicPublicSet', false],
+            ['publicProtectedSet', true],
+            ['publicPrivateSet', false],
+            ['protected', false],
+            ['publicImplicitProtectedSet', true],
+        ];
+    }
+
+    /** @param non-empty-string $propertyName */
+    #[DataProvider('asymmetricVisibilityImplicitProtectedSetProvider')]
+    public function testAsymmetricVisibilityImplicitProtectedSet(string $propertyName, bool $isProtectedSet): void
+    {
+        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/AsymmetricVisibilityImplicitProtectedSet.php', $this->astLocator));
+        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\AsymmetricVisibilityImplicitProtectedSet');
+        $property  = $classInfo->getProperty($propertyName);
+
+        self::assertSame($isProtectedSet, $property->isProtectedSet());
+    }
+
     public function testIsAbstract(): void
     {
         $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/PropertyHooks.php', $this->astLocator));