Replace implement notes with inheritdoc tag

Yes, this needs to be used.

So the first thing would be to find all of these, hopefully with a script that would go ahead and replace them.

There are also some that say "Overrides ..." if they are overriding a method from a base class; "Implements... " is for interface methods.

The reason these are there, by the way, is that for a while that was our standard way of documenting these methods, until we adopted @inheritdoc.

See also #1392754: Comply with new documentation standards for @file for namespaced class files, which is doing a similar thing for @file doc blocks (whose standards also changed). Maybe you can use a similar script?

An initial effort to replace all overridden methods comment block to {@inheritdoc}. Tried to find all overrides and implements in the comment block and manually did the replacement in most case.

It looks like the PHP 5.4 tests failed on Drupal 8 because Drupal 8 requires PHP 5.5. I do not know why that test was run? It looks like it was added manually. Why?

Anyway... the patch.

+++ b/core/lib/Drupal/Component/Gettext/PoMemoryWriter.php
@@ -62,33 +62,25 @@ public function getData() {
-   * Implements Drupal\Component\Gettext\PoMetadataInterface:setLangcode().
-   *
-   * Not implemented. Not relevant for the MemoryWriter.
+   * {@inheritdoc}
    */

In this case, if we make this change, we are losing a valuable comment that says why this method is not implemented. So we should not make this type of change. Only update if the "Implements..." or "Overrides..." is the only thing in the doc block.

This needs to be fixed throughout the patch.

I didn't look through the whole patch in detail, but I that is the only type of problem I found.

Also... I commend your effort doing all of this by hand, but we may eventually need a script to do this because this patch will conflict with other patches, so it will need to be rerolled and may need to be delayed before it is committed.

I ran the test mistakenly. I will reroll the patch again based on your suggestion.

Rerolling first.

Also, it might make sense to break this issue up in smaller patches for easy review.

Since the last patch was just a reroll apparently, still needs work for #5.

Breaking this up into several issues would be OK too. (There should always only be one patch per issue, so if you want to have multiple patches it needs multiple issues. If you do that, figure out some logical way to break it up into issues, and have just a few issues and not something like one issue per file that would be a huge number of very small patches.)

I had a go at creating a patch for this, using a regular expression to find the offending comment blocks and replace the relevant line with {@inheritdoc}. The regex addresses the issue raised in #6 by looking for comment blocks that contain just the Implements... or Overrides... line.

I used a search and replace (in PhpStorm) to make the changes, using
Text to find /\*\*\n\s+\* (Implements|Overrides)\s+.*::.*\n\s+\*/
Replace with /**\n * {@inheritdoc}\n */

I couldn't think of any logical way to break this up into multiple smaller patches, so it's still one big patch.

Phew! That looks fine to me. THANKS for making the patch!

Well done @Peacog! That's an excellent regex -- strict to only change the single-line docblock matches for method names only.

I reviewed the patch carefully to ensure each change was only the intended type of replacement. I did not (obviously) check every one to ensure that it actually was an overridden method that could {@inheritdoc}, but that is not in scope here because if there are any that are incorrect, they are also incorrect in HEAD.

I also checked to see if we had a DrupalCS rule for this and it doesn't appear that we do:
http://cgit.drupalcode.org/drupal/tree/core/phpcs.xml.dist
http://cgit.drupalcode.org/coder/tree/coder_sniffer/Drupal/Sniffs/Commen...
http://cgit.drupalcode.org/coder/tree/coder_sniffer/Drupal/Sniffs/Commen...
(...but also a hard rule to write since a correct one would have to understand the class inheritance).

As a large but well-scoped and low-risk coding standards cleanup, this is a good change to make during the RC phase. Committed and pushed to 8.0.x. Thanks!

I do think this broke docs of all trait methods which correctly *did not* use inheritdoc. As a Trait cannot "officially" extend an interface, @inheritdoc does not work and we must use the implements variant.

Example of a wrong change:

--- a/core/lib/Drupal/Core/Database/Query/QueryConditionTrait.php
+++ b/core/lib/Drupal/Core/Database/Query/QueryConditionTrait.php
@@ -26,7 +26,7 @@
   protected $condition;
 
   /**
-   * Implements Drupal\Core\Database\Query\ConditionInterface::condition().
+   * {@inheritdoc}
    */
   public function condition($field, $value = NULL, $operator = '=') {
     $this->condition->condition($field, $value, $operator);
@@ -34,7 +34,7 @@ public function condition($field, $value = NULL, $operator = '=') {
   }

I guess it would be best to revert this and re-roll the patch without touching traits.

It might be easier to leave this commited and roll a new patch that removes @inheritdoc from all files named *Trait.php. I'll have a go at making a patch if someone can confirm that this is the right thing to do. I see that the vast majority of docblocks in *Trait.php files look like this:

/**
  * {@inheritdoc}
  */

Should these be removed completely?

Yes let's make a new issue. I don't think we can easily revert this patch now.

I'll mark this back to Fixed for now and please open a new issue. We cannot use @inheritdoc in traits. We need to replace them with an actual doc block that says something like:

Implements \Drupal\Whatever\Whatever\Whatever::whatever().

So let's open up a new issue that does this for Traits.

By the way, there's a discussion going on about how to properly document traits:
#2206175: Document traits that use methods on interfaces

Hi , As per the suggestion in #18 , shall I create an issue in order to add a patch for *Trait.php ?