Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error messages mentioning parameters instead of arguments #5999

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,10 @@ PHP 8.0 UPGRADE NOTES
. Uncaught exceptions now go through "clean shutdown", which means that
destructors will be called after an uncaught exception.
. Compile time fatal error "Only variables can be passed by reference" has
been delayed until runtime and converted to "Cannot pass parameter by
been delayed until runtime and converted to "Argument cannot be passed by
reference" exception.
. Some "Only variables should be passed by reference" notices have been
converted to "Cannot pass parameter by reference" exception.
converted to "Argument cannot be passed by reference" exception.
. The generated name for anonymous classes has changed. It will now include
the name of the first parent or interface:

Expand Down
2 changes: 1 addition & 1 deletion Zend/tests/bug46106.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ try {
?>
DONE
--EXPECT--
str_pad() expects at least 2 parameters, 1 given
str_pad() expects at least 2 arguments, 1 given
DONE
2 changes: 1 addition & 1 deletion Zend/tests/bug51827.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ register_shutdown_function('exploDe');
--EXPECT--
int(1)

Fatal error: Uncaught ArgumentCountError: explode() expects at least 2 parameters, 0 given in [no active file]:0
Fatal error: Uncaught ArgumentCountError: explode() expects at least 2 arguments, 0 given in [no active file]:0
Stack trace:
#0 [internal function]: explode()
#1 {main}
Expand Down
21 changes: 9 additions & 12 deletions Zend/tests/bug72038.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,25 @@ Bug #72038 (Function calls with values to a by-ref parameter don't always throw
try {
test($foo = new stdClass);
var_dump($foo);
} catch (Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
} catch (Error $e) {
echo $e->getMessage() . "\n";
}
try {
test($bar = 2);
var_dump($bar);
} catch (Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
}
try {
test($baz = &$bar);
var_dump($baz);
} catch (Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
} catch (Error $e) {
echo $e->getMessage() . "\n";
}

test($baz = &$bar);
var_dump($baz);

function test(&$param) {
$param = 1;
}

?>
--EXPECT--
Exception: Cannot pass parameter 1 by reference
Exception: Cannot pass parameter 1 by reference
test(): Argument #1 ($param) cannot be passed by reference
test(): Argument #1 ($param) cannot be passed by reference
int(1)
2 changes: 1 addition & 1 deletion Zend/tests/bug72107.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ try {
}
?>
--EXPECT--
func_get_args() expects exactly 0 parameters, 4 given
func_get_args() expects exactly 0 arguments, 4 given
2 changes: 1 addition & 1 deletion Zend/tests/bug73663_2.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ change(list($val) = $array);
var_dump($array);
?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Fatal error: Uncaught Error: change(): Argument #1 ($ref) cannot be passed by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
4 changes: 2 additions & 2 deletions Zend/tests/bug78154.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ namespace Foo {

?>
--EXPECT--
Exception: Cannot pass parameter 3 by reference
Exception: Cannot pass parameter 3 by reference
Exception: similar_text(): Argument #3 ($percent) cannot be passed by reference
Exception: similar_text(): Argument #3 ($percent) cannot be passed by reference
2 changes: 1 addition & 1 deletion Zend/tests/bug79783.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Bug #79783: Segfault in php_str_replace_common
str_replace("a", "b", "c", strlen("d"));
?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot pass parameter 4 by reference in %s:%d
Fatal error: Uncaught Error: str_replace(): Argument #4 ($replace_count) cannot be passed by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
2 changes: 1 addition & 1 deletion Zend/tests/closure_019.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ int(9)
Notice: Only variable references should be returned by reference in %sclosure_019.php on line 4
int(81)

Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Fatal error: Uncaught Error: {closure}(): Argument #1 ($x) cannot be passed by reference in %s:%d
Stack trace:
#0 %s(%d): test()
#1 {main}
Expand Down
2 changes: 1 addition & 1 deletion Zend/tests/errmsg_022.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ foo(1);
echo "Done\n";
?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Fatal error: Uncaught Error: foo(): Argument #1 ($var) cannot be passed by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ try {
}
?>
--EXPECT--
substr() expects at least 2 parameters, 1 given
substr() expects at least 2 arguments, 1 given
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ try {
?>
--EXPECT--
ArgumentCountError
substr() expects at least 2 parameters, 1 given
substr() expects at least 2 arguments, 1 given
ArgumentCountError
At least 2 parameters are required, 1 given
At least 2 arguments are required, 1 given
2 changes: 1 addition & 1 deletion Zend/tests/generators/errors/count_error.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ try {

?>
--EXPECTF--
Warning: count(): Parameter must be an array or an object that implements Countable in %s on line %d
Warning: count(): Argument #1 ($var) must be of type Countable|array, Generator given in %s on line %d
6 changes: 3 additions & 3 deletions Zend/tests/match/027.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ main();
usesValue 0
i is 0

Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s027.php:20
Fatal error: Uncaught Error: Test::usesRef(): Argument #1 ($x) cannot be passed by reference in %s:%d
Stack trace:
#0 %s027.php(23): main()
#0 %s(%d): main()
#1 {main}
thrown in %s027.php on line 20
thrown in %s on line %d
2 changes: 1 addition & 1 deletion Zend/tests/match/028.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ try {
usesValue 42
usesValue 42
int(42)
Caught Cannot pass parameter 1 by reference
Caught Test::usesRef(): Argument #1 ($x) cannot be passed by reference
2 changes: 1 addition & 1 deletion Zend/tests/named_params/cannot_pass_by_ref.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ try {
}
?>
--EXPECT--
Cannot pass parameter 2 by reference
test(): Argument #2 ($e) cannot be passed by reference
6 changes: 3 additions & 3 deletions Zend/tests/nullsafe_operator/013.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ int(0)
string(98) "call_user_func_array(): Argument #1 ($function) must be a valid callback, no array or string given"
string(77) "call_user_func_array(): Argument #2 ($args) must be of type array, null given"
string(69) "get_class(): Argument #1 ($object) must be of type object, null given"
string(56) "get_called_class() expects exactly 0 parameters, 1 given"
string(55) "get_called_class() expects exactly 0 arguments, 1 given"
string(4) "NULL"
string(53) "func_num_args() expects exactly 0 parameters, 1 given"
string(53) "func_get_args() expects exactly 0 parameters, 1 given"
string(52) "func_num_args() expects exactly 0 arguments, 1 given"
string(52) "func_get_args() expects exactly 0 arguments, 1 given"
string(69) "array_slice(): Argument #1 ($array) must be of type array, null given"
array(1) {
[0]=>
Expand Down
8 changes: 4 additions & 4 deletions Zend/tests/nullsafe_operator/016.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ test(new Foo());

?>
--EXPECT--
Cannot pass parameter 1 by reference
Cannot pass parameter 1 by reference
Cannot pass parameter 1 by reference
Cannot pass parameter 1 by reference
set(): Argument #1 ($ref) cannot be passed by reference
set(): Argument #1 ($ref) cannot be passed by reference
set(): Argument #1 ($ref) cannot be passed by reference
set(): Argument #1 ($ref) cannot be passed by reference
2 changes: 1 addition & 1 deletion Zend/tests/variadic/by_ref_error.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ test(1);

?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Fatal error: Uncaught Error: test(): Argument #1 cannot be passed by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
63 changes: 30 additions & 33 deletions Zend/zend_API.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,33 +183,29 @@ ZEND_API zend_string *zend_zval_get_legacy_type(const zval *arg) /* {{{ */
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_parameters_none_error(void) /* {{{ */
{
int num_args = ZEND_CALL_NUM_ARGS(EG(current_execute_data));
zend_function *active_function = EG(current_execute_data)->func;
const char *class_name = active_function->common.scope ? ZSTR_VAL(active_function->common.scope->name) : "";
zend_string *func_name = get_active_function_or_method_name();

zend_argument_count_error(
"%s%s%s() expects exactly 0 parameters, %d given",
class_name, \
class_name[0] ? "::" : "", \
ZSTR_VAL(active_function->common.function_name),
num_args);
zend_argument_count_error("%s() expects exactly 0 arguments, %d given", ZSTR_VAL(func_name), num_args);

zend_string_release(func_name);
}
/* }}} */

ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_parameters_count_error(uint32_t min_num_args, uint32_t max_num_args) /* {{{ */
{
uint32_t num_args = ZEND_CALL_NUM_ARGS(EG(current_execute_data));
zend_function *active_function = EG(current_execute_data)->func;
const char *class_name = active_function->common.scope ? ZSTR_VAL(active_function->common.scope->name) : "";
zend_string *func_name = get_active_function_or_method_name();

zend_argument_count_error(
"%s%s%s() expects %s %d parameter%s, %d given",
class_name, \
class_name[0] ? "::" : "", \
ZSTR_VAL(active_function->common.function_name),
min_num_args == max_num_args ? "exactly" : num_args < min_num_args ? "at least" : "at most",
num_args < min_num_args ? min_num_args : max_num_args,
(num_args < min_num_args ? min_num_args : max_num_args) == 1 ? "" : "s",
num_args);
"%s() expects %s %d argument%s, %d given",
ZSTR_VAL(func_name),
min_num_args == max_num_args ? "exactly" : num_args < min_num_args ? "at least" : "at most",
num_args < min_num_args ? min_num_args : max_num_args,
(num_args < min_num_args ? min_num_args : max_num_args) == 1 ? "" : "s",
num_args
);

zend_string_release(func_name);
}
/* }}} */

Expand Down Expand Up @@ -325,23 +321,23 @@ ZEND_API ZEND_COLD void ZEND_FASTCALL zend_unexpected_extra_named_error(void)

static ZEND_COLD void ZEND_FASTCALL zend_argument_error_variadic(zend_class_entry *error_ce, uint32_t arg_num, const char *format, va_list va) /* {{{ */
{
const char *space;
const char *class_name;
zend_string *func_name;
const char *arg_name;
char *message = NULL;
if (EG(exception)) {
return;
}

class_name = get_active_class_name(&space);
func_name = get_active_function_or_method_name();
arg_name = get_active_function_arg_name(arg_num);

zend_vspprintf(&message, 0, format, va);
zend_throw_error(error_ce, "%s%s%s(): Argument #%d%s%s%s %s",
class_name, space, get_active_function_name(), arg_num,
zend_throw_error(error_ce, "%s(): Argument #%d%s%s%s %s",
ZSTR_VAL(func_name), arg_num,
arg_name ? " ($" : "", arg_name ? arg_name : "", arg_name ? ")" : "", message
);
efree(message);
zend_string_release(func_name);
}
/* }}} */

Expand Down Expand Up @@ -1005,16 +1001,17 @@ static zend_result zend_parse_va_args(uint32_t num_args, const char *type_spec,

if (num_args < min_num_args || num_args > max_num_args) {
if (!(flags & ZEND_PARSE_PARAMS_QUIET)) {
zend_function *active_function = EG(current_execute_data)->func;
const char *class_name = active_function->common.scope ? ZSTR_VAL(active_function->common.scope->name) : "";
zend_argument_count_error("%s%s%s() expects %s %d parameter%s, %d given",
class_name,
class_name[0] ? "::" : "",
ZSTR_VAL(active_function->common.function_name),
min_num_args == max_num_args ? "exactly" : num_args < min_num_args ? "at least" : "at most",
num_args < min_num_args ? min_num_args : max_num_args,
(num_args < min_num_args ? min_num_args : max_num_args) == 1 ? "" : "s",
num_args);
zend_string *func_name = get_active_function_or_method_name();

zend_argument_count_error("%s() expects %s %d argument%s, %d given",
ZSTR_VAL(func_name),
min_num_args == max_num_args ? "exactly" : num_args < min_num_args ? "at least" : "at most",
num_args < min_num_args ? min_num_args : max_num_args,
(num_args < min_num_args ? min_num_args : max_num_args) == 1 ? "" : "s",
num_args
);

zend_string_release(func_name);
}
return FAILURE;
}
Expand Down
13 changes: 13 additions & 0 deletions Zend/zend_execute.c
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,19 @@ static zend_never_inline ZEND_COLD bool zend_wrong_assign_to_variable_reference(
return 1;
}

ZEND_API void zend_cannot_pass_by_reference(uint32_t arg_num)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ZEND_API void zend_cannot_pass_by_reference(uint32_t arg_num)
ZEND_API ZEND_COLD void zend_cannot_pass_by_reference(uint32_t arg_num)

{
const zend_execute_data *execute_data = EG(current_execute_data);
zend_string *func_name = get_function_or_method_name(EX(call)->func);
const char *param_name = get_function_arg_name(EX(call)->func, arg_num);

zend_throw_error(NULL, "%s(): Argument #%d%s%s%s cannot be passed by reference",
ZSTR_VAL(func_name), arg_num, param_name ? " ($" : "", param_name ? param_name : "", param_name ? ")" : ""
);

zend_string_release(func_name);
}

static zend_never_inline ZEND_COLD void zend_throw_auto_init_in_prop_error(zend_property_info *prop, const char *type) {
zend_string *type_str = zend_type_to_string(prop->type);
zend_type_error(
Expand Down
3 changes: 3 additions & 0 deletions Zend/zend_execute.h
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,14 @@ ZEND_API const char *get_active_class_name(const char **space);
ZEND_API const char *get_active_function_name(void);
ZEND_API const char *get_active_function_arg_name(uint32_t arg_num);
ZEND_API const char *get_function_arg_name(const zend_function *func, uint32_t arg_num);
ZEND_API zend_string *get_active_function_or_method_name();
ZEND_API zend_string *get_function_or_method_name(const zend_function *func);
ZEND_API const char *zend_get_executed_filename(void);
ZEND_API zend_string *zend_get_executed_filename_ex(void);
ZEND_API uint32_t zend_get_executed_lineno(void);
ZEND_API zend_class_entry *zend_get_executed_scope(void);
ZEND_API zend_bool zend_is_executing(void);
ZEND_API void zend_cannot_pass_by_reference(uint32_t arg_num);

ZEND_API void zend_set_timeout(zend_long seconds, bool reset_signals);
ZEND_API void zend_unset_timeout(void);
Expand Down
21 changes: 21 additions & 0 deletions Zend/zend_execute_API.c
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ ZEND_API const char *get_active_class_name(const char **space) /* {{{ */
}

func = EG(current_execute_data)->func;

switch (func->type) {
case ZEND_USER_FUNCTION:
case ZEND_INTERNAL_FUNCTION:
Expand Down Expand Up @@ -470,7 +471,9 @@ ZEND_API const char *get_active_function_name(void) /* {{{ */
if (!zend_is_executing()) {
return NULL;
}

func = EG(current_execute_data)->func;

switch (func->type) {
case ZEND_USER_FUNCTION: {
zend_string *function_name = func->common.function_name;
Expand All @@ -491,6 +494,24 @@ ZEND_API const char *get_active_function_name(void) /* {{{ */
}
/* }}} */

ZEND_API zend_string *get_active_function_or_method_name(void) /* {{{ */
{
ZEND_ASSERT(zend_is_executing());

return get_function_or_method_name(EG(current_execute_data)->func);
}
/* }}} */

ZEND_API zend_string *get_function_or_method_name(const zend_function *func) /* {{{ */
{
if (func->common.scope) {
return zend_create_member_string(func->common.scope->name, func->common.function_name);
}

return func->common.function_name ? zend_string_copy(func->common.function_name) : zend_string_init("main", sizeof("main") - 1, 0);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or alternatively, should we could add an assert that function_name is not null?

}
/* }}} */

ZEND_API const char *get_active_function_arg_name(uint32_t arg_num) /* {{{ */
{
zend_function *func;
Expand Down
5 changes: 3 additions & 2 deletions Zend/zend_vm_def.h
Original file line number Diff line number Diff line change
Expand Up @@ -4596,7 +4596,8 @@ ZEND_VM_COLD_HELPER(zend_cannot_pass_by_ref_helper, ANY, ANY, uint32_t _arg_num,
USE_OPLINE

SAVE_OPLINE();
zend_throw_error(NULL, "Cannot pass parameter %d by reference", _arg_num);

zend_cannot_pass_by_reference(_arg_num);
FREE_OP1();
ZVAL_UNDEF(_arg);
HANDLE_EXCEPTION();
Expand Down Expand Up @@ -8918,7 +8919,7 @@ ZEND_VM_COLD_CONST_HANDLER(190, ZEND_COUNT, CONST|TMPVAR|CV, UNUSED)
} else {
count = 1;
}
zend_error(E_WARNING, "%s(): Parameter must be an array or an object that implements Countable", opline->extended_value ? "sizeof" : "count");
zend_error(E_WARNING, "%s(): Argument #1 ($var) must be of type Countable|array, %s given", opline->extended_value ? "sizeof" : "count", zend_zval_type_name(op1));
break;
}

Expand Down
Loading