Create a "setDestination" method on \Drupal\Core\Url to make it easier to set the destination

I think this is a good improvement. I think that this does make the DX better. But as you said, this needs tests.

1. +++ b/core/lib/Drupal/Core/Url.php
   @@ -887,4 +887,27 @@ public function setUnroutedUrlAssembler(UnroutedUrlAssemblerInterface $url_assem
   +  public function setDestination($path = NULL) {
   +    if (!isset($path)) {
   +      $destination = \Drupal::destination()->getAsArray();
   +    }
   +    else {
   +      $destination = ['destination' => $path];
   +    }
   

   Instead of having one method with an optional parameter I think it would be a bit nicer to have two: $this->setDestination and $this->setCurrentDestination. This way it is way more obvious what is going on.

2. +++ b/core/lib/Drupal/Core/Url.php
   @@ -887,4 +887,27 @@ public function setUnroutedUrlAssembler(UnroutedUrlAssemblerInterface $url_assem
   +    if ($query = $this->getOption('query')) {
   +      $query = array_merge($query, $destination);
   +    }
   +    else {
   +      $query = $destination;
   +    }
   +    $this->setOption('query', $query);
   

   What about using $url->mergeOptions for that bit?

That's much nicer!

Needs work so we can add the tests.

Was looking for the generic methods for the same. I have raised an MR with some additional checks, like the path should be internal only. Once this is reviewed, we can go ahead with writing tests also.

Current changes need to be reviewed

Seems to have pipeline failures. May be worth converting a spot where this is needed to help show why it should be added.

Test coverage is good, but I mean where in core could this be needed? Typically I see that functions that aren't called anywhere in core get deprecated and removed. So think we need least a spot or two in core where this is needed.

I just did a keyword search in the codebase, and a few files in which this could be used are:

• https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/block...
• https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
• https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
• https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/conte...

There could be many more like these.

Don't have to convert all but a few help show usefulness

Was trying to find occurences of

$url->setOption('query', \Drupal::destination()->getAsArray())

and $url->setOption('query', ['destination' => 'other/path'), could only find one, pushed that change.

Tests were added, so I am removing the tag for now.

Left some comments.

But proposed solution mentions just 1 method. If there's only 1 instance in core that could use it is this fully needed? If so will also require a CR.

If there's only 1 instance in core that could use it is this fully needed?

It's an API helper function that can be used by developers. I often need to set the destination on URLs.

I agree that this is valuable beyond just core. Today I need it for drush site:install - https://github.com/drush-ops/drush/issues/6310.