DX: Insufficient error message "The form argument is not a valid form."

Ah just got bitten by that, looking everywhere when It was a simple typo (missing uppercase in the class name).

This splits the exception into 2, for the 2 different potential causes.

Although, I wonder if this is an appropriate place to use `assert()` instead?

This updates the test, and adds coverage for the new logic and exception text.

Patch for Drupal 8.8.x

This patch helped a lot

I uploaded a new version of the last patch where it adds a space after an elseif()

+++ b/core/lib/Drupal/Core/Form/FormBuilder.php
@@ -193,8 +193,11 @@ public function getFormId($form_arg, FormStateInterface &$form_state) {
+    elseif (!($form_arg instanceof FormInterface)) {
+      throw new \InvalidArgumentException('The form argument ' . get_class($form_arg) . ' must be an instance of \Drupal\Core\Form\FormInterface.');

Does this need to be an elseif?

I also feel like the patch in #9 by @jhedstrom had more test coverage thats missing from this patch. I think that needs to be added back in.

+++ b/core/lib/Drupal/Core/Form/FormBuilder.php
@@ -193,8 +193,11 @@ public function getFormId($form_arg, FormStateInterface &$form_state) {
+      throw new \InvalidArgumentException('The form argument ' . get_class($form_arg) . ' must be an instance of \Drupal\Core\Form\FormInterface.');
     }

I think we use ::class now and not get_class()

Here's the new patch with the suggested changes. I won't enable tests until you confirm the changes are the correct

@DanielVeza the elseif must be elseif because the if (!is_object($form_arg)) { checks $form_arg is object, but then we have to make sure it's instanceof FormInterface because here $form_state->setFormObject($form_arg); it expects an object of FormInterface

Sounds like a good addition, tests cover the new exception. Looks good.

Committed/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5.x, thanks!

This has broken compatibility with PHP 7.3 in 9.5.x, technically we don't support it any more but this currently causes so many errors DrupalCI cannot report them: https://www.drupal.org/pift-ci-job/2509655

The error is

PHP Fatal error:  Dynamic class names are not allowed in compile-time ::class fetch in /var/www/html/core/lib/Drupal/Core/Form/FormBuilder.php on line 200

Ah interesting

So shall we use get_class() instead?

I think get_class() would work fine.

Reverted from 9.5.x, leaving needs work in case we want to backport a 7.3-friendly version, but we could also just leave this in 10.0.x

Here's the patch again, using get_class() instead of ::class to support PHP 7.3 and 7.4

Looks good, thank you.

Committed 83d2b0b and pushed to 9.5.x. Thanks!