Problem/Motivation

In Drupal, Drupal\Core\Routing\Router::matchCollection() is declared as:

   * @return array|null
   *   An array of parameters. NULL when there is no match.
   */
  protected function matchCollection($pathinfo, RouteCollection $routes) {

In Symfony 6, the signature changes to

    protected function matchCollection(string $pathinfo, RouteCollection $routes): array

ie. we can no longer return NULL when there is no match. The upstream docblock gives a clue as to a possible fix:

     * @throws ResourceNotFoundException If the resource could not be found

Steps to reproduce

Proposed resolution

Convert the NULL return to an exception.

Remaining tasks

See what side effects this change has.
Figure out if this is only affects core or could affect contrib/custom code.
If needed, figure out how to warn downstream users of this change.

User interface changes

API changes

Router::matchCollection() will throw an exception instead of returning NULL when there is no match.

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3259674-2.patch1.97 KBlongwave

Comments

longwave created an issue. See original summary.

longwave’s picture

Status: Active » Needs review
StatusFileSize
new1.97 KB

So it turns out there is a throw statement already, we can just move it from matchRequest() down to matchCollection(). As matchCollection() is protected I think this is safe to do with no effect to downstream users - callers will be using matchRequest() and the end result will be the same.

This patch also adds the return type to matchCollection() as required by Symfony 6.

catch’s picture

Status: Needs review » Reviewed & tested by the community

Nice easy one in the end.

andypost’s picture

Maybe it needs to update docblock to point exceptions for another method ?

longwave’s picture

@andypost docblocks are both @inheritdoc now, and the Symfony interfaces mention both exceptions, so I think we are OK.

  • catch committed faa6580 on 10.0.x
    Issue #3259674 by longwave, andypost: [Symfony 6] Drupal\Core\Routing\...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Been a few days since I RTBCed this so I think this can just go in.

Committed faa6580 and pushed to 10.0.x. Thanks!

Status: Fixed » Closed (fixed)

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