Use PHP attributes for form route discovery

Got a start on this that seems to work OK: MR 15476.

Introduced a new attribute FormRoute that just extends the Symfony Route attribute. This is done to separate form route discovery from regular controller discovery, and also FormRoute is limited to targeting only classes, not methods. I've allowed multiple FormRoute attribute to be applied on one class, to allow for multiple routes to use the same form, but I'm not sure whether that's necessary or should be supported. It's a straightforward change if not.

Also, this adds all the classes/files in the Form namespace/directory to discovery. I think it should be technically possible to use Route attributes on classes (and methods) in \Form, and FormRoute attributes on classes in \Controller as well, though in the later case it won't work unless those classes implement FormInterface. That being said, I haven't tested this and I don't think this kind of usage should be promoted.

I think I found a bug in AttributeRouteDiscovery;

   // See https://symfony.com/doc/current/controller/service.html#invokable-controllers.
    if ($collection->count() && $class->hasMethod('__invoke') === 0) {

hasMethod returns a boolean, which would make the identity comparison to 0 always FALSE, so I changed it to

    if ($collection->count() === 0 && $class->hasMethod('__invoke')) {

I think this is the expected result, but not sure.

Lastly, ran into test failures because layout_builder does not have a dependency on field_ui, but Drupal\layout_builder\Form\LayoutBuilderEntityViewDisplayForm extends Drupal\field_ui\Form\EntityViewDisplayEditForm. So checking whether LayoutBuilderEntityViewDisplayForm exists with class_exists throws an exception when field_ui is not installed. In PHP 8.5, it's straightforward to handle this by catching the exception. For PHP < 8.5, we've had much sadness in plugin attribute discovery dealing with the fact that missing traits are fatal errors, not exceptions. I'm not sure if we want to bring that sadness over to route discovery, but it might be necessary.

For easier DX can we reuse the #[Route] attribute and detect internally whether we need to treat it as a form or a controller?

Yes, we could base it off either namespace detection or whether an object implements FormInterface.

If the first thing, then no form routes would be allowed in \Controller, and no controller routes allowed in \Form, which should be fine I think.

And in either case, that means that Route attributes applied to methods on a form would be ignored, which I think would be fine too.

Edited to add: Actually, we should always do the FormInterface detection. We could add the namespace detection as well if we don't want any forms in \Controller.

Actually, we should always do the FormInterface detection.

Removed FormRoute and made this change.

Started a CR: https://www.drupal.org/node/3585342.

Since I removed the new attribute and a sample form route was converted, not sure what's left for tests, so I'll move to NR and add any tests from feedback.

Added some test coverage.

Looking good, thanks for working on this. Added some questions/suggestions.

Applied the suggestion. Not sure either way whether it's worth detecting Route attributes on methods in the Form class to throw an error, but I can make that change if we think it's best.

What do you think we should do about the traits issue in older PHP?

We could use the MissingClassDetectionClassLoader the same as plugin discovery. I think it's worked pretty well, other than being a bit ugly to implement. There is one case it would not handle:

• Module A has Controller or Form class that uses a Trait in Module B, both uninstalled.
• If within one request:
  • Module A is installed individually, routes discovery runs, and the Module B Trait is aliased to Drupal\Component\Discovery\StubTrait
  • Module B is installed, route discovery runs, and the route in Module A should now be discovered
  • If somehow the controller or form class needs to be instantiated, then errors could occur because the methods that the Trait is supposed to define do not exist, since StubTrait does not have any code

This popped up in plugin discovery per #3255804-35: Hidden dependency on block_content in layout_builder, and we ended up having to move the trait. It seems less likely to me that a controller or form class would need to be instantiated somehow in the same request as module installation, so maybe it's more of an edge case here?

If we land this in 11.4 we should perhaps consider combining the change record with https://www.drupal.org/node/3324758 instead of two separate ones that kinda say the same thing.

Yeah, is that something we can do right before it's committed? Change the URLs in the MR and edit the original CR?

Yeah, is that something we can do right before it's committed? Change the URLs in the MR and edit the original CR?

Actually, there aren't any CR URLs in the MR, so if this lands in time, we can just update the original CR instead.

Started to copy the trait handling stuff from plugin discovery over, but it's a lot. That'll wait for another day, or maybe a follow up.

I stubbed a follow up #3586328: [11.x] Handle attribute discovery fatal errors on missing traits for routing and other subsystems to handle missing traits, to keep scope here down. We can consider there whether just to copy out what we need from plugin discovery to route discovery, or whether it's possible to extract the missing trait handling to somewhere it can be used for attribute discovery across multiple subsystems, including hooks now or possibly services in the future.

I think feedback otherwise has been addressed, so back to NR.

Addressed MR feedback. Back to NR.

Resolved a merge conflict in system.routing.yml after #3584823: Convert system module routes to use attributes was merged to main.

@berdir brought up in Slack with me and @longwave that he hit an error with route discovery because of a controller class with an optional dependency. The exception thrown by a missing class/interface/trait wasn't caught in the origin attribute routing issue, but it is addressed in this issue's MR. Noting here that catching that exception should probably be a priority for 11.4 beta, so if this one doesn't make it, we should create a separate issue to handle that.

Made fixes for or otherwise addressed MR comments.

If this goes in 11.4, I think the CR here shouldn't be published. We'll just add to https://www.drupal.org/node/3324758.

Rebased for merge conflict after #3592887: AttributeRouteDiscovery does not cleanly handle invalid classes

I guess we could move SearchBlockForm, but it might be unintuitive for it not to be in the Form namespace.

Alternatively, once we have #3582628: Add a generic way of scanning classes for attributes to scan for the attribute name, it should be skipped from being reflected, based on the str_contains check for the attribute.

11.x MR: https://git.drupalcode.org/project/drupal/-/merge_requests/16185

Committed 5bc84e8 and pushed to 11.x. Thanks!