Improve error handling of \Drupal class

Someone should find the other issues which do the same.

I really like the patch.

7: 2160655_7.patch queued for re-testing.

Rerolled.

Fixed the test.

13: drupal-2160655-13.patch queued for re-testing.

13: drupal-2160655-13.patch queued for re-testing.

Fixed the tests.

rmdir(sites/simpletest/353397): Directory not empty: drupal_rmdir('sites/simpletest/353397') file_unmanaged_delete_recursive('sites/simpletest/353397', Array) Drupal\simpletest\TestBase->restoreEnvironment() Drupal\simpletest\TestBase->run() simpletest_script_run_one_test('269', 'Drupal\field\Tests\reEnableModuleFieldTest') now what :S

19: drupal-2160655-19.patch queued for re-testing.

That's certainly a much more useful error message. The pattern seems a bit weird, but only because php should provide a better way (it doesn't AFAIK).

+++ b/core/lib/Drupal/Core/Plugin/PluginBase.php
@@ -103,11 +103,13 @@ public function __sleep() {
-    $container = \Drupal::getContainer();
-    foreach ($this->_serviceIds as $key => $service_id) {
-      $this->$key = $container->get($service_id);
+    if (\Drupal::hasContainer()) {
+      $container = \Drupal::getContainer();
+      foreach ($this->_serviceIds as $key => $service_id) {
+        $this->$key = $container->get($service_id);
+      }
+      unset($this->_serviceIds);
     }
-    unset($this->_serviceIds);

I don't understand these changes. Surely we need to put the services into the object, and if we can't do that because we haven't got a container yet, then we need to find a way of having a container? Won't just ignoring it cause us problems later?

+++ b/core/lib/Drupal.php
@@ -115,9 +115,28 @@ public static function setContainer(ContainerInterface $container) {
+      $backtrace[1]['class'] . $backtrace[1]['type'] . $backtrace[1]['function']

These might not be set depending on where getContainer is called from. But we're already killing the page, so do we care about a couple of notices?

It's probably beyond the scope of this issue, but I don't think getContainer() should be deprecated. protected-unless-youre-a-test would be more accurate.

I have raised #2195725: Un-deprecate \Drupal()::getContainer() to look at whether getContainer() should be deprecated.

It's probably beyond the scope of this issue, but I don't think getContainer() should be deprecated. protected-unless-youre-a-test would be more accurate.

Afaik the problem is that we have no proper semantics to tell people that they really should not do it that way unless there is no alternative solution.

Possible duplicate of #2363341: Throw exception in Drupal::service() and friends, if container not initialized yet.