Make ReflectionProperty and ReflectionMethod access private/protected symbols by default

With this patch, it is no longer required to call `ReflectionProperty#setAccessible()` or
`ReflectionMethod#setAccessible()` with `true`.

If a userland consumer already got to the point of accessing object/class information via reflection,
it makes little sense for `ext/reflection` to disallow accessing `private`/`protected` symbols by
default.

After this patch, calling `ReflectionProperty#setAccessible(true)` or
`ReflectionMethod#setAccessible(true)` on newly instantiated `ReflectionProperty` or
`ReflectionMethod` respectively will have no effect.

Full RFC text for the voted upon https://wiki.php.net/rfc/make-reflection-setaccessible-no-op is as follows:

```
====== PHP RFC: Make reflection setAccessible() no-op ======

  * Version: 1.0
  * Date: 2021-06-13
  * Author: Marco Pivetta
  * Status: Accepted
  * First Published at: https://wiki.php.net/rfc/make-reflection-setaccessible-no-op

===== Introduction =====

The `ext-reflection` API is designed to inspect static details of a code-base, as well as reading and manipulating runtime state and calling internal details of objects that are otherwise inaccessible.

These methods are most notably:

  * `ReflectionMethod#invoke(): mixed`
  * `ReflectionMethod#invokeArgs(mixed ...$args): mixed`
  * `ReflectionProperty#getValue(object $object): mixed`
  * `ReflectionProperty#setValue(object $object, mixed $value): void`

While breaking encapsulation principles that allow for safe coding practices, these methods are extremely valuable to tools like:

  * mappers
  * serializers
  * debuggers
  * etc.

Infrastructural instrumentation is often required to do things that are in direct conflict with encapsulation itself.

The 4 methods listed above change behavior depending on the only mutable state within the scope of `ext-reflection` classes, which is an "accessible" flag. This "accessibility" flag is steered by:

  * `ReflectionMethod#setAccessible(bool $accessible): void`
  * `ReflectionProperty#setAccessible(bool $accessible): void`

Attempting to use any of the above listed methods without configuring accessibility first will lead to an exception being thrown. For example:

<code php>
class Foo { private $bar = 'a'; }

(new ReflectionProperty(Foo::class, 'bar'))->getValue();
</code>

https://3v4l.org/ousrD :

<code>
Fatal error: Uncaught ReflectionException: Cannot access non-public property Foo::$bar in <SNIP>
</code>

==== The problem with mutability ====

By having `ReflectionProperty#setAccessible()` and `ReflectionMethod#setAccessible()`, any consumer of a `ReflectionMethod` or `ReflectionProperty` that is given by a third party must ensure that `#setAccessible()` is called:

<code php>
function doSomethingWithState(MyObject $o, ReflectionProperty $p) : void
{
    $p->setAccessible(true); // wasteful safety check

    doSomethingWith($p->getValue($o));
}
</code>

In addition to that, any developer that is intentionally using the reflection API (after having evaluated its trade-off) will have to use this obnoxious syntax in order to use it at its fullest:

<code php>
$p = new ReflectionProperty(MyClass::class, 'propertyName');

$p->setAccessible(true);

// now $p is usable
</code>

===== Proposal =====

This RFC proposes to:

  * make `ReflectionProperty` and `ReflectionMethod` behave as if `#setAccessible(true)` had been called upfront
  * make `ReflectionProperty#setAccessible()` and `ReflectionMethod#setAccessible()` no-op operations, with no side-effects nor state mutation involved

After the RFC is successfully accepted/implemented, the following code should no longer throw, improving therefore the ergonomics around reflection.

<code php>
class Foo { private $bar = 'a'; }

(new ReflectionProperty(Foo::class, 'bar'))->getValue();
</code>

==== Deprecations ====

In order to ease migration to PHP 8.1, and minimize runtime side-effects, a deprecation is explicitly avoided in this RFC.

Instead, a deprecation should be introduced when a new/separate RFC plans for the removal of `ReflectionProperty#setAccessible()` and `ReflectionMethod#setAccessible()`.

Such RFC will be raised **after** the release of PHP 8.1, if this RFC is accepted.

===== Backward Incompatible Changes =====

Although of minimal concern, it is true that some behavior will change:

  * `ReflectionProperty#getValue()` will no longer throw an exception when used against a protected/private property
  * `ReflectionProperty#setValue()` will no longer throw an exception when used against a protected/private property
  * `ReflectionMethod#invoke()` will no longer throw an exception when used against a protected/private method
  * `ReflectionMethod#invokeArgs()` will no longer throw an exception when used against a protected/private method
  * for extensions developers, `reflection_object->ignore_visibility` no longer exists

===== Proposed PHP Version(s) =====

8.1

===== RFC Impact =====
==== To SAPIs ====
None

==== To Existing Extensions ====
None

==== To Opcache ====

None

==== New Constants ====

None

==== php.ini Defaults ====

None

===== Open Issues =====

None

===== Proposed Voting Choices =====

Accept turning `ReflectionProperty#setAccessible()` and `ReflectionMethod#setAccessible()` into a no-op? (yes/no)

===== Patches and Tests =====

#5412

===== Vote =====

This is a Yes/No vote, requiring a 2/3 majority. Voting started on 2021-06-23 and ends on 2021-07-07.

<doodle title="Make reflection setAccessible() no-op" auth="ocramius" voteType="single" closed="true">
   * Yes
   * No
</doodle>
```

Author: Ocramius

Zend/tests/bug63762.phpt

@@ -5,7 +5,6 @@ Bug #63762 - Sigsegv when Exception::$trace is changed by user
 $e = new Exception();
 
 $ref = new ReflectionProperty($e, 'trace');
-$ref->setAccessible(TRUE);
 
 echo "Array of NULL:\n";
 $ref->setValue($e, array(NULL));

Zend/tests/bug72177.phpt

@@ -21,7 +21,6 @@ class Parnt
         $this->child = new Child();
 
         $prop = new \ReflectionProperty($this, 'child');
-        $prop->setAccessible(true);
         $prop->setValue($this, null);
     }
 }

Zend/tests/bug72177_2.phpt

@@ -27,7 +27,6 @@ class Bar extends Foo
 
 $r = new ReflectionProperty(Foo::class, 'bar');
 
-$r->setAccessible(true);
 echo "OK\n";
 ?>
 --EXPECT--

Zend/tests/bug78921.phpt

@@ -27,7 +27,6 @@ class OtherClass
 
 $reflectionClass = new ReflectionClass('OtherClass');
 $reflectionProperty = $reflectionClass->getProperty('prop');
-$reflectionProperty->setAccessible(true);
 $value = $reflectionProperty->getValue();
 echo "Value is $value\n";
 

ext/reflection/php_reflection.c

@@ -171,7 +171,6 @@ typedef struct {
 	void *ptr;
 	zend_class_entry *ce;
 	reflection_type_t ref_type;
-	unsigned int ignore_visibility:1;
 	zend_object zo;
 } reflection_object;
 
@@ -1440,7 +1439,6 @@ static void reflection_property_factory(zend_class_entry *ce, zend_string *name,
 	intern->ptr = reference;
 	intern->ref_type = REF_TYPE_PROPERTY;
 	intern->ce = ce;
-	intern->ignore_visibility = 0;
 	ZVAL_STR_COPY(reflection_prop_name(object), name);
 	ZVAL_STR_COPY(reflection_prop_class(object), prop ? prop->ce->name : ce->name);
 }
@@ -1463,7 +1461,6 @@ static void reflection_class_constant_factory(zend_string *name_str, zend_class_
 	intern->ptr = constant;
 	intern->ref_type = REF_TYPE_CLASS_CONSTANT;
 	intern->ce = constant->ce;
-	intern->ignore_visibility = 0;
 
 	ZVAL_STR_COPY(reflection_prop_name(object), name_str);
 	ZVAL_STR_COPY(reflection_prop_class(object), constant->ce->name);
@@ -1482,7 +1479,6 @@ static void reflection_enum_case_factory(zend_class_entry *ce, zend_string *name
 	intern->ptr = constant;
 	intern->ref_type = REF_TYPE_CLASS_CONSTANT;
 	intern->ce = constant->ce;
-	intern->ignore_visibility = 0;
 
 	ZVAL_STR_COPY(reflection_prop_name(object), name_str);
 	ZVAL_STR_COPY(reflection_prop_class(object), constant->ce->name);
@@ -3313,15 +3309,6 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic)
 		RETURN_THROWS();
 	}
 
-	if (!(mptr->common.fn_flags & ZEND_ACC_PUBLIC) && intern->ignore_visibility == 0) {
-		zend_throw_exception_ex(reflection_exception_ptr, 0,
-			"Trying to invoke %s method %s::%s() from scope %s",
-			mptr->common.fn_flags & ZEND_ACC_PROTECTED ? "protected" : "private",
-			ZSTR_VAL(mptr->common.scope->name), ZSTR_VAL(mptr->common.function_name),
-			ZSTR_VAL(Z_OBJCE_P(ZEND_THIS)->name));
-		RETURN_THROWS();
-	}
-
 	if (variadic) {
 		ZEND_PARSE_PARAMETERS_START(1, -1)
 			Z_PARAM_OBJECT_OR_NULL(object)
@@ -3696,16 +3683,11 @@ ZEND_METHOD(ReflectionMethod, getPrototype)
 /* {{{ Sets whether non-public methods can be invoked */
 ZEND_METHOD(ReflectionMethod, setAccessible)
 {
-	reflection_object *intern;
 	bool visible;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "b", &visible) == FAILURE) {
 		RETURN_THROWS();
 	}
-
-	intern = Z_REFLECTION_P(ZEND_THIS);
-
-	intern->ignore_visibility = visible;
 }
 /* }}} */
 
@@ -3745,7 +3727,6 @@ ZEND_METHOD(ReflectionClassConstant, __construct)
 	intern->ptr = constant;
 	intern->ref_type = REF_TYPE_CLASS_CONSTANT;
 	intern->ce = constant->ce;
-	intern->ignore_visibility = 0;
 	ZVAL_STR_COPY(reflection_prop_name(object), constname);
 	ZVAL_STR_COPY(reflection_prop_class(object), constant->ce->name);
 }
@@ -5453,7 +5434,6 @@ ZEND_METHOD(ReflectionProperty, __construct)
 	intern->ptr = reference;
 	intern->ref_type = REF_TYPE_PROPERTY;
 	intern->ce = ce;
-	intern->ignore_visibility = 0;
 }
 /* }}} */
 
@@ -5580,13 +5560,6 @@ ZEND_METHOD(ReflectionProperty, getValue)
 
 	GET_REFLECTION_OBJECT_PTR(ref);
 
-	if (!(prop_get_flags(ref) & ZEND_ACC_PUBLIC) && intern->ignore_visibility == 0) {
-		zend_throw_exception_ex(reflection_exception_ptr, 0,
-			"Cannot access non-public property %s::$%s",
-			ZSTR_VAL(intern->ce->name), ZSTR_VAL(ref->unmangled_name));
-		RETURN_THROWS();
-	}
-
 	if (prop_get_flags(ref) & ZEND_ACC_STATIC) {
 		member_p = zend_read_static_property_ex(intern->ce, ref->unmangled_name, 0);
 		if (member_p) {
@@ -5630,13 +5603,6 @@ ZEND_METHOD(ReflectionProperty, setValue)
 
 	GET_REFLECTION_OBJECT_PTR(ref);
 
-	if (!(prop_get_flags(ref) & ZEND_ACC_PUBLIC) && intern->ignore_visibility == 0) {
-		zend_throw_exception_ex(reflection_exception_ptr, 0,
-			"Cannot access non-public property %s::$%s",
-			ZSTR_VAL(intern->ce->name), ZSTR_VAL(ref->unmangled_name));
-		RETURN_THROWS();
-	}
-
 	if (prop_get_flags(ref) & ZEND_ACC_STATIC) {
 		if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "z", &value) == FAILURE) {
 			if (zend_parse_parameters(ZEND_NUM_ARGS(), "zz", &tmp, &value) == FAILURE) {
@@ -5669,13 +5635,6 @@ ZEND_METHOD(ReflectionProperty, isInitialized)
 
 	GET_REFLECTION_OBJECT_PTR(ref);
 
-	if (!(prop_get_flags(ref) & ZEND_ACC_PUBLIC) && intern->ignore_visibility == 0) {
-		zend_throw_exception_ex(reflection_exception_ptr, 0,
-			"Cannot access non-public property %s::$%s",
-			ZSTR_VAL(intern->ce->name), ZSTR_VAL(ref->unmangled_name));
-		RETURN_THROWS();
-	}
-
 	if (prop_get_flags(ref) & ZEND_ACC_STATIC) {
 		member_p = zend_read_static_property_ex(intern->ce, ref->unmangled_name, 1);
 		if (member_p) {
@@ -5762,16 +5721,11 @@ ZEND_METHOD(ReflectionProperty, getAttributes)
 /* {{{ Sets whether non-public properties can be requested */
 ZEND_METHOD(ReflectionProperty, setAccessible)
 {
-	reflection_object *intern;
 	bool visible;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "b", &visible) == FAILURE) {
 		RETURN_THROWS();
 	}
-
-	intern = Z_REFLECTION_P(ZEND_THIS);
-
-	intern->ignore_visibility = visible;
 }
 /* }}} */
 

ext/reflection/tests/ReflectionClass_getProperty_003.phpt

@@ -49,13 +49,9 @@ function showInfo($name) {
         echo $e->getMessage() . "\n";
         return;
     }
-    try {
-        var_dump($rp);
-        var_dump($rp->getValue($myC));
-    } catch (Exception $e) {
-        echo $e->getMessage() . "\n";
-        return;
-    }
+
+    var_dump($rp);
+    var_dump($rp->getValue($myC));
 }
 
 
@@ -110,7 +106,7 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "A"
 }
-Cannot access non-public property C::$protA
+string(10) "protA in A"
 --- (Reflecting on privA) ---
 Property C::$privA does not exist
 --- (Reflecting on pubB) ---
@@ -128,7 +124,7 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "B"
 }
-Cannot access non-public property C::$protB
+string(10) "protB in B"
 --- (Reflecting on privB) ---
 Property C::$privB does not exist
 --- (Reflecting on pubC) ---
@@ -146,15 +142,15 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "C"
 }
-Cannot access non-public property C::$protC
+string(10) "protC in C"
 --- (Reflecting on privC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
   string(5) "privC"
   ["class"]=>
   string(1) "C"
 }
-Cannot access non-public property C::$privC
+string(10) "privC in C"
 --- (Reflecting on doesNotExist) ---
 Property C::$doesNotExist does not exist
 --- (Reflecting on A::pubC) ---
@@ -172,15 +168,15 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "A"
 }
-Cannot access non-public property A::$protC
+string(10) "protC in A"
 --- (Reflecting on A::privC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
   string(5) "privC"
   ["class"]=>
   string(1) "A"
 }
-Cannot access non-public property A::$privC
+string(10) "privC in A"
 --- (Reflecting on B::pubC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
@@ -196,15 +192,15 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "B"
 }
-Cannot access non-public property B::$protC
+string(10) "protC in B"
 --- (Reflecting on B::privC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
   string(5) "privC"
   ["class"]=>
   string(1) "B"
 }
-Cannot access non-public property B::$privC
+string(10) "privC in B"
 --- (Reflecting on c::pubC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
@@ -230,15 +226,15 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "C"
 }
-Cannot access non-public property C::$protC
+string(10) "protC in C"
 --- (Reflecting on C::privC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
   string(5) "privC"
   ["class"]=>
   string(1) "C"
 }
-Cannot access non-public property C::$privC
+string(10) "privC in C"
 --- (Reflecting on X::pubC) ---
 Fully qualified property name X::$pubC does not specify a base class of C
 --- (Reflecting on X::protC) ---

ext/reflection/tests/ReflectionClass_getProperty_004.phpt

@@ -110,7 +110,7 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "A"
 }
-Cannot access non-public property C::$protA
+string(10) "protA in A"
 --- (Reflecting on privA) ---
 Property C::$privA does not exist
 --- (Reflecting on pubB) ---
@@ -128,7 +128,7 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "B"
 }
-Cannot access non-public property C::$protB
+string(10) "protB in B"
 --- (Reflecting on privB) ---
 Property C::$privB does not exist
 --- (Reflecting on pubC) ---
@@ -146,15 +146,15 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "C"
 }
-Cannot access non-public property C::$protC
+string(10) "protC in C"
 --- (Reflecting on privC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
   string(5) "privC"
   ["class"]=>
   string(1) "C"
 }
-Cannot access non-public property C::$privC
+string(10) "privC in C"
 --- (Reflecting on doesNotExist) ---
 Property C::$doesNotExist does not exist
 --- (Reflecting on A::pubC) ---
@@ -172,15 +172,15 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "A"
 }
-Cannot access non-public property A::$protC
+string(10) "protC in C"
 --- (Reflecting on A::privC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
   string(5) "privC"
   ["class"]=>
   string(1) "A"
 }
-Cannot access non-public property A::$privC
+string(10) "privC in A"
 --- (Reflecting on B::pubC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
@@ -196,15 +196,15 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "B"
 }
-Cannot access non-public property B::$protC
+string(10) "protC in C"
 --- (Reflecting on B::privC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
   string(5) "privC"
   ["class"]=>
   string(1) "B"
 }
-Cannot access non-public property B::$privC
+string(10) "privC in B"
 --- (Reflecting on c::pubC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
@@ -230,15 +230,15 @@ object(ReflectionProperty)#%d (2) {
   ["class"]=>
   string(1) "C"
 }
-Cannot access non-public property C::$protC
+string(10) "protC in C"
 --- (Reflecting on C::privC) ---
 object(ReflectionProperty)#%d (2) {
   ["name"]=>
   string(5) "privC"
   ["class"]=>
   string(1) "C"
 }
-Cannot access non-public property C::$privC
+string(10) "privC in C"
 --- (Reflecting on X::pubC) ---
 Fully qualified property name X::$pubC does not specify a base class of C
 --- (Reflecting on X::protC) ---

ext/reflection/tests/ReflectionMethod_invokeArgs_error3.phpt

@@ -49,11 +49,7 @@ echo "\nStatic method:\n";
 var_dump($staticMethod->invokeArgs(null, array()));
 
 echo "\nPrivate method:\n";
-try {
-    var_dump($privateMethod->invokeArgs($testClassInstance, array()));
-} catch (ReflectionException $e) {
-    var_dump($e->getMessage());
-}
+var_dump($privateMethod->invokeArgs($testClassInstance, array()));
 
 echo "\nAbstract method:\n";
 $abstractMethod = new ReflectionMethod("AbstractClass::foo");
@@ -79,7 +75,8 @@ Exception: Using $this when not in object context
 NULL
 
 Private method:
-string(86) "Trying to invoke private method TestClass::privateMethod() from scope ReflectionMethod"
+Called privateMethod()
+NULL
 
 Abstract method:
 string(53) "Trying to invoke abstract method AbstractClass::foo()"