Improve error messages related to void/never return types of methods

devnexen · Jan 14, 2024 · d84ed03 · d84ed03
1 parent b5a23d8
commit d84ed03
Show file tree

Hide file tree

Showing 10 changed files with 68 additions and 14 deletions.
diff --git a/Zend/tests/return_types/never_disallowed1.phpt b/Zend/tests/return_types/never_disallowed1.phpt
@@ -1,5 +1,5 @@
 --TEST--
-never return type: unacceptable cases: any return
+never return type: unacceptable cases: any return in a function
 --FILE--
 <?php
 

diff --git a/Zend/tests/return_types/never_disallowed2.phpt b/Zend/tests/return_types/never_disallowed2.phpt
@@ -1,5 +1,5 @@
 --TEST--
-never return type: unacceptable cases: empty return
+never return type: unacceptable cases: empty return in a function
 --FILE--
 <?php
 

diff --git a/Zend/tests/return_types/never_disallowed3.phpt b/Zend/tests/return_types/never_disallowed3.phpt
@@ -1,5 +1,5 @@
 --TEST--
-never return type: unacceptable cases: implicit return
+never return type: unacceptable cases: implicit return in a function
 --FILE--
 <?php
 

diff --git a/Zend/tests/return_types/never_disallowed4.phpt b/Zend/tests/return_types/never_disallowed4.phpt
@@ -0,0 +1,14 @@
+--TEST--
+never return type: unacceptable cases: empty return in a method
+--FILE--
+<?php
+
+class Foo {
+    public function bar(): never {
+        return;
+    }
+}
+
+?>
+--EXPECTF--
+Fatal error: A never-returning method must not return in %s on line %d
diff --git a/Zend/tests/return_types/never_disallowed5.phpt b/Zend/tests/return_types/never_disallowed5.phpt
@@ -0,0 +1,21 @@
+--TEST--
+never return type: unacceptable cases: implicit return in a method
+--FILE--
+<?php
+
+class Foo {
+    public static function bar(): never {
+        if (false) {
+            throw new Exception('bad');
+        }
+    }
+}
+
+try {
+    Foo::bar();
+} catch (TypeError $e) {
+    echo $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Foo::bar(): never-returning method must not implicitly return
diff --git a/Zend/tests/return_types/void_disallowed1.phpt b/Zend/tests/return_types/void_disallowed1.phpt
@@ -1,5 +1,5 @@
 --TEST--
-void return type: unacceptable cases: explicit NULL return
+void return type: unacceptable cases: explicit NULL return in a function
 --FILE--
 <?php
 

diff --git a/Zend/tests/return_types/void_disallowed2.phpt b/Zend/tests/return_types/void_disallowed2.phpt
@@ -1,5 +1,5 @@
 --TEST--
-void return type: unacceptable cases: explicit return of some other value
+void return type: unacceptable cases: explicit return of some other value in a function
 --FILE--
 <?php
 

diff --git a/Zend/tests/return_types/void_disallowed3.phpt b/Zend/tests/return_types/void_disallowed3.phpt
@@ -0,0 +1,14 @@
+--TEST--
+void return type: unacceptable cases: explicit null return in a method
+--FILE--
+<?php
+
+class Foo {
+    public function bar(): void {
+        return -1; // not permitted in a void function
+    }
+}
+
+?>
+--EXPECTF--
+Fatal error: A void method must not return a value in %s on line %d
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -2571,10 +2571,12 @@ static void zend_emit_return_type_check(
 			if (expr) {
 				if (expr->op_type == IS_CONST && Z_TYPE(expr->u.constant) == IS_NULL) {
 					zend_error_noreturn(E_COMPILE_ERROR,
-						"A void function must not return a value "
-						"(did you mean \"return;\" instead of \"return null;\"?)");
+						"A void %s must not return a value "
+						"(did you mean \"return;\" instead of \"return null;\"?)",
+						CG(active_class_entry) != NULL ? "method" : "function");
 				} else {
-					zend_error_noreturn(E_COMPILE_ERROR, "A void function must not return a value");
+					zend_error_noreturn(E_COMPILE_ERROR, "A void %s must not return a value",
+					CG(active_class_entry) != NULL ? "method" : "function");
 				}
 			}
 			/* we don't need run-time check */
@@ -2585,18 +2587,21 @@ static void zend_emit_return_type_check(
 		if (ZEND_TYPE_CONTAINS_CODE(type, IS_NEVER)) {
 			/* Implicit case handled separately using VERIFY_NEVER_TYPE opcode. */
 			ZEND_ASSERT(!implicit);
-			zend_error_noreturn(E_COMPILE_ERROR, "A never-returning function must not return");
+			zend_error_noreturn(E_COMPILE_ERROR, "A never-returning %s must not return",
+				CG(active_class_entry) != NULL ? "method" : "function");
 			return;
 		}
 
 		if (!expr && !implicit) {
 			if (ZEND_TYPE_ALLOW_NULL(type)) {
 				zend_error_noreturn(E_COMPILE_ERROR,
-					"A function with return type must return a value "
-					"(did you mean \"return null;\" instead of \"return;\"?)");
+					"A %s with return type must return a value "
+					"(did you mean \"return null;\" instead of \"return;\"?)",
+					CG(active_class_entry) != NULL ? "method" : "function");
 			} else {
 				zend_error_noreturn(E_COMPILE_ERROR,
-					"A function with return type must return a value");
+					"A %s with return type must return a value",
+					CG(active_class_entry) != NULL ? "method" : "function");
 			}
 		}
 

diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
@@ -1411,8 +1411,8 @@ ZEND_API ZEND_COLD void zend_verify_never_error(const zend_function *zf)
 {
 	zend_string *func_name = get_function_or_method_name(zf);
 
-	zend_type_error("%s(): never-returning function must not implicitly return",
-		ZSTR_VAL(func_name));
+	zend_type_error("%s(): never-returning %s must not implicitly return",
+		ZSTR_VAL(func_name), zf->common.scope ? "method" : "function");
 
 	zend_string_release(func_name);
 }