Make type error messages more consistent

Closes GH-5092

1 files changed, 30 insertions, 112 deletions

diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
index 3fbbcef0fe..7da6810a11 100644
--- a/Zend/zend_execute.c
+++ b/Zend/zend_execute.c
@@ -640,30 +640,12 @@ static zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_throw_non_object_erro
 	}
 }
 
-/* Test used to preserve old error messages for non-union types.
- * We might want to canonicalize all type errors instead. */
-static zend_bool is_union_type(zend_type type) {
-	if (ZEND_TYPE_HAS_LIST(type)) {
-		return 1;
-	}
-	uint32_t type_mask_without_null = ZEND_TYPE_PURE_MASK_WITHOUT_NULL(type);
-	if (ZEND_TYPE_HAS_CLASS(type)) {
-		return type_mask_without_null != 0;
-	}
-	if (type_mask_without_null == MAY_BE_BOOL) {
-		return 0;
-	}
-	/* Check that only one bit is set. */
-	return (type_mask_without_null & (type_mask_without_null - 1)) != 0;
-}
-
 static ZEND_COLD void zend_verify_type_error_common(
 		const zend_function *zf, const zend_arg_info *arg_info,
 		void **cache_slot, zval *value,
 		const char **fname, const char **fsep, const char **fclass,
-		zend_string **need_msg, const char **given_msg, const char **given_kind)
+		zend_string **need_msg, const char **given_kind)
 {
-	smart_str str = {0};
 	*fname = ZSTR_VAL(zf->common.function_name);
 	if (zf->common.scope) {
 		*fsep =  "::";
@@ -673,90 +655,18 @@ static ZEND_COLD void zend_verify_type_error_common(
 		*fclass = "";
 	}
 
-	if (is_union_type(arg_info->type)) {
-		zend_string *type_str = zend_type_to_string_resolved(arg_info->type, zf->common.scope);
-		smart_str_appends(&str, "be of type ");
-		smart_str_append(&str, type_str);
-		zend_string_release(type_str);
-	} else if (ZEND_TYPE_HAS_CLASS(arg_info->type)) {
-		zend_bool is_interface = 0;
-		zend_class_entry *ce = *cache_slot;
-		if (!ce) {
-			ce = zend_fetch_class(ZEND_TYPE_NAME(arg_info->type),
-				(ZEND_FETCH_CLASS_AUTO | ZEND_FETCH_CLASS_NO_AUTOLOAD));
-		}
-		if (ce) {
-			if (ce->ce_flags & ZEND_ACC_INTERFACE) {
-				smart_str_appends(&str, "implement interface ");
-				is_interface = 1;
-			} else {
-				smart_str_appends(&str, "be an instance of ");
-			}
-			smart_str_append(&str, ce->name);
-		} else {
-			/* We don't know whether it's a class or interface, assume it's a class */
-			smart_str_appends(&str, "be an instance of ");
-			smart_str_append(&str, ZEND_TYPE_NAME(arg_info->type));
-		}
-
-		if (ZEND_TYPE_ALLOW_NULL(arg_info->type)) {
-			smart_str_appends(&str, is_interface ? " or be null" : " or null");
-		}
-	} else {
-		uint32_t type_mask = ZEND_TYPE_PURE_MASK_WITHOUT_NULL(arg_info->type);
-		switch (type_mask) {
-			case MAY_BE_OBJECT:
-				smart_str_appends(&str, "be an object");
-				break;
-			case MAY_BE_CALLABLE:
-				smart_str_appends(&str, "be callable");
-				break;
-			case MAY_BE_ITERABLE:
-				smart_str_appends(&str, "be iterable");
-				break;
-			case MAY_BE_STATIC: {
-				zend_class_entry *called_scope = zend_get_called_scope(EG(current_execute_data));
-				smart_str_appends(&str, "be an instance of ");
-				if (called_scope) {
-					smart_str_append(&str, called_scope->name);
-				} else {
-					smart_str_appends(&str, "static");
-				}
-				break;
-			}
-			default:
-			{
-				/* Hack to print the type without null */
-				zend_type type = arg_info->type;
-				ZEND_TYPE_FULL_MASK(type) &= ~MAY_BE_NULL;
-				zend_string *type_str = zend_type_to_string(type);
-				smart_str_appends(&str, "be of the type ");
-				smart_str_append(&str, type_str);
-				zend_string_release(type_str);
-				break;
-			}
-		}
-
-		if (ZEND_TYPE_ALLOW_NULL(arg_info->type)) {
-			smart_str_appends(&str, " or null");
-		}
-	}
-
-	*need_msg = smart_str_extract(&str);
+	*need_msg = zend_type_to_string_resolved(arg_info->type, zf->common.scope);
 
 	if (value) {
 		zend_bool has_class = ZEND_TYPE_HAS_CLASS(arg_info->type)
 			|| (ZEND_TYPE_FULL_MASK(arg_info->type) & MAY_BE_STATIC);
 		if (has_class && Z_TYPE_P(value) == IS_OBJECT) {
-			*given_msg = "instance of ";
 			*given_kind = ZSTR_VAL(Z_OBJCE_P(value)->name);
 		} else {
-			*given_msg = zend_zval_type_name(value);
-			*given_kind = "";
+			*given_kind = zend_zval_type_name(value);
 		}
 	} else {
-		*given_msg = "none";
-		*given_kind = "";
+		*given_kind = "none";
 	}
 }
 
@@ -767,7 +677,7 @@ ZEND_API ZEND_COLD void zend_verify_arg_error(
 	zend_execute_data *ptr = EG(current_execute_data)->prev_execute_data;
 	const char *fname, *fsep, *fclass;
 	zend_string *need_msg;
-	const char *given_msg, *given_kind;
+	const char *given_msg;
 
 	if (EG(exception)) {
 		/* The type verification itself might have already thrown an exception
@@ -778,18 +688,20 @@ ZEND_API ZEND_COLD void zend_verify_arg_error(
 	if (value) {
 		zend_verify_type_error_common(
 			zf, arg_info, cache_slot, value,
-			&fname, &fsep, &fclass, &need_msg, &given_msg, &given_kind);
+			&fname, &fsep, &fclass, &need_msg, &given_msg);
 
 		if (zf->common.type == ZEND_USER_FUNCTION) {
 			if (ptr && ptr->func && ZEND_USER_CODE(ptr->func->common.type)) {
-				zend_type_error("Argument %d passed to %s%s%s() must %s, %s%s given, called in %s on line %d",
-						arg_num, fclass, fsep, fname, ZSTR_VAL(need_msg), given_msg, given_kind,
+				zend_type_error("%s%s%s() expects argument #%d ($%s) to be of type %s, %s given, called in %s on line %d",
+						fclass, fsep, fname, arg_num, ZSTR_VAL(arg_info->name), ZSTR_VAL(need_msg), given_msg,
 						ZSTR_VAL(ptr->func->op_array.filename), ptr->opline->lineno);
 			} else {
-				zend_type_error("Argument %d passed to %s%s%s() must %s, %s%s given", arg_num, fclass, fsep, fname, ZSTR_VAL(need_msg), given_msg, given_kind);
+				zend_type_error("%s%s%s() expects argument #%d ($%s) to be of type %s, %s given",
+					fclass, fsep, fname, arg_num, ZSTR_VAL(arg_info->name), ZSTR_VAL(need_msg), given_msg);
 			}
 		} else {
-			zend_type_error("Argument %d passed to %s%s%s() must %s, %s%s given", arg_num, fclass, fsep, fname, ZSTR_VAL(need_msg), given_msg, given_kind);
+			zend_type_error("%s%s%s() expects argument #%d ($%s) to be of type %s, %s given",
+				fclass, fsep, fname, arg_num, ((zend_internal_arg_info*) arg_info)->name, ZSTR_VAL(need_msg), given_msg);
 		}
 
 		zend_string_release(need_msg);
@@ -1259,14 +1171,14 @@ ZEND_API ZEND_COLD void zend_verify_return_error(
 	const zend_arg_info *arg_info = &zf->common.arg_info[-1];
 	const char *fname, *fsep, *fclass;
 	zend_string *need_msg;
-	const char *given_msg, *given_kind;
+	const char *given_msg;
 
 	zend_verify_type_error_common(
 		zf, arg_info, cache_slot, value,
-		&fname, &fsep, &fclass, &need_msg, &given_msg, &given_kind);
+		&fname, &fsep, &fclass, &need_msg, &given_msg);
 
-	zend_type_error("Return value of %s%s%s() must %s, %s%s returned",
-		fclass, fsep, fname, ZSTR_VAL(need_msg), given_msg, given_kind);
+	zend_type_error("Return value of %s%s%s() must be of type %s, %s returned",
+		fclass, fsep, fname, ZSTR_VAL(need_msg), given_msg);
 
 	zend_string_release(need_msg);
 }
@@ -1278,14 +1190,14 @@ static ZEND_COLD void zend_verify_internal_return_error(
 	const zend_arg_info *arg_info = &zf->common.arg_info[-1];
 	const char *fname, *fsep, *fclass;
 	zend_string *need_msg;
-	const char *given_msg, *given_kind;
+	const char *given_msg;
 
 	zend_verify_type_error_common(
 		zf, arg_info, cache_slot, value,
-		&fname, &fsep, &fclass, &need_msg, &given_msg, &given_kind);
+		&fname, &fsep, &fclass, &need_msg, &given_msg);
 
-	zend_error_noreturn(E_CORE_ERROR, "Return value of %s%s%s() must %s, %s%s returned",
-		fclass, fsep, fname, ZSTR_VAL(need_msg), given_msg, given_kind);
+	zend_error_noreturn(E_CORE_ERROR, "Return value of %s%s%s() must be of type %s, %s returned",
+		fclass, fsep, fname, ZSTR_VAL(need_msg), given_msg);
 }
 
 static ZEND_COLD void zend_verify_void_return_error(const zend_function *zf, const char *returned_msg, const char *returned_kind)
@@ -2011,11 +1923,17 @@ static zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_non_static_method_cal
 
 static zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_param_must_be_ref(const zend_function *func, uint32_t arg_num)
 {
-	zend_error(E_WARNING, "Parameter %d to %s%s%s() expected to be a reference, value given",
-		arg_num,
+	const char *arg_name = get_function_arg_name(func, arg_num);
+
+	zend_error(E_WARNING, "%s%s%s() expects argument #%d%s%s%s to be passed by reference, value given",
 		func->common.scope ? ZSTR_VAL(func->common.scope->name) : "",
 		func->common.scope ? "::" : "",
-		ZSTR_VAL(func->common.function_name));
+		ZSTR_VAL(func->common.function_name),
+		arg_num,
+		arg_name ? " ($" : "",
+		arg_name ? arg_name : "",
+		arg_name ? ")" : ""
+	);
 }
 
 static zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_use_scalar_as_array(void)
@@ -2632,7 +2550,7 @@ static ZEND_COLD void ZEND_FASTCALL zend_array_key_exists_error(
 		ZVAL_UNDEFINED_OP2();
 	}
 	if (!EG(exception)) {
-		zend_type_error("array_key_exists() expects parameter 2 to be array, %s given",
+		zend_type_error("array_key_exists() expects argument #2 ($array) to be of type array, %s given",
 			zend_get_type_by_const(Z_TYPE_P(subject)));
 	}
 }