ext/intl: idn.c use ValueErrors where appropriate

Drive-by refactoring

Author: Girgias

ext/intl/idn/idn.c

@@ -37,7 +37,7 @@ enum {
 };
 
 /* like INTL_CHECK_STATUS, but as a function and varying the name of the func */
-static int php_intl_idn_check_status(UErrorCode err, const char *msg)
+static zend_result php_intl_idn_check_status(UErrorCode err, const char *msg)
 {
 	intl_error_set_code(NULL, err);
 	if (U_FAILURE(err)) {
@@ -53,11 +53,6 @@ static int php_intl_idn_check_status(UErrorCode err, const char *msg)
 	return SUCCESS;
 }
 
-static inline void php_intl_bad_args(const char *msg)
-{
-	php_intl_idn_check_status(U_ILLEGAL_ARGUMENT_ERROR, msg);
-}
-
 static void php_intl_idn_to_46(INTERNAL_FUNCTION_PARAMETERS,
 		const zend_string *domain, uint32_t option, int mode, zval *idna_info)
 {
@@ -128,18 +123,17 @@ static void php_intl_idn_handoff(INTERNAL_FUNCTION_PARAMETERS, int mode)
 		RETURN_THROWS();
 	}
 
-	if (variant != INTL_IDN_VARIANT_UTS46) {
-		php_intl_bad_args("invalid variant, must be INTL_IDNA_VARIANT_UTS46");
-		RETURN_FALSE;
-	}
-
-	if (ZSTR_LEN(domain) < 1) {
-		php_intl_bad_args("empty domain name");
-		RETURN_FALSE;
+	if (ZSTR_LEN(domain) == 0) {
+		zend_argument_value_error(1, "cannot be empty");
+		RETURN_THROWS();
 	}
 	if (ZSTR_LEN(domain) > INT32_MAX - 1) {
-		php_intl_bad_args("domain name too large");
-		RETURN_FALSE;
+		zend_argument_value_error(1, "must be less than " PRId32 " bytes", INT32_MAX);
+		RETURN_THROWS();
+	}
+	if (variant != INTL_IDN_VARIANT_UTS46) {
+		zend_argument_value_error(2, "must be INTL_IDNA_VARIANT_UTS46");
+		RETURN_THROWS();
 	}
 	/* don't check options; it wasn't checked before */
 

ext/intl/tests/idn_uts46_errors.phpt

@@ -13,10 +13,18 @@ ini_set("intl.error_level", E_WARNING);
 echo "=> PHP level errors", "\n";
 
 echo "bad variant:", "\n";
-var_dump(idn_to_ascii("", 0, INTL_IDNA_VARIANT_UTS46 + 10));
+try {
+    var_dump(idn_to_ascii("domain", 0, INTL_IDNA_VARIANT_UTS46 + 10));
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
 
 echo "empty domain:", "\n";
-var_dump(idn_to_ascii("", 0, INTL_IDNA_VARIANT_UTS46));
+try {
+    var_dump(idn_to_ascii("", 0, INTL_IDNA_VARIANT_UTS46));
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
 
 echo "with error, but no details arg:", "\n";
 var_dump(idn_to_ascii("www.fußball.com-", 0, INTL_IDNA_VARIANT_UTS46));
@@ -26,7 +34,7 @@ var_dump(idn_to_ascii("www.fußball.com-", IDNA_NONTRANSITIONAL_TO_ASCII,
     INTL_IDNA_VARIANT_UTS46, $foo));
 var_dump($foo);
 
-echo "with error, with details arg, contextj:", "\n";
+echo "with error, with details arg, context:", "\n";
 var_dump(idn_to_ascii(
         html_entity_decode("www.a‍b.com", 0, "UTF-8"),
         IDNA_NONTRANSITIONAL_TO_ASCII | IDNA_CHECK_CONTEXTJ,
@@ -35,16 +43,12 @@ var_dump($foo);
 var_dump($foo["errors"]==IDNA_ERROR_CONTEXTJ);
 
 ?>
---EXPECTF--
+--EXPECT--
 => PHP level errors
 bad variant:
-
-Warning: idn_to_ascii(): idn_to_ascii: invalid variant, must be INTL_IDNA_VARIANT_UTS46 in %s on line %d
-bool(false)
+ValueError: idn_to_ascii(): Argument #2 ($flags) must be INTL_IDNA_VARIANT_UTS46
 empty domain:
-
-Warning: idn_to_ascii(): idn_to_ascii: empty domain name in %s on line %d
-bool(false)
+ValueError: idn_to_ascii(): Argument #1 ($domain) cannot be empty
 with error, but no details arg:
 bool(false)
 with error, with details arg:
@@ -57,7 +61,7 @@ array(3) {
   ["errors"]=>
   int(16)
 }
-with error, with details arg, contextj:
+with error, with details arg, context:
 bool(false)
 array(3) {
   ["result"]=>