Skip to content

[PHP] keyword return omitted in a constructor override - #9604

Merged
matthiasblaesing merged 1 commit into
apache:masterfrom
DamImpr:php_construct_without_return
Sep 15, 2026
Merged

matthiasblaesing merged 1 commit into
apache:masterfrom
DamImpr:php_construct_without_return

Conversation

@DamImpr

@DamImpr DamImpr commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

In connection with the fix I made in PR 9395, which correctly resolved the issue with the return type in the constructor, IDE still generates return parent::__construct(); when overriding a parent constructor. .

Although the return statement in the constructor does not actually cause a runtime error, I’d like to explain why I believe it is still incorrect to include it, and why I am submitting this PR:

The RFC “Ensure correct signatures of magic methods” explicitly states that

__construct() and __destruct() remain unchanged and continue to permit no declared return type — not even void — because, as the rationale states: almost all languages, including PHP, do not have the concept of constructors and destructors that “return” anything upon completion of their execution.

This is therefore the officially accepted source which establishes, at the level of the PHP language RFC, the principle that, conceptually, a constructor must not return anything.

Credit for the unit test of this feature: @matthiasblaesing

@mbien mbien added PHP [ci] enable extra PHP tests (php/php.editor) ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Sep 7, 2026
@apache apache locked and limited conversation to collaborators Sep 7, 2026
@apache apache unlocked this conversation Sep 7, 2026

@matthiasblaesing matthiasblaesing left a comment

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.

@DamImpr thanks for the change. To me this makes sense and looking at the PHP documentation the calls to parent are all without return.
One thing needs to change though: we need tests. The PHP Editor module has an exceptional test coverage and we should keep it that way. Please have a look here: matthiasblaesing@36bc8e9. Feel free to squash into your change.

DamImpr added a commit to DamImpr/netbeans that referenced this pull request Sep 13, 2026
Add unittest for constructor override fix
… a constructor.

Removed the return type from the constructor in the list of methods to override

Add unittest for constructor override fix
@DamImpr
DamImpr force-pushed the php_construct_without_return branch from 414ceaa to 08e48ab Compare September 13, 2026 16:59
@DamImpr

DamImpr commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

@matthiasblaesing Thank you for adding the unit test, i have included it in the PR.
Next time, I’ll make sure to take care of this aspect.

@matthiasblaesing

Copy link
Copy Markdown
Contributor

@junichi11 @tmysik this makes sense and i think with the unittest this is good to go.

@mbien mbien added this to the NB32 milestone Sep 13, 2026

@tmysik tmysik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed, makes sense to me. Thank you for this PR!

@matthiasblaesing
matthiasblaesing merged commit 1aa627b into apache:master Sep 15, 2026
30 checks passed
@matthiasblaesing

Copy link
Copy Markdown
Contributor

@tmysik thanks for review, @DamImpr thanks for implementation

@DamImpr
DamImpr deleted the php_construct_without_return branch September 15, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) PHP [ci] enable extra PHP tests (php/php.editor)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants