Skip to content

PHPORM-146: Add override attribute everywhere #3412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: 5.x
Choose a base branch
from

Conversation

paulinevos
Copy link
Collaborator

Closes PHPORM-146

Adds the override attribute in every class that extends a Laravel base class, so that PHP will show errors if ever the base methods are renamed or removed.

Also adds in a few corrections to phpdoc declarations where necessary.

Checklist

  • Add tests and ensure they pass (not applicable)

@paulinevos paulinevos requested a review from a team as a code owner June 18, 2025 13:16
@paulinevos paulinevos requested a review from alcaeus June 18, 2025 13:16
@paulinevos
Copy link
Collaborator Author

Apparently git commit -m does not like backticks... will rectify commit messages I guess.

Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

This is huge, and will be very helpful to detect when a parent method is removed.
I spotted some tiny changes.

@GromNaN
Copy link
Member

GromNaN commented Jun 18, 2025

Apparently git commit -m does not like backticks... will rectify commit messages I guess.

We squash-merge. So you can refine the commit message in the merge box in GitHub.

@paulinevos paulinevos force-pushed the 146_override-attribute branch from d5e0610 to 99ea95c Compare June 19, 2025 09:43
@paulinevos paulinevos requested a review from a team as a code owner June 19, 2025 09:43
@paulinevos paulinevos requested a review from lindseymoore June 19, 2025 09:43
@GromNaN GromNaN removed the request for review from lindseymoore June 19, 2025 09:46
@paulinevos paulinevos force-pushed the 146_override-attribute branch 2 times, most recently from da2f424 to 5c296f9 Compare June 19, 2025 12:17
@paulinevos paulinevos requested a review from GromNaN June 19, 2025 12:21
*
* @return Collection
*/
/** Shorthand to get the results of the relationship. */
Copy link
Member

Choose a reason for hiding this comment

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

Without @inheritdoc you lose the phpdoc for the parameter and the return type that are described in the parent method; isn't it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm maybe I misunderstood your previous comment, but didn't you say there's no point in just having inheritdoc when we already have #[Override]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah wait, you said "when a method is not documented". So you're saying it won't parse the parent's phpdoc in this case because there is a comment.

I think the comment is identical to the parent's, so I should probably just remove it then

@paulinevos paulinevos force-pushed the 146_override-attribute branch from 5c296f9 to 51526a5 Compare June 19, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants