Skip to content

Allow reinitialization of a readonly property in __clone since PHP8.3#5731

Open
grizzm0 wants to merge 1 commit into
phpstan:2.1.xfrom
grizzm0:bug-11495
Open

Allow reinitialization of a readonly property in __clone since PHP8.3#5731
grizzm0 wants to merge 1 commit into
phpstan:2.1.xfrom
grizzm0:bug-11495

Conversation

@grizzm0
Copy link
Copy Markdown

@grizzm0 grizzm0 commented May 22, 2026

@phpstan-bot
Copy link
Copy Markdown
Collaborator

You've opened the pull request against the latest branch 2.2.x. PHPStan 2.2 is not going to be released for months. If your code is relevant on 2.1.x and you want it to be released sooner, please rebase your pull request and change its target to 2.1.x.

Signed-off-by: Kristofer Karlsson <karlsson.kristofer@gmail.com>
@grizzm0 grizzm0 changed the base branch from 2.2.x to 2.1.x May 22, 2026 12:12

$function = $scope->getFunction();
if (
$function instanceof MethodReflection
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.

Is the instanceof MethodReflection really needed given the fact there is a previous $this->isInClass() check and you're checking the method name ?

Copy link
Copy Markdown
Contributor

@staabm staabm May 25, 2026

Choose a reason for hiding this comment

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

$function !== null

edit: Ohh I see now why it is done like that. we don't want to react on regular functions named __clone and we did similar checks across the codebase.

so its fine to me.

}

$methodName = $scopeMethod->getName();
$inClone = $this->phpVersion->supportsReadonlyPropertyReinitializationOnClone() && strtolower($methodName) === '__clone';
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.

Given the fact that CloneReinitializationExpr is only added if supportsReadonlyPropertyReinitializationOnClone is true, we might don't need to check it here again ?

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.

it seems check is still needed for the 1st occurence, which does not react on CloneReinitializationExpr

@VincentLanglet VincentLanglet requested a review from staabm May 24, 2026 19:04
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.

Reinitialization of a readonly property in __clone

4 participants