Stop using {@inheritdoc} in DocBlocks

Is {@inheritdoc} used by the API module to create the documentation pages on api.drupal.org?
IDEs could be able to find the documentation from a parent method, but I think that the API module still uses {@inheritdoc} to decide what documentation is shown for a method.

I am glad that I found this already existing issue and I did not need to open a new one :) let's go for this!

@apaderno: Yes, I think so, too.

Instead of dropping {@inheritdoc}, how about we replace it with the PHP 8.3 #[Override] attribute?

API module can use that instead of the {@inheritdoc} tag, and we get the benefit that attribute brings (see https://stitcher.io/blog/override-in-php-83).

Something we're considering for https://github.com/previousnext/coding-standard/issues/21 is removing these tags (without replacement) and using SlevomatCodingStandard.Commenting.UselessInheritDocComment as it suggests inheritance is implied.

So a vote for removal without replacement here.

When 8.3 becomes our minium, #5 sounds interesting!

Do we need to wait for that?

We need API module to understand that attribute, yes.
IDEs detect inheritance with PHP directly I think?

We don't have to wait, but we would have to define the attribute ourselves and then deprecate it and remove it favor of the built in attribute. Maybe I'm missing something.

An attribute doesn't need to have a class defining it: https://www.php.net/manual/en/language.attributes.classes.php

PHP < 8.3 will just treat it as a custom attribute without a class and do nothing with it.
PHP < 8 will just treat it as a comment.

Yeah pretty sure we've done similar for other attributes in the past.

Seems like we should move forward with adopting it since it serves the same propose we use inheritDoc for with additional language features that theoretically makes it more useful.

Thinking through more this actually opens up some options for developers too. The phpdoc standard specifies inheritDoc should be the only documentation which can be annoying when you want to provide additional context to your method. The attribute shouldn't come with this limitation so we can improve documentation in some cases.

> The attribute shouldn't come with this limitation so we can improve documentation in some cases.

I thought of that too, but the reason that inheritDoc doesn't allow additional docs is because how you merge them is a complex can of worms. That problem would exist with the Override attribute too.

Had to think on that some. I don't know if we run into the same problem precisely because this solves one of the problems causing that complexity. Basically the problem is a classic case of complexity, we use {@inheritDoc} to mean two thing.

1. Document everything/"copy" from parent
2. Explicit override documentation

   In some respects, #1 is kinda pointless for all the reasons being discussed. Implicit documentation is the norm, tools are better, etc. If that where the only reason we did this #6 and this issue would make a lot of sense. It does fulfill the Drupal "always have _some_ documentation" idea but I feel that might be a dying sentiment.

   #2 however is pretty powerful especially for something like Drupal because we can see when a method is stranded by a refactor or vendor change without hours of digging through git logs possibly through multiple repositories.

   By separating these concepts we can require #Override to explicitly capture #2 and be more flexible with one #1. And if you're using @inheritDoc and we've basically solved #1994890: Allow {@inheritdoc} and additional documentation → because if you want to provide additional documentation just drop it per the phpdoc standard and the explicit inheritance isn't lost.

Jumping in mid discussion I didn't actually read the summary and though this was a different issues not about removing inherit[dD]oc.

I guess it goes without saying I'm strongly of the opinion inheritDoc(or some analog) provides real value. The Symfony change actually came from me pushing for better adoption in their project so I've got a bad track record though.

Maybe the summary is out of date but I'd like to address some points.

> PHPCS Fixer also dropped it after Symfony
We don't use a lot of PHPCS' rules so 🤷

> In addition. see https://docs.phpdoc.org/guide/guides/inheritance.html#the-inheritdoc-tag (note 'Doc' with capital D):
Yeah we chose to stick with the older standard though the phpdocumenation implementation claims to be case insensitive. I remember pushing to update it but I can't find the issue. 🤷

> is kind of Drupalism in both usage and syntax.
The wording can be confusing but the documentation actually explicitly acknowledges the way Drupal uses it. It just says it doesn't match their normal pattern for tag naming since its "inline" and that they'd like to add a new one, @inheritDoc, without the brackets. But 10 years after that comment was added I don't think we should hold our breath.

So while its dropped from Symfony but its still pretty widely used in this manner and our usage follows the PSR draft document. Its usage is not only not a Drupalism, its not even really a phpDocism since its the same tag in jsdoc, typedoc, javadoc, and even a similar annotation in .net. All with similar if not identical meaning and usage to Drupal's usage.

I think the strongest argument for moving away from it is actually that most of those languages also have moved to annotations which is also seems to be the push in PHP.

I kinda brain dumped on this issue and probably stalled it. Sorry.

tl;dnr. IMHO we should move forward with #[Override].

I do that too on issues sometimes :)

I think in summary:

- for people who want to keep {@inheritdoc} (such as me!), #[Override] is acceptable, because it still looks like docs and it conveys the same thing
- for people who want to remove {@inheritdoc}, #[Override] is acceptable, because it performs a useful DX function.

What we need now is support for the attribute in API module.

I'm surprised that #5 has made [#Override] such a popular choice in this issue. Especially since the linked Stitcher blogpost makes a good case against its existence and usage.

I realise we should not prematurely optimise, but I'm quite hesitant in adding an attribute, which is a runtime check, for something that's essentially documentation (and thus belongs in comments). I'm assuming the impact of an attribute is generally negligible but I do wonder whether that's still true if we're talking about 1000s of annotations. grep -R '{@inheritdoc}' core | wc -l for 11.x shows 119507 instances of {@inheritdoc}.

As for the existance/usage of {@inheritdoc} itself, it currently provides an easy cmd + click target in IDEs to go to the first class/interface in the chain that provides docs (in PHPStorm). I can see that possible becoming a bit more challenging if it's not there, but I'm sure I can figure out what other keyboard shortcut for that exists.

Re #18 and it being a runtime check, some excerpts from the PHP internals discussion

Just to make sure there is no misunderstanding I'd like to clarify that the proposed check of this RFC is not a runtime error. It's a compile time error, just like the check for incompatible method signatures during inheritance. In fact it shares a non-trivial amount of the existing logic of that check, because it's so similar.

My understanding is that once the class is successfully sitting in Opcache there will not be any further overhead.
— source

and a response

Although OPcache is ubiquitous in production environments now, it's not obligated and the cache only lasts as long as the SAPI process.
— source

This may already be clear to everyone but it wasn't for me (: