Inject redirect.destination service instead of using \Drupal:: destination() in non-tests

Seems there still two instances of
\Drupal::destination()->getAsArray
That should be replaced.

Updated patch #29 by addressing #30, please review it.

Thanks!

Seems all instances have been replaced.

+++ b/core/lib/Drupal/Core/Render/Element/SystemCompactLink.php
@@ -66,7 +66,7 @@ public static function preRenderCompactLink($element) {
-        'query' => \Drupal::destination()->getAsArray(),
+        'query' => \Drupal::service('redirect.destination')->getAsArray(),

+++ b/core/modules/workspaces/workspaces.module
@@ -227,7 +227,7 @@ function workspaces_toolbar() {
-      '#url' => Url::fromRoute('entity.workspace.collection', [], ['query' => \Drupal::destination()->getAsArray()]),
+      '#url' => Url::fromRoute('entity.workspace.collection', [], ['query' => \Drupal::service('redirect.destination')->getAsArray()]),

All changes should be reverted and are invalid. Replacing \Drupal::destination() with \Drupal::service('redirect.destination') does not in scope here and makes no sense. They are almost the same. And it's not using IoC injection.

An example coming in the next comment.

Taking core/modules/views/src/Plugin/views/field/Links.php as an example.

diff --git a/core/modules/views/src/Plugin/views/field/Links.php b/core/modules/views/src/Plugin/views/field/Links.php
index 899d082504..2f4d975218 100644
--- a/core/modules/views/src/Plugin/views/field/Links.php
+++ b/core/modules/views/src/Plugin/views/field/Links.php
@@ -4,7 +4,9 @@
 
 use Drupal\Component\Utility\Html;
 use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\Routing\RedirectDestinationInterface;
 use Drupal\Core\Url as UrlObject;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * An abstract handler which provides a collection of links.
@@ -13,6 +15,26 @@
  */
 abstract class Links extends FieldPluginBase {
 
+  /**
+   * Constructs a Links object.
+   *
+   * @param array $configuration
+   *   A configuration array containing information about the plugin instance.
+   * @param string $plugin_id
+   *   The plugin_id for the plugin instance.
+   * @param mixed $plugin_definition
+   *   The plugin implementation definition.
+   * @param \Drupal\Core\Routing\RedirectDestinationInterface|null $redirectDestination
+   *   The redirect destination service.
+   */
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, protected ?RedirectDestinationInterface $redirectDestination = NULL) {
+    parent::__construct($configuration, $plugin_id, $plugin_definition);
+    if ($redirectDestination === NULL) {
+      $this->redirectDestination = \Drupal::destination();
+      @trigger_error('Calling ' .  __METHOD__ . '() without the $redirectDestination argument is deprecated in drupal:10.1.0 and will be required in drupal:11.0.0. See https://www.drupal.org/node/CHANGE-RECORD-NODE-ID', E_USER_DEPRECATED);
+    }
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -20,6 +42,18 @@ public function usesGroupBy() {
     return FALSE;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
+    return new static(
+      $configuration,
+      $plugin_id,
+      $plugin_definition,
+      $container->get('redirect.destination')
+    );
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -84,7 +118,7 @@ protected function getLinks() {
         'title' => $title,
       ];
       if (!empty($this->options['destination'])) {
-        $links[$field]['query'] = \Drupal::destination()->getAsArray();
+        $links[$field]['query'] = $this->redirectDestination->getAsArray();
       }
     }

Note:

1. I might pick the wrong class, as the example. It's an abstract class, not a normal one. Not sure if abstract classes should be handled specially. Please just treat it as a normal class.

+      @trigger_error('Calling ' .  __METHOD__ . '() without the $redirectDestination argument is deprecated in drupal:10.1.0 and will be required in drupal:11.0.0. See https://www.drupal.org/node/CHANGE-RECORD-NODE-ID', E_USER_DEPRECATED);

2. A change record is required, and please update the placeholder CHANGE-RECORD-NODE-ID with its node ID.

3. IoC is class specific, do not touch .module files etc.

See parent issue for more details.

Continue with #34

 public function __construct(array $configuration, $plugin_id, $plugin_definition, protected ?RedirectDestinationInterface $redirectDestination = NULL) {

4. Constructor property promotion is used. see https://stitcher.io/blog/constructor-promotion-in-php-8

Switching to MR and re-wording the title without the term IoC which some contributors might be unfamiliar with. And updating the issue summary to add a fixed issue and its commit as an example.

Add Command to get classes to work with to IS

$ grep -nR '\\Drupal::destination()' . | grep -v Test | grep -v "\.module" | grep -v '\.inc' | grep -v 'Trait\.php' | grep -v 'api.php' | grep -v SystemCompactLink | grep -v RedirectDestinationInterface | grep -v UserLoginBlock

The command is used to find the target classes.

Note: Usages in SystemCompactLink and UserLoginBlock are inside static methods. And usage in RedirectDestinationInterface is in the comment.

CR updated

Thanks @jungle!

Committed 9ed7086 and pushed to 10.1.x. Thanks!

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

FWIW, the changes to the workspace forms from this issue were not correct, see #3376293-12: WorkspacePublishForm $redirectDestination parameter appears not to be used → .

Published the CR.