Skip to content

Commit 8227299

Browse files
staabmphpstan-bot
authored andcommitted
Mark array_filter(), array_map(), array_reduce() and sibling non-mutating callback functions as having no side effects
- Add `hasSideEffects => false` entries for `array_filter`, `array_map` and `array_reduce` to `bin/functionMetadata_original.php` and the generated `resources/functionMetadata.php`. These functions do not mutate their arguments and have no side effects of their own; any side effects come from the supplied callback, whose impure points already propagate to the caller because callable parameters of functions are treated as immediately invoked. - Sweep the rest of the non-mutating array callback family that had the same problem: `array_find`, `array_find_key`, `array_any`, `array_all`. - Also mark the analogous `preg_replace_callback` and `preg_replace_callback_array` as side-effect-free. - An impure callback (e.g. one that echoes) still suppresses the "no effect" report because its impure points are merged into the statement; only genuinely pure callbacks make the whole call a no-op. - Update `tests/.../data/file-without-errors.php` and `tests/.../data/discussion-7124.php` so they use the result of the now-pure calls instead of relying on the previous (buggy) behavior where these calls were never reported. - Considered but deliberately excluded `call_user_func`/`call_user_func_array`: they are the general-purpose invocation primitive rather than part of the array-callback family; the propagation machinery would make them safe, but the behavior change is broader than this issue.
1 parent 1fdf7f9 commit 8227299

7 files changed

Lines changed: 133 additions & 9 deletions

File tree

bin/functionMetadata_original.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
'apcu_key_info' => ['hasSideEffects' => true],
2121
'apcu_sma_info' => ['hasSideEffects' => true],
2222
'apcu_store' => ['hasSideEffects' => true],
23+
'array_all' => ['hasSideEffects' => false],
24+
'array_any' => ['hasSideEffects' => false],
2325
'array_change_key_case' => ['hasSideEffects' => false],
2426
'array_chunk' => ['hasSideEffects' => false],
2527
'array_column' => ['hasSideEffects' => false],
@@ -32,6 +34,9 @@
3234
'array_diff_ukey' => ['hasSideEffects' => false],
3335
'array_fill' => ['hasSideEffects' => false],
3436
'array_fill_keys' => ['hasSideEffects' => false],
37+
'array_filter' => ['hasSideEffects' => false],
38+
'array_find' => ['hasSideEffects' => false],
39+
'array_find_key' => ['hasSideEffects' => false],
3540
'array_flip' => ['hasSideEffects' => false],
3641
'array_intersect' => ['hasSideEffects' => false],
3742
'array_intersect_assoc' => ['hasSideEffects' => false],
@@ -42,13 +47,15 @@
4247
'array_key_last' => ['hasSideEffects' => false],
4348
'array_key_exists' => ['hasSideEffects' => false],
4449
'array_keys' => ['hasSideEffects' => false],
50+
'array_map' => ['hasSideEffects' => false],
4551
'array_merge' => ['hasSideEffects' => false],
4652
'array_merge_recursive' => ['hasSideEffects' => false],
4753
'array_pad' => ['hasSideEffects' => false],
4854
'array_pop' => ['hasSideEffects' => true],
4955
'array_product' => ['hasSideEffects' => false],
5056
'array_push' => ['hasSideEffects' => true],
5157
'array_rand' => ['hasSideEffects' => false],
58+
'array_reduce' => ['hasSideEffects' => false],
5259
'array_replace' => ['hasSideEffects' => false],
5360
'array_replace_recursive' => ['hasSideEffects' => false],
5461
'array_reverse' => ['hasSideEffects' => false],
@@ -237,6 +244,8 @@
237244
'output_reset_rewrite_vars' => ['hasSideEffects' => true],
238245
'pclose' => ['hasSideEffects' => true],
239246
'popen' => ['hasSideEffects' => true],
247+
'preg_replace_callback' => ['hasSideEffects' => false],
248+
'preg_replace_callback_array' => ['hasSideEffects' => false],
240249
'readfile' => ['hasSideEffects' => true],
241250
'rename' => ['hasSideEffects' => true],
242251
'rewind' => ['hasSideEffects' => true],

resources/functionMetadata.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,8 @@
725725
'apcu_key_info' => ['hasSideEffects' => true],
726726
'apcu_sma_info' => ['hasSideEffects' => true],
727727
'apcu_store' => ['hasSideEffects' => true],
728+
'array_all' => ['hasSideEffects' => false],
729+
'array_any' => ['hasSideEffects' => false],
728730
'array_change_key_case' => ['hasSideEffects' => false],
729731
'array_chunk' => ['hasSideEffects' => false],
730732
'array_column' => ['hasSideEffects' => false],
@@ -737,6 +739,9 @@
737739
'array_diff_ukey' => ['hasSideEffects' => false],
738740
'array_fill' => ['hasSideEffects' => false],
739741
'array_fill_keys' => ['hasSideEffects' => false],
742+
'array_filter' => ['hasSideEffects' => false],
743+
'array_find' => ['hasSideEffects' => false],
744+
'array_find_key' => ['hasSideEffects' => false],
740745
'array_first' => ['hasSideEffects' => false],
741746
'array_flip' => ['hasSideEffects' => false],
742747
'array_intersect' => ['hasSideEffects' => false],
@@ -750,13 +755,15 @@
750755
'array_key_last' => ['hasSideEffects' => false],
751756
'array_keys' => ['hasSideEffects' => false],
752757
'array_last' => ['hasSideEffects' => false],
758+
'array_map' => ['hasSideEffects' => false],
753759
'array_merge' => ['hasSideEffects' => false],
754760
'array_merge_recursive' => ['hasSideEffects' => false],
755761
'array_pad' => ['hasSideEffects' => false],
756762
'array_pop' => ['hasSideEffects' => true],
757763
'array_product' => ['hasSideEffects' => false],
758764
'array_push' => ['hasSideEffects' => true],
759765
'array_rand' => ['hasSideEffects' => false],
766+
'array_reduce' => ['hasSideEffects' => false],
760767
'array_replace' => ['hasSideEffects' => false],
761768
'array_replace_recursive' => ['hasSideEffects' => false],
762769
'array_reverse' => ['hasSideEffects' => false],
@@ -1616,6 +1623,8 @@
16161623
'preg_last_error' => ['hasSideEffects' => true],
16171624
'preg_last_error_msg' => ['hasSideEffects' => true],
16181625
'preg_quote' => ['hasSideEffects' => false],
1626+
'preg_replace_callback' => ['hasSideEffects' => false],
1627+
'preg_replace_callback_array' => ['hasSideEffects' => false],
16191628
'preg_split' => ['hasSideEffects' => false],
16201629
'property_exists' => ['hasSideEffects' => false],
16211630
'quoted_printable_decode' => ['hasSideEffects' => false],

tests/PHPStan/Analyser/data/discussion-7124.php

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,30 +33,30 @@ function filter(array $array, ?callable $callback = null, int $mode = ARRAY_FILT
3333

3434
function () {
3535
// This one does fail, as both the value + key is asked and the key + value is used
36-
filter(
36+
echo count(filter(
3737
[false, true, false],
3838
static fn (int $key, bool $value): bool => 0 === $key % 2 && $value,
3939
mode: ARRAY_FILTER_USE_BOTH,
40-
);
40+
));
4141

4242
// This one should fail, as both the value + key is asked but only the key is used
43-
filter(
43+
echo count(filter(
4444
[false, true, false],
4545
static fn (int $key): bool => 0 === $key % 2,
4646
mode: ARRAY_FILTER_USE_BOTH,
47-
);
47+
));
4848

4949
// This one should fail, as only the key is asked but the value is used
50-
filter(
50+
echo count(filter(
5151
[false, true, false],
5252
static fn (bool $value): bool => $value,
5353
mode: ARRAY_FILTER_USE_KEY,
54-
);
54+
));
5555

5656
// This one should fail, as only the value is asked but the key is used
57-
filter(
57+
echo count(filter(
5858
[false, true, false],
5959
static fn (int $key): bool => 0 === $key % 2,
6060
mode: 0,
61-
);
61+
));
6262
};
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
<?php
22

3-
array_filter([0, 1, 2]);
3+
echo count(array_filter([0, 1, 2]));

tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,51 @@ public function testBug12224(): void
9191
$this->analyse([__DIR__ . '/data/bug-12224.php'], []);
9292
}
9393

94+
public function testBug11101(): void
95+
{
96+
$this->analyse([__DIR__ . '/data/bug-11101.php'], [
97+
[
98+
'Call to function array_filter() on a separate line has no effect.',
99+
8,
100+
],
101+
[
102+
'Call to function array_map() on a separate line has no effect.',
103+
9,
104+
],
105+
[
106+
'Call to function array_reduce() on a separate line has no effect.',
107+
10,
108+
],
109+
[
110+
'Call to function array_filter() on a separate line has no effect.',
111+
13,
112+
],
113+
[
114+
'Call to function preg_replace_callback() on a separate line has no effect.',
115+
14,
116+
],
117+
[
118+
'Call to function preg_replace_callback_array() on a separate line has no effect.',
119+
15,
120+
],
121+
]);
122+
}
123+
124+
#[RequiresPhp('>= 8.4.0')]
125+
public function testBug11101Php84(): void
126+
{
127+
$this->analyse([__DIR__ . '/data/bug-11101-php84.php'], [
128+
[
129+
'Call to function array_any() on a separate line has no effect.',
130+
8,
131+
],
132+
[
133+
'Call to function array_all() on a separate line has no effect.',
134+
9,
135+
],
136+
]);
137+
}
138+
94139
public function testBug4455(): void
95140
{
96141
require_once __DIR__ . '/data/bug-4455.php';
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
namespace Bug11101Php84;
4+
5+
function doFoo(array $array): void
6+
{
7+
// pure callbacks - reported as no effect
8+
array_any($array, 'is_string');
9+
array_all($array, 'is_string');
10+
}
11+
12+
function doBar(array $array): void
13+
{
14+
// impure callbacks - NOT reported
15+
array_any($array, function ($v) {
16+
echo $v;
17+
return true;
18+
});
19+
array_all($array, function ($v) {
20+
echo $v;
21+
return true;
22+
});
23+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
namespace Bug11101;
4+
5+
function doFoo(array $array): void
6+
{
7+
// pure callbacks - reported as no effect
8+
array_filter($array, 'is_string');
9+
array_map('is_string', $array);
10+
array_reduce($array, function ($carry, $item) {
11+
return $carry + $item;
12+
}, 0);
13+
array_filter($array);
14+
preg_replace_callback('/\d/', 'strtoupper', 'abc');
15+
preg_replace_callback_array(['/\d/' => 'strtoupper'], 'abc');
16+
}
17+
18+
function doBar(array $array, callable $cb): void
19+
{
20+
// impure callbacks - NOT reported
21+
array_filter($array, function ($v) {
22+
echo $v;
23+
return true;
24+
});
25+
array_map(function ($v) {
26+
echo $v;
27+
return $v;
28+
}, $array);
29+
array_reduce($array, function ($carry, $item) {
30+
echo $item;
31+
return $carry;
32+
}, 0);
33+
array_map('printf', $array);
34+
35+
// unknown callable purity - NOT reported
36+
array_map($cb, $array);
37+
array_filter($array, $cb);
38+
}

0 commit comments

Comments
 (0)