Fix issue1385: IS_INDIRECT when fetching private properties

[nikita2206]

https://bugs.xdebug.org/view.php?id=1385

[nikita2206]

@derickr shouldn't we also call zend_read_property() with a silent flag (e.g. https://gist.github.com/nikita2206/af8a6c36693e96ef7a897d7f3b224395 )? I was always wondering why do I sometimes get these Notices when I inspect variables in PHPStorm, turns out there's a lot happening in there. It would be perfect if there was no need for poking around and trying to find the value (like with the zend_read_property, then zend_hash_str_find, then the asterisk), but for that the property_get (from DBGP functions) would need to be changed somehow (or a new function could be introduced) in order to exhaustively tell which property we need to take (e.g. we shouldn't try public properties table if there is a *Classname* prefix, but we have to right now because there could be a public property with the name *Classname*foo).
What do you think about it?

[derickr]

Looks good, I just have some suggested changes for the CS in the test file.

Would you also mind changing the commit message to be exactly "Fixed issue #1385: Can not fetch IS_INDIRECT private properties", and change the test file names to "tests/bug01385.phpt" and "tests/bug01385.inc" with as test file "test for bug #1385: Can not fetch IS_INDIRECT private properties" please?

[derickr]

The Coding Standards I use have a space after ( and before ), as well as the { on a new line (see line 40 below).

[nikita2206]

Done. What do you think about what I said above?

[derickr]

I think it makes sense to use the "silent flag", and I would welcome a PR with the GIST that you linked to.

I am not sure whether I want to change how property_get works. Adding a new command to the protocol is probably not very useful either, as almost no IDE will add support for it (as it's not really necessary?).

[nikita2206]

@derickr

I think it makes sense to use the "silent flag", and I would welcome a PR with the GIST that you linked to.

Nice, you'll get it tomorrow.

I am not sure whether I want to change how property_get works. Adding a new command to the protocol is probably not very useful either, as almost no IDE will add support for it (as it's not really necessary?).

It is kinda necessary... Well, those are rare use cases, but basically the way property_get works right now - it can not correctly cover all possible use cases. Here's a snippet for you to try with PHPStorm or manually calling "property_get":
https://gist.github.com/nikita2206/4cca974d599348c84be5e6f45de949c7 or
https://gist.github.com/nikita2206/dc512860e896c1ec2281cb7ca54bd544

This shows that we, at least, need some escaping capability.
That is something you're not opposed to though, yeah I wasn't even talking about it in the previous comment. (I guess I'll make a PR later about escape sequences in property paths, if you're ok with it)

But here's a snippet that makes it impossible to get certain property with property_get just because there's no exhaustive syntax for property paths and it uses guesswork instead:
https://gist.github.com/nikita2206/915cac6efa1e25e5ded76accf718549a
It is obviously is a rare use case, but as a programmer it strikes me as unreliable behavior.