Improve magic __get and property type inconsistency error message

Fixes GH-9388
Closes GH-9436

NEWS

@@ -2,6 +2,10 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.3.0alpha1
 
+- Core:
+  . Fixed bug GH-9388 (Improve unset property and __get type incompatibility
+    error message). (ilutov)
+
 - Opcache:
   . Added start, restart and force restart time to opcache's
   phpinfo section. (Mikhail Galanin)

Zend/tests/type_declarations/typed_properties_030.phpt

@@ -19,7 +19,7 @@ unset($foo->bar); # ok
 var_dump($foo->bar); # not okay, __get is nasty
 ?>
 --EXPECTF--
-Fatal error: Uncaught TypeError: Cannot assign string to property Foo::$bar of type int in %s:%d
+Fatal error: Uncaught TypeError: Value of type string returned from Foo::__get() must be compatible with unset property Foo::$bar of type int in %s:%d
 Stack trace:
 #0 {main}
   thrown in %s on line %d

Zend/tests/type_declarations/typed_properties_040.phpt

@@ -20,7 +20,7 @@ var_dump($foo->bar);
 --EXPECTF--
 string(3) "bar"
 
-Fatal error: Uncaught TypeError: Cannot assign null to property Foo::$bar of type int in %s:%d
+Fatal error: Uncaught TypeError: Value of type null returned from Foo::__get() must be compatible with unset property Foo::$bar of type int in %s:%d
 Stack trace:
 #0 {main}
   thrown in %s on line %d

Zend/tests/type_declarations/typed_properties_074.phpt

@@ -32,7 +32,7 @@ object(Test)#1 (1) {
   ["val"]=>
   uninitialized(int)
 }
-Cannot assign string to property Test::$val of type int
+Value of type string returned from Test::__get() must be compatible with unset property Test::$val of type int
 object(Test)#1 (1) {
   ["prop"]=>
   &string(1) "x"

Zend/zend_execute.c

@@ -830,6 +830,23 @@ ZEND_COLD zend_never_inline void zend_verify_property_type_error(zend_property_i
 	zend_string_release(type_str);
 }
 
+ZEND_COLD zend_never_inline void zend_magic_get_property_type_inconsistency_error(zend_property_info *info, zval *property)
+{
+	/* we _may_ land here in case reading already errored and runtime cache thus has not been updated (i.e. it contains a valid but unrelated info) */
+	if (EG(exception)) {
+		return;
+	}
+
+	zend_string *type_str = zend_type_to_string(info->type);
+	zend_type_error("Value of type %s returned from %s::__get() must be compatible with unset property %s::$%s of type %s",
+		zend_zval_type_name(property),
+		ZSTR_VAL(info->ce->name),
+		ZSTR_VAL(info->ce->name),
+		zend_get_unmangled_property_name(info->name),
+		ZSTR_VAL(type_str));
+	zend_string_release(type_str);
+}
+
 ZEND_COLD void zend_match_unhandled_error(zval *value)
 {
 	smart_str msg = {0};
@@ -3560,7 +3577,7 @@ ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, ze
 	return variable_ptr;
 }
 
-ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_info *prop_info, zval *orig_val, bool strict) {
+ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref_ex(zend_property_info *prop_info, zval *orig_val, bool strict, zend_verify_prop_assignable_by_ref_context context) {
 	zval *val = orig_val;
 	if (Z_ISREF_P(val) && ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(val))) {
 		int result;
@@ -3591,10 +3608,20 @@ ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_inf
 		}
 	}
 
-	zend_verify_property_type_error(prop_info, val);
+	if (EXPECTED(context == ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_ASSIGNMENT)) {
+		zend_verify_property_type_error(prop_info, val);
+	} else {
+		ZEND_ASSERT(context == ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_MAGIC_GET);
+		zend_magic_get_property_type_inconsistency_error(prop_info, val);
+	}
+
 	return 0;
 }
 
+ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_info *prop_info, zval *orig_val, bool strict) {
+	return zend_verify_prop_assignable_by_ref_ex(prop_info, orig_val, strict, ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_ASSIGNMENT);
+}
+
 ZEND_API void ZEND_FASTCALL zend_ref_add_type_source(zend_property_info_source_list *source_list, zend_property_info *prop)
 {
 	zend_property_info_list *list;

Zend/zend_execute.h

@@ -65,6 +65,12 @@ ZEND_COLD void ZEND_FASTCALL zend_param_must_be_ref(const zend_function *func, u
 ZEND_API ZEND_COLD void ZEND_FASTCALL zend_use_resource_as_offset(const zval *dim);
 
 ZEND_API bool ZEND_FASTCALL zend_verify_ref_assignable_zval(zend_reference *ref, zval *zv, bool strict);
+
+typedef enum {
+	ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_ASSIGNMENT,
+	ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_MAGIC_GET,
+} zend_verify_prop_assignable_by_ref_context;
+ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref_ex(zend_property_info *prop_info, zval *orig_val, bool strict, zend_verify_prop_assignable_by_ref_context context);
 ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_info *prop_info, zval *orig_val, bool strict);
 
 ZEND_API ZEND_COLD void zend_throw_ref_type_error_zval(zend_property_info *prop, zval *zv);
@@ -449,6 +455,7 @@ ZEND_API int ZEND_FASTCALL zend_handle_undef_args(zend_execute_data *call);
 
 ZEND_API bool zend_verify_property_type(zend_property_info *info, zval *property, bool strict);
 ZEND_COLD void zend_verify_property_type_error(zend_property_info *info, zval *property);
+ZEND_COLD void zend_magic_get_property_type_inconsistency_error(zend_property_info *info, zval *property);
 
 #define ZEND_REF_ADD_TYPE_SOURCE(ref, source) \
 	zend_ref_add_type_source(&ZEND_REF_TYPE_SOURCES(ref), source)

Zend/zend_object_handlers.c

@@ -726,7 +726,7 @@ call_getter:
 			}
 
 			if (UNEXPECTED(prop_info)) {
-				zend_verify_prop_assignable_by_ref(prop_info, retval, (zobj->ce->__get->common.fn_flags & ZEND_ACC_STRICT_TYPES) != 0);
+				zend_verify_prop_assignable_by_ref_ex(prop_info, retval, (zobj->ce->__get->common.fn_flags & ZEND_ACC_STRICT_TYPES) != 0, ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_MAGIC_GET);
 			}
 
 			OBJ_RELEASE(zobj);