Remove all usages of drupal_set_message and drupal_get_messages in core lib

#2935254: Remove all usages of drupal_get_message and drupal_set_message in core includes commited into 8.6.x. Retest patch from #2

Re-roll.

Trying to fix the test failures.

Fix some of the test failures.

Fix one of the test failures.

Here's a possible fix to avoid the remaining deprecation notice, but it is a real hack.

Fix coding standards errors.

#2937945: Add messenger to ControllerBase and FormBase fixed

Patch in #16 no longer applies. Re-roll.

+1 for RTBC

grep -inr -e "drupal_set_message" -e "drupal_set_message" core/lib/
core/lib/Drupal/Core/Render/Renderer.php:652:    // change the way drupal_set_message() works in the Drupal 8 cycle. So we

There are some usages remaining.

grep -inr -e "drupal_set_message" -e "drupal_set_message" core/tests/
core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php:173:      'drupal_set_message() is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead. See https://www.drupal.org/node/2774931',
core/tests/Drupal/KernelTests/Core/Theme/MessageTest.php:27:    drupal_set_message('An error occurred', 'error');
core/tests/Drupal/KernelTests/Core/Theme/MessageTest.php:28:    drupal_set_message('But then something nice happened');
core/tests/Drupal/KernelTests/Core/Common/DrupalSetMessageTest.php:8: * @covers ::drupal_set_message
core/tests/Drupal/KernelTests/Core/Common/DrupalSetMessageTest.php:14:   * The basic functionality of drupal_set_message().
core/tests/Drupal/KernelTests/Core/Common/DrupalSetMessageTest.php:17:    drupal_set_message(t('A message: @foo', ['@foo' => 'bar']));

Sure.

Verified the patch +1 for RTBC, good to go.

+++ b/core/lib/Drupal/Core/Render/Element/StatusMessages.php
@@ -74,7 +74,13 @@ public static function generatePlaceholder(array $element) {
    */
   public static function renderMessages($type) {
     $render = [];
-    $messages = drupal_get_messages($type);
+    if (isset($type)) {
+      $messages = \Drupal::messenger()->deleteByType($type);
+    }
+    else {
+      $messages = \Drupal::messenger()->deleteAll();
+    }
+

Given $type isn't optional, when would it be null?

Yep we need to support NULL here for BC. We probably want to deprecate that at some point in the future but not in this issue.

Actually this needs a reroll and also there are still instances for drupal_set_message and drupal_get_message outside of the modules directories. I think this issue should clean up everything that the modules issue does not. We should leave \Drupal\KernelTests\Core\Common\DrupalSetMessageTest alone though and make this the legacy test once we've removed the entry from \Drupal\Tests\Listeners\DeprecationListenerTrait::getSkippedDeprecations().

The files which contain stuff to fix are:

• core/lib/Drupal/Core/Render/Element/StatusMessages.php
• core/lib/Drupal/Core/Render/Renderer.php
• core/core.api.php
• core/tests/Drupal/KernelTests/Core/Theme/MessageTest.php
• core/scripts/run-tests.sh

Did #32.

+++ b/core/modules/menu_ui/src/MenuForm.php
@@ -240,6 +240,10 @@ protected function buildOverviewForm(array &$form, FormStateInterface $form_stat
         ],
+        [
+          'data' => $this->t('Expanded'),
+          'class' => ['checkbox', 'priority-medium'],
+        ],
         $this->t('Weight'),

@@ -298,8 +302,19 @@ protected function buildOverviewForm(array &$form, FormStateInterface $form_stat
         ];
...
         $form['links'][$id]['enabled'] = $element['enabled'];
-        $form['links'][$id]['enabled']['#wrapper_attributes']['class'] = ['checkbox', 'menu-enabled'];
+        $form['links'][$id]['enabled']['#wrapper_attributes']['class'] = [
+          'checkbox',
+          'menu-enabled',
+        ];
+
+        $form['links'][$id]['expanded'] = $element['expanded'];
+        $form['links'][$id]['expanded']['#wrapper_attributes']['class'] = [
+          'checkbox',
+          'menu-expanded',
+          'priority-medium',
+        ];
 

@@ -361,6 +376,12 @@ protected function buildOverviewTreeForm($tree, $delta) {
         ];
+        $form[$id]['expanded'] = [
+          '#type' => 'checkbox',
+          '#title' => $this->t('Show @title as expanded', ['@title' => $link->getTitle()]),
+          '#title_display' => 'invisible',
+          '#default_value' => $link->isExpanded(),
+        ];
         $form[$id]['weight'] = [

@@ -455,7 +476,7 @@ protected function submitOverviewForm(array $complete_form, FormStateInterface $
     // Update our original form with the new order.
     $form = array_intersect_key(array_merge($order, $form), $form);
 
-    $fields = ['weight', 'parent', 'enabled'];
+    $fields = ['weight', 'parent', 'enabled', 'expanded'];
     $form_links = $form['links'];

Should it be in the current scope?

Limit the messages returned by type. Defaults to NULL, meaning all types.
* Passed on to drupal_get_messages(). These values are supported:

This says defaults to NULL, but there is no default. I think it'd be more self-explanatory to add a default arg, or to change the comment to not say 'defaults to NULL'.

Rerolled #33

Also addressed:
#34: Reverted changes that are out of scope.
#35: Added arg default value.

This issue should also be able to remove the messages from \Drupal\Tests\Listeners\DeprecationListenerTrait::getSkippedDeprecations() for drupal_set_message(). Any legacy tests of drupal_set_message() should be in the legacy test group and have an @expectedDeprecation annotation.

Should comments in core/modules/system/tests/src/Functional/Bootstrap/DrupalSetMessageTest.php also be updated?
Update:
I guess not. Because those changes are out of scope. Maybe should this be updated in the followup issue if changes are required?

Anyway, +1 for RTBC.

It think there is a overcomplication that should be stripped out

SiteConfigureForm extends ConfigFormBase extends FormBase

FormBase makes use of MessengerTrait.

So there is not need to inject the messenger service into SiteConfigureForm

Less talk more do.

PS BTW This was close to RTBC ... I think it is now.

+1 for RTBC

Can we get a follow up to fix the naming of:

• The system_test.drupal_set_message route
• The /system-test/drupal-set-message path
• The SystemTestController::drupalSetMessageTest() method
• The \Drupal\Tests\system\Functional\Bootstrap\DrupalSetMessageTest() test class

None of these things test drupal_set_message() anymore. Actually, lets use the parent issue to finally finish that one - #2924538: [META] Remove all usages of drupal_set_message and drupal_get_messages.

Crediting @catch and @jibran for reviews.

Committed 3c82d71 and pushed to 8.6.x. Thanks!

diff --git a/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestContextAwareBlock.php b/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestContextAwareBlock.php
index 9d05c27407..51f60b473a 100644
--- a/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestContextAwareBlock.php
+++ b/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestContextAwareBlock.php
@@ -3,7 +3,6 @@
 namespace Drupal\block_test\Plugin\Block;
 
 use Drupal\Core\Block\BlockBase;
-use Drupal\Core\Messenger\MessengerTrait;
 use Drupal\Core\Session\AccountInterface;
 use Drupal\user\UserInterface;
 
diff --git a/core/modules/views/tests/modules/views_test_data/src/Plugin/views/field/FieldFormButtonTest.php b/core/modules/views/tests/modules/views_test_data/src/Plugin/views/field/FieldFormButtonTest.php
index 82eaaeff5d..2367981ad5 100644
--- a/core/modules/views/tests/modules/views_test_data/src/Plugin/views/field/FieldFormButtonTest.php
+++ b/core/modules/views/tests/modules/views_test_data/src/Plugin/views/field/FieldFormButtonTest.php
@@ -3,7 +3,6 @@
 namespace Drupal\views_test_data\Plugin\views\field;
 
 use Drupal\Core\Form\FormStateInterface;
-use Drupal\Core\Messenger\MessengerTrait;
 use Drupal\views\Plugin\views\field\FieldPluginBase;
 use Drupal\views\Plugin\views\field\UncacheableFieldHandlerTrait;
 use Drupal\views\ResultRow;

fixed unused uses on commit.

Can we get a change notice please, to notify us that MessengerTrait was added to a number of base classes? (which doesn't seem to be mentioned in the issue summary or discussed here ...)

drupal_set_message() was removed in 8.5.0, so I for one already converted my code to use the messenger service, which includes using MessengerTrait in my own plugin bases and list builders.

But as part of this patch for 8.6.x MessengerTrait was added to the core PluginBase, EntityListBuilder, BlockBase, etc. So now I get all sorts of errors from previously-working code. Adding MessengerTrait is a change in the core API for these important base classes, and there should be a change notice for this.

This should not have been committed without a change record. See Change records now needed before commit

#2937945: Add messenger to ControllerBase and FormBase added this trait and there is a CR for it: https://www.drupal.org/node/2774931

I've added a note (and referenced this issue) mentioning that a few base classes now have this trait.