Refactor ReflectionMethod::__construct()

Closes GH-6098

7 files changed, 72 insertions, 71 deletions

diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c
index 3b7c29b087..e8938157c0 100644
--- a/ext/reflection/php_reflection.c
+++ b/ext/reflection/php_reflection.c
@@ -2976,81 +2976,83 @@ ZEND_METHOD(ReflectionUnionType, getTypes)
 /* {{{ Constructor. Throws an Exception in case the given method does not exist */
 ZEND_METHOD(ReflectionMethod, __construct)
 {
-	zval *classname;
-	zval *object, *orig_obj;
-	reflection_object *intern;
+	zend_object *arg1_obj;
+	zend_string *arg1_str;
+	zend_string *arg2_str = NULL;
+
+	zend_object *orig_obj = NULL;
+	zend_class_entry *ce = NULL;
+	zend_string *class_name = NULL;
+	char *method_name;
+	size_t method_name_len;
 	char *lcname;
-	zend_class_entry *ce;
+
+	zval *object;
+	reflection_object *intern;
 	zend_function *mptr;
-	char *name_str, *tmp;
-	size_t name_len, tmp_len;
-	zval ztmp;
 
-	if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "zs", &classname, &name_str, &name_len) == FAILURE) {
-		if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &name_str, &name_len) == FAILURE) {
+	ZEND_PARSE_PARAMETERS_START(1, 2)
+		Z_PARAM_STR_OR_OBJ(arg1_str, arg1_obj)
+		Z_PARAM_OPTIONAL
+		Z_PARAM_STR_OR_NULL(arg2_str)
+	ZEND_PARSE_PARAMETERS_END();
+
+	if (arg1_obj) {
+		if (!arg2_str) {
+			zend_argument_value_error(2, "cannot be null when argument #1 ($objectOrMethod) is an object");
 			RETURN_THROWS();
 		}
 
-		if ((tmp = strstr(name_str, "::")) == NULL) {
+		orig_obj = arg1_obj;
+		ce = arg1_obj->ce;
+		method_name = ZSTR_VAL(arg2_str);
+		method_name_len = ZSTR_LEN(arg2_str);
+	} else if (arg2_str) {
+		class_name = zend_string_copy(arg1_str);
+		method_name = ZSTR_VAL(arg2_str);
+		method_name_len = ZSTR_LEN(arg2_str);
+	} else {
+		char *tmp;
+		size_t tmp_len;
+		char *name = ZSTR_VAL(arg1_str);
+
+		if ((tmp = strstr(name, "::")) == NULL) {
 			zend_argument_error(reflection_exception_ptr, 1, "must be a valid method name");
 			RETURN_THROWS();
 		}
-		classname = &ztmp;
-		tmp_len = tmp - name_str;
-		ZVAL_STRINGL(classname, name_str, tmp_len);
-		name_len = name_len - (tmp_len + 2);
-		name_str = tmp + 2;
-		orig_obj = NULL;
-	} else if (Z_TYPE_P(classname) == IS_OBJECT) {
-		orig_obj = classname;
-	} else {
-		orig_obj = NULL;
-	}
+		tmp_len = tmp - name;
 
-	object = ZEND_THIS;
-	intern = Z_REFLECTION_P(object);
-
-	switch (Z_TYPE_P(classname)) {
-		case IS_STRING:
-			if ((ce = zend_lookup_class(Z_STR_P(classname))) == NULL) {
-				if (!EG(exception)) {
-					zend_throw_exception_ex(reflection_exception_ptr, 0,
-							"Class \"%s\" does not exist", Z_STRVAL_P(classname));
-				}
-				if (classname == &ztmp) {
-					zval_ptr_dtor_str(&ztmp);
-				}
-				RETURN_THROWS();
-			}
-			break;
-
-		case IS_OBJECT:
-			ce = Z_OBJCE_P(classname);
-			break;
+		class_name = zend_string_init(name, tmp_len, 0);
+		method_name = tmp + 2;
+		method_name_len = ZSTR_LEN(arg1_str) - tmp_len - 2;
+	}
 
-		default:
-			if (classname == &ztmp) {
-				zval_ptr_dtor_str(&ztmp);
+	if (class_name) {
+		if ((ce = zend_lookup_class(class_name)) == NULL) {
+			if (!EG(exception)) {
+				zend_throw_exception_ex(reflection_exception_ptr, 0, "Class \"%s\" does not exist", ZSTR_VAL(class_name));
 			}
-			zend_argument_error(reflection_exception_ptr, 1, "must be of type object|string, %s given", zend_zval_type_name(classname));
+			zend_string_release(class_name);
 			RETURN_THROWS();
-	}
+		}
 
-	if (classname == &ztmp) {
-		zval_ptr_dtor_str(&ztmp);
+		zend_string_release(class_name);
 	}
 
-	lcname = zend_str_tolower_dup(name_str, name_len);
+	object = ZEND_THIS;
+	intern = Z_REFLECTION_P(object);
+
+	lcname = zend_str_tolower_dup(method_name, method_name_len);
 
-	if (ce == zend_ce_closure && orig_obj && (name_len == sizeof(ZEND_INVOKE_FUNC_NAME)-1)
+	if (ce == zend_ce_closure && orig_obj && (method_name_len == sizeof(ZEND_INVOKE_FUNC_NAME)-1)
 		&& memcmp(lcname, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1) == 0
-		&& (mptr = zend_get_closure_invoke_method(Z_OBJ_P(orig_obj))) != NULL)
+		&& (mptr = zend_get_closure_invoke_method(orig_obj)) != NULL)
 	{
 		/* do nothing, mptr already set */
-	} else if ((mptr = zend_hash_str_find_ptr(&ce->function_table, lcname, name_len)) == NULL) {
+	} else if ((mptr = zend_hash_str_find_ptr(&ce->function_table, lcname, method_name_len)) == NULL) {
 		efree(lcname);
 		zend_throw_exception_ex(reflection_exception_ptr, 0,
-			"Method %s::%s() does not exist", ZSTR_VAL(ce->name), name_str);
+			"Method %s::%s() does not exist", ZSTR_VAL(ce->name), method_name);
 		RETURN_THROWS();
 	}
 	efree(lcname);
diff --git a/ext/reflection/php_reflection.stub.php b/ext/reflection/php_reflection.stub.php
index 311e748e9b..d1ba2cf624 100644
--- a/ext/reflection/php_reflection.stub.php
+++ b/ext/reflection/php_reflection.stub.php
@@ -147,8 +147,7 @@ final class ReflectionGenerator
 
 class ReflectionMethod extends ReflectionFunctionAbstract
 {
-    /** @param object|string $objectOrMethod */
-    public function __construct($objectOrMethod, string $method = UNKNOWN) {}
+    public function __construct(object|string $objectOrMethod, ?string $method = null) {}
 
     public function __toString(): string {}
 
diff --git a/ext/reflection/php_reflection_arginfo.h b/ext/reflection/php_reflection_arginfo.h
index c0ff2d75e2..aab74783c8 100644
--- a/ext/reflection/php_reflection_arginfo.h
+++ b/ext/reflection/php_reflection_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 1311fc5c498d6f16afb5a18aee2d60e72048174f */
+ * Stub hash: d698afd338e4bf7c782f0edddfcbe95859eef477 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Reflection_getModifierNames, 0, 0, 1)
 	ZEND_ARG_TYPE_INFO(0, modifiers, IS_LONG, 0)
@@ -101,8 +101,8 @@ ZEND_END_ARG_INFO()
 #define arginfo_class_ReflectionGenerator_getExecutingGenerator arginfo_class_ReflectionFunctionAbstract___clone
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_ReflectionMethod___construct, 0, 0, 1)
-	ZEND_ARG_INFO(0, objectOrMethod)
-	ZEND_ARG_TYPE_INFO(0, method, IS_STRING, 0)
+	ZEND_ARG_TYPE_MASK(0, objectOrMethod, MAY_BE_OBJECT|MAY_BE_STRING, NULL)
+	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, method, IS_STRING, 1, "null")
 ZEND_END_ARG_INFO()
 
 #define arginfo_class_ReflectionMethod___toString arginfo_class_ReflectionFunction___toString
diff --git a/ext/reflection/tests/008.phpt b/ext/reflection/tests/008.phpt
index da600f0016..d33aa23a9f 100644
--- a/ext/reflection/tests/008.phpt
+++ b/ext/reflection/tests/008.phpt
@@ -34,6 +34,6 @@ string(24) "Class "a" does not exist"
 string(23) "Class "" does not exist"
 string(24) "Class "a" does not exist"
 string(23) "Class "" does not exist"
-string(103) "ReflectionMethod::__construct(): Argument #1 ($objectOrMethod) must be of type object|string, int given"
+string(24) "Class "1" does not exist"
 string(23) "Class "" does not exist"
 Done
diff --git a/ext/reflection/tests/ReflectionMethod_006.phpt b/ext/reflection/tests/ReflectionMethod_006.phpt
index e32c73b8be..4f76e8c338 100644
--- a/ext/reflection/tests/ReflectionMethod_006.phpt
+++ b/ext/reflection/tests/ReflectionMethod_006.phpt
@@ -8,16 +8,16 @@ Steve Seear <stevseea@php.net>
 
 try {
     new ReflectionMethod();
-} catch (TypeError $re) {
+} catch (ArgumentCountError $re) {
     echo "Ok - ".$re->getMessage().PHP_EOL;
 }
 try {
     new ReflectionMethod('a', 'b', 'c');
-} catch (TypeError $re) {
+} catch (ArgumentCountError $re) {
     echo "Ok - ".$re->getMessage().PHP_EOL;
 }
 
 ?>
 --EXPECT--
-Ok - ReflectionMethod::__construct() expects exactly 1 argument, 0 given
-Ok - ReflectionMethod::__construct() expects exactly 1 argument, 3 given
+Ok - ReflectionMethod::__construct() expects at least 1 argument, 0 given
+Ok - ReflectionMethod::__construct() expects at most 2 arguments, 3 given
diff --git a/ext/reflection/tests/ReflectionMethod_constructor_error1.phpt b/ext/reflection/tests/ReflectionMethod_constructor_error1.phpt
index 322ea9bddd..8c5bd7139c 100644
--- a/ext/reflection/tests/ReflectionMethod_constructor_error1.phpt
+++ b/ext/reflection/tests/ReflectionMethod_constructor_error1.phpt
@@ -75,9 +75,9 @@ Stack trace:
 #0 %s ReflectionMethod->__construct('3')
 #1 {main}
 Wrong type of argument (bool, string):
-ReflectionException: ReflectionMethod::__construct(): Argument #1 ($objectOrMethod) must be of type object|string, bool given in %s:%d
+ReflectionException: Class "1" does not exist in %s:%d
 Stack trace:
-#0 %s ReflectionMethod->__construct(true, 'foo')
+#0 %s ReflectionMethod->__construct('1', 'foo')
 #1 {main}
 Wrong type of argument (string, bool):
 ReflectionException: Method TestClass::1() does not exist in %s:%d
diff --git a/ext/reflection/tests/ReflectionMethod_constructor_error2.phpt b/ext/reflection/tests/ReflectionMethod_constructor_error2.phpt
index 2a8b30bf03..a7115893a5 100644
--- a/ext/reflection/tests/ReflectionMethod_constructor_error2.phpt
+++ b/ext/reflection/tests/ReflectionMethod_constructor_error2.phpt
@@ -16,13 +16,13 @@ class TestClass
 try {
     echo "Too few arguments:\n";
     $methodInfo = new ReflectionMethod();
-} catch (TypeError $re) {
+} catch (ArgumentCountError $re) {
     echo "Ok - ".$re->getMessage().PHP_EOL;
 }
 try {
     echo "\nToo many arguments:\n";
     $methodInfo = new ReflectionMethod("TestClass", "foo", true);
-} catch (TypeError $re) {
+} catch (ArgumentCountError $re) {
     echo "Ok - ".$re->getMessage().PHP_EOL;
 }
 
@@ -38,7 +38,7 @@ try {
 try {
     //invalid 1st param
     $methodInfo = new ReflectionMethod([], "foo");
-} catch (ReflectionException $re) {
+} catch (TypeError $re) {
     echo "Ok - ".$re->getMessage().PHP_EOL;
 }
 
@@ -52,10 +52,10 @@ try{
 ?>
 --EXPECT--
 Too few arguments:
-Ok - ReflectionMethod::__construct() expects exactly 1 argument, 0 given
+Ok - ReflectionMethod::__construct() expects at least 1 argument, 0 given
 
 Too many arguments:
-Ok - ReflectionMethod::__construct() expects exactly 1 argument, 3 given
+Ok - ReflectionMethod::__construct() expects at most 2 arguments, 3 given
 Ok - Class "InvalidClassName" does not exist
 Ok - ReflectionMethod::__construct(): Argument #1 ($objectOrMethod) must be of type object|string, array given
-Ok - ReflectionMethod::__construct() expects exactly 1 argument, 2 given
+Ok - ReflectionMethod::__construct(): Argument #2 ($method) must be of type ?string, array given