Skip to content

Implement @pure-unless-parameter-passed#6018

Open
zonuexe wants to merge 9 commits into
phpstan:2.2.xfrom
zonuexe:feature/pure-unless-parameter-passed
Open

Implement @pure-unless-parameter-passed#6018
zonuexe wants to merge 9 commits into
phpstan:2.2.xfrom
zonuexe:feature/pure-unless-parameter-passed

Conversation

@zonuexe

@zonuexe zonuexe commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

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

continue;
}

if (!$hasNamedParameter && $i === $parameterIndex) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing named arg test. Did we also miss it in the @pure-unless-callable-is-impure PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #6022 (we also missed this in #3482).


namespace PureUnlessParameterPassedBuiltin;

/**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like we are missing most tests, which the @pure-unless-callable-is-impure PR contained?

zonuexe added 9 commits July 8, 2026 19:50
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.
@zonuexe zonuexe force-pushed the feature/pure-unless-parameter-passed branch from a4f91d3 to 52e31bd Compare July 8, 2026 10:52
@zonuexe

zonuexe commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

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.

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

Copy link
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

str_replace considered impure

3 participants