Skip to content

[RFC] Final Property Promotion#17861

Merged
DanielEScherzer merged 5 commits intophp:masterfrom
DanielEScherzer:final-promoted-props
Jun 22, 2025
Merged

[RFC] Final Property Promotion#17861
DanielEScherzer merged 5 commits intophp:masterfrom
DanielEScherzer:final-promoted-props

Conversation

@DanielEScherzer
Copy link
Copy Markdown
Member

@DanielEScherzer DanielEScherzer commented Feb 19, 2025 •
edited
Loading

@DanielEScherzer DanielEScherzer mentioned this pull request Feb 19, 2025
Merged
11 tasks
@TimWolla TimWolla requested a review from iluuu1994 February 19, 2025 07:43
@TimWolla
Copy link
Copy Markdown
Member

TimWolla commented Feb 19, 2025

This does not appear to be explicitly specified in the RFC and is a language change. I'm not sure this is an appropriate bugfix for 8.4.

@TimWolla
Copy link
Copy Markdown
Member

TimWolla commented Feb 19, 2025

/cc @Crell

@iluuu1994
Copy link
Copy Markdown
Member

iluuu1994 commented Feb 19, 2025

This came up, and there were some concerns it could imply const parameters as in Java. So we just left it out.

@staabm staabm mentioned this pull request Feb 19, 2025
Merged
@iluuu1994
Copy link
Copy Markdown
Member

iluuu1994 commented Feb 19, 2025

So no, this is not a bug. If you think final should be a promoted keyword, please open a new discussion on the mailing list.

@iluuu1994 iluuu1994 closed this Feb 19, 2025
@DanielEScherzer DanielEScherzer mentioned this pull request Feb 21, 2025
Closed
@iluuu1994 iluuu1994 reopened this Mar 21, 2025
@DanielEScherzer DanielEScherzer changed the base branch from PHP-8.4 to master March 24, 2025 18:26
@DanielEScherzer DanielEScherzer changed the title GH-17860: allow final in constructor property promotion [RFC] Final Property Promotion Mar 24, 2025
mvorisek
mvorisek reviewed Apr 24, 2025
View reviewed changes
jorgsowa
jorgsowa reviewed Jun 3, 2025
View reviewed changes
ext/reflection/tests/ReflectionProperty_isFinal.phpt Outdated
public private(set) mixed $p7;
public private(set) final mixed $p8;

public function __construct( final $p9 ) {}
Copy link
Copy Markdown
Contributor

@jorgsowa jorgsowa Jun 3, 2025

Choose a reason for hiding this comment

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

    public function __construct( $p9 ) {}

It would be good to test also the case without the final property in the constructor.

Copy link
Copy Markdown
Member Author

@DanielEScherzer DanielEScherzer Jun 20, 2025

Choose a reason for hiding this comment

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

Done

@iluuu1994 iluuu1994 linked an issue Jun 19, 2025 that may be closed by this pull request
Allow constructor property promotion with final properties #17860
Closed
@DanielEScherzer
Copy link
Copy Markdown
Member Author

DanielEScherzer commented Jun 20, 2025 •
edited
Loading

Any more reviewers? If not I'll probably merge this next week

@TimWolla TimWolla self-requested a review June 21, 2025 09:43
iluuu1994
iluuu1994 approved these changes Jun 21, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

Only nits, the implementation looks correct. Thanks @DanielEScherzer!

TimWolla
TimWolla approved these changes Jun 22, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

Don't forget to also add NEWS before merging.

@DanielEScherzer DanielEScherzer merged commit 4dfba7a into php:master Jun 22, 2025
9 checks passed
@DanielEScherzer DanielEScherzer deleted the final-promoted-props branch June 22, 2025 19:45
@DanielEScherzer DanielEScherzer mentioned this pull request Oct 1, 2025
Closed
@DanielEScherzer DanielEScherzer mentioned this pull request Nov 3, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TimWolla TimWolla TimWolla approved these changes

@iluuu1994 iluuu1994 iluuu1994 approved these changes

+2 more reviewers

@mvorisek mvorisek mvorisek left review comments

@jorgsowa jorgsowa jorgsowa left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Category: Engine Extension: reflection RFC

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow constructor property promotion with final properties

5 participants

@DanielEScherzer @TimWolla @iluuu1994 @mvorisek @jorgsowa