API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Form%21Fo...

> Builds and processes a form for a given form ID.

> \Drupal\Core\Form\FormInterface|string $form_id: The value must be one of the following:

> public function buildForm($form_id, FormStateInterface &$form_state) {

But $form_id is NOT a form ID. The form ID is the arbitrary string returned by getFormId() that's used in alter hooks.

The parameter here is either the name of the form class, or an instance of it.

This parameter should be renamed $form_arg to match with:

> public function getForm($form_arg);
> public function getFormId($form_arg, FormStateInterface &$form_state);

CommentFileSizeAuthor
#2 3070604-2.patch1.75 KBchristinlepson

Comments

joachim created an issue. See original summary.

christinlepson’s picture

StatusFileSize
new1.75 KB

- Renamed $form_id parameter to $form_arg in Drupal\Core\Form\FormBuilderInterface::buildForm()

- Renamed $form_id parameter to $form_arg in Drupal\Core\Form\FormBuilder::buildForm() and used the new parameter to get the form ID.

christinlepson’s picture

Status: Active » Needs review
joachim’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the patch! LGTM.

jhodgdon’s picture

This is confusing, but after carefully reading the code for this function, as well as the function it calls, I think this patch is correct.

As another note, the value that is passed into this function (which is now renamed to $form_arg) is passed into
$this->getFormId()
and that function calls it $form_arg. So this is a good change that will make the function and its documentation less confusing. +1.

  • webchick committed 9a0ebb8 on 8.8.x
    Issue #3070604 by christinlepson, joachim, jhodgdon: badly named and...

  • webchick committed 07aa6db on 8.7.x
    Issue #3070604 by christinlepson, joachim, jhodgdon: badly named and...
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome! Thanks a lot for this improvement!

Committed and pushed to 8.8.x and backported to 8.7.x. Thanks!

Status: Fixed » Closed (fixed)

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