Skip to content

Commit ce9c0ef

Browse files
alganetalganet
committed
Align array_pop() and array_shift() to official PHP behavior
1 parent 2feefab commit ce9c0ef

19 files changed

+230
-62
lines changed

‎src/ph7/hashmap.c‎

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2561,20 +2561,33 @@ static int ph7_hashmap_key_exists(ph7_context *pCtx,int nArg,ph7_value **apArg)
25612561
static int ph7_hashmap_pop(ph7_context *pCtx,int nArg,ph7_value **apArg)
25622562
{
25632563
ph7_hashmap *pMap;
2564-
if( nArg < 1 ){
2565-
/* Missing arguments,return null */
2566-
ph7_result_null(pCtx);
2567-
return PH7_OK;
2564+
/* PHP requires exactly one argument and it must be passed by reference */
2565+
if( nArg != 1 ){
2566+
return PH7_VmThrowException(pCtx,
2567+
"ArgumentCountError",
2568+
"array_pop() expects exactly 1 argument, %d given",
2569+
nArg
2570+
);
2571+
}
2572+
/* Passing a constant (including literals) or non-variable triggers the same
2573+
* error message as official PHP. Check the index to detect constants. */
2574+
if( apArg[0]->nIdx == SXU32_HIGH ){
2575+
return PH7_VmThrowException(pCtx,
2576+
"Error",
2577+
"array_pop(): Argument #1 ($array) could not be passed by reference"
2578+
);
25682579
}
25692580
/* Make sure we are dealing with a valid hashmap */
25702581
if( !ph7_value_is_array(apArg[0]) ){
2571-
/* Invalid argument,return null */
2572-
ph7_result_null(pCtx);
2573-
return PH7_OK;
2582+
return PH7_VmThrowException(pCtx,
2583+
"TypeError",
2584+
"array_pop(): Argument #1 ($array) must be of type array, %s given",
2585+
ph7_type_name(apArg[0])
2586+
);
25742587
}
25752588
pMap = (ph7_hashmap *)apArg[0]->x.pOther;
25762589
if( pMap->nEntry < 1 ){
2577-
/* Noting to pop,return NULL */
2590+
/* Nothing to pop,return NULL */
25782591
ph7_result_null(pCtx);
25792592
}else{
25802593
ph7_hashmap_node *pLast = pMap->pLast;
@@ -2644,16 +2657,28 @@ static int ph7_hashmap_push(ph7_context *pCtx,int nArg,ph7_value **apArg)
26442657
static int ph7_hashmap_shift(ph7_context *pCtx,int nArg,ph7_value **apArg)
26452658
{
26462659
ph7_hashmap *pMap;
2647-
if( nArg < 1 ){
2648-
/* Missing arguments,return null */
2649-
ph7_result_null(pCtx);
2650-
return PH7_OK;
2660+
/* PHP requires exactly one argument and it must be passed by reference */
2661+
if( nArg != 1 ){
2662+
return PH7_VmThrowException(pCtx,
2663+
"ArgumentCountError",
2664+
"array_shift() expects exactly 1 argument, %d given",
2665+
nArg
2666+
);
2667+
}
2668+
/* Detect constants or literals, which cannot be passed by reference. */
2669+
if( apArg[0]->nIdx == SXU32_HIGH ){
2670+
return PH7_VmThrowException(pCtx,
2671+
"Error",
2672+
"array_shift(): Argument #1 ($array) could not be passed by reference"
2673+
);
26512674
}
26522675
/* Make sure we are dealing with a valid hashmap */
26532676
if( !ph7_value_is_array(apArg[0]) ){
2654-
/* Invalid argument,return null */
2655-
ph7_result_null(pCtx);
2656-
return PH7_OK;
2677+
return PH7_VmThrowException(pCtx,
2678+
"TypeError",
2679+
"array_shift(): Argument #1 ($array) must be of type array, %s given",
2680+
ph7_type_name(apArg[0])
2681+
);
26572682
}
26582683
/* Point to the internal representation of the hashmap */
26592684
pMap = (ph7_hashmap *)apArg[0]->x.pOther;

‎tests/ph7/001-smoke/function/array_pop/array_pop.phpt‎

Lines changed: 0 additions & 24 deletions
This file was deleted.

‎tests/ph7/001-smoke/function/array_pop/array_pop_empty.phpt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
--CREDITS--
2+
SPDX-FileCopyrightText: 2025 Alexandre Gomes Gaigalas <alganet@gmail.com>
3+
SPDX-License-Identifier: BSD-3-Clause
4+
--TEST--
5+
array_pop on an empty array returns NULL
6+
--FILE--
7+
<?php
8+
$empty = array();
9+
$val = array_pop($empty);
10+
echo (is_null($val) ? 'NULL' : $val) . PHP_EOL;
11+
?>
12+
--EXPECT--
13+
NULL

‎tests/ph7/001-smoke/function/array_pop/array_pop_mutate.phpt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
--CREDITS--
2+
SPDX-FileCopyrightText: 2025 Alexandre Gomes Gaigalas <alganet@gmail.com>
3+
SPDX-License-Identifier: BSD-3-Clause
4+
--TEST--
5+
array_pop modifies the original array by removing its last element
6+
--FILE--
7+
<?php
8+
$a = array('x','y','z');
9+
array_pop($a);
10+
echo implode(',', $a) . PHP_EOL;
11+
?>
12+
--EXPECT--
13+
x,y

‎tests/ph7/001-smoke/function/array_pop/array_pop_return.phpt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--CREDITS--
2+
SPDX-FileCopyrightText: 2025 Alexandre Gomes Gaigalas <alganet@gmail.com>
3+
SPDX-License-Identifier: BSD-3-Clause
4+
--TEST--
5+
array_pop returns the last element of a non-empty array
6+
--FILE--
7+
<?php
8+
$a = array('x','y','z');
9+
echo array_pop($a) . PHP_EOL;
10+
?>
11+
--EXPECT--
12+
z

‎tests/ph7/001-smoke/function/array_shift/array_shift.phpt‎

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎tests/ph7/001-smoke/function/array_shift/array_shift_empty.phpt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
--CREDITS--
2+
SPDX-FileCopyrightText: 2025 Alexandre Gomes Gaigalas <alganet@gmail.com>
3+
SPDX-License-Identifier: BSD-3-Clause
4+
--TEST--
5+
array_shift on an empty array returns NULL
6+
--FILE--
7+
<?php
8+
$b = array();
9+
$val = array_shift($b);
10+
echo (is_null($val) ? 'NULL' : $val) . PHP_EOL;
11+
?>
12+
--EXPECT--
13+
NULL

‎tests/ph7/001-smoke/function/array_shift/array_shift_mutate.phpt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
--CREDITS--
2+
SPDX-FileCopyrightText: 2025 Alexandre Gomes Gaigalas <alganet@gmail.com>
3+
SPDX-License-Identifier: BSD-3-Clause
4+
--TEST--
5+
array_shift modifies the original array by removing the first element
6+
--FILE--
7+
<?php
8+
$a = array('first','second','third');
9+
array_shift($a);
10+
echo implode(',', $a) . PHP_EOL;
11+
?>
12+
--EXPECT--
13+
second,third

‎tests/ph7/001-smoke/function/array_shift/array_shift_return.phpt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--CREDITS--
2+
SPDX-FileCopyrightText: 2025 Alexandre Gomes Gaigalas <alganet@gmail.com>
3+
SPDX-License-Identifier: BSD-3-Clause
4+
--TEST--
5+
array_shift returns first element of a non-empty array
6+
--FILE--
7+
<?php
8+
$a = array('first','second','third');
9+
echo array_shift($a) . PHP_EOL;
10+
?>
11+
--EXPECT--
12+
first

‎tests/ph7/002-integration/function/array_pop/array_pop_extra_args.phpt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--CREDITS--
2+
SPDX-FileCopyrightText: 2025 Alexandre Gomes Gaigalas <alganet@gmail.com>
3+
SPDX-License-Identifier: BSD-3-Clause
4+
--TEST--
5+
Calling array_pop with too many arguments triggers ArgumentCountError
6+
--FILE--
7+
<?php
8+
$a = array(1,2);
9+
array_pop($a, 123);
10+
?>
11+
--EXPECTF--
12+
%s Fatal error: Uncaught ArgumentCountError: array_pop() expects exactly 1 argument, %d given in %s

0 commit comments

Comments
 (0)