Replace @inheritdoc annotations with #[\Override]

Created on 18 April 2024, 5 months ago
Updated 19 April 2024, 5 months ago

Problem/Motivation

PHP 8.3 introduced the Override attribute which, in general, is a signal to the PHP compiler that a method is intended to override a parent method of the same name or implement an interface with the named method. If it does not match either of these conditions, an exception is thrown. The intent is to help catch errors in inheritance and changes in class heirarchy/interface shape and ensure better hygiene.

See the RFC description here.

Also, @inheritdoc annotations are looking pretty old-school these days especially given smarter IDEs, static analysis tools and a more general move to less-verbose code.

Proposed resolution

Replace most/all instances of @inheritdoc with an #[\Override] method annotation.

Remaining tasks

Do the thing.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

TBD.

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component
OtherΒ  β†’

Last updated about 8 hours ago

Created by

πŸ‡ΊπŸ‡ΈUnited States bradjones1 Digital Nomad Life

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @bradjones1
  • πŸ‡ΊπŸ‡ΈUnited States bradjones1 Digital Nomad Life
  • πŸ‡¦πŸ‡ΊAustralia mstrelan

    FWIW there is a rector rule AddOverrideAttributeToOverriddenMethodsRector for adding the attribute. It won't remove the @inheritdoc annotation though.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    I am not sure we could do this until Drupal 10 EOL because it feels like it would make cherry-picking backports from 11 to 10 much more difficult.

  • πŸ‡¬πŸ‡§United Kingdom joachim

    It's worth mentioning the opinion here: https://stitcher.io/blog/override-in-php-83

    > we're adding runtime checks for something that could be determined by static analysers.

  • πŸ‡ΊπŸ‡ΈUnited States bradjones1 Digital Nomad Life

    I don't feel particularly strongly about this, mostly created the issue from a thread in Slack proposing this change.

    From Brandt's blog, as pointed out in #5 -

    Once again we're adding runtime checks for something that could be determined by static analysers.

    I'm not sure I actually follow this. One of the main reasons for using the annotation is to identify where you _think_ you are overriding a method on a parent class or implementing an interface. If the parent class(es) or interface change, then you will get an error (and static analysis failure and/or notification in your IDE.) I think you would have to implement specific static analysis rules to do the same, and that seems a bit more complex than replacing a rather useless/outdated @inheritdoc with something that is more meaningful in modern PHP with Override.

Production build 0.71.5 2024