From 509482a78a53625ffdc6d353cdfd69d4c7f1cd09 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 9 Jan 2020 12:21:02 +0100 Subject: [PATCH 1/2] Throw warning is required param follows optional --- Zend/tests/bug71428.3.phpt | 2 ++ Zend/tests/call_user_func_005.phpt | 1 + Zend/tests/traits/bug60717.phpt | 4 ++-- Zend/zend_compile.c | 6 +++++ .../tests/ReflectionClass_isArray.phpt | 8 ++----- ext/reflection/tests/ReflectionType_001.phpt | 24 +++++++++---------- ext/reflection/tests/bug62715.phpt | 3 ++- 7 files changed, 27 insertions(+), 21 deletions(-) diff --git a/Zend/tests/bug71428.3.phpt b/Zend/tests/bug71428.3.phpt index ab3cc9ae1601a..084209f9e9061 100644 --- a/Zend/tests/bug71428.3.phpt +++ b/Zend/tests/bug71428.3.phpt @@ -7,4 +7,6 @@ class B { public function m(A $a = NULL, $n) { echo "B.m";} }; class C extends B { public function m(A $a , $n) { echo "C.m";} }; ?> --EXPECTF-- +Deprecated: Required parameter $n follows optional parameter in %s on line %d + Fatal error: Declaration of C::m(A $a, $n) must be compatible with B::m(?A $a, $n) in %sbug71428.3.php on line 4 diff --git a/Zend/tests/call_user_func_005.phpt b/Zend/tests/call_user_func_005.phpt index fd130eb773a49..9a54c42545900 100644 --- a/Zend/tests/call_user_func_005.phpt +++ b/Zend/tests/call_user_func_005.phpt @@ -18,6 +18,7 @@ var_dump(call_user_func(array('foo', 'teste'))); ?> --EXPECTF-- +Deprecated: Required parameter $b follows optional parameter in %s on line %d string(1) "x" array(1) { [0]=> diff --git a/Zend/tests/traits/bug60717.phpt b/Zend/tests/traits/bug60717.phpt index 0f5cadb0661c2..1ea146c5a0a37 100644 --- a/Zend/tests/traits/bug60717.phpt +++ b/Zend/tests/traits/bug60717.phpt @@ -9,7 +9,7 @@ namespace HTML { function text($text); function attributes(array $attributes = null); - function textArea(array $attributes = null, $value); + function textArea(?array $attributes, $value); } trait TextUTF8 @@ -19,7 +19,7 @@ namespace HTML trait TextArea { - function textArea(array $attributes = null, $value) {} + function textArea(?array $attributes, $value) {} abstract function attributes(array $attributes = null); abstract function text($text); } diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 5a705c19475c4..581997e91b248 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -5679,6 +5679,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast) /* {{{ */ uint32_t i; zend_op_array *op_array = CG(active_op_array); zend_arg_info *arg_infos; + zend_bool have_optional_param = 0; if (return_type_ast) { /* Use op_array->arg_info[-1] for return type */ @@ -5747,10 +5748,15 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast) /* {{{ */ default_node.op_type = IS_CONST; zend_const_expr_to_zval(&default_node.u.constant, default_ast); CG(compiler_options) = cops; + have_optional_param = 1; } else { opcode = ZEND_RECV; default_node.op_type = IS_UNUSED; op_array->required_num_args = i + 1; + if (have_optional_param) { + zend_error(E_DEPRECATED, "Required parameter $%s follows optional parameter", + ZSTR_VAL(name)); + } } arg_info = &arg_infos[i]; diff --git a/ext/reflection/tests/ReflectionClass_isArray.phpt b/ext/reflection/tests/ReflectionClass_isArray.phpt index 3eec0dac54f5b..7c6093a55c9fe 100644 --- a/ext/reflection/tests/ReflectionClass_isArray.phpt +++ b/ext/reflection/tests/ReflectionClass_isArray.phpt @@ -4,7 +4,8 @@ public bool ReflectionParameter::isArray ( void ); marcosptf - - @phpsp - sao paulo - br --FILE-- getParameters() as $parameter) { } ?> --EXPECT-- -bool(false) -bool(false) bool(true) -bool(false) bool(true) bool(false) bool(false) -bool(true) -bool(false) diff --git a/ext/reflection/tests/ReflectionType_001.phpt b/ext/reflection/tests/ReflectionType_001.phpt index e31dc11348415..d0f327d0466ff 100644 --- a/ext/reflection/tests/ReflectionType_001.phpt +++ b/ext/reflection/tests/ReflectionType_001.phpt @@ -2,7 +2,7 @@ ReflectionParameter::get/hasType and ReflectionType tests --FILE-- getParameters() as $p) { } } ?> ---EXPECT-- +--EXPECTF-- +Deprecated: Required parameter $c follows optional parameter in %s on line %d bool(true) bool(true) bool(false) From b63436ab67e1644d3611fba91c89c45a7a845626 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 5 Feb 2020 10:45:15 +0100 Subject: [PATCH 2/2] Don't throw deprecation for poor-mans nullable types --- Zend/tests/bug71428.3.phpt | 2 -- Zend/tests/call_user_func_005.phpt | 2 +- Zend/tests/required_param_after_optional.phpt | 14 ++++++++++++++ Zend/zend_compile.c | 19 ++++++++++++++----- ext/reflection/tests/bug62715.phpt | 2 +- 5 files changed, 30 insertions(+), 9 deletions(-) create mode 100644 Zend/tests/required_param_after_optional.phpt diff --git a/Zend/tests/bug71428.3.phpt b/Zend/tests/bug71428.3.phpt index 084209f9e9061..ab3cc9ae1601a 100644 --- a/Zend/tests/bug71428.3.phpt +++ b/Zend/tests/bug71428.3.phpt @@ -7,6 +7,4 @@ class B { public function m(A $a = NULL, $n) { echo "B.m";} }; class C extends B { public function m(A $a , $n) { echo "C.m";} }; ?> --EXPECTF-- -Deprecated: Required parameter $n follows optional parameter in %s on line %d - Fatal error: Declaration of C::m(A $a, $n) must be compatible with B::m(?A $a, $n) in %sbug71428.3.php on line 4 diff --git a/Zend/tests/call_user_func_005.phpt b/Zend/tests/call_user_func_005.phpt index 9a54c42545900..2f5220db6291b 100644 --- a/Zend/tests/call_user_func_005.phpt +++ b/Zend/tests/call_user_func_005.phpt @@ -18,7 +18,7 @@ var_dump(call_user_func(array('foo', 'teste'))); ?> --EXPECTF-- -Deprecated: Required parameter $b follows optional parameter in %s on line %d +Deprecated: Required parameter $b follows optional parameter $a in %s on line %d string(1) "x" array(1) { [0]=> diff --git a/Zend/tests/required_param_after_optional.phpt b/Zend/tests/required_param_after_optional.phpt new file mode 100644 index 0000000000000..cd715e77d4819 --- /dev/null +++ b/Zend/tests/required_param_after_optional.phpt @@ -0,0 +1,14 @@ +--TEST-- +Required parameter after optional is deprecated +--FILE-- + +--EXPECTF-- +Deprecated: Required parameter $testC follows optional parameter $testA in %s on line %d + +Deprecated: Required parameter $test2C follows optional parameter $test2B in %s on line %d diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 581997e91b248..b2bb74a49cab5 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -5679,7 +5679,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast) /* {{{ */ uint32_t i; zend_op_array *op_array = CG(active_op_array); zend_arg_info *arg_infos; - zend_bool have_optional_param = 0; + zend_string *optional_param = NULL; if (return_type_ast) { /* Use op_array->arg_info[-1] for return type */ @@ -5748,14 +5748,23 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast) /* {{{ */ default_node.op_type = IS_CONST; zend_const_expr_to_zval(&default_node.u.constant, default_ast); CG(compiler_options) = cops; - have_optional_param = 1; + + if (!optional_param) { + /* Ignore parameters of the form "Type $param = null". + * This is the PHP 5 style way of writing "?Type $param", so allow it for now. */ + zend_bool is_implicit_nullable = + type_ast && Z_TYPE(default_node.u.constant) == IS_NULL; + if (!is_implicit_nullable) { + optional_param = name; + } + } } else { opcode = ZEND_RECV; default_node.op_type = IS_UNUSED; op_array->required_num_args = i + 1; - if (have_optional_param) { - zend_error(E_DEPRECATED, "Required parameter $%s follows optional parameter", - ZSTR_VAL(name)); + if (optional_param) { + zend_error(E_DEPRECATED, "Required parameter $%s follows optional parameter $%s", + ZSTR_VAL(name), ZSTR_VAL(optional_param)); } } diff --git a/ext/reflection/tests/bug62715.phpt b/ext/reflection/tests/bug62715.phpt index 52691d1588e56..63339cddc1ec1 100644 --- a/ext/reflection/tests/bug62715.phpt +++ b/ext/reflection/tests/bug62715.phpt @@ -17,7 +17,7 @@ foreach ($r->getParameters() as $p) { } ?> --EXPECTF-- -Deprecated: Required parameter $c follows optional parameter in %s on line %d +Deprecated: Required parameter $c follows optional parameter $b in %s on line %d bool(true) bool(true) bool(false)