Roave/BetterReflection

Philosophy behind methods like PropertyReflection::getValue()

vudaltsov opened this issue · 3 comments

vudaltsov commented

The current implementation of PropertyReflection::getValue() does trigger autoloading when calling Closure::bind. And that makes sense, because this methods either acts on a static property of a class that should be loaded to write or non-static property of an object, so class is already loaded by the time the method is called with an instance.

Question 1. Why not using native reflection here? It will give exactly the same results and side-effects (class loading).

Question 2. Why ReflectionClass::newInstance like methods are not implemented via native reflection if PropertyReflection::getValue() already breaks the "don't autoload" rule? I mean, if the developer calls $reflectionClass->newInstance(), he obviously realizes that this cannot be done only without loading the class (same with PropertyReflection::getValue($object)). So it's his responsibility to check what side effects this will have.

Ocramius commented

Question 1. Why not using native reflection here? It will give exactly the same results and side-effects (class loading).

Very much feasible, yes: I don't remember why we decided to instead do closure binding.
Autoloading doesn't really happen here, other than for static methods.

Question 2.

To be fair, that's a sensible question: I'd say that #getValue() does work without autoloading with object instances, whilst instantiation starts from potentially a non-loaded class-string.

Practically, both are in the adapter layer, and could in theory be implemented.

  • 👍1
autaut03 commented

Just stumbled upon this randomly. The reason for Closure::bind in getValue is most likely for consistency with setValue, and the reason for Closure::bind in setValue is because it respects the declare(strict_types=1);, unlike reflection:

<?php declare(strict_types=1);

class Test {
	private string $prop;
}

$test = new \Test();

(new \ReflectionProperty(\Test::class, 'prop'))->setValue($test, 123);

var_dump($test);

(fn () => $this->prop = 123)->call($test);

var_dump($test);

outputs

object(Test)#1 (1) {
  ["prop":"Test":private]=>
  string(3) "123"
}

Fatal error: Uncaught TypeError: Cannot assign int to property Test::$prop of type string in /in/I3mDb:15
  • 👍1
Ocramius commented