Skip to content

Commit

Permalink
Improve error messages mentioning parameters instead of arguments
Browse files Browse the repository at this point in the history
Closes GH-5999
  • Loading branch information
kocsismate committed Sep 9, 2020
1 parent c5b42be commit 9975986
Show file tree
Hide file tree
Showing 143 changed files with 473 additions and 452 deletions.
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 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 ZEND_COLD 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);
}
/* }}} */

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

0 comments on commit 9975986

Please sign in to comment.