Updated: Comment #0

Problem/Motivation

Follow-up from #2087279-25: Add a config() method to FormBase.

+++ b/core/lib/Drupal/Core/Form/FormBase.php
@@ -104,6 +135,20 @@ public function setTranslationManager(TranslationInterface $translation_manager)
+   * @return self
+   *   The form.
...
+    return $this;

Usually setter don't return anything. @see FormBase::setRequest or FormBase::setUrlGenerator. But then we have FormBase::setTranslationManager returning $this. We should make a rule and follow it. We can do it in followup issue.

Proposed resolution

Discuss and try to remove this inconsistency before it turns into DrupalWTF

Remaining tasks

Discuss and patch

User interface changes

None

API changes

None

None

CommentFileSizeAuthor
#2 form-2089729-2.patch1.97 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Title: Finalize setter return value » Formalize setter return value

Please, return $this; unless there is some meaningful return value.
It allows fluent code like

$controller
  ->setTranslationManager($this->translationManager)
  ->setModuleHandler($this->moduleHandler)
  ->setOperation($operation);

Whereas returning nothing would have no benefit.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new form-2089729-2.patch1.97 KB

See #2158497: [policy, no patch] Use "$this" and "static" in documentation for @return types for @return self/static/$this

Crell’s picture

Crell commented
Status: Needs review » Reviewed & tested by the community

Allowing chaining by default is a good general case. +1

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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