Problem/Motivation

The \Drupal\comment\CommentInterface::getCommentedEntity() definition is the following:

  /**
   * Returns the entity to which the comment is attached.
   *
   * @return \Drupal\Core\Entity\FieldableEntityInterface
   *   The entity on which the comment is attached.
   */
  public function getCommentedEntity();

The PHPDoc states that the method always returns a FieldableEntityInterface; however, in fact, the method may also return NULL in some cases like when a host entity was removed but the comment left orphan.

The incorrect documentation may lead to bugs caused by an assumption that a return value is always not empty.

An example of such bug is #2565247: Fatal error: Call to a member function url() on null

Proposed resolution

Correct the method doc.

User interface changes

None.

API changes

The \Drupal\comment\CommentInterface::getCommentedEntity() method PHPDoc @return will be updated; the API is not changed though.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

abramm created an issue. See original summary.

abramm’s picture

Component: comment.module » documentation
Assigned: abramm » Unassigned
Status: Active » Needs review
FileSize
687 bytes

Here's a simple patch fixing this.

Changed the issue Component to 'documentation'.

longwave’s picture

Status: Needs review » Needs work

Looks good, one tiny point:

+++ b/core/modules/comment/src/CommentInterface.php
@@ -56,8 +56,9 @@ public function getParentComment();
+   *   The entity on which the comment is attached or NULL if the comment is
+   *   orphan.

"is an orphan" is better English.

abramm’s picture

Thank you @longwave.
Easiest fix ever.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for the quick fix, looks great!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed f8f869f223 to 8.7.x and 98bc15d7ed to 8.6.x. Thanks!

  • alexpott committed f8f869f on 8.7.x
    Issue #3001851 by abramm, longwave: Incorrect documented...

  • alexpott committed 98bc15d on 8.6.x
    Issue #3001851 by abramm, longwave: Incorrect documented...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.