Problem/Motivation
We are using the Autowire attribute in a number of places (e.g. #[Autowire(service: 'foo')]) because we don't have a service alias for the interface/class name.
Let's add the aliases so the Autowire attribute isn't needed wherever we can.
From #6 the list is:
$ grep -RhoP '#\[Autowire\(\K[^)]+' core | grep -v 'param:' | grep -v 'logger\.' | grep -v 'cache\.' | grep -v 'router'| grep -v 'keyvalue' | grep -v 'lock' | sort -u
'@plugin.manager.config_action'
'@workspaces.negotiator.query_parameter'
service: 'access_check.theme'
service: 'asset.css.collection_optimizer'
service: 'entity.memory_cache'
service: 'file.mime_type.guesser'
service: 'file.recursive_validator'
service: 'kernel'
service: 'node.view_all_nodes_memory_cache'
service: 'plugin.manager.field.formatter'
service: 'plugin.manager.help_section'
service: 'plugin.manager.views.join'
service: 'transliteration'
service: 'validation.constraint'
service: TranslationInterface::class
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3559978
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
kim.pepperComment #4
mstrelan commentedI've had to manually autowire the lock service a few times, and there are several instances of it in core. I don't think it needed to be reverted, it looks like the failing tests are relying on it as an example of a service that's not autowireable. They should be updated to use a different service or even a dummy service in a test module.
Comment #5
kim.pepperI guess I was trying to keep the scope of this issue to what is needed in the file module services. I bet there are a lot of other missing aliases. Do we want to try and find them and add them in this issue?
Comment #6
mstrelan commentedHow about a compromise of anywhere that is currently using the Autowire attribute? Can find with this command below. Not sure all of these can be autowired.
Comment #7
kim.pepperOK sounds good. I updated the title and IS.
From #6
Comment #8
kim.pepperI think we can take
logger.channel.foooff the list.Comment #9
longwaveWe can't autowire any of the cache or logger services, this is by design that you have to explicitly select which bin or channel that you want to use.
Similarly the reason I am not sure we can do
lockorlock.persistentis because they both implement the same interface, and it felt better to force the developer to select which case. From what I remember it's the same withkeyvalueandrouter, there are multiple implementations of the interface available. We could select a default implementation in these cases, but is that the right thing to do?Comment #10
kim.pepperUpdated the list in the IS to exclude those services listed above.
Comment #11
kim.pepperI had another review of this. I think we should avoid adding service aliases for classes and just focus on adding aliases for interfaces if they are missing. Adding aliases for plugin managers doesn't make sense to me. We might want to consider updating classes injecting plugin managers to use
PluginManagerInterfaceinstead?I added a couple of things I think should be changed.
Comment #12
nicxvan commentedShould we somehow enforce or document the ones that are intentionally not aliased?