Asymmetric visibility Reflection API problems

Hi, internals! Recently I've started testing the new features of the PHP 8.4 Reflection API and found some of them unintuitive. At first, I reported this as a bug [1], but then Ilija explained that the behavior is intentional. So I've decided to discuss the issue publicly. We are going to talk about the asymmetric visibility RFC [2] and the new ReflectionProperty::is(Private|Protected)Set() methods. Consider a class and reflected facts about it's properties [3]: ```php class A { // isPrivateSet() = true (OK) public private(set) mixed $public_private_set; // isPrivateSet() = false (I would expect true) private private(set) mixed $private_private_set; // isPrivateSet() = false (I would expect true) private mixed $private; // isProtectedSet() = true (OK, explained in the RFC) public readonly mixed $public_readonly; // isProtectedSet() = false (I would expect true) protected readonly mixed $protected_readonly; // isProtectedSet() = false (I would expect true) protected protected(set) readonly mixed $protected_protected_set_readonly; // isPrivateSet() = false (OK), isProtectedSet() = false (OK) public bool $virtual_no_set_hook { get => true; } } ``` Here's why it works this way. Internally properties with symmetric visibility do not have an asymmetric flag. That is why private private(set) and private both have `isPrivateSet() = false. Readonly properties without explicit set` imply protected(set) [4] and thus they are symmetric when protected and asymmetric otherwise. I personally don't agree that Reflection API should stick to the internal implementation details. On the contrary, it should hide them and make it easier to study the facts about the code. Alright. Let's keep that in mind and jump into another issue. I've suddenly realised, that the new Reflection API breaks backward compatibility. In PHP <8.4 you can check `$reflectionProperty->isPublic() && !$reflectionProperty->isReadonly()` and then safely write to that property from global scope. Now this is not the case anymore: if a public property has a private set(), a simple Hydrator working on these assumptions will break in 8.4 and require refactoring. The reason for this is that before 8.4 ReflectionProperty::isPublic() = true guaranteed that the property has public get and set, now it only guarantees that property has a public get. Here's what I propose: - ReflectionProperty::isPublic() returns true only if the property is symmetrically public. For public readonly $prop it will return false, because it's implicitly `public protected(set) readonly $prop`. This is a BC break, but a smaller one, given that libraries now take "readonly" into account. - ReflectionProperty::isProtected() returns true only if the property is symmetrically protected. For protected readonly $prop it will return `true, because it's implicitly protected protected(set) readonly $prop`. Fully backward compatible. - ReflectionProperty::isPrivate() returns true only if the property is symmetrically private. Fully backward compatible. - Add ReflectionProperty::isPublicGet(): returns true if property is public or public XXX(set). - Add ReflectionProperty::isProtectedGet(): returns true if property is protected or protected XXX(set). - Add ReflectionProperty::isPrivateGet(): returns true if property is private or private private(set). - Add ReflectionProperty::isPublicSet(): returns true if property is symmetrically public (asymmetric public set is not possible). - Change ReflectionProperty::isProtectedSet(): returns true if property is symmetrically protected or has an asymmetric protected set. - Change ReflectionProperty::isPrivate(): returns true if property is symmetrically private or has an asymmetric private set. ```php class A { // isPublic() = false, isPrivate() = false, isPublicGet() = true, isPrivateSet() = true public private(set) mixed $public_private_set; // isPrivate() = true, isPrivateGet() = true, isPrivateSet() = true private private(set) mixed $private_private_set; // isPrivate() = true, isPrivateGet() = true, isPrivateSet() = true private mixed $private; // isPublic() = false, isProtected() = false, isPublicGet() = true, isProtectedSet() = true public readonly mixed $public_readonly; // isProtected() = true, isProtectedGet() = true, isProtectedSet() = true protected readonly mixed $protected_readonly; // isProtected() = true, isProtectedGet() = true, isProtectedSet() = true protected protected(set) readonly mixed $protected_protected_set_readonly; // isPublic() = false, isPublicGet() = true, isPublicSet() = false, isProtectedSet() = false, isPrivateSet() = false public bool $virtual_no_set_hook { get => true; } } ``` The most difficult question is whether we can have these or similar changes in 8.4 after RC1... Does a BC break in the current implementation (if we agree that it exists) give us such an option? Also, Ilija has a brilliant idea to add isReadable($scope): bool, isWritable($scope): bool methods [5], but this can be done in PHP 9. 1. https://github.com/php/php-src/issues/16175 2. https://wiki.php.net/rfc/asymmetric-visibility-v2 3. https://3v4l.org/O3FNN/rfc#vgit.master 4. https://wiki.php.net/rfc/asymmetric-visibility-v2#relationship_with_readonly 5. https://github.com/php/php-src/issues/16175#issuecomment-2389966021 -- Best regards, Valentin