Deprecate EntityResolverManager procedural controllers

Some relevant test failures.

I think the CR needs updating to tell us where we can't use these keys. Is this in route definitions (yaml / route provider)?

> I think the CR needs updating to tell us where we can't use these keys. Is this in route definitions (yaml / route provider)?

I honestly have no idea. This is about _controller and form definitions. forms are already limited to being a class. I've never used a function as a _controller and wasn't even aware that this is supported. Maybe it isn't.

The purpose of it is essentially to automatically set upcasting information for entity parameters on routes and forms, something that in my experience works pretty badly and typically needs to be done by hand.

So essentially the CR is that something no longer works on functions, maybe never did, and it rarely works anyway.

I'm not sure we even want to do a CR here. If we're confused about it, then people viewing it will be even more confused.

So one example that causes the deprecation is this route definition in system_test.routing.yml:

system_test.always_denied:
  path: '/system-test/always-denied'
  defaults:
    _title: 'Always denied'
    _controller: 'chop'
  requirements:
    _access: 'FALSE'

so it apparently is possible to set a controller to a function, except this will never be called anyway.

The problem with the deprecation is that this runs on every single defined route. If we want to do a deprecation, we should only do it on a route with a function where we actually would do something. So we need to trigger it in \Drupal\Core\Entity\EntityResolverManager::setParametersFromReflection, when expand parameters and $controller is a function.

I don't get why we'd need to change where this is, we only throw it if it is a function and the function exists where we have it. How are these being called on all routes?

Because this is called from a route event subscriber that calls it on every route. See \Drupal\Core\EventSubscriber\EntityRouteAlterSubscriber::onRoutingRouteAlterSetType().

Note that this is different from the other issues. This is a fallback and it is not exclusively supporting only functions, it just also supports functions. While weird and not used in core except for one test, it's not something we have to remove.

so we have 3 options:

1. Deprecate just functions, that's fairly complex although possible as explained above.
2. Deprecate the whole functionality, As mentioned, in my experience, it rarely works, is unpredictable and hard to debug, especially when combined with title and access callbacks. Deprecation in same place, when we actually add the options, but unconditionally. This might actually be beneficial for performance, because this logic requires that on a router rebuild, we need to load and inspect any controller class/callback. On plain core, blackfire reports that this event subscriber uses about 100ms and it's more the more routes you have.
3. Won't fix this. It's not blocking anything.

IMHO, I'd either do 2 or 3. We could see just how many routes are affected in core if we trigger a deprecation for that? Many entity routes are generic and if necessary, we could add the options explicitly if not done yet?