Skip to content

Commit 983a066

Browse files
westonruterwestonruter
committed
Code Quality: Improve typing for wp_parse_list() et al.
This adds precise `@param` and `@return` typing for the `wp_parse_list()`, `wp_parse_id_list()`, and `wp_parse_slug_list()` functions; native `array` return types are also added. It also adds casting and type guarding to guarantee the types of the values involved. Descriptions are updated to indicate that lists may not be returned, which may be unexpected given the function names; instead, sparse arrays or even associative arrays may be returned. Additionally, typically invalid ID values like zero may be in the array returned by `wp_parse_id_list()` and an empty string may be in the array returned by `wp_parse_slug_list()`. Tests are added to ensure existing behavior is preserved. This fixes 6 PHPStan errors at rule level 10. Developed in WordPress#12588. Follow-up to r38832, r44546, r57737, r62647, r62771. See #64898. git-svn-id: https://develop.svn.wordpress.org/trunk@62797 602fd350-edb4-49c9-b593-d223f7449a82
1 parent e35eb99 commit 983a066

4 files changed

Lines changed: 186 additions & 41 deletions

File tree

‎src/wp-includes/functions.php‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5021,12 +5021,15 @@ function wp_parse_args( $args, $defaults = array() ) {
50215021
*
50225022
* @since 5.1.0
50235023
*
5024-
* @param array|string $input_list List of values.
5025-
* @return array Array of values.
5024+
* @param mixed[]|string $input_list List of values.
5025+
* @return array Array of values. A string is split into a list, while an array
5026+
* keeps its keys, so the result is not necessarily a list.
5027+
* @phpstan-return ( $input_list is string ? list<string> : array<scalar> )
50265028
*/
5027-
function wp_parse_list( $input_list ) {
5029+
function wp_parse_list( $input_list ): array {
50285030
if ( ! is_array( $input_list ) ) {
5029-
return preg_split( '/[\s,]+/', $input_list, -1, PREG_SPLIT_NO_EMPTY );
5031+
$parsed_list = preg_split( '/[\s,]+/', $input_list, -1, PREG_SPLIT_NO_EMPTY );
5032+
return is_array( $parsed_list ) ? $parsed_list : array();
50305033
}
50315034

50325035
// Validate all entries of the list are scalar.
@@ -5041,10 +5044,13 @@ function wp_parse_list( $input_list ) {
50415044
* @since 3.0.0
50425045
* @since 5.1.0 Refactored to use wp_parse_list().
50435046
*
5044-
* @param array|string $input_list List of IDs.
5045-
* @return int[] Sanitized array of IDs.
5047+
* @param mixed[]|string $input_list List of IDs.
5048+
* @return int[] Sanitized array of IDs. May include zero. Keys are preserved
5049+
* from the input and `array_unique()` may leave gaps, so the
5050+
* result is not necessarily a list.
5051+
* @phpstan-return array<non-negative-int>
50465052
*/
5047-
function wp_parse_id_list( $input_list ) {
5053+
function wp_parse_id_list( $input_list ): array {
50485054
$input_list = wp_parse_list( $input_list );
50495055

50505056
return array_unique( array_map( 'absint', $input_list ) );
@@ -5056,13 +5062,27 @@ function wp_parse_id_list( $input_list ) {
50565062
* @since 4.7.0
50575063
* @since 5.1.0 Refactored to use wp_parse_list().
50585064
*
5059-
* @param array|string $input_list List of slugs.
5060-
* @return string[] Sanitized array of slugs.
5065+
* @param mixed[]|string $input_list List of slugs.
5066+
* @return string[] Sanitized array of slugs. May include an empty string. Keys
5067+
* are preserved from the input and `array_unique()` may leave
5068+
* gaps, so the result is not necessarily a list.
50615069
*/
5062-
function wp_parse_slug_list( $input_list ) {
5070+
function wp_parse_slug_list( $input_list ): array {
50635071
$input_list = wp_parse_list( $input_list );
50645072

5065-
return array_unique( array_map( 'sanitize_title', $input_list ) );
5073+
return array_unique(
5074+
array_map(
5075+
'sanitize_title',
5076+
array_map(
5077+
/*
5078+
* Cast booleans, integers, and floats to strings. Non-scalar types
5079+
* (including null) have already been filtered out by wp_parse_list().
5080+
*/
5081+
'strval',
5082+
$input_list
5083+
)
5084+
)
5085+
);
50665086
}
50675087

50685088
/**

‎tests/phpunit/tests/functions/wpParseIdList.php‎

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,31 @@ class Tests_Functions_wpParseIdList extends WP_UnitTestCase {
1515
*
1616
* @dataProvider data_wp_parse_id_list
1717
* @dataProvider data_unexpected_input
18+
*
19+
* @param mixed[]|string $input_list
20+
* @param array<non-negative-int> $expected
1821
*/
19-
public function test_wp_parse_id_list( $input_list, $expected ) {
20-
$this->assertSameSets( $expected, wp_parse_id_list( $input_list ) );
22+
public function test_wp_parse_id_list( $input_list, array $expected ): void {
23+
$parsed_list = wp_parse_id_list( $input_list );
24+
$this->assertThat(
25+
$parsed_list,
26+
$this->callback(
27+
static fn ( array $arr ) => array_all(
28+
$arr,
29+
static fn ( $v ) => is_int( $v ) && $v >= 0
30+
)
31+
),
32+
'Array should contain only non-negative ints.'
33+
);
34+
$this->assertSame( $expected, $parsed_list );
2135
}
2236

2337
/**
2438
* Data provider.
2539
*
26-
* @return array[]
40+
* @return array<string, array{ input_list: mixed[]|string, expected: array<non-negative-int> }>
2741
*/
28-
public function data_wp_parse_id_list() {
42+
public function data_wp_parse_id_list(): array {
2943
return array(
3044
'regular' => array(
3145
'input_list' => '1,2,3,4',
@@ -37,7 +51,12 @@ public function data_wp_parse_id_list() {
3751
),
3852
'duplicate id in a string' => array(
3953
'input_list' => '1,2,2,3,4',
40-
'expected' => array( 1, 2, 3, 4 ),
54+
'expected' => array(
55+
0 => 1,
56+
1 => 2,
57+
3 => 3,
58+
4 => 4,
59+
),
4160
),
4261
'duplicate id in an array' => array(
4362
'input_list' => array( '1', '2', '3', '4', '3' ),
@@ -61,9 +80,9 @@ public function data_wp_parse_id_list() {
6180
/**
6281
* Data provider.
6382
*
64-
* @return array[]
83+
* @return array<string, array{ input_list: mixed[]|string, expected: array<non-negative-int> }>
6584
*/
66-
public function data_unexpected_input() {
85+
public function data_unexpected_input(): array {
6786
return array(
6887
'string with commas' => array(
6988
'input_list' => '1,2,string with spaces',
@@ -97,6 +116,25 @@ public function data_unexpected_input() {
97116
'input_list' => array( 1, 2, false ),
98117
'expected' => array( 1, 2, 0 ),
99118
),
119+
'array with array' => array(
120+
'input_list' => array( 1, array(), 2 ),
121+
'expected' => array(
122+
0 => 1,
123+
2 => 2,
124+
),
125+
),
126+
'passed assoc array' => array(
127+
'input_list' => array(
128+
'one' => 1,
129+
'two' => '2',
130+
'three' => '3 is company',
131+
),
132+
'expected' => array(
133+
'one' => 1,
134+
'two' => 2,
135+
'three' => 3,
136+
),
137+
),
100138
);
101139
}
102140
}

‎tests/phpunit/tests/functions/wpParseList.php‎

Lines changed: 55 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,62 +13,102 @@ class Tests_Functions_wpParseList extends WP_UnitTestCase {
1313
* @ticket 43977
1414
*
1515
* @dataProvider data_wp_parse_list
16+
*
17+
* @param mixed[]|string $input_list
18+
* @param array<scalar> $expected
1619
*/
17-
public function test_wp_parse_list( $input_list, $expected ) {
18-
$this->assertSameSets( $expected, wp_parse_list( $input_list ) );
20+
public function test_wp_parse_list( $input_list, array $expected ): void {
21+
$parsed_list = wp_parse_list( $input_list );
22+
$this->assertThat(
23+
$parsed_list,
24+
$this->callback(
25+
static fn ( array $arr ) => array_all(
26+
$arr,
27+
static fn ( $v ) => is_scalar( $v )
28+
)
29+
),
30+
'Array should contain only scalars.'
31+
);
32+
$this->assertSame( $expected, $parsed_list );
1933
}
2034

2135
/**
2236
* Data provider.
2337
*
24-
* @return array[]
38+
* @return array<string, array{ input_list: mixed[]|string, expected: array<scalar> }>
2539
*/
26-
public function data_wp_parse_list() {
40+
public function data_wp_parse_list(): array {
2741
return array(
28-
'ids only' => array(
42+
'ids only' => array(
2943
'input_list' => '1,2,3,4',
3044
'expected' => array( '1', '2', '3', '4' ),
3145
),
32-
'slugs only' => array(
46+
'slugs only' => array(
3347
'input_list' => 'apple,banana,carrot,dog',
3448
'expected' => array( 'apple', 'banana', 'carrot', 'dog' ),
3549
),
36-
'ids and slugs' => array(
50+
'ids and slugs' => array(
3751
'input_list' => '1,2,apple,banana',
3852
'expected' => array( '1', '2', 'apple', 'banana' ),
3953
),
40-
'space after comma' => array(
54+
'space after comma' => array(
4155
'input_list' => '1, 2,apple,banana',
4256
'expected' => array( '1', '2', 'apple', 'banana' ),
4357
),
44-
'double comma' => array(
58+
'double comma' => array(
4559
'input_list' => '1,2,apple,,banana',
4660
'expected' => array( '1', '2', 'apple', 'banana' ),
4761
),
48-
'leading comma' => array(
62+
'leading comma' => array(
4963
'input_list' => ',1,2,apple,banana',
5064
'expected' => array( '1', '2', 'apple', 'banana' ),
5165
),
52-
'trailing comma' => array(
66+
'trailing comma' => array(
5367
'input_list' => '1,2,apple,banana,',
5468
'expected' => array( '1', '2', 'apple', 'banana' ),
5569
),
56-
'space before comma' => array(
70+
'space before comma' => array(
5771
'input_list' => '1,2 ,apple,banana',
5872
'expected' => array( '1', '2', 'apple', 'banana' ),
5973
),
60-
'empty string' => array(
74+
'empty string' => array(
6175
'input_list' => '',
6276
'expected' => array(),
6377
),
64-
'comma only' => array(
78+
'comma only' => array(
6579
'input_list' => ',',
6680
'expected' => array(),
6781
),
68-
'double comma only' => array(
82+
'double comma only' => array(
6983
'input_list' => ',,',
7084
'expected' => array(),
7185
),
86+
'passed scalar array' => array(
87+
'input_list' => array( 'foo', true, false, 1, 3.14 ),
88+
'expected' => array( 'foo', true, false, 1, 3.14 ),
89+
),
90+
'passed mixed array' => array(
91+
'input_list' => array( null, 'foo', array(), true, new stdClass(), false, 1, 3.14 ),
92+
'expected' => array(
93+
1 => 'foo',
94+
3 => true,
95+
5 => false,
96+
6 => 1,
97+
7 => 3.14,
98+
),
99+
),
100+
'passed assoc array' => array(
101+
'input_list' => array(
102+
'foo' => 1,
103+
'bar' => true,
104+
'baz' => 3.14,
105+
),
106+
'expected' => array(
107+
'foo' => 1,
108+
'bar' => true,
109+
'baz' => 3.14,
110+
),
111+
),
72112
);
73113
}
74114
}

‎tests/phpunit/tests/functions/wpParseSlugList.php‎

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,31 @@ class Tests_Functions_WpParseSlugList extends WP_UnitTestCase {
1515
*
1616
* @dataProvider data_wp_parse_slug_list
1717
* @dataProvider data_unexpected_input
18+
*
19+
* @param mixed[]|string $input_list
20+
* @param array<string> $expected
1821
*/
19-
public function test_wp_parse_slug_list( $input_list, $expected ) {
20-
$this->assertSameSets( $expected, wp_parse_slug_list( $input_list ) );
22+
public function test_wp_parse_slug_list( $input_list, array $expected ): void {
23+
$parsed_list = wp_parse_slug_list( $input_list );
24+
$this->assertThat(
25+
$parsed_list,
26+
$this->callback(
27+
static fn ( array $arr ) => array_all(
28+
$arr,
29+
static fn ( $v ) => is_string( $v )
30+
)
31+
),
32+
'Array should contain only strings.'
33+
);
34+
$this->assertSame( $expected, $parsed_list );
2135
}
2236

2337
/**
2438
* Data provider.
2539
*
26-
* @return array[]
40+
* @return array<string, array{ input_list: mixed[]|string, expected: array<string> }>
2741
*/
28-
public function data_wp_parse_slug_list() {
42+
public function data_wp_parse_slug_list(): array {
2943
return array(
3044
'regular' => array(
3145
'input_list' => 'apple,banana,carrot,dog',
@@ -37,11 +51,21 @@ public function data_wp_parse_slug_list() {
3751
),
3852
'duplicate slug in a string' => array(
3953
'input_list' => 'apple,banana,carrot,carrot,dog',
40-
'expected' => array( 'apple', 'banana', 'carrot', 'dog' ),
54+
'expected' => array(
55+
0 => 'apple',
56+
1 => 'banana',
57+
2 => 'carrot',
58+
4 => 'dog',
59+
),
4160
),
4261
'duplicate slug in an array' => array(
4362
'input_list' => array( 'apple', 'banana', 'carrot', 'carrot', 'dog' ),
44-
'expected' => array( 'apple', 'banana', 'carrot', 'dog' ),
63+
'expected' => array(
64+
0 => 'apple',
65+
1 => 'banana',
66+
2 => 'carrot',
67+
4 => 'dog',
68+
),
4569
),
4670
'string with spaces' => array(
4771
'input_list' => 'apple banana carrot dog',
@@ -51,15 +75,27 @@ public function data_wp_parse_slug_list() {
5175
'input_list' => array( 'apple ', 'banana carrot', 'd o g' ),
5276
'expected' => array( 'apple', 'banana-carrot', 'd-o-g' ),
5377
),
78+
'passed assoc array' => array(
79+
'input_list' => array(
80+
'one' => 'foo',
81+
'two' => 'bar',
82+
'three' => 'baz',
83+
),
84+
'expected' => array(
85+
'one' => 'foo',
86+
'two' => 'bar',
87+
'three' => 'baz',
88+
),
89+
),
5490
);
5591
}
5692

5793
/**
5894
* Data provider.
5995
*
60-
* @return array[]
96+
* @return array<string, array{ input_list: mixed[]|string, expected: array<string> }>
6197
*/
62-
public function data_unexpected_input() {
98+
public function data_unexpected_input(): array {
6399
return array(
64100
'string with commas' => array(
65101
'input_list' => '1,2,string with spaces',
@@ -93,6 +129,17 @@ public function data_unexpected_input() {
93129
'input_list' => array( 1, 2, false ),
94130
'expected' => array( '1', '2', '' ),
95131
),
132+
'array with array' => array(
133+
'input_list' => array( 1, array(), 2 ),
134+
'expected' => array(
135+
0 => '1',
136+
2 => '2',
137+
),
138+
),
139+
'array with tag' => array(
140+
'input_list' => array( 1, '<br>', 2 ),
141+
'expected' => array( '1', '', '2' ),
142+
),
96143
);
97144
}
98145
}

0 commit comments

Comments
 (0)