Implement @pure-unless-parameter-passed by zonuexe · Pull Request #6018 · phpstan/phpstan-src

zonuexe · 2026-07-07T17:25:50Z

Implements the @pure-unless-parameter-passed PHPDoc tag: a parameter-level annotation declaring that the function/method is pure unless an argument is passed for that (by-ref out) parameter. Built on top of 2.2.x, following the same architecture as @pure-unless-callable-is-impure (#3482) — the parameter-level flag is TrinaryLogic from the start so it composes correctly on union/intersection method variants.

Background

This closes @staabm's phpstan/phpstan#11884, where he pointed out that str_replace() (and str_ireplace/preg_replace) could be treated as pure when the by-ref $count argument is omitted, and asked for "a solution independent of concrete function signatures" rather than one-off metadata flags. ondrejmirtes suggested the @phpstan-pure-unless-parameter-passed annotation name in response. @staabm then implemented the parser-side syntax support at phpstan/phpdoc-parser#259, which is still open — ondrejmirtes asked there for the phpstan-src implementation to be ready first before merging the parser side. This PR is that implementation.

Parser support

Since phpstan/phpdoc-parser#259 is not merged yet, this PR parses the tag from the generic tag value as a stopgap (PhpDocNodeResolver::resolveParamPureUnlessParameterPassed(), marked with a TODO to switch to PhpDocNode::getPureUnlessParameterIsPassedTagValues() once the parser PR merges).

How it works

The tag (and its @phpstan- prefixed alias) is resolved into per-parameter flags, threaded through the reflection layer (ExtendedParameterReflection::isPureUnlessParameterPassedParameter(): TrinaryLogic), and populated for builtins via functionMetadata (str_replace, str_ireplace, preg_replace, preg_match, preg_match_all, similar_text).
SimpleImpurePoint::resolvePureUnlessParameterPassedVerdict() checks whether an argument was actually passed for each flagged parameter:
- the by-ref parameter is omitted (or explicitly null) → the call stays pure;
- an argument is passed → unchanged (possibly-impure) behavior.
combineAcceptors() merges the flag across union method variants with equals-or-Maybe, so a parameter tagged in some members but not others correctly becomes Maybe rather than being silently dropped.

/** @phpstan-pure */
function normalize(string $s): string
{
    return str_replace('_', '-', $s); // no longer "Possibly impure"
}

/** @phpstan-pure */
function normalizeAndCount(string $s, int &$count): string
{
    return str_replace('_', '-', $s, $count); // still "Possibly impure" - $count is written
}

Add function map (str_replace, str_ireplace, preg_replace, preg_match, preg_match_all, similar_text)
Add tests (userland functions/methods, builtins, union-variant Maybe combine)
Add rules (no new rule needed — the impure-point suppression feeds the existing pure-context check)

refs #3482, phpstan/phpdoc-parser#259

Closes phpstan/phpstan#11884

zonuexe · 2026-07-08T11:25:08Z

Rebased onto latest 2.2.x. While reworking the tests I found and fixed a real asymmetry in SimpleImpurePoint::createFromVariant(): when an argument was passed for a @pure-unless-parameter-passed by-ref parameter, the call was only reported as possibly impure, whereas passing it is a definite side effect. It now reports certain impurity, matching both the @pure-unless-callable-is-impure block right above it and the constructor path in NewHandler. Added coverage for methods, intersection receivers, inheritance (including a renamed parameter), and first-class callables.

Current CI status — the remaining red checks are all unrelated to this PR's code:

Mutation Testing passes on 8.4 (0 escaped; the two NewHandler TrinaryLogicMutator mutants are killed by static analysis). On 8.3 those same two mutants escape and an unrelated mutant times out — a nondeterministic SA-step/coverage artifact on the slower runner, not a genuine test gap. The two mutants are not killable by a test because a Maybe verdict is unreachable at the constructor call site by construction (single-variant constructors; dynamic new bails out on a union of classes).
Rector integration fails with undefined method ...getPureUnlessParameterIsPassedTagValues(). This is expected: phpdoc-parser 2.3.3 (which adds that method) was released today, and no PHPStan/Rector release bundling it exists yet, so the integration environment resolves an older phpdoc-parser. It will clear once a release catches up.
E2E result-cache-restore-without-reflection fails on 2.2.x itself, from an upstream phpstan-phpunit@dev constructor-parameter change.
Benchmark and the old-PHPUnit Windows job failures are pre-existing/flaky (the latter is an unrelated IntersectionTypeTest data set) and reproduce independently of this PR.

phpstan-bot · 2026-07-08T11:26:10Z

This pull request has been marked as ready for review.

staabm · 2026-07-09T05:07:11Z

would it make sense to emit a new error when @pure-unless-parameter-passed is used on a non-optional parameter?

staabm · 2026-07-09T05:09:54Z

+		// TODO: drop this once phpstan/phpdoc-parser#259 is merged and the parser
+		// recognizes the tag natively (like @phpstan-pure-unless-callable-is-impure).
+		'@phpstan-pure-unless-parameter-passed',


the mentioned PR was merged, so this TODO can be resolved

Removed the TODO. The entry itself stays though: the rule matches by name, so dropping it would report the tag as unknown again. @phpstan-pure-unless-callable-is-impure had the same gap, added it too.

Resolve the tag into per-parameter flags and merge them across parent PHPDocs. Until phpstan/phpdoc-parser#259 is merged the parser does not understand the tag natively, so parse it from the generic tag value and whitelist the @phpstan- alias in InvalidPHPStanDocTagRule.

Add isPureUnlessParameterPassedParameter(): TrinaryLogic on parameter reflections, populated from PHPDoc and function metadata, mirroring the already-merged @pure-unless-callable-is-impure threading (same TrinaryLogic typing from the start, so combineAcceptors() merges it with equals-or-Maybe instead of ->or(), giving correct behavior on union method variants).

Thread the raw parameter flags through enterClassMethod()/enterFunction() and getPhpDocs(), and consult them in SimpleImpurePoint::resolvePureUnlessParameterPassedVerdict(): a call to a function flagged with @pure-unless-parameter-passed stays pure as long as none of those by-ref parameters received an argument.

str_replace, str_ireplace, preg_replace, preg_match, preg_match_all and similar_text are pure unless their optional by-ref out parameter (count / matches / percent) receives an argument. Add the pureUnlessParameterPassedParameters metadata shape to the generator, the metadata schema test, and the metadata files.

…tins str_replace/str_ireplace's count and preg_match/preg_match_all's matches resolve to different parameter names (replace_count/subpatterns) when the target PHP version is below 8.0: NativeFunctionReflectionProvider falls back from php-8-stubs to the legacy functionMap.php names in that case. List both names in the metadata so the by-ref-omitted suppression works regardless of the analysed PHP version. Found via the old-PHPUnit (7.4) CI job, which runs the test suite against phpVersion 7.4 by default.

…d named arguments NewHandler now applies the parameter-passed verdict to 'new' the same way the sibling callable verdict is applied (constructors return void, so createFromVariant cannot be reused). The verdict also treats argument unpacking conservatively (an unpacked argument might cover the flagged by-ref parameter, so the call stays possibly impure) and respects the flag's own TrinaryLogic certainty from union-variant composition (an uncertain flag downgrades a passed argument to Maybe instead of No).

phpstan/phpdoc-parser#259 is merged and released, so replace the generic-tag stopgap with PhpDocNode::getPureUnlessParameterIsPassedTagValues() and bump the dependency.

… is passed createFromVariant suppressed the impure point when the flagged parameter was omitted, but never promoted the verdict to certain when it was passed, unlike its @pure-unless-callable-is-impure sibling right above it and unlike NewHandler's own constructor handling. Passing the by-ref out-parameter is a definite side effect, not a possible one. Also covers intersection types, method inheritance (incl. a renamed parameter), and first-class callables.

InvalidPHPStanDocTagRule matches @phpstan-* tags by name against a hardcoded list, so native phpdoc-parser recognition does not exempt them. @phpstan-pure-unless-callable-is-impure was never added to that list and is currently reported as an unknown tag; add it alongside @phpstan-pure-unless-parameter-passed and drop the stale TODO.

staabm · 2026-07-09T06:56:20Z

+class InheritedReplacerChild extends InheritedReplacerParent
+{
+
+	public function replace(string $subject, int &$count = 0): string


I think it would be useful to test a union of 2 object types, where both have a same named, same arguments method (including a by-ref arg), but only one of them declares @pure-unless-parameter-passed

This is already covered above by pureUnionMethodOmittingCount / pureUnionMethodPassingCount: PureUnlessParameterPassedA declares the tag, PureUnlessParameterPassedB does not, and both have the same m(string $s, int &$count = 0) signature. Omitting $count stays pure; passing it is reported as possibly impure, since the merged flag is Maybe.

A non-optional parameter is always passed, so the tag can never keep the function or method pure. Report it as a misuse, mirroring the existing @pure-unless-callable-is-impure redundancy check. The declaration-side flag was dropped for functions: enterFunction() was never given the @pure-unless-parameter-passed parameters (the method path already passed them), so FunctionPurityCheck could not see it. Thread it through as well.

FunctionPurityCheck read the per-parameter TrinaryLogic flag via ->yes() to find the flagged parameters, which is an equivalent mutation at a declaration site (the flag is only ever Yes or No there, never Maybe). Mirror the @pure-unless-callable-is-impure sibling instead: expose the flagged parameters as a function/method-level array<string, TrinaryLogic> getter and look them up with array_key_exists.

phpstan-bot · 2026-07-09T08:41:48Z

This pull request has been marked as ready for review.

zonuexe · 2026-07-09T08:42:19Z

Update after the latest review round:

Non-optional parameter (suggested above): a non-optional parameter is always passed, so the tag can never keep the function/method pure. This is now reported as a misuse (pureFunction.nonOptionalParameterPassed / pureMethod.nonOptionalParameterPassed), mirroring the existing @pure-unless-callable-is-impure redundancy check. Wiring it up surfaced a gap: the declaration-side flag was dropped for functions (enterFunction() never received it, while the method path already did), fixed as well.
Function-level map: to find the flagged parameters at a declaration, the check now uses a getPureUnlessParameterPassedParameters() map with array_key_exists, exactly like the callable sibling, instead of the per-parameter ->yes(). This removes an equivalent TrinaryLogicMutator mutant in FunctionPurityCheck (at a declaration site the flag is only ever Yes or No, never Maybe).
@phpstan-pure-unless-callable-is-impure was missing from the known-tags list and was reported as an unknown tag; added it there too.

Mutation status: 8.4 is green (0 escaped). 8.3 still reports the two NewHandler constructor-path mutants; these are equivalent ($passedVerdict can only be Yes or No at a new call site: a userland constructor has a single variant, and dynamic new bails out on a union of classes, so Maybe is unreachable) and are killed by static analysis on 8.4. The remaining red checks (Benchmark, Rector integration) are unrelated as noted earlier; E2E now passes after picking up the phpstan-phpunit update.

staabm · 2026-07-09T13:09:57Z


-/** @var array<string, array{hasSideEffects: bool}|array{pureUnlessCallableIsImpureParameters: array<string, bool>}> */
+/** @var array<string, array{hasSideEffects: bool}|array{pureUnlessCallableIsImpureParameters: array<string, bool>}|array{pureUnlessParameterPassedParameters: array<string, bool>}> */
 return [


can we add

exec

exif_thumbnail

flock

fsockopen

?

searching with '&[^']+= thru resources/functionMap.php could reveal even more

These optional by-ref parameters do have a side effect on the caller variable scope, but these functions have side effects as their primary purpose, so their purity does not change depending on whether the parameter is passed. In some cases it can be worth treating a function as #[\NoDiscard], but these do not seem to be such cases.

Did a broader sweep of all optional by-ref out-params in functionMap.php. preg_filter is a clean match (same signature as the already-covered preg_replace, pure regex substitution with an optional count), so I added it. The remaining plausible candidates (grapheme_extract, Spoofchecker::areConfusable/isSuspicious, IntlTimeZone::getCanonicalID) are borderline — their purity depends on ICU/locale process globals — so I left them out for now; happy to revisit if you think they qualify.

For reference, here is the full breakdown of the borderline candidates I left out, in case any are worth a follow-up.

FITS but deferred (medium confidence — purity depends on ICU/locale process globals):

Function out param Why deferred

grapheme_extract next GRAPHEME_EXTR_WORD/SENTENCE use ICU break-iterators bound to the process-global default locale (Locale::setDefault()), so identical args can differ across calls.

Spoofchecker::areConfusable error Pure evaluation against ICU confusables data, but an obscure extension whose runtime behavior is hard to verify.

Spoofchecker::isSuspicious error Same as above.

IntlTimeZone::getCanonicalID isSystemID Looks up ICU's compiled tz database (no PHP-mutable setting), safer than the others but still ICU-build dependent.

Looks pure at first glance, but classified impure (clear side effect / non-determinism):

Function out param Why impure

is_callable callable_name A "Class::method" string can trigger autoloading, independent of the by-ref param.

mb_ereg / mb_eregi registers Match result depends on the mutable global mb_regex_encoding().

idn_to_ascii / idn_to_utf8 idna_info Depends on the ICU build's IDNA/UTS46 tables.

IntlDateFormatter::parse / ::localtime parse_pos Falls back to the process-global date_default_timezone_get() when the formatter has no timezone.

xdiff_string_patch error In-memory on its face, but the extension is long unmaintained and its internal temp-file usage can't be verified.

yaml_parse ndocs Its callbacks array takes arbitrary user callables (same class as preg_replace_callback); that would need pureUnlessCallableIsImpureParameters on callbacks, not this mechanism.

ldap_parse_exop, ldap_control_paged_result_response retdata/retoid, cookie/estimated Documented as decode-only, but I could not verify they don't update the connection's cached last-error/status (ldap_control_paged_result_response is also removed in PHP 8.0).

Totals across the 79 optional/required by-ref out-param entries: 12 required-by-ref (skipped), 11 already handled, 51 impure, 5 fit (of which preg_filter is added here).

thanks for looking into it. I agree these are mostly borderline.

one stood out for me: preg_replace_callback, preg_replace_callback_array

Good call. preg_replace_callback now carries both flags — the two verdicts were checked in sequence, so a pure callback short-circuited before the $count check; I combined them with TrinaryLogic::and() (in both the call and new paths) and flagged its $count. preg_replace_callback_array I left out: its callbacks live inside an array<string, callable> parameter, which the pureUnlessCallableIsImpureParameters mechanism (single callable param) cannot inspect, so flagging only its $count would wrongly treat it as pure when a callback in the array is impure. That one needs a separate mechanism.

Cover a function whose flagged by-ref parameter is followed by a trailing variadic: omitting the parameter stays pure, passing it is impure, and the extra variadic arguments do not affect the flagged parameter.

preg_filter() has the same signature as the already-covered preg_replace() (a pure regex substitution with an optional by-ref $count out-parameter), so it stays pure unless $count is passed.

A function can carry both @pure-unless-callable-is-impure and @pure-unless-parameter-passed at once - preg_replace_callback() is pure unless its callback is impure or its $count is passed. The two verdicts were checked in sequence, so a pure callback short-circuited to "pure" and never looked at $count. Combine them with TrinaryLogic::and() in both SimpleImpurePoint::createFromVariant() and NewHandler, and flag preg_replace_callback()'s $count.

phpstan-bot · 2026-07-09T16:01:45Z

This pull request has been marked as ready for review.

zonuexe mentioned this pull request Jul 8, 2026

Support for @pure-unless-parameter-passed phpstan/phpdoc-parser#259

Merged

staabm reviewed Jul 8, 2026

View reviewed changes

Comment thread src/Reflection/Callables/SimpleImpurePoint.php

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

zonuexe mentioned this pull request Jul 8, 2026

Add named argument tests for @pure-unless-callable-is-impure #6022

Merged

zonuexe force-pushed the feature/pure-unless-parameter-passed branch from a4f91d3 to 52e31bd Compare July 8, 2026 10:52

zonuexe marked this pull request as ready for review July 8, 2026 11:25

staabm reviewed Jul 9, 2026

View reviewed changes

zonuexe added 10 commits July 9, 2026 15:10

Add tests for @pure-unless-parameter-passed

9e0daad

Parse @pure-unless-parameter-passed via phpdoc-parser 2.3.3

1cde339

phpstan/phpdoc-parser#259 is merged and released, so replace the generic-tag stopgap with PhpDocNode::getPureUnlessParameterIsPassedTagValues() and bump the dependency.

zonuexe force-pushed the feature/pure-unless-parameter-passed branch from 52e31bd to 4902e4b Compare July 9, 2026 06:31

staabm reviewed Jul 9, 2026

View reviewed changes

zonuexe marked this pull request as draft July 9, 2026 08:03

zonuexe marked this pull request as ready for review July 9, 2026 08:41

staabm reviewed Jul 9, 2026

View reviewed changes

Comment thread src/Reflection/Callables/SimpleImpurePoint.php

Test @pure-unless-parameter-passed with a variadic parameter

850d542

Cover a function whose flagged by-ref parameter is followed by a trailing variadic: omitting the parameter stays pure, passing it is impure, and the extra variadic arguments do not affect the flagged parameter.

Mark preg_filter() as @pure-unless-parameter-passed

d78c3df

preg_filter() has the same signature as the already-covered preg_replace() (a pure regex substitution with an optional by-ref $count out-parameter), so it stays pure unless $count is passed.

zonuexe marked this pull request as draft July 9, 2026 15:33

zonuexe marked this pull request as ready for review July 9, 2026 16:01

Function	out param	Why deferred
`grapheme_extract`	`next`	`GRAPHEME_EXTR_WORD`/`SENTENCE` use ICU break-iterators bound to the process-global default locale (`Locale::setDefault()`), so identical args can differ across calls.
`Spoofchecker::areConfusable`	`error`	Pure evaluation against ICU confusables data, but an obscure extension whose runtime behavior is hard to verify.
`Spoofchecker::isSuspicious`	`error`	Same as above.
`IntlTimeZone::getCanonicalID`	`isSystemID`	Looks up ICU's compiled tz database (no PHP-mutable setting), safer than the others but still ICU-build dependent.

Function	out param	Why impure
`is_callable`	`callable_name`	A `"Class::method"` string can trigger autoloading, independent of the by-ref param.
`mb_ereg` / `mb_eregi`	`registers`	Match result depends on the mutable global `mb_regex_encoding()`.
`idn_to_ascii` / `idn_to_utf8`	`idna_info`	Depends on the ICU build's IDNA/UTS46 tables.
`IntlDateFormatter::parse` / `::localtime`	`parse_pos`	Falls back to the process-global `date_default_timezone_get()` when the formatter has no timezone.
`xdiff_string_patch`	`error`	In-memory on its face, but the extension is long unmaintained and its internal temp-file usage can't be verified.
`yaml_parse`	`ndocs`	Its `callbacks` array takes arbitrary user callables (same class as `preg_replace_callback`); that would need `pureUnlessCallableIsImpureParameters` on `callbacks`, not this mechanism.
`ldap_parse_exop`, `ldap_control_paged_result_response`	`retdata`/`retoid`, `cookie`/`estimated`	Documented as decode-only, but I could not verify they don't update the connection's cached last-error/status (`ldap_control_paged_result_response` is also removed in PHP 8.0).

Uh oh!

Conversation

zonuexe commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Parser support

How it works

Uh oh!

Uh oh!

Uh oh!

zonuexe commented Jul 8, 2026

Uh oh!

phpstan-bot commented Jul 8, 2026

Uh oh!

staabm commented Jul 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

staabm Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented Jul 9, 2026

Uh oh!

zonuexe commented Jul 9, 2026

Uh oh!

staabm Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phpstan-bot commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zonuexe commented Jul 7, 2026 •

edited

Loading

staabm Jul 9, 2026 •

edited

Loading

staabm Jul 9, 2026 •

edited

Loading