Hi Michał, On Mon, Jan 16, 2017 at 10:30 AM, Michał Brzuchalski <mic...@brzuchalski.com > wrote:
> As I am familiar with those interceptions, I tend to point out some dirty > hacks > when reflection tells you property exists while getting notice on set, see > https://3v4l.org/VDMHm > > <?php > > class Foo > { > public $bar = 'bar'; > public $baz = 'baz'; > } > > class FooHack extends Foo > { > public function __construct(Foo $wrapped) > { > unset($this->bar); > } > } > > $foo = new FooHack(new Foo); > > $reflectionFooBar = new \ReflectionProperty(Foo::class, 'bar'); > var_dump((new ReflectionClass(Foo::class))->getProperties()); > var_dump($reflectionFooBar->getValue($foo)); > var_dump( > property_exists(FooHack::class, 'bar'), > property_exists($foo, 'bar') > ); > > Funny PHP7 has different behaviour from 7.0.7 - 7.1 with raising a notice. > Yes, some cleanups were applied to PHP 7.1 (Nikic worked on them, I think). Specifically around trying to access reflection properties across incompatible object types. Unsure if that also touched `property_exists()` > I may be just looking for dirty example without purpose right now so, don't > listen to me. > No, that's fine. These are dirty hacks, but it really is the only (currently) available way to build userland property accessors. Without this kind of approach, the only alternative is relying on https://pecl.php.net/package/AOP, and that's not really something that is going to happen. > But I do feel like this could bring someone crazy when something should > exists while it's not. > This is not something that users would do directly. AOP-ish libraries are extremely well tested in these scenarios, because if something magic breaks it is really hard to understand what is going on. > Those property_exists($foo, 'bar') shouldn't return false in that example?! > I'd say that `property_exists()` should always return `true` for static (class definition) defined fields, since the primary use-case for `property_exists()` is to provide information for consumers of a certain object for BC/FC. Most frameworks/libraries rely on `function_exists()`, `method_exists()` and `property_exists()` to skip tests, run tests, load polyfills, etc. My best guess is that the API shouldn't change though. That's the safest BC approach, whereas new behavior can be defined with a completely new function (for example: `property_is_statically_defined()`, `property_value_is_assigned()`, etc.). > > 2017-01-16 10:17 GMT+01:00 Marco Pivetta <ocram...@gmail.com>: > > > On Mon, Jan 16, 2017 at 9:49 AM, Michał Brzuchalski < > > mic...@brzuchalski.com> wrote: > > > >> Hi Marco, > >> > >> 2017-01-16 0:27 GMT+01:00 Marco Pivetta <ocram...@gmail.com>: > >> > >>> Hi Wes, > >>> > >>> This has been discussed before, and it's currently used to intercept > >>> access > >>> to properties. Since we don't have property accessors (sigh), the code > >>> (simplified version) would look like following: > >>> > >>> class Foo > >>> { > >>> public $bar = 'baz'; > >>> } > >>> > >>> class FooInterceptor extends Foo > >>> { > >>> private $wrapped; > >>> public function __construct(Foo $wrapped) > >>> { > >>> $this->wrapped = $wrapped; > >>> unset($this->bar); > >>> } > >>> public function __get(string $name) > >>> { > >>> var_dump('reading ' . $name); > >>> return $this->wrapped->$name; > >>> } > >>> } > >>> > >>> $foo = new FooInterceptor(new Foo); > >>> > >>> var_dump($foo->bar); > >>> > >>> You can see a working example at https://3v4l.org/UtugD > >> > >> > >> There is one more thing might be confusing - reflection tells there > still > >> exists bar property after unset while it's realy not. > >> For example https://3v4l.org/NAg1l > >> > >> $class = new ReflectionClass(FooInterceptor::class); > >> $property = $class->getProperty('bar'); > >> var_dump($property); // still exists while actually being unset may > cause > >> errors > >> > >> I'm sticking to extending class without magic _get method implemented. > >> > >> > >>> > >>> > >>> This behavior is protected from regressions since PHP 5.4, but has been > >>> working since 5.0: > >>> https://github.com/php/php-src/blob/cd2b462a2742c79256668d47 > >>> 36644e34573c33d9/tests/classes/unset_properties.phpt > >>> > >>> We can most probably get rid of this weird behavior once property > >>> accessors > >>> are in the language. > >>> > >>> Greets, > >>> > >>> Marco Pivetta > >>> > >>> http://twitter.com/Ocramius > >>> > >>> http://ocramius.github.com/ > >>> > >>> On Mon, Jan 16, 2017 at 12:20 AM, Wes <netmo....@gmail.com> wrote: > >>> > >>> > Hello elephpants. > >>> > > >>> > Currently PHP allows explicitly declared fields (eg public $foo = > 10;) > >>> to > >>> > be removed entirely through unset (eg unset($obj->foo)). > >>> > > >>> > Now that isn't really an issue as properties in php are currently > >>> untyped > >>> > and therefore nullable; at worst you would get a notice. But it would > >>> > become an issue with typed fields... that might get a second chance > >>> sooner > >>> > or later. > >>> > > >>> > But regardless of that, it looks very strange to me that this is > >>> allowed > >>> > for fields that are explicitly declared. I think unset() should set > the > >>> > field to null if it's declared in the class, and remove the field > >>> > altogether only if it was defined dynamically. > >>> > > >>> > On the other hand, this is just one of many ways of hacking php that > >>> just > >>> > exist and we accept / don't care because we have faith in other > people > >>> not > >>> > doing nasty stuff with our code. This might sound ironic it is > >>> actually not > >>> > :P > >>> > > >>> > However, I am curious: what you think about this? Should PHP do > >>> something > >>> > in regard? Should this continue to work like it does now? Why do you > >>> feel > >>> > it should do the one or the other? > >>> > > >>> > >> > > Hi Michał, > > > > Reflection will also trigger `__get` in this scenario, which is expected > > and was also reverted multiple times in "fixes" that worked around or > > forgot to call the property access guards. > > > > class Foo > > { > > public $bar = 'baz'; > > } > > > > class FooInterceptor extends Foo > > { > > private $wrapped; > > public function __construct(Foo $wrapped) > > { > > $this->wrapped = $wrapped; > > unset($this->bar); > > } > > public function __get(string $name) > > { > > var_dump('reading ' . $name); > > return $this->wrapped->$name; > > } > > } > > > > $foo = new FooInterceptor(new Foo); > > > > $reflectionFooBar = new \ReflectionProperty(Foo::class, 'bar'); > > > > var_dump($reflectionFooBar->getValue($foo)); > > > > See https://3v4l.org/6JtWT for a working example. > > > > You can see https://github.com/Ocramius/ProxyManager/tree/ > > cce5477857504997baf3168974b8f1283516a686/tests/language-feature-scripts > > for > > > > As I already mentioned, this hack is currently necessary to make property > > access interception transparent, which is common for most AOP-oriented > > code. We need an alternate approach to make this happen, before such a > > feature can be dropped. > > > > Marco Pivetta > > > > http://twitter.com/Ocramius > > > > http://ocramius.github.com/ > > > Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/
