Problem/Motivation
As per #3490713: Return the correct typehint when Drupal::service() is called with a class string, but for \Drupal::classResolver().
Calling \Drupal::classResolver($class) does not return a type hint for PHPStan validation and IDE completion.
We can provide PHPStan annotations to infer the return type when the class string passed in as an argument.
Steps to reproduce
Proposed resolution
We can use Generics and conditional return types to acheive this.
For example:
<?php
/**
* ...
*
* @param class-string<T> $class
* (optional) A class name to instantiate.
*
* @template T of object
*
* @return ($class is class-string<T> ? T : \Drupal\Core\DependencyInjection\ClassResolverInterface)
* The class resolver or if $class is provided, a class instance with a
* given class definition.
*
* ...
*/
public static function classResolver($class = NULL) {
?>
Since \Drupal::classResolver() is effectively a wrapper around ClassResolverInterface::getInstanceFromDefinition() we should update the annotation there too. Note it is slightly different as that does not allow NULL, and explicitly allows a service id.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3606411
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:
- 3606411-drupal-classresolver
changes, plain diff MR !16150
Comments
Comment #3
mstrelan commentedComment #4
danielvezaVery nice! This looks good, I can't see any other asserts around the other usages of classResolver that would need to be removed. I think this is ready.
Comment #5
mstrelan commentedActually we should update
\Drupal\Core\DependencyInjection\ClassResolverInterface::getInstanceFromDefinitiontoo.Comment #6
kieran.cottLooks good @mstrelan, I've updated
ClassResolverInterface::getInstanceFromDefinitionto match.Comment #7
kieran.cottComment #8
smustgrave commentedNitpicky but can the summary be completed please
Comment #9
mstrelan commentedUpdated issue summary as per the related issue.
Comment #10
smustgrave commentedThanks LGTM.
Comment #11
longwaveAdded a question about further simplification.
Comment #12
mstrelan commentedIn-lined the vars as per longwave's feedback. FWIW we could inject the classResolver and moduleHandler in to the hooks class as well, but that's not really in scope.
Comment #13
longwaveTBH all code from InlineBlockEntityOperations and LayoutOverrideFieldHelper could be inlined directly into LayoutBuilderHooks, or they could be converted individually into their own hook classes, but refactoring all that is followup material for sure.
Comment #14
longwaveComment #19
godotislateSince the
$classargument ofclassResolver()defaults to NULL, made a small change to add a union type|null.Committed and pushed 0daf93c to main and ec8c3ab to 11.x. Thanks!