Issue #3085751 by alexpott, Deepak Goyal, rpayanm, longwave, volkerk, kristiaanvandeneynde, dww: Setter injection arguments are not checked for unmet dependencies

core/lib/Drupal/Core/Update/UpdateCompilerPass.php

@@ -27,16 +27,28 @@ class UpdateCompilerPass implements CompilerPassInterface {
     do {
       $has_changed = FALSE;
       foreach ($container->getDefinitions() as $key => $definition) {
+        // Ensure all the definition's arguments are valid.
         foreach ($definition->getArguments() as $argument) {
-          if ($argument instanceof Reference) {
-            $argument_id = (string) $argument;
-            if (!$container->has($argument_id) && $argument->getInvalidBehavior() === ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE) {
-              // If the container does not have the argument and would throw an
-              // exception, remove the service.
+          if ($this->isArgumentMissingService($argument, $container)) {
+            $container->removeDefinition($key);
+            $container->log($this, sprintf('Removed service "%s"; reason: depends on non-existent service "%s".', $key, (string) $argument));
+            $has_changed = TRUE;
+            $process_aliases = TRUE;
+            // Process the next definition.
+            continue 2;
+          }
+        }
+
+        // Ensure all the method call arguments are valid.
+        foreach ($definition->getMethodCalls() as $call) {
+          foreach ($call[1] as $argument) {
+            if ($this->isArgumentMissingService($argument, $container)) {
               $container->removeDefinition($key);
-              $container->log($this, sprintf('Removed service "%s"; reason: depends on non-existent service "%s".', $key, $argument_id));
+              $container->log($this, sprintf('Removed service "%s"; reason: method call "%s" depends on non-existent service "%s".', $key, $call[0], (string) $argument));
               $has_changed = TRUE;
               $process_aliases = TRUE;
+              // Process the next definition.
+              continue 3;
             }
           }
         }
@@ -58,4 +70,26 @@ class UpdateCompilerPass implements CompilerPassInterface {
     }
   }
 
+  /**
+   * Checks if a reference argument is to a missing service.
+   *
+   * @param mixed $argument
+   *   The argument to check.
+   * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container
+   *   The container.
+   *
+   * @return bool
+   *   TRUE if the argument is a reference to a service that is missing from the
+   *   container and the reference is required, FALSE if not.
+   */
+  protected function isArgumentMissingService($argument, ContainerBuilder $container) {
+    if ($argument instanceof Reference) {
+      $argument_id = (string) $argument;
+      if (!$container->has($argument_id) && $argument->getInvalidBehavior() === ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE) {
+        return TRUE;
+      }
+    }
+    return FALSE;
+  }
+
 }

core/modules/system/tests/modules/new_dependency_test/new_dependency_test.install

@@ -17,6 +17,7 @@ function new_dependency_test_update_8001() {
     'new_dependency_test.alias_dependency',
     'new_dependency_test.alias2',
     'new_dependency_test.alias_dependency2',
+    'new_dependency_test.setter_injection',
   ];
 
   // Gather the state of the services prior to installing the

core/modules/system/tests/modules/new_dependency_test/new_dependency_test.services.yml

@@ -34,3 +34,7 @@ services:
     decorates: new_dependency_test.another_service_two
     decoration_inner_name: new_dependency_test.foo
     arguments: ['@new_dependency_test.foo']
+  new_dependency_test.setter_injection:
+    class: Drupal\new_dependency_test\SetterInjection
+    calls:
+      - [setter, ['@new_dependency_test_with_service.service']]

core/modules/system/tests/modules/new_dependency_test/src/SetterInjection.php

@@ -0,0 +1,39 @@
+<?php
+
+namespace Drupal\new_dependency_test;
+
+use Drupal\new_dependency_test_with_service\NewService;
+
+/**
+ * Generic service which uses setter injection.
+ */
+class SetterInjection {
+
+  /**
+   * The injected service.
+   *
+   * @var \Drupal\new_dependency_test_with_service\NewService
+   */
+  protected $service;
+
+  /**
+   * SetterInjection constructor.
+   *
+   * @param \Drupal\new_dependency_test_with_service\NewService $service
+   *   The service of the new module.
+   */
+  public function setter(NewService $service) {
+    $this->service = $service;
+  }
+
+  /**
+   * Get the simple greeting from the service.
+   *
+   * @return string
+   *   The greeting.
+   */
+  public function greet() {
+    return $this->service->greet();
+  }
+
+}

core/modules/system/tests/src/Functional/UpdateSystem/UpdatePathNewDependencyTest.php

@@ -55,6 +55,7 @@ class UpdatePathNewDependencyTest extends BrowserTestBase {
     $this->assertEquals('Hello', $this->container->get('new_dependency_test.alias')->greet());
     $this->assertEquals('Hello World', $this->container->get('new_dependency_test.hard_dependency')->greet());
     $this->assertEquals('Hello World', $this->container->get('new_dependency_test.optional_dependency')->greet());
+    $this->assertEquals('Hello', $this->container->get('new_dependency_test.setter_injection')->greet());
 
     // Tests that existing decorated services work as expected during update.
     $this->assertTrue(\Drupal::state()->get('new_dependency_test_update_8001.decorated_service'), 'The new_dependency_test.another_service service is decorated');
@@ -70,6 +71,7 @@ class UpdatePathNewDependencyTest extends BrowserTestBase {
       'new_dependency_test.alias_dependency' => FALSE,
       'new_dependency_test.alias2' => FALSE,
       'new_dependency_test.alias_dependency2' => FALSE,
+      'new_dependency_test.setter_injection' => FALSE,
     ], $before_install);
 
     $after_install = \Drupal::state()->get('new_dependency_test_update_8001.has_after_install', []);
@@ -81,6 +83,7 @@ class UpdatePathNewDependencyTest extends BrowserTestBase {
       'new_dependency_test.alias_dependency' => TRUE,
       'new_dependency_test.alias2' => TRUE,
       'new_dependency_test.alias_dependency2' => TRUE,
+      'new_dependency_test.setter_injection' => TRUE,
     ], $after_install);
   }