Skip to content

Commit d7f1891

Browse files
committed
Leave the PCRE functions out of @pure-unless-parameter-passed
preg_match(), preg_match_all(), preg_replace() and preg_filter() raise a warning on an invalid pattern and set preg_last_error(), so they are not pure even when their out parameter is omitted. Code relies on that: a bare preg_match($pattern, '') under set_error_handler() validates a pattern. preg_replace_callback() goes back to its callback-only entry. No built-in carries both conditions any more, so a userland function tagged with both covers the combined verdict.
1 parent 9b8f8ec commit d7f1891

7 files changed

Lines changed: 106 additions & 145 deletions

File tree

‎bin/functionMetadata_original.php‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@
1818
* (keyed by parameter name) receives an argument, e.g. str_replace()
1919
* whose only side effect is writing to its optional 'count' argument.
2020
*
21-
* The last two can be combined for a call that is pure unless either happens,
22-
* e.g. preg_replace_callback() (impure callback or a passed 'count').
21+
* The last two can be combined for a call that is pure unless either happens.
2322
*/
2423

2524
/** @var array<string, array{hasSideEffects?: bool, pureUnlessCallableIsImpureParameters?: array<string, bool>, pureUnlessParameterPassedParameters?: array<string, bool>}> */
@@ -271,13 +270,7 @@
271270
'output_reset_rewrite_vars' => ['hasSideEffects' => true],
272271
'pclose' => ['hasSideEffects' => true],
273272
'popen' => ['hasSideEffects' => true],
274-
'preg_filter' => ['pureUnlessParameterPassedParameters' => ['count' => true]],
275-
// 'matches'/'subpatterns': PHP 8+ uses the php-8-stubs parameter name, PHP <8 falls
276-
// back to the legacy functionMap.php name.
277-
'preg_match' => ['pureUnlessParameterPassedParameters' => ['matches' => true, 'subpatterns' => true]],
278-
'preg_match_all' => ['pureUnlessParameterPassedParameters' => ['matches' => true, 'subpatterns' => true]],
279-
'preg_replace' => ['pureUnlessParameterPassedParameters' => ['count' => true]],
280-
'preg_replace_callback' => ['pureUnlessCallableIsImpureParameters' => ['callback' => true], 'pureUnlessParameterPassedParameters' => ['count' => true]],
273+
'preg_replace_callback' => ['pureUnlessCallableIsImpureParameters' => ['callback' => true]],
281274
'similar_text' => ['pureUnlessParameterPassedParameters' => ['percent' => true]],
282275
'readfile' => ['hasSideEffects' => true],
283276
'rename' => ['hasSideEffects' => true],

‎resources/functionMetadata.php‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1633,15 +1633,11 @@
16331633
'posix_ttyname' => ['hasSideEffects' => false],
16341634
'posix_uname' => ['hasSideEffects' => false],
16351635
'pow' => ['hasSideEffects' => false],
1636-
'preg_filter' => ['pureUnlessParameterPassedParameters' => ['count' => true]],
16371636
'preg_grep' => ['hasSideEffects' => false],
16381637
'preg_last_error' => ['hasSideEffects' => true],
16391638
'preg_last_error_msg' => ['hasSideEffects' => true],
1640-
'preg_match' => ['pureUnlessParameterPassedParameters' => ['matches' => true, 'subpatterns' => true]],
1641-
'preg_match_all' => ['pureUnlessParameterPassedParameters' => ['matches' => true, 'subpatterns' => true]],
16421639
'preg_quote' => ['hasSideEffects' => false],
1643-
'preg_replace' => ['pureUnlessParameterPassedParameters' => ['count' => true]],
1644-
'preg_replace_callback' => ['pureUnlessCallableIsImpureParameters' => ['callback' => true], 'pureUnlessParameterPassedParameters' => ['count' => true]],
1640+
'preg_replace_callback' => ['pureUnlessCallableIsImpureParameters' => ['callback' => true]],
16451641
'preg_split' => ['hasSideEffects' => false],
16461642
'property_exists' => ['hasSideEffects' => false],
16471643
'quoted_printable_decode' => ['hasSideEffects' => false],

‎src/Reflection/Callables/SimpleImpurePoint.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,9 @@ public static function createFromVariant(FunctionReflection|ExtendedMethodReflec
137137
}
138138

139139
/**
140-
* A function can carry both flags at once (e.g. preg_replace_callback, which is
141-
* pure unless its callback is impure or its $count is passed), so the two
142-
* verdicts are combined. Returns null when the variant declares neither flag.
140+
* A function can carry both flags at once (pure unless a callback is impure
141+
* or an out parameter is passed), so the two verdicts are combined. Returns
142+
* null when the variant declares neither flag.
143143
*
144144
* @param Arg[] $args
145145
*/

‎tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php‎

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -488,51 +488,55 @@ public function testPureUnlessParameterPassed(): void
488488
'Impure echo in pure function PureUnlessParameterPassedFunction\sideEffectUnlessParameterPassed().',
489489
470,
490490
],
491-
]);
492-
}
493-
494-
public function testPureUnlessParameterPassedBuiltin(): void
495-
{
496-
$this->analyse([__DIR__ . '/data/pure-unless-parameter-passed-builtin.php'], [
497491
[
498-
'Impure call to function str_replace() in pure function PureUnlessParameterPassedBuiltin\pureStrReplaceWithCount().',
499-
22,
492+
'Impure call to function PureUnlessParameterPassedFunction\myReplaceCallback() in pure function PureUnlessParameterPassedFunction\pureBothPureCallbackPassingCount().',
493+
505,
500494
],
501495
[
502-
'Impure call to function preg_match() in pure function PureUnlessParameterPassedBuiltin\purePregMatchWithMatches().',
503-
40,
496+
'Impure call to function PureUnlessParameterPassedFunction\myReplaceCallback() in pure function PureUnlessParameterPassedFunction\pureBothImpureCallbackOmittingCount().',
497+
514,
504498
],
505499
[
506-
'Impure call to function preg_filter() in pure function PureUnlessParameterPassedBuiltin\purePregFilterWithCount().',
507-
60,
500+
'Impure echo in pure function PureUnlessParameterPassedFunction\pureBothImpureCallbackOmittingCount().',
501+
515,
508502
],
509503
[
510-
'Impure call to function preg_replace_callback() in pure function PureUnlessParameterPassedBuiltin\purePregReplaceCallbackWithCount().',
511-
80,
504+
'Possibly impure call to a callable in pure function PureUnlessParameterPassedFunction\pureBothOpaqueCallbackOmittingCount().',
505+
528,
512506
],
513507
[
514-
'Impure call to function preg_replace_callback() in pure function PureUnlessParameterPassedBuiltin\purePregReplaceCallbackImpureCallback().',
515-
89,
508+
'Possibly impure call to function PureUnlessParameterPassedFunction\myReplaceCallback() in pure function PureUnlessParameterPassedFunction\pureBothOpaqueCallbackOmittingCount().',
509+
528,
516510
],
517511
[
518-
'Impure echo in pure function PureUnlessParameterPassedBuiltin\purePregReplaceCallbackImpureCallback().',
519-
90,
512+
'Impure call to function PureUnlessParameterPassedFunction\myReplaceCallback() in pure function PureUnlessParameterPassedFunction\pureBothOpaqueCallbackPassingCount().',
513+
539,
520514
],
521515
[
522-
'Impure call to function str_ireplace() in pure function PureUnlessParameterPassedBuiltin\pureStrIreplaceWithCount().',
523-
113,
516+
'Possibly impure call to a callable in pure function PureUnlessParameterPassedFunction\pureBothOpaqueCallbackPassingCount().',
517+
539,
524518
],
519+
]);
520+
}
521+
522+
public function testPureUnlessParameterPassedBuiltin(): void
523+
{
524+
$this->analyse([__DIR__ . '/data/pure-unless-parameter-passed-builtin.php'], [
525525
[
526-
'Impure call to function preg_replace() in pure function PureUnlessParameterPassedBuiltin\purePregReplaceWithCount().',
527-
133,
526+
'Impure call to function str_replace() in pure function PureUnlessParameterPassedBuiltin\pureStrReplaceWithCount().',
527+
22,
528528
],
529529
[
530-
'Impure call to function preg_match_all() in pure function PureUnlessParameterPassedBuiltin\purePregMatchAllWithMatches().',
531-
151,
530+
'Possibly impure call to function preg_match() in pure function PureUnlessParameterPassedBuiltin\purePregMatchWithoutMatches().',
531+
32,
532+
],
533+
[
534+
'Impure call to function str_ireplace() in pure function PureUnlessParameterPassedBuiltin\pureStrIreplaceWithCount().',
535+
52,
532536
],
533537
[
534538
'Impure call to function similar_text() in pure function PureUnlessParameterPassedBuiltin\pureSimilarTextWithPercent().',
535-
169,
539+
70,
536540
],
537541
]);
538542
}

‎tests/PHPStan/Rules/Pure/data/pure-unless-parameter-passed-builtin.php‎

Lines changed: 2 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -27,72 +27,11 @@ function pureStrReplaceWithCount(string $s): string
2727
*/
2828
function purePregMatchWithoutMatches(string $s): int
2929
{
30-
// The by-ref $matches is omitted, so preg_match() is pure.
30+
// preg_match() is left out of the metadata even with $matches omitted: an invalid
31+
// pattern raises a warning and sets preg_last_error(), so the call stays possibly impure.
3132
return (int) preg_match('/a/', $s);
3233
}
3334

34-
/**
35-
* @phpstan-pure
36-
*/
37-
function purePregMatchWithMatches(string $s): int
38-
{
39-
// The by-ref $matches is passed, so preg_match() is impure (the flag is certain).
40-
return (int) preg_match('/a/', $s, $matches);
41-
}
42-
43-
/**
44-
* @phpstan-pure
45-
*/
46-
function purePregFilterWithoutCount(string $s): ?string
47-
{
48-
// The by-ref $count is omitted, so preg_filter() is pure.
49-
return preg_filter('/a/', 'b', $s);
50-
}
51-
52-
/**
53-
* @phpstan-pure
54-
*/
55-
function purePregFilterWithCount(string $s): ?string
56-
{
57-
// The by-ref $count is passed, so preg_filter() is impure (the flag is certain).
58-
$count = 0;
59-
60-
return preg_filter('/a/', 'b', $s, -1, $count);
61-
}
62-
63-
/**
64-
* @phpstan-pure
65-
*/
66-
function purePregReplaceCallbackPureCallback(string $s): ?string
67-
{
68-
// preg_replace_callback() carries both flags: the callback is pure and the by-ref
69-
// $count is omitted, so the call stays pure.
70-
return preg_replace_callback('/a/', static fn (array $m): string => $m[0], $s);
71-
}
72-
73-
/**
74-
* @phpstan-pure
75-
*/
76-
function purePregReplaceCallbackWithCount(string $s): ?string
77-
{
78-
$count = 0;
79-
// The callback is pure but the by-ref $count is passed, so the call is impure.
80-
return preg_replace_callback('/a/', static fn (array $m): string => $m[0], $s, -1, $count);
81-
}
82-
83-
/**
84-
* @phpstan-pure
85-
*/
86-
function purePregReplaceCallbackImpureCallback(string $s): ?string
87-
{
88-
// The callback itself is impure, so the call is impure regardless of $count.
89-
return preg_replace_callback('/a/', static function (array $m): string {
90-
echo $m[0];
91-
92-
return $m[0];
93-
}, $s);
94-
}
95-
9635
/**
9736
* @phpstan-pure
9837
*/
@@ -113,44 +52,6 @@ function pureStrIreplaceWithCount(string $s): string
11352
return str_ireplace('a', 'b', $s, $count);
11453
}
11554

116-
/**
117-
* @phpstan-pure
118-
*/
119-
function purePregReplaceWithoutCount(string $s): ?string
120-
{
121-
// The by-ref $count is omitted, so preg_replace() is pure.
122-
return preg_replace('/a/', 'b', $s);
123-
}
124-
125-
/**
126-
* @phpstan-pure
127-
*/
128-
function purePregReplaceWithCount(string $s): ?string
129-
{
130-
// The by-ref $count is passed, so preg_replace() is impure (the flag is certain).
131-
$count = 0;
132-
133-
return preg_replace('/a/', 'b', $s, -1, $count);
134-
}
135-
136-
/**
137-
* @phpstan-pure
138-
*/
139-
function purePregMatchAllWithoutMatches(string $s): int
140-
{
141-
// The by-ref $matches is omitted, so preg_match_all() is pure.
142-
return (int) preg_match_all('/a/', $s);
143-
}
144-
145-
/**
146-
* @phpstan-pure
147-
*/
148-
function purePregMatchAllWithMatches(string $s): int
149-
{
150-
// The by-ref $matches is passed, so preg_match_all() is impure (the flag is certain).
151-
return (int) preg_match_all('/a/', $s, $matches);
152-
}
153-
15455
/**
15556
* @phpstan-pure
15657
*/

‎tests/PHPStan/Rules/Pure/data/pure-unless-parameter-passed.php‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,3 +472,69 @@ function sideEffectUnlessParameterPassed(string $subject, int &$count = 0): stri
472472

473473
return $subject;
474474
}
475+
476+
/**
477+
* @param callable(string): string $f
478+
* @param-out int $count
479+
* @pure-unless-callable-is-impure $f
480+
* @pure-unless-parameter-passed $count
481+
*/
482+
function myReplaceCallback(callable $f, string $subject, int &$count = 0): string
483+
{
484+
$count = 1;
485+
486+
return $f($subject);
487+
}
488+
489+
/**
490+
* @phpstan-pure
491+
*/
492+
function pureBothConditionsMet(string $s): string
493+
{
494+
// The callback is pure and $count is omitted, so the call stays pure.
495+
return myReplaceCallback(static fn (string $x): string => $x, $s);
496+
}
497+
498+
/**
499+
* @phpstan-pure
500+
*/
501+
function pureBothPureCallbackPassingCount(string $s): string
502+
{
503+
$count = 0;
504+
// The callback is pure but $count is passed, so the call is impure.
505+
return myReplaceCallback(static fn (string $x): string => $x, $s, $count);
506+
}
507+
508+
/**
509+
* @phpstan-pure
510+
*/
511+
function pureBothImpureCallbackOmittingCount(string $s): string
512+
{
513+
// $count is omitted but the callback is impure, so the call is impure.
514+
return myReplaceCallback(static function (string $x): string {
515+
echo $x;
516+
517+
return $x;
518+
}, $s);
519+
}
520+
521+
/**
522+
* @param callable(string): string $f
523+
* @phpstan-pure
524+
*/
525+
function pureBothOpaqueCallbackOmittingCount(callable $f, string $s): string
526+
{
527+
// $count is omitted but the callback's purity is unknown, so the call stays possibly impure.
528+
return myReplaceCallback($f, $s);
529+
}
530+
531+
/**
532+
* @param callable(string): string $f
533+
* @phpstan-pure
534+
*/
535+
function pureBothOpaqueCallbackPassingCount(callable $f, string $s): string
536+
{
537+
$count = 0;
538+
// $count is passed, so the call is impure whatever the callback does.
539+
return myReplaceCallback($f, $s, $count);
540+
}

‎turbo-ext/tests/type-family.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6890,7 +6890,7 @@ public function getResolvedPhpDoc(): ?\PHPStan\PhpDoc\ResolvedPhpDocBlock { retu
68906890
// arguments), the pure-unless-callable-is-impure parameters of array_filter()
68916891
// and array_reduce() fed pure, impure, maybe-pure, null, non-callable and
68926892
// omitted callbacks (positional and named), the pure-unless-parameter-passed
6893-
// out parameters of str_replace() and preg_match() passed and omitted
6893+
// out parameters of str_replace() and similar_text() passed and omitted
68946894
// (positional, named and unpacked), both verdicts combined and applied to a
68956895
// keyed list of certain and uncertain points, the transformed and plain
68966896
// fixture methods, a missing scope or variant, and unconstructed instances
@@ -6931,10 +6931,11 @@ public function getResolvedPhpDoc(): ?\PHPStan\PhpDoc\ResolvedPhpDocBlock { retu
69316931
'named count' => [$arg('string'), $arg('string'), $arg('string'), $arg('true', 'count')],
69326932
'named matches' => [$arg('string', 'pattern'), $arg('string', 'subject'), $arg('true', 'matches')],
69336933
'named subject only' => [$arg('string', 'pattern'), $arg('string', 'subject')],
6934+
'named percent' => [$arg('string', 'string1'), $arg('string', 'string2'), $arg('true', 'percent')],
69346935
'unpacked' => [new \PhpParser\Node\Arg(new \PhpParser\Node\Expr\Variable('array'), unpack: true)],
69356936
];
69366937
$functions = [];
6937-
foreach (['strlen', 'usleep', 'print_r', 'var_export', 'highlight_string', 'array_filter', 'array_reduce', 'array_map', 'rand', 'str_replace', 'preg_match'] as $functionName) {
6938+
foreach (['strlen', 'usleep', 'print_r', 'var_export', 'highlight_string', 'array_filter', 'array_reduce', 'array_map', 'rand', 'str_replace', 'similar_text'] as $functionName) {
69386939
$functions[$functionName] = $stringReflectionProvider->getFunction(new \PhpParser\Node\Name($functionName), null);
69396940
}
69406941
foreach ($functions as $functionName => $function) {

0 commit comments

Comments
 (0)