Re: [RFC][Discussion] Objects can be declared falsifiable

Hi Josh, I'd recommend looking at https://github.com/php/php-src/pull/5156 (linked to in your RFC's reference) when implementing this PR. Additionally, I didn't see if this was brought up - PHP already has a class which is falsifiable - SimpleXMLElement. You can see the source of ext/simplexml/simplexml.c for _IS_BOOL ``` static int sxe_object_cast_ex(zend_object *readobj, zval *writeobj, int type) { php_sxe_object *sxe; xmlChar *contents = NULL; xmlNodePtr node; int rv; sxe = php_sxe_fetch_object(readobj); if (type == _IS_BOOL) { node = php_sxe_get_first_node(sxe, NULL); if (node) { ZVAL_TRUE(writeobj); } else { ZVAL_BOOL(writeobj, !sxe_prop_is_empty(readobj)); } return SUCCESS; } ``` ``` static php_sxe_object* php_sxe_object_new(zend_class_entry *ce, zend_function *fptr_count) intern->zo.handlers = &sxe_object_handlers; // ... elsewhere in ext/simplexml/simplexml.c sxe_object_handlers.cast_object = sxe_object_cast; ``` The default handlers would be overridden, so this would probably need to fall back to the original handlers for types other than bool. Also, this might need to copy all of the handlers from the parent class entry's zend_object_handlers, to meet the expected behavior of other cast types and other magic behaviors from SimpleXMLElement and other classes continuing to work. From the perspective of static analysis, a few things to note: - Static analyzers such as psalm/phan currently deliberately don't bother handling the possibility that object can be false even after it was checked for falsiness. There's hundreds of other things that could be implemented, and adding the special casing and performance overhead checking for FFI objects and SimpleXMLElement, subclasses of those is currently low priority compared to those things. Code using SimpleXMLElement/FFI is generally limited to a few files in practice. Definitely possible for analyzers to support this for known base classes, though, and the priority would increase if the RFC passed. I am a maintainer of Phan. - For BC reasons, internal data structures such as ArrayObject probably wouldn't get changed in any php release (e.g. could break code using falsiness check instead of null check). So this might lead to inconsistencies with newer extensions if half of the data structures treat emptiness as false and half don't. - This feature may end up getting adopted in cases where it's convenient but turns out prone to accidental bugs and later is deprecated and removed. For example, https://stackoverflow.com/questions/25031236/if-elem-vs-elem-is-not-none was seen in python (e.g. if (!$this->resultSetObject) { $this->resultSetObject = slow_db_operation(); } would not behave the way people would previously expect for most objects (executed repeatedly instead of once)) ``` <?php function test(SimpleXMLElement $e) { // False positive RedundantCondition in psalm if ($e) { } } ``` > And I don’t know of a way to add an interface to new stdClass() - but thought this might be a > valid use case: stdClass isn't a final class. I assume they meant this ``` class EmptyStdClass extends stdClass implements Falsifiable { public function __toBool() { return false; } } function example(stdClass $s) { if (!$s) { throw new Exception("Previously impossible"); } } ``` Thanks, - Tyson