Skip to content

Commit 68ef393

Browse files
committed
Fix missing "Optional parameter before required" deprecation on union null type
The check would only work for the ?type syntax, but not type|null. Switch to a check during type compilation instead. Fixes GH-11488 Closes GH-11497
1 parent a94216d commit 68ef393

File tree

3 files changed

+59
-25
lines changed

3 files changed

+59
-25
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ PHP NEWS
77
(nielsdos)
88
. Fixed oss-fuzz #60011 (Mis-compilation of by-reference nullsafe operator).
99
(ilutov)
10+
. Fixed GH-11488 (Missing "Optional parameter before required" deprecation on
11+
union null type). (ilutov)
1012

1113
- DOM:
1214
. Fixed bug GH-11500 (Namespace reuse in createElementNS() generates wrong

Zend/tests/gh11488.phpt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
GH-11488: "Optional parameter before required" warning for union nullable type
3+
--FILE--
4+
<?php
5+
function a(
6+
string|null $a = null,
7+
$b,
8+
) {}
9+
function b(
10+
Foo&Bar $c = null,
11+
$d,
12+
) {}
13+
function c(
14+
(Foo&Bar)|null $e = null,
15+
$f,
16+
) {}
17+
?>
18+
--EXPECTF--
19+
Deprecated: Optional parameter $a declared before required parameter $b is implicitly treated as a required parameter in %s on line %d
20+
21+
Deprecated: Optional parameter $e declared before required parameter $f is implicitly treated as a required parameter in %s on line %d

Zend/zend_compile.c

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6503,8 +6503,10 @@ static void zend_is_type_list_redundant_by_single_type(zend_type_list *type_list
65036503
}
65046504
}
65056505

6506-
static zend_type zend_compile_typename(
6507-
zend_ast *ast, bool force_allow_null) /* {{{ */
6506+
static zend_type zend_compile_typename(zend_ast *ast, bool force_allow_null);
6507+
6508+
static zend_type zend_compile_typename_ex(
6509+
zend_ast *ast, bool force_allow_null, bool *forced_allow_null) /* {{{ */
65086510
{
65096511
bool is_marked_nullable = ast->attr & ZEND_TYPE_NULLABLE;
65106512
zend_ast_attr orig_ast_attr = ast->attr;
@@ -6707,6 +6709,10 @@ static zend_type zend_compile_typename(
67076709
zend_error_noreturn(E_COMPILE_ERROR, "null cannot be marked as nullable");
67086710
}
67096711

6712+
if (force_allow_null && !is_marked_nullable && !(type_mask & MAY_BE_NULL)) {
6713+
*forced_allow_null = true;
6714+
}
6715+
67106716
if (is_marked_nullable || force_allow_null) {
67116717
ZEND_TYPE_FULL_MASK(type) |= MAY_BE_NULL;
67126718
type_mask = ZEND_TYPE_PURE_MASK(type);
@@ -6725,6 +6731,12 @@ static zend_type zend_compile_typename(
67256731
}
67266732
/* }}} */
67276733

6734+
static zend_type zend_compile_typename(zend_ast *ast, bool force_allow_null)
6735+
{
6736+
bool forced_allow_null;
6737+
return zend_compile_typename_ex(ast, force_allow_null, &forced_allow_null);
6738+
}
6739+
67286740
/* May convert value from int to float. */
67296741
static bool zend_is_valid_default_value(zend_type type, zval *value)
67306742
{
@@ -6954,28 +6966,6 @@ static void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32
69546966
zend_const_expr_to_zval(
69556967
&default_node.u.constant, default_ast_ptr, /* allow_dynamic */ true);
69566968
CG(compiler_options) = cops;
6957-
6958-
if (last_required_param != (uint32_t) -1 && i < last_required_param) {
6959-
/* Ignore parameters of the form "Type $param = null".
6960-
* This is the PHP 5 style way of writing "?Type $param", so allow it for now. */
6961-
bool is_implicit_nullable =
6962-
type_ast && !(type_ast->attr & ZEND_TYPE_NULLABLE)
6963-
&& Z_TYPE(default_node.u.constant) == IS_NULL;
6964-
if (!is_implicit_nullable) {
6965-
zend_ast *required_param_ast = list->child[last_required_param];
6966-
zend_error(E_DEPRECATED,
6967-
"Optional parameter $%s declared before required parameter $%s "
6968-
"is implicitly treated as a required parameter",
6969-
ZSTR_VAL(name), ZSTR_VAL(zend_ast_get_str(required_param_ast->child[1])));
6970-
}
6971-
6972-
/* Regardless of whether we issue a deprecation, convert this parameter into
6973-
* a required parameter without a default value. This ensures that it cannot be
6974-
* used as an optional parameter even with named parameters. */
6975-
opcode = ZEND_RECV;
6976-
default_node.op_type = IS_UNUSED;
6977-
zval_ptr_dtor(&default_node.u.constant);
6978-
}
69796969
} else {
69806970
opcode = ZEND_RECV;
69816971
default_node.op_type = IS_UNUSED;
@@ -6993,12 +6983,13 @@ static void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32
69936983
);
69946984
}
69956985

6986+
bool forced_allow_nullable = false;
69966987
if (type_ast) {
69976988
uint32_t default_type = *default_ast_ptr ? Z_TYPE(default_node.u.constant) : IS_UNDEF;
69986989
bool force_nullable = default_type == IS_NULL && !property_flags;
69996990

70006991
op_array->fn_flags |= ZEND_ACC_HAS_TYPE_HINTS;
7001-
arg_info->type = zend_compile_typename(type_ast, force_nullable);
6992+
arg_info->type = zend_compile_typename_ex(type_ast, force_nullable, &forced_allow_nullable);
70026993

70036994
if (ZEND_TYPE_FULL_MASK(arg_info->type) & MAY_BE_VOID) {
70046995
zend_error_noreturn(E_COMPILE_ERROR, "void cannot be used as a parameter type");
@@ -7017,6 +7008,26 @@ static void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32
70177008
ZSTR_VAL(name), ZSTR_VAL(type_str));
70187009
}
70197010
}
7011+
if (last_required_param != (uint32_t) -1
7012+
&& i < last_required_param
7013+
&& default_node.op_type == IS_CONST) {
7014+
/* Ignore parameters of the form "Type $param = null".
7015+
* This is the PHP 5 style way of writing "?Type $param", so allow it for now. */
7016+
if (!forced_allow_nullable) {
7017+
zend_ast *required_param_ast = list->child[last_required_param];
7018+
zend_error(E_DEPRECATED,
7019+
"Optional parameter $%s declared before required parameter $%s "
7020+
"is implicitly treated as a required parameter",
7021+
ZSTR_VAL(name), ZSTR_VAL(zend_ast_get_str(required_param_ast->child[1])));
7022+
}
7023+
7024+
/* Regardless of whether we issue a deprecation, convert this parameter into
7025+
* a required parameter without a default value. This ensures that it cannot be
7026+
* used as an optional parameter even with named parameters. */
7027+
opcode = ZEND_RECV;
7028+
default_node.op_type = IS_UNUSED;
7029+
zval_ptr_dtor(&default_node.u.constant);
7030+
}
70207031

70217032
opline = zend_emit_op(NULL, opcode, NULL, &default_node);
70227033
SET_NODE(opline->result, &var_node);

0 commit comments

Comments
 (0)