Should be \Drupal\Core\Entity\ContentEntityInterface instead of \Drupal\Core\Entity\ContentEntityTypeInterface IMHO.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Patrick R. created an issue. See original summary.

kevinvhengst’s picture

Hi Patrick,

I believe you are right.

Drupal/Core/Entity/ContentEntityForm.php is the only class implementing ContentEntityFormInterface and when inspecting that implementation of the validateForm() method in Drupal/Core/Entity/ContentEntityForm.php you will notice this piece of code:

public function validateForm(array &$form, FormStateInterface $form_state) {
    parent::validateForm($form, $form_state);
    /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
    $entity = $this->buildEntity($form, $form_state);

    // Removed code that does not apply to our issue.

    return $entity;

It clearly states $entity is of type \Drupal\Core\Entity\ContentEntityInterface. In my opinion it does return the correct type and the return type of validateForm() method in ContentEntityFormInterface is incorrect.

Your patch solves this error.

This issue is also applicable on Drupal 9.0.x.

kevinvhengst’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Category: Task » Bug report
Status: Reviewed & tested by the community » Fixed
Issue tags: +Documentation

So I think what's really odd here is that the return is never used. We do return an entity in \Drupal\Core\Entity\ContentEntityConfirmFormBase::validateForm() and \Drupal\contact\MessageForm::validateForm() and \Drupal\Core\Entity\ContentEntityForm::validateForm() but nearly every contrib implementation I have locally does not do this. They are all more like \Drupal\taxonomy\TermForm::validateForm() which ignores the return value.

This is a bit of a twisted web because now we're in a world where people are relying on \Drupal\Core\Entity\ContentEntityForm::validateForm to return an entity but the form system definitely doesn't require or use this return value. I guess given that some forms that extend ContentEntityForm are relying on the return value we have to fix this and then when strict return types come along we'll have to fix the broken implementations.

Committed and pushed 74d51c95ee to 9.1.x and 8ecae1e36d to 9.0.x and 96ec6856b8 to 8.9.x. Thanks!

  • alexpott committed 8ecae1e on 9.0.x
    Issue #3154125 by Patrick R., kevinvhengst: Return type of...

  • alexpott committed 96ec685 on 8.9.x
    Issue #3154125 by Patrick R., kevinvhengst: Return type of...

  • alexpott committed 74d51c9 on 9.1.x
    Issue #3154125 by Patrick R., kevinvhengst: Return type of...

Status: Fixed » Closed (fixed)

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