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

Command icon 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

mstrelan created an issue. See original summary.

mstrelan’s picture

Status: Active » Needs review
danielveza’s picture

Status: Needs review » Reviewed & tested by the community

Very 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.

mstrelan’s picture

Status: Reviewed & tested by the community » Needs work

Actually we should update \Drupal\Core\DependencyInjection\ClassResolverInterface::getInstanceFromDefinition too.

kieran.cott’s picture

Looks good @mstrelan, I've updated ClassResolverInterface::getInstanceFromDefinition to match.

kieran.cott’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Needs work

Nitpicky but can the summary be completed please

mstrelan’s picture

Issue summary: View changes
Status: Needs work » Needs review
Related issues: +#3490713: Return the correct typehint when Drupal::service() is called with a class string

Updated issue summary as per the related issue.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks LGTM.

longwave’s picture

Status: Reviewed & tested by the community » Needs review

Added a question about further simplification.

mstrelan’s picture

In-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.

longwave’s picture

TBH 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.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

godotislate made their first commit to this issue’s fork.

  • godotislate committed ec8c3abd on 11.x
    task: #3606411 Return the correct typehint when Drupal::classResolver()...

  • godotislate committed 0daf93c5 on main
    task: #3606411 Return the correct typehint when Drupal::classResolver()...
godotislate’s picture

Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Since the $class argument of classResolver() 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!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.