Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Philosophy behind methods like PropertyReflection::getValue() #1367

Open
vudaltsov opened this issue Sep 7, 2023 · 3 comments
Open

Philosophy behind methods like PropertyReflection::getValue() #1367

vudaltsov opened this issue Sep 7, 2023 · 3 comments

Comments

@vudaltsov
Copy link

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
Copy link
Member

Ocramius commented Sep 7, 2023

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.

@autaut03
Copy link

autaut03 commented Oct 1, 2023

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

@Ocramius
Copy link
Member

Ocramius commented Oct 1, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants