Use PHP attributes for route discovery

As I mentioned in #3317792-11: Make controllers invokable for DX and maintainability , we should make sure our implementation works with classes (tests!), as the Route attribute we are using is \Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD.

As in, working Symfony code:

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Annotation\Route;

#[Route('/test')]
final class TestController extends AbstractController {

  public function __invoke(): Response {
    return new Response('Hello world!');
  }

}

No longer need route names - these can be automatically provided based on the class and method name

That's what I faced with in #3159210: Support route aliasing (Symfony 5.4) and allow deprecating the route name

Aliases should replace alias route name with a name from target route in route match.

So if no route names available then how to alter routes? I mean the class name will be hard to deprecate instead of yaml file

PHP 8.1 and 8.2 both green using last patch, patch needs some polishing of docs but mostly looks ready

Route rebuilding is not much often happening so no reason to measure performance effect on descovery

PS: not sure is that a task or feature

This is a great idea! +1

The routing.yml file can give a good overview of what a module does and makes reviewing route access a bit simpler

I actually don't think that the problem is unique to this issue, it technically already exists thanks to Route Providers and Enhancers. I'm thinking Entity Types which have Route Providers and routes which are added by Route Enhancers (e.g. Field Ui and Layout Builder). In most cases the routing.yml file already does not provide a good overview of all the routes associated with a given module, so if anything the solution here just continues this existing pattern.

We should and can build CLI tools to expose all the routes provided by a module. This would be good not just for routes determined by attributes but also dynamic routes

This is also a good idea, and as I noted the current situation would definitely benefit from this. Maybe that would help gather momentum for #3089277: Provide core CLI commands for the most common features of Drush.

Can we use Psr4DirectoryLoader? See https://github.com/symfony/symfony/blob/6.2/src/Symfony/Component/Routin... and https://symfony.com/blog/new-in-symfony-6-2-psr-4-route-loader

Add a new static routing event for the yaml discovery to listen to
Add a new service for PHP attribute discovery and listen to the same event. This reflects on all classes in a module's src/Controllers directory to find routes

Why an event and not a tagged service?

Don't think we need ordering, ability to stop propogation etc.

Think the MR got altered. Seeing 900+ changes for ckeditor, composer, etc. Is that expected?

@smustgrave seems ok now. I saw something similar on a different issue. I think its a gitlab problem.

Will add to my list.

I can take a look at #13:

a unit test of AttributeRouteDiscovery would be good.

I've opened an issue to decide on where to put annotation classes, so all annotation issues can work to a common standard: #3344303: [policy, no patch] Decide on a namespace for PHP Attribute classes.

A couple of extracted comments from @alexpott and @berdir in slack:

alexpott
  3 days ago
OTOH we could teach POTX to extract _title from Route attributes like it does with routing yml


alexpott
  3 days ago
I have no idea how _title ends up in TranslatableMarkup :smile:


alexpott
  3 days ago
Oh that happens here… \Drupal\Core\Controller\TitleResolver::getTitle

berdir
  3 days ago
you're too fast, was just about to paste that


berdir
  3 days ago
didn't know we make any raw variable available as a placeholder, can't imagine that this is often used and seems pretty scary


alexpott
  3 days ago
Yeah so I think we’ll need to open an issue to extend POTX to support discovery of certain attributes… which given what it already does shouldn’t be too bad.

Before reviewing can the remaining tasks be crossed out for what has been completed please.

I've added more test coverage so crossed that out. Not sure if it's enough test coverage though.

Seems to have some open threads.

Addressed feedback.

Left a review and nitpicks, not sure core need to adopt sophisticated logic from SF with automatic route name generation

Somehow it discovery should report about routes without name

@andypost think there are some questions in the MR for you? There are still some open threads.

I mean if route name will be generated from class-name or whatever it will be DX-- to use such names in URL generator and making links

> Why not? Naming thing with IDs opens the possibility of namespace conflicts. Less hard coded IDs would be a good thing.

All it means is that instead of having a code ID that shouldn't change, you have a class that you can't rename.

Routes need to be referred to by other routes, child menu items etc etc.

I actually think the introduction of route IDs in D8 was a really good thing -- much easier to deal with route IDs rather than paths.

As a custom module developer and site builder, I think I'd be fairly averse to auto generated route names based on the controller class.

Will this be optional? Would it include the namespace? I might have more than one DownloadController across a suite of modules for example.

• I shouldn't need a cli tool to discover a route name.
• I definitely don't want a route name to change if a controller class changes.
• I do want to be able to define a route name explicitly and search for its definition or usages in my IDE.

What about _title_callback?

As a custom module developer and site builder, I think I'd be fairly averse to auto generated route names based on the controller class.

Personally I think I like automatic route names based on the class name.
Obviously it should include the namespace.

But if we want to keep custom ids, one pattern that can be useful in combination with attributes is class constants.

class MyController {
  const ROUTE_ID = 'mymodule.myroute';

  #[Route(id: self::ROUTE_ID, ...)]
  public function page(..) {}
}

Then other code can target that constant to refer to the route.

$url = Url::fromRoute(MyController::ROUTE_ID);

Of course, this again makes that code dependent on the class holding the constant.
To avoid that, the constant could live in a separate class..

A big question is also whether we want to convert the other yml files to attributes, e.g. for local tasks, menu links etc.
And as a consequence, will we refer to routes more often from yml files or from php code?
In yml files we cannot use constants (afaik), we cannot use class imports, and string ids generally look nicer than class names in yml.
In php code, I prefer class constants over string literals.

I wonder if we could have local tasks, menu links, etc, also defined in the controller attribute. When creating a controller, and you want a local task to appear, it might be quite nice DX to just simply attach an attribute to the controller, rather than having to edit YAML files and have to manage the references. If things like local tasks can be defined in an attribute with the controller, that also simplifies it slightly because then you don't need to know or care what the route name is.

@AaronMcHale great idea, ++ to add to summary and probably as follow-up issue

Missing hook_menu()? :)))

> Personally I think I like automatic route names based on the class name.
> Obviously it should include the namespace.

It's not explicitly stated in the BC policy, but route names have to be public API so that you can work with them to generate links, add extra pages to the UI, etc.

Automatic route names would mean the controller class name becomes API. I'm not automatically against that, but it's not what we're used to.

Also, it would worsen DX because how is a developer supposed to find the route name?

Building on my previous comment, and the concerns about automatic route naming. If we're going to use attributes anyway, then why not just define the route name in the attribute.

That would address the BC and DX problems stated above, automatic route naming sounds nice in theory but it doesn't sound like it's worth it for the trade-offs mentioned.

It could also make it harder for new developers to understand what's going on, if the route names are generated automatically, we're abstracting away a key piece of information that could really help someone new understand routes.

The big DX win here is the ability to define the route (and potentially links, etc) right there in the controller class. Just that in itself is a huge win!

Not sure if this is irrelevant but a contrib module is already using PHP attributes for hook discovery - https://www.drupal.org/project/hux

> If we're going to use attributes anyway, then why not just define the route name in the attribute.

+1

> Missing hook_menu()? :)))

Hmmm yes and no! Having to mess about in THREE different Yaml files to add a new admin tab is tedious. But hook_menu() wasn't simple, and there were plenty of ways that could go wrong too. I seem to remember complications around the MENU_DEFAULT_TASK value or something like that.

So I'd cautiously say yes to some sort of additional attributes to say 'Put this route in the menu' and 'Put this route in a tab set'. But as a follow-up issue - let's keep this one reasonably simple, and allow addition of future attributes when we design it.

> The defaults._controller value is determined by the method you add the attribute to

I don't want to add complexity to this issue, so it should be a follow-up, but we should bear in mind in the building of this that we might want to be able to also use a Route attribute on a form class, where defaults._form would be the route property that gets set to the form class, rather than defaults._controller set to the method.

I don't want to add complexity to this issue, so it should be a follow-up, but we should bear in mind in the building of this that we might want to be able to also use a Route attribute on a form class, where defaults._form would be the route property that gets set to the form class, rather than defaults._controller set to the method.

I support it. But this should be a different attribute class.

So I'd cautiously say yes to some sort of additional attributes to say 'Put this route in the menu' and 'Put this route in a tab set'. But as a follow-up issue - let's keep this one reasonably simple, and allow addition of future attributes when we design it.

A question is do we "pollute" the route list with these meta keys, or do we collect a different kind of item that we then use to generate routes _and_ links?

Very interesting — this looks indeed … very ready. Posted a few questions on the MR :)

We should and can build CLI tools to expose all the routes provided by a module. This would be good not just for routes determined by attributes but also dynamic routes

This is true for not just routes, but also for all plugins of any given type: that is a frequent challenge when working on migrations (Which migration destination/source/process plugins exist?), validation (Which validation constraint plugins exists?), and more.

Related may need some work too

@alexpott
You created two change records:
- https://www.drupal.org/node/3324758
- https://www.drupal.org/node/3314769

Neither of them had any information in it, so I started filling out some information on the first.
Is there a reason for us to keep the second one around?

Good question. I think I'd expect these to be declared in a modules services file - maybe an event subscriber. Yeah - this change introduces RoutingEvents::STATIC so these callbacks could be changed to an event subscriber listening to that event and then add their routes in.

Added https://www.drupal.org/project/drupal/issues/3401172

@duadua I think I got a bit change-record-keen :)

For a while I was trying to figure out whether there is a logical space in which the slightly different title might exist.
I tried to delete the particular change record, but there are some additional permissions required. Someone will probably see the updated title.

I really like the idea of copying Symfony 6.4 and adding an always there FQCN route alias. That would mean getting #3159210: Support route aliasing (Symfony 5.4) and allow deprecating the route name done. Definitely think it is worth a follow-up. I wonder if we should forgo the automatically generated names like 'MethodRouteLocale' and instead where no name is provide automatically name using the FQCN that Symfony would create an alias for?

What feels very sensible for me. Looking at the current way its generated, a FQCN name would be no way longer compared to the current autogeneration.
Looking at this it just let me realize that auto-generation has the advantage of avoiding accidental re-definitions of the same route name.

CREATE TABLE IF NOT EXISTS "router" (
"name" VARCHAR(255) NOT NULL DEFAULT '',
"path" VARCHAR(255) NOT NULL DEFAULT '',
"pattern_outline" VARCHAR(255) NOT NULL DEFAULT '',
"fit" INTEGER NOT NULL DEFAULT 0,
"route" BLOB DEFAULT NULL,
"number_parts" INTEGER NOT NULL DEFAULT 0,
"alias" VARCHAR(255) NOT NULL DEFAULT '',
 PRIMARY KEY ("name")
);
CREATE INDEX "router_pattern_outline_parts" ON "router" ("pattern_outline", "number_parts");
CREATE INDEX "router_alias" ON "router" ("alias");

Looking at the schema for the router, I see there is a limit of length for the route name. Looking at everything, 255 seems plenty of space though for:

• Drupal
• Module name
• "Controller"
• Controller name
• Method name

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Couple of scenarios which comes in my mind, which we should address:

• Currently multiple route definition can point to a single class method, How we will define routes like that with php attributes?
• Route names are used to define menu links, menu tabs and at various other places also, I think we are aiming to generate auto route names but it also kind of relates to above point, How we are going to address that?
• What would be the process of overriding an existing route? Using route subscribers or something else?

Would love to see this happen, this would be a massive DX improvement! Bumping priority 😇

I suspect this is also a de facto hard blocker for #3317792: Make controllers invokable for DX and maintainability ? 🤔

P.S.: would also help Experience Builder: #3490069-13: [upstream] [PP-1] Auto-register XB's controllers' invokable services.

Rebased, needs the deprecations bringing up to date again.

Re #22 and the test failures I wonder if we should just allow strings only in _title for now, and move to TranslatableMarkup there in a followup?

Re #22/#64 actually I think the fix is simple, if _title is already TranslatableMarkup then we can use it directly; there is no need for a separate _title_arguments or _title_context when we are using attributes. If we want to remove that _raw_variables usage in TitleResolver::getTitle() then we can do that separately in another issue.

Also backported the changes from more recent Symfony versions, which adds automatic FQCN aliases for invokable controllers, and the route aliases feature from Symfony 7.3: https://symfony.com/blog/new-in-symfony-7-3-routing-improvements#route-a...

As suggested in #44 I think it would be trivial to add support for _form alongside _controller here, we likely just have to scan the Form directory as well and check whether the class implements FormInterface.

Some comments on the MR, mostly small.

Do we need a follow up to add a filecache?

From issue summary:

We should and can build CLI tools to expose all the routes provided by a module. This would be good not just for routes determined by attributes but also dynamic routes

Might be worth noting that Drush provides a core:route command which lists all routes, actually very useful sometimes! So given that we may conclude that we can do that in a follow-up, if we need it at all.

The routing.yml file can give a good overview of what a module does and makes reviewing route access a bit simpler

While this is true, in a lot of cases the routing.yml file gives at best an incomplete picture, because most entity types will use a route provider, and so those routes won't be in the routing.yml file. So you could argue that this actually isn't really a strong advantage of the routing.yml file.

Addressed all MR feedback, this is ready for re-review. Also updated the deprecations and untagged as this didn't make 11.3.0; are the deprecations are niche enough that they can still be in 11.4 for removal in 12, or should we bump to 13?

Was a bit concerned about extra loading of PHP files for reflection during route rebuilds, but every time I make a typo in a controller class, I'm reminded that tends to happen during route rebuild already, so this might not result in much extra class loading at all. Haven't checked exactly where the controller classes get loaded from yet.

I think that's because of \Drupal\Core\Entity\EntityResolverManager, see #3539922: Deprecate EntityResolverManager procedural controllers, but I think if we add this then the argument about performance there goes away, so that might be one more reason to just won't fix that issue.

One small comment on the MR.

Some questions:

• Does there need to be any test routes having the other route properties (schemes, methods, options, host, condition, priority), and/or how they are merged or replaced with values on the class?
• Does there need to be documentation of the prefixing/merging/replacing of class attribute values with method attribute values in the CR? Or maybe a link to Symfony routing documentation is enough?
• Should support for _form be done here or in a follow up? Would the attribute on form classes be put on the class? Or the buildForm method?
• Should attribute discovery use a file cache? If so, does there need to be a follow up for that?

Otherwise, I think this looks pretty close, but I'm not sure what other comments in this issue might be outstanding. (It's a long comment thread.)

> Note: The route name is optional and will be generated based upon the class name.

How? Where does this happen? For routes which omit the name (and I think this is a bad idea), developers will need to understand how to get the route name.

Added some more test coverage, but three properties don't appear to be used in core. host is checked in the route matcher (but I don't remember ever seeing it used), schemes is only used for URL generation (but not matching that I can see) and condition is never checked at all. Should we drop support for some or all these here? We can always add them later.

I think _form should be handled in a followup so we can decide the best DX - maybe it even needs a separate attribute.

I also think we shouldn't bother with file cache for now, again we could do that in a followup if it turns out that we need it.

I'm in two minds about the auto name generation, while Symfony supports it does it makes sense for us? Is there actually a use case in Drupal? We could require it for now, and then make it optional later if we wanted to.

host is checked in the route matcher (but I don't remember ever seeing it used), schemes is only used for URL generation (but not matching that I can see) and condition is never checked at all. Should we drop support for some or all these here? We can always add them later.

I have used schemes on a project before where Drupal generated an ics-formatted response using field data from an Event content type in an "addToCalendar" controller method. IIRC, (it was a long time ago), setting the first schemes value to "webcal" made it possible that clicking on the link would open the link in the device's default calendar app. Also, I'm not sure about dropping host, but condition probably could be dropped.

I'm in two minds about the auto name generation, while Symfony supports it does it makes sense for us? Is there actually a use case in Drupal? We could require it for now, and then make it optional later if we wanted to.

We need to know route names for alters and for menus, local tasks, and local actions. While not every route will have those, I think it's hard to be confident when creating a route that it will never be used in any of those contexts. Also, the automate route name generation does not result in something using the FQCN (in the MR, looks like \ is replaced with _), so the route names are still magic strings that need to be looked up, just with more difficulty now. Though per #70, drush provides a core:route command to list all routes, so that probably would help.

I dropped support for condition because while you can set it, it does nothing (this was the case in YAML, but we can be more explicit here).

I also made name required for the method attribute (but it's still optional for class level). We can always add auto-naming later but I don't think it's very useful and the name is not discoverable.

Updated the CR with more examples.

> Also, the automate route name generation does not result in something using the FQCN (in the MR, looks like \ is replaced with _), so the route names are still magic strings that need to be looked up, just with more difficulty now. Though per #70, drush provides a core:route command to list all routes, so that probably would help.

That would make it quite a bit harder to go from seeing something that affects a route, or uses a route, and finding the controller for that route. You'd need to reverse-engineer the FQCN from the route name, instead of just grepping the codebase for the string.

Added test coverage for the additional properties looks good.
Additional examples in https://www.drupal.org/node/3324758 look good as well. I also updated the version numbers in the other 2 CRs.
The change to require the route name also looks good.

I think there was a previous concern about POTX picking up the title, but seems like it was addressed in #66.

This issue is already RTBC, but it looks to me as though the issue summary could use some love:

1. One of the two Remaining tasks has not been crossed off.
2. The API changes section still says "TBD".
3. The Release notes snippet still says "@todo".

Also from the issue summary:

We should and can build CLI tools to expose all the routes provided by a module. This would be good not just for routes determined by attributes but also dynamic routes

I believe that drush route already handles dynamic routes. It can list all routes or details about a specific route, but not all routes for a module. For example:

$ ddev drush route | head
'<button>': /
'<current>': /<current>
'<front>': /
'<nolink>': /
'<none>': /
announcements_feed.announcement: /admin/announcements_feed
big_pipe.nojs: /big_pipe/no-js
block.admin_add: '/admin/structure/block/add/{plugin_id}/{theme}'
block.admin_demo: '/admin/structure/block/demo/{theme}'
block.admin_display: /admin/structure/block
$ ddev drush route --path=/admin/structure/types/manage/page/fields | head
name: entity.node.field_ui_fields
path: '/admin/structure/types/manage/{node_type}/fields'
defaults:
  _controller: '\Drupal\field_ui\Controller\FieldConfigListController::listing'
  _title: 'Manage fields'
  entity_type_id: node
  bundle: ''
requirements:
  _permission: 'administer node fields'
options:

Thanks - updated the IS, I think the Symfony issues are solved, I made a note of the API changes, and we don't need a release note for this as it doesn't affect site owners.

Re #83 - for this task, "Decide which of the Symfony route features to keep in - env, localized_paths, priority, _locale, _canonical_route stuff... not sure what to do with this", the MR supports these arguments from the Symfony Route attribute contructor:
path, name, requirements, options, defaults, host, methods, schemes, priority, and alias.

Of the remaining arguments, exceptions are thrown if _locale or condition is set. The others (format, utf8, stateless, and env) are just ignored. Maybe exceptions should be thrown for those as well?

_canonical_route isn't an argument to Route, and since there's no existing support in Drupal routing AFAICT for defaults['_canonical_route'], it can continue to be ignored?

Overall this looks great, couple of minor comments on the MR, and do we want to address #85?

Also I don't see a follow-up for converting core routes, but I would think we'd want to do probably all of them?

The CR needs to say that controllers need to be in the Controller namespace.

Made the CR change.

Also, I did a quick skim of comments, and I think these are follow ups that have been mentioned to be created:

• Support for form routes
• Converting core routes
• File cache(?)

Is it also the case that translated strings need to be translated explicitly in the attribute, unlike in routes where it's implicitly done for certain properties?

> Other attributes will be merged together

Deep merge of arrays? Who wins if a clash? CR should say. And is this documented in the code?

Whether or not we add file cache support should probably depend on whether the controller classes get loaded for other reasons, and if we think it's possible to remove that too - if we do then it might be worth doing. Although there's also the issue that most router rebuilds happen on the cli, it's directly rebuilt when you call drush cr (as opposed to most other caches which get built on the next request that needs them). Which goes back to #3486503: Add a persistent cache for file-based discovery based on FileCache and having a file cache implementation that doesn't use apcu. All reasons not to do it here but worth a follow-up definitely.

Addressed MR feedback, also added an explanation of attribute merging to the CR.

@joachim plain text titles will still be passed through TranslatableMarkup, but they will not be picked up by potx, so best practice is to use TranslatableMarkup as we do elsewhere with translatable strings in attributes.

We could also have followup(s) for this point from #58:

Route names are used to define menu links, menu tabs and at various other places also, I think we are aiming to generate auto route names but it also kind of relates to above point, How we are going to address that?

Once we've added this feature I think we could also add attributes in place of module.links.*.yml?

Two small suggestions and I think we're there.

Accepted both suggestions, thanks!

Think it looks good.

NW for merge conflict.

Collided with #3579890: Convert expectation-less test mocks to stubs - Database, Routing, rebased, back to RTBC.

Opened a second MR for 11.x as the issue in #99 means we have diverged a bit.

Needs a rebase/update for phpunit 12:

Running PHPStan on changed files.
 ------ ----------------------------------------------------------------------- 
  Line   tests/Drupal/Tests/Core/Routing/AttributeRouteDiscoveryTest.php        
 ------ ----------------------------------------------------------------------- 
  101    Call to an undefined method                                            
         Drupal\Tests\Core\Routing\AttributeRouteDiscoveryTest::expectDeprecat  
         ion().                                                                 
         🪪  method.notFound                                                     
 ------ ----------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------ 
  Line   tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php              
 ------ ------------------------------------------------------------------ 
  377    Call to an undefined method                                       
         Drupal\Tests\Core\Routing\RouteBuilderTest::expectDeprecation().  
         🪪  method.notFound                                                
 ------ ------------------------------------------------------------------ 

I already reviewed this before and it's had an even more recent review from @godotislate so happy to commit this as soon as it's back to RTBC.

Would also be good to open the follow-ups - e.g. attributes for links/tasks/actions all got discussed here and we should make sure we have issues for them so those ideas don't get lost.

Also one to evaluate performance once we've done lots of conversions, investigate file cache (but might need a non-apcu backend due to drush) etc.

Fixed the deprecation messages on the main MR, but got these now:

PHPUnit 12.5.14 by Sebastian Bergmann and contributors.
    
    Runtime:       PHP 8.5.4
    Configuration: /builds/core/phpunit.xml.dist
    
    ......N                                                             7 / 7 (100%)
    
    Time: 00:00.395, Memory: 12.00 MB
    
    Route Builder (Drupal\Tests\Core\Routing\RouteBuilder)
     ✔ Rebuild locking unlocking
     ✔ Rebuild blocking lock
     ✔ Rebuild with static module routes
     ✔ Rebuild with provider based routes
     ✔ Rebuild if needed
     ✔ Rebuild with overridden route class
     ✔ Deprecated constructor args
    
    1 test triggered 2 PHPUnit notices:
    
    1) Drupal\Tests\Core\Routing\RouteBuilderTest::testDeprecatedConstructorArgs
    * No expectations were configured for the mock object for Drupal\Core\Lock\LockBackendInterface. Consider refactoring your test code to use a test stub instead. The #[AllowMockObjectsWithoutExpectations] attribute can be used to opt out of this check.
    
    * No expectations were configured for the mock object for \Drupal\Core\Discovery\YamlDiscovery. Consider refactoring your test code to use a test stub instead. The #[AllowMockObjectsWithoutExpectations] attribute can be used to opt out of this check.
    
    /builds/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php:376

Added #[AllowMockObjectsWithoutExpectations] to the deprecation test, it seems the most pragmatic way to solve it.

Also rebased the 11.x branch.

Self re-RTBCing, this has only had minor test fixes.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I guess I need to make the bot check the MR with the target branch the same as the issue.

The main branch MR did have a merge conflict, resolved and rebased.

Committed/pushed to main and 11.x, thanks!

I think there was a cross-post with the bot, so setting back to Fixed per #112.

Stubbed some follow ups, hopefully didn't miss any:

• #3584791: Convert route callbacks to event subscribers
• #3584793: Use PHP attributes for form route discovery
• #3584794: Convert core routes to PHP attributes
• #3584795: Remove non-controller classes from module Controller namespaces/folders
• #3584798: Use PHP attributes to define menu links

Fantastic to see this get in. Another potential follow-up, use attributes to define services. Thoughts?

@aaronmchale that's already possible with autowiring. We're using it for several newish services in core but haven't gone back to convert everything else yet. Not sure where the meta is for that.

Similar for plugins.

@godotislate I think we probably want an issue to evaluate whether to use file cache for this or not too.

@aaronmchale: for service autodiscovery, there's #3422359: Directory based automatic service creation (inactive for 2 years), but I think a major issue with autodiscovery is the performance hit on container rebuild for iterating through entire directory trees and reflecting/loading into memory every file/class.

@catch: opened #3584878: [PP-2] Look into adding a file cache for route discovery.

Thanks both, that's useful to know.

@catch do you have an example to hand of one of those new services I could look at?

@aaronmchale so e.g.

class WorkspaceOperationFactory {

  public function __construct(
    protected EntityTypeManagerInterface $entityTypeManager,
    protected Connection $database,
    protected WorkspaceManagerInterface $workspaceManager,
    protected WorkspaceTrackerInterface $workspaceTracker,
    protected EventDispatcherInterface $eventDispatcher,
    #[Autowire(service: 'logger.channel.workspaces')]
    protected LoggerInterface $logger,
    protected TimeInterface $time,
  ) {}

but the YAML just looks like:

  workspaces.operation_factory:
    class: Drupal\workspaces\WorkspaceOperationFactory
    autowire: true
  Drupal\workspaces\WorkspaceOperationFactory: '@workspaces.operation_factory'

Or an even shorter one:

  Drupal\Core\Extension\ThemeSettingsProvider:
    autowire: true

vs the constructor.

  /**
   * Builds a new service instance.
   */
  public function __construct(
    protected readonly ThemeManagerInterface $themeManager,
    protected readonly ThemeInitializationInterface $themeInitialization,
    protected readonly ThemeHandlerInterface $themeHandler,
    protected readonly ConfigFactoryInterface $configFactory,
    protected readonly FileUrlGeneratorInterface $fileUrlGenerator,
    #[Autowire(service: 'cache.memory')]
    protected readonly CacheBackendInterface $memoryCache,
  ) {
  }

#3376163: Support #[AsEventListener] attribute on classes and methods is open to register event listeners via attributes, without any services.yml entry at all.

However what I think you actually asked for is #3422359: Directory based automatic service creation which @godotislate mentioned already.

#3376163: Support #[AsEventListener] attribute on classes and methods is open to register event listeners via attributes, without any services.yml entry at all.

Unfortunately not. You will still need to register the class making use of #[AsEventListener] as service. Otherwise Symfony couldn't find it.

So to confirm, unlike in a YAML file where title strings are assumed to require translation, in an attribute you have to explicitly translate?

That should be pointed out in the CR if that's the case.

CR updated.

Added #3586832: Allow kernel test classes to define their own routes as another follow up.