[6.x] Support PHP 8.5 clone-with-properties#11893
Draft
alies-dev wants to merge 3 commits into
Draft
Conversation
PHP 8.5 adds the function form of clone, clone(object $object, array $withProperties), which php-parser routes as a FuncCall rather than a Clone_ node. It never reached CloneAnalyzer, so the cloned type was lost and reported as a flat object by the CallMap entry, and the property array went unchecked. Extract CloneAnalyzer's clone-validity core into a reusable method and route the clone() call (positional or named arguments) through it. The cloned type is now preserved and the bare-clone checks (InvalidClone/PossiblyInvalidClone/ MixedClone, __clone visibility, immutability) apply. The withProperties array is validated against the cloned class: each literal key must be an accessible property and each value assignable to its declared type, while replacing readonly properties is permitted per the RFC.
The function form of clone only exists on PHP 8.5+; on an older target it is a compile error rather than a clone of an object with replaced properties. Emit a ParseError when the analysis target is below 8.5, mirroring how ClassConstAnalyzer gates dynamic class-constant fetches, while still analysing the call so tooling keeps the inferred type.
alies-dev
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bb10c15. Configure here.
| $with_properties_arg, | ||
| $object_type, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Skipped withProperties type check
Medium Severity
analyzeWithProperties runs only when both object and withProperties are present, so a call like clone(withProperties: 5) never validates the second argument and misses InvalidArgument, unlike clone($o, 5) which is checked.
Reviewed by Cursor Bugbot for commit bb10c15. Configure here.
…alues The clone() intercept resolves arguments itself, so the standard call validation never ran: extra positional arguments, unknown named arguments, and an argument passed both by position and by name were accepted silently. Report TooManyArguments / InvalidNamedArgument for those. Also check the value assigned to a declared @property-write key against its write type, which a regular property assignment already validates but the clone-with path was skipping.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PHP 8.5 adds a function form of clone,
clone(object $object, array $withProperties = []), that returns a copy with some properties replaced. php-parser routes it as a regularclone(...)function call rather than aClone_node, so it never reachedCloneAnalyzer: the cloned type collapsed to the flatobjectfrom theCallMap_85entry, none of the usual clone-validity checks ran, and thewithPropertiesarray went unchecked.This routes the
clone(...)call (positional or named arguments) intoCloneAnalyzer. The bare-clone validity core is extracted into a reusable method shared by both forms, soclone($o, [...])now keeps the type of$o, still reportsInvalidClone/PossiblyInvalidClone/MixedCloneand__clonevisibility, and carries the same immutability flags. ThewithPropertiesarray is validated against the cloned class: each literal-string key must name an accessible property, and each value must be assignable to that property's declared type. Replacingreadonlyproperties is allowed, which is the whole point of the RFC, so that validation is replicated locally rather than routed throughInstancePropertyAssignmentAnalyzer, whose readonly-write guard requires$context->selfand would reject a top-level clone-with. Generic, intersection, union, dynamic-array and magic-__settargets are handled conservatively so they do not produce false positives.The second commit gates the function form to PHP 8.5: analysing it against an older target reports a
ParseError, since the two-argument call does not compile before 8.5 (this mirrors howClassConstAnalyzergates dynamic class-constant fetches to 8.3). This is a deliberate addition that is stricter than #11884 and reverses an earlier decision to leave version-gating out of a first cut.Relationship to #11884: that PR solves the type-preservation half declaratively with a templated stub in
stubs/Php85.phpstub. This PR is the superset, since it also performs the clone-validity andwithPropertieschecks that a stub cannot express (unknown keys, value-type mismatches, visibility, readonly exemption). The two are alternatives rather than complementary: the intercept resolves before CallMap/stub resolution, so if this lands the stub is not needed for the two-argument form.