Replace calls to format_string() with Drupal\Component\Render\FormattableMarkup

I've changed this to just format_string(), since the others have either been done already or are trivial so can be done in the deprecation patch.

I have replace format_string with String:format in core

Upss i think i have replaced th delegated funcion format_string to. I will fix it

I have added the "use" line to all files that need it.

Given the number of things that need changing here, I strongly suggest using #2208607: Write script to automate replacement of deprecated functions where possible

Maybe it is the correct way

The fails seems easy to solve. I´ll try.

Fatal error: Cannot use Drupal\Component\Utility\String as String because the name is already in use in ./core/modules/simpletest/src/KernelTestBase.php on line 22
Errors parsing ./core/modules/simpletest/src/KernelTestBase.php].
[07:21:35] Encountered error on [syntax], details:

Error fixed

@malotor you must change the status to "Needs review" for test the patch.

ok rpayanm , thanks.

Tonight, I will try to install in order to find this install failure

Maybe splitting the issue for each core module makes easy to create the a working patch-

The patch should be easy to make if you use the script from #2208607: Write script to automate replacement of deprecated functions where possible

When a patch fails testing the testbot will set the issue to needs work - there's no point setting it back to needs review at that point because the testbot has already found a problem with the issue. There are links below the dropdowns if you don't understand what they mean.

Let me try :)

ups... hidden the files again...

Again...

Patch in 31 didn't apply, so a reroll. It was a bit tedious (maybe working too late).

The conflicts were in these files:
core/modules/text/src/Tests/TextFieldTest.php
core/modules/system/src/Tests/Common/SystemListingTest.php
core/modules/field_ui/src/Tests/ManageFieldsTest.php
core/modules/entity_reference/src/Tests/EntityReferenceFormatterTest.php

Not bad, only one small error.

A patch of this size would presumably be considered disruptive according to https://www.drupal.org/core/beta-changes, and therefore not committed at this time. Our time would be better used on the other issues on the meta (particularly those needing review), then we can come back to this when there are fewer critical patches to disrupt.

(see also #2350615-49: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase? to comment #52)

Also postponed on #2454447: Split Utility\String class to support PHP 7 (String is a reserved word)

And the call String::format would by converted to SafeMarkup::format() ?

@rpayanm yes, that need discussion how to split the units to cover all #2454439-14: [META] Support PHP 7

Round 1.

Of course.
Round 2.

Nice.
Round 3.

Yes, yes, yes :)
Round 4.

Let me see.
Round 5.

And again.
Round 6.

@rpayanm
Have a look at my previous patch: https://www.drupal.org/files/issues/2454447-23.patch and search for '\Drupal\Compo
This will let you find a couple of more instances.

this should be green.

@dawehner this issue id for format_string, if I search for \Drupal\Compo I got, for example:

-    // \Drupal\Component\Utility\String::checkPlain() by
+    // \Drupal\Component\Utility\StringHelper::checkPlain() by

And not objective of this issue.

I search on PHPStorm for format_string and String::format and not found any occurrence, only the function.

Patch rerolled.

Sorry... :-)

$ git apply replace_calls_to-2228741-59_0.patch 
error: patch failed: core/modules/tracker/src/Tests/TrackerTest.php:9
error: core/modules/tracker/src/Tests/TrackerTest.php: patch does not apply

You can probably blame #2401191: Activity Tracker shows 'Last updated' status as '45 years 1 week ago'

No need to blame :-) I am happy to contribute. Patch rerolled.

Patch size reduced. Thanks to #2457887: Use Utility\SafeMarkup class instead of Utility\String for placeholder(), checkPlain(),format() functions

Uploaded the patch in haste. Fixed all unmerged issues & duplicate errors.

Removed more duplicates of "use Drupal\Component\Utility\SafeMarkup;" which crept in during reroll.

Looks good, thanks for sticking with it.

After applying the patch, NetBeans can't find any occurrence of format_string(), other than the function definition which still remains. Probably need a follow-up issue to remove it.

Thanks everyone for your work on this! Unfortunately, this patch can no longer go in during the beta phase of the Drupal 8 release cycle:

• Per #2221771-22: Mark all simple wrappers in bootstrap.inc and common.inc as deprecated, it is too late to remove this function from 8.0.x.
• The function is also not yet even deprecated.
• This patch is somewhat disruptive by its sheer size alone (many changes across many files).

Following the flowchart in the Allowed beta changes:

1. The issue is not a feature request.
2. The issue is not unfrozen (it changes more than just CSS, strings, markup, or automated tests).
3. The issue is not critical.
4. The issue is not a prioritized change (it does not remove previously deprecated code, and it does not improve usability, accessibility, performace, documentation, etc., nor is it a bugfix).
5. The issue is not major.
6. The issue does not reduce fragility (format_string() is just a wrapper anyway).

Whenever an issue is a normal task, be sure to evaluate right away if it can be accepted during the beta. We can clean up these uses of deprecated code again before 8.1.x.

xjm, thanks for the evaluation. I agree with it!

@xjm: Would you accept a patch for 8.0.x that just affected the tests and didn't touch the rest of core?

We have 5 of them left, IMHO this should not be 8.1 material

Now that #2221771: Mark all simple wrappers in bootstrap.inc and common.inc as deprecated has gone in and format_string() is deprecated, I think it's important to re-open this issue and get this finished.

I think any disruption to current core issues would be a good thing - we don't want to be committing *more* code with the deprecated format_string() function. format_string() was obsoleted more than a year ago - we really *should* be breaking any pending patches that still use format_string().

Agreed this is not a priority, but as core serves as an example (and sometimes the only available documentation) for what can be done in contrib, it makes sense to get core usage of format_string() removed BEFORE 8.0 is released, otherwise all of contrib is going to start copying core and it will become far *more* disruptive to try to change this. At the very least, removing format_string() from tests is within the allowed beta changes, and this accounts for most of the core usage.

@dawehner: What do you mean by 5? I count ~750 instances of format_string() in core ...

Again, it's deprecated for 9.0.0 at present, which means we are committed to keeping it in core for the full 8.x.x cycle. Unless we also end up removing SafeMarkup::format() as part of #2549943: [plan] Remove as much of the SafeMarkup class's methods as possible (which I think would be too disruptive and @alexpott said probably was not on the table as of at least last week), we don't gain much by converting core usages.

Not re-postponing yet though; I'll ping alexpott asking for a second opinion. But by itself; this is not eligible as a change during the beta (see #67); the only reason to consider it would be as part of #2549943: [plan] Remove as much of the SafeMarkup class's methods as possible.

Such a huge patch and so much effort, and now SafeMarkup is deprecated in favor of FormattableMarkup. :-)

Some of the changes are more narrowly addressed in #2549805: Remove all usage of FormattableMarkup in tests apart from explicit tests of that API

Woops. Meant to add that as related.

grep returns 724 places which is too much for single patch to review properly

Better to create a meta(plan) issue to split this conversion into subtasks like #2549805: Remove all usage of FormattableMarkup in tests apart from explicit tests of that API

@andypost I disagree. This can and should be scripted.

@alexpott scripting will require to be smart to modify use (kinda usecase for pharborist or ast) but idea is interesting
Also it should exclude tests that in progress of conversion in #2549805: Remove all usage of FormattableMarkup in tests apart from explicit tests of that API

@andypost I'd go the other way around here. And fix this before #2549805: Remove all usage of FormattableMarkup in tests apart from explicit tests of that API - I'd also fix #2958429: Properly deprecate SafeMarkup::format() first and then the test issue only has to deal with FormattableMarkup.

Here's a fresh patch against 8.8. General regex and replace, plus a script to add in the use statements. Added a test.

I used the below to fix the use statements after the global S&R. It took care of all but about a dozen files which I updated manually.

#!/usr/bin/env bash

files=`git status | grep modified | awk '{print $2}'`
for i in $files
do
    if [ -z "$(cat $i | grep -n "use Drupal\\\\Component\\\\Render\\\\FormattableMarkup;")" ]
       then
       line=$(expr "$(cat $i | egrep -n "^use")" : '\([0-9]*\)')
       if [ -z "$line" ]
       then
         echo "no use block in $i add manually"
       else
       echo "$line $i"
       sed -i '' "${line}i\\
       use Drupal\\\Component\\\Render\\\FormattableMarkup;
       " $i
       fi
    fi
done

Reroll

My only other question for this is do we want to remove some of the redundant usages of markup in test messages as part of this patch, or is that relegated to one of the followups, which will remove new FormattableMarkup now instead of format_string?

Reroll

Curious what a bulk S&R removing Formattable markup messages from any ->assertEqual/assertEquals calls looks like

Fix tests, remove some extra hunks.

Also, you know, the patch file, just for fun...

Rerolling

+++ b/core/modules/aggregator/tests/src/Functional/UpdateFeedItemTest.php
@@ -2,6 +2,7 @@
+use Drupal\Component\Render\FormattableMarkup;
 use Drupal\aggregator\Entity\Feed;
 use Drupal\aggregator\Entity\Item;

+++ b/core/modules/block_content/tests/src/Functional/BlockContentRevisionsTest.php
@@ -2,6 +2,7 @@

As discussed on slack, this is not in alphabetical order, but that is not a hard standard, so this will do, since a fair number are not in alphabetical order to begin with (possibly from other scripts). So no action needed here.

+++ b/core/modules/comment/tests/src/Functional/CommentLanguageTest.php
@@ -119,7 +119,7 @@ public function testCommentLanguage() {
-        $this->assertEqual($comment->langcode->value, $langcode, format_string('The comment posted with content language %langcode and belonging to the node with language %node_language has language %comment_language', $args));
+        $this->assertEquals($comment->langcode->value, $langcode);

as discussed on slack, lets stick with assertEqual so we don't have to flip arguments.

Went through most of the patch (10% in detail, lots in scan mode), and these were the only issues that I saw. The added test coverage looks good.

scripted restoration of the assertEqual

Fixed remaining test (just missing use statements) . Then manually wen through each test file, and cleared out additional unneeded usages in test messages.

Fixing a couple typos

As mentioned on slack, sorry for this being so huge, but I'm prepared to RTBC after this is addressed.

1. +++ b/core/modules/config_translation/tests/src/Functional/ConfigTranslationUiTest.php
   @@ -1155,9 +1155,7 @@ protected function assertDisabledTextarea($id) {
        $expected = 'This field has been disabled because you do not have sufficient permissions to edit it.';
   -    $this->assertEqual($textarea->getText(), $expected, new FormattableMarkup('Disabled textarea @id hides text in an inaccessible text format.', [
   -      '@id' => $id,
   -    ]));
   +    $this->assertEqual($textarea->getText(), $expected);
   

   This is not a format_string(), so not really in context, maybe was a merge conflict or so. Also IMHO useful information.

2. +++ b/core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php
   @@ -103,8 +103,7 @@ protected function doTestBasicTranslation() {
          $value = is_array($value) ? $value[0]['value'] : $value;
   -      $message = format_string('@property correctly stored in the default language.', ['@property' => $property]);
   -      $this->assertEqual($stored_value, $value, $message);
   +      $this->assertEqual($stored_value, $value);
   

   This seems useful too, as it is in a loop and you otherwise don't know which property that didn't match.

   I also suggested to exclude all tests in src/Tests because those are legacy simpletest tests that we'll remove anyway and it helps to make the patch a bit smaller.

3. +++ b/core/modules/content_translation/tests/src/Functional/ContentTranslationUITestBase.php
   @@ -96,8 +96,7 @@ protected function doTestBasicTranslation() {
          $value = is_array($value) ? $value[0]['value'] : $value;
   -      $message = format_string('@property correctly stored in the default language.', ['@property' => $property]);
   -      $this->assertEqual($stored_value, $value, $message);
   +      $this->assertEqual($stored_value, $value);
        }
   
   @@ -201,8 +200,7 @@ protected function doTestBasicTranslation() {
            $value = is_array($value) ? $value[0]['value'] : $value;
   -        $message = format_string('%property correctly stored with language %language.', ['%property' => $property, '%language' => $langcode]);
   -        $this->assertEqual($stored_value, $value, $message);
   +        $this->assertEqual($stored_value, $value);
          }
   

   these are the non-deprecated versions of the above.

4. +++ b/core/modules/field/tests/src/Kernel/EntityReference/EntityReferenceFormatterTest.php
   @@ -163,7 +163,7 @@ public function testAccess() {
    
          // Verify the un-accessible item still exists.
   -      $this->assertEqual($referencing_entity->{$field_name}->target_id, $this->referencedEntity->id(), format_string('The un-accessible item still exists after @name formatter was executed.', ['@name' => $name]));
   +      $this->assertEqual($referencing_entity->{$field_name}->target_id, $this->referencedEntity->id());
        }
      }
    
   @@ -215,7 +215,7 @@ public function testEntityFormatter() {
   
   @@ -215,7 +215,7 @@ public function testEntityFormatter() {
        $this->assertEqual($build[0]['#markup'], 'default | ' . $this->referencedEntity->label() . $expected_rendered_name_field_1 . $expected_rendered_body_field_1, sprintf('The markup returned by the %s formatter is correct for an item with a saved entity.', $formatter));
        $expected_cache_tags = Cache::mergeTags(\Drupal::entityTypeManager()->getViewBuilder($this->entityType)->getCacheTags(), $this->referencedEntity->getCacheTags());
        $expected_cache_tags = Cache::mergeTags($expected_cache_tags, FilterFormat::load('full_html')->getCacheTags());
   -    $this->assertEqual($build[0]['#cache']['tags'], $expected_cache_tags, format_string('The @formatter formatter has the expected cache tags.', ['@formatter' => $formatter]));
   +    $this->assertEqual($build[0]['#cache']['tags'], $expected_cache_tags);
   

   the first seems useful too as this is inside a loop that is testing multiple ones, so not obvious to see which one failed. The second one has an argument too but that's actually hardcoded above and only one is tested in that method, so seems fine to remove.

5. +++ b/core/modules/field/tests/src/Kernel/FieldAttachStorageTest.php
   @@ -54,17 +54,17 @@ public function testFieldAttachSaveLoad() {
          // The field value loaded matches the one inserted or updated.
   -      $this->assertEqual($entity->{$this->fieldTestData->field_name}[$delta]->value, $values[$current_revision][$delta]['value'], format_string('Current revision: expected value %delta was found.', ['%delta' => $delta]));
   +      $this->assertEqual($entity->{$this->fieldTestData->field_name}[$delta]->value, $values[$current_revision][$delta]['value']);
        }
    
        // Confirm each revision loads the correct data.
        foreach (array_keys($values) as $revision_id) {
          $entity = $storage->loadRevision($revision_id);
          // Number of values per field loaded equals the field cardinality.
   -      $this->assertEqual(count($entity->{$this->fieldTestData->field_name}), $cardinality, format_string('Revision %revision_id: expected number of values.', ['%revision_id' => $revision_id]));
   +      $this->assertEqual(count($entity->{$this->fieldTestData->field_name}), $cardinality);
          for ($delta = 0; $delta < $cardinality; $delta++) {
            // The field value loaded matches the one inserted or updated.
   -        $this->assertEqual($entity->{$this->fieldTestData->field_name}[$delta]->value, $values[$revision_id][$delta]['value'], format_string('Revision %revision_id: expected value %delta was found.', ['%revision_id' => $revision_id, '%delta' => $delta]));
   +        $this->assertEqual($entity->{$this->fieldTestData->field_name}[$delta]->value, $values[$revision_id][$delta]['value']);
          }
   

   same here, on the first it's the delta, those below have a loop over revisions and deltas.

6. +++ b/core/modules/field/tests/src/Kernel/TranslationTest.php
   @@ -189,7 +190,7 @@ public function testTranslatableFieldSaveLoad() {
    
          foreach ($entity->getTranslationLanguages() as $langcode => $language) {
   -        $this->assertEquals([], $entity->getTranslation($langcode)->{$field_name_default}->getValue(), format_string('Empty value correctly populated for language %language.', ['%language' => $langcode]));
   +        $this->assertEquals([], $entity->getTranslation($langcode)->{$field_name_default}->getValue());
          }
   

   another one of these.

7. +++ b/core/modules/file/tests/src/Functional/FileTokenReplaceTest.php
   @@ -81,7 +81,7 @@ public function testFileTokenReplacement() {
          $output = $token_service->replace($input, ['file' => $file], ['langcode' => $language_interface->getId()], $bubbleable_metadata);
   -      $this->assertEqual($output, $expected, format_string('Sanitized file token %token replaced.', ['%token' => $input]));
   +      $this->assertEqual($output, $expected);
          $this->assertEqual($bubbleable_metadata, $metadata_tests[$input]);
        }
    
   @@ -93,7 +93,7 @@ public function testFileTokenReplacement() {
   
   @@ -93,7 +93,7 @@ public function testFileTokenReplacement() {
    
        foreach ($tests as $input => $expected) {
          $output = $token_service->replace($input, ['file' => $file], ['langcode' => $language_interface->getId(), 'sanitize' => FALSE]);
   -      $this->assertEqual($output, $expected, format_string('Unsanitized file token %token replaced.', ['%token' => $input]));
   +      $this->assertEqual($output, $expected);
        }
      }
   

   those two and other similar token tests loop over pretty long lists of tokens, so knowing which token actually failed is important.

8. +++ b/core/modules/filter/tests/src/Kernel/FilterCrudTest.php
   @@ -94,11 +94,11 @@ public function verifyTextFormat($format) {
        // Verify the loaded filter has all properties.
        $filter_format = FilterFormat::load($format->id());
   -    $this->assertEqual($filter_format->id(), $format->id(), format_string('filter_format_load: Proper format id for text format %format.', $t_args));
   -    $this->assertEqual($filter_format->label(), $format->label(), format_string('filter_format_load: Proper title for text format %format.', $t_args));
   -    $this->assertEqual($filter_format->get('weight'), $format->get('weight'), format_string('filter_format_load: Proper weight for text format %format.', $t_args));
   +    $this->assertEqual($filter_format->id(), $format->id());
   +    $this->assertEqual($filter_format->label(), $format->label());
   +    $this->assertEqual($filter_format->get('weight'), $format->get('weight'));
        // Check that the filter was created in site default language.
   -    $this->assertEqual($format->language()->getId(), $default_langcode, format_string('filter_format_load: Proper language code for text format %format.', $t_args));
   +    $this->assertEqual($format->language()->getId(), $default_langcode);
   

   not sure about this. it's a helper function, but it's relatively easy to see from backtrace which call that failed. If we do remove it then you can also remove $t_args which is now unused.

9. +++ b/core/modules/filter/tests/src/Kernel/FilterSettingsTest.php
   @@ -36,11 +36,7 @@ public function testFilterDefaults() {
          $expected_weight = $filter_info[$name]['weight'];
   -      $this->assertEqual($filter->weight, $expected_weight, format_string('@name filter weight %saved equals %default', [
   -        '@name' => $name,
   -        '%saved' => $filter->weight,
   -        '%default' => $expected_weight,
   -      ]));
   +      $this->assertEqual($filter->weight, $expected_weight);
          $saved_settings[$name]['weight'] = $expected_weight;
        }
    
   @@ -51,11 +47,7 @@ public function testFilterDefaults() {
   
   @@ -51,11 +47,7 @@ public function testFilterDefaults() {
    
        // Verify that saved filter settings have not been changed.
        foreach ($filter_defaults_format->filters() as $name => $filter) {
   -      $this->assertEqual($filter->weight, $saved_settings[$name]['weight'], format_string('@name filter weight %saved equals %previous', [
   -        '@name' => $name,
   -        '%saved' => $filter->weight,
   -        '%previous' => $saved_settings[$name]['weight'],
   -      ]));
   +      $this->assertEqual($filter->weight, $saved_settings[$name]['weight']);
   

   these two have the filter name in a loop, might be useful?

10. +++ b/core/modules/image/tests/src/Functional/ImageAdminStylesTest.php
    @@ -198,7 +198,7 @@ public function testStyle() {
         $image_path = $this->createSampleImage($style);
    -    $this->assertEqual($this->getImageCount($style), 1, format_string('Image style %style image %file successfully generated.', ['%style' => $style->label(), '%file' => $image_path]));
    +    $this->assertEqual($this->getImageCount($style), 1);
     
    

    also tricky, this seems like useful information at first, but the image style is a random string and there's only one it seems. But $image_path seems unused now, so we could remove that (a few times)

11. +++ b/core/modules/image/tests/src/Functional/ImageFieldDefaultImagesTest.php
    @@ -166,14 +167,7 @@ public function testDefaultImages() {
         $article = $this->drupalCreateNode(['type' => 'article']);
         $article_built = $this->drupalBuildEntityView($article);
    -    $this->assertEqual(
    -      $article_built[$field_name][0]['#item']->target_id,
    -      $default_images['field']->id(),
    -      format_string(
    -        'A new article node without an image has the expected default image file ID of @fid.',
    -        ['@fid' => $default_images['field']->id()]
    -      )
    -    );
    +    $this->assertEqual($article_built[$field_name][0]['#item']->target_id, $default_images['field']->id());
     
    

    hm, in this file you keep it on assertFieldByXpath() but remove it from assertEqual() but those are basically doing kind of the same, so we should either remove both or keep both (keeping seems easier to review, involves less thinking :))

12. +++ b/core/modules/language/tests/src/Functional/LanguageNegotiationInfoTest.php
    @@ -112,10 +113,10 @@ public function testInfoAlterations() {
    -        $this->assertFieldByName($form_field, NULL, format_string('Type-specific test language negotiation method available for %type.', ['%type' => $type]));
    +        $this->assertFieldByName($form_field, NULL);
           }
           else {
    -        $this->assertNoFieldByName($form_field, NULL, format_string('Type-specific test language negotiation method unavailable for %type.', ['%type' => $type]));
    +        $this->assertNoFieldByName($form_field, NULL);
    

    if we remove it then we can remove NULL to, that was just here so we could have a message. $type is part of the field name, so safe to remove I'd say.

13. +++ b/core/modules/language/tests/src/Functional/LanguageNegotiationInfoTest.php
    @@ -125,7 +126,7 @@ public function testInfoAlterations() {
         foreach ($this->languageManager()->getDefinedLanguageTypes() as $type) {
           $langcode = $last[$type];
           $value = $type == LanguageInterface::TYPE_CONTENT || strpos($type, 'test') !== FALSE ? 'it' : 'en';
    -      $this->assertEqual($langcode, $value, format_string('The negotiated language for %type is %language', ['%type' => $type, '%language' => $value]));
    +      $this->assertEqual($langcode, $value);
    

    useful?

14. +++ b/core/modules/language/tests/src/Functional/LanguageUILanguageNegotiationTest.php
    @@ -513,19 +513,19 @@ public function testLanguageDomain() {
         $italian_url = Url::fromRoute('system.admin', [], ['https' => TRUE, 'language' => $languages['it']])->toString();
         $correct_link = 'https://' . $link;
    -    $this->assertTrue($italian_url == $correct_link, format_string('The right HTTPS URL (via options) (@url) in accordance with the chosen language', ['@url' => $italian_url]));
    +    $this->assertTrue($italian_url == $correct_link);
     
    ...
         $correct_link = 'https://' . $link;
    -    $this->assertTrue($italian_url == $correct_link, format_string('The right URL (via current URL scheme) (@url) in accordance with the chosen language', ['@url' => $italian_url]));
    +    $this->assertTrue($italian_url == $correct_link);
    

    hm, we could make these two assertEquals(), then we get useful default output.

15. +++ b/core/modules/node/tests/src/Functional/NodeLoadMultipleTest.php
    @@ -45,12 +46,12 @@ public function testNodeMultipleLoad() {
    -    $this->assertTrue($count == 2, format_string('@count nodes loaded.', ['@count' => $count]));
    +    $this->assertTrue($count == 2, new FormattableMarkup('@count nodes loaded.', ['@count' => $count]));
     
         // Load nodes by nid. Nodes 1, 2 and 4 will be loaded.
         $nodes = Node::loadMultiple([1, 2, 4]);
         $count = count($nodes);
    -    $this->assertTrue(count($nodes) == 3, format_string('@count nodes loaded', ['@count' => $count]));
    +    $this->assertTrue(count($nodes) == 3, new FormattableMarkup('@count nodes loaded', ['@count' => $count]));
         $this->assertTrue(isset($nodes[$node1->id()]), 'Node is correctly keyed in the array');
         $this->assertTrue(isset($nodes[$node2->id()]), 'Node is correctly keyed in the array');
    

    phpunit has assertCount, we could use that and drop the messages?

16. +++ b/core/modules/node/tests/src/Kernel/NodeAccessRecordsTest.php
    @@ -73,7 +73,7 @@ public function testNodeAccessRecords() {
           $altered_grants = $grants;
           \Drupal::moduleHandler()->alter('node_grants', $altered_grants, $web_user, $op);
    -      $this->assertNotEqual($grants, $altered_grants, format_string('Altered the %op grant for a user.', ['%op' => $op]));
    +      $this->assertNotEqual($grants, $altered_grants);
    

    useful?

17. +++ b/core/modules/node/tests/src/Kernel/NodeTokenReplaceTest.php
    @@ -104,7 +104,7 @@ public function testNodeTokenReplacement() {
           $bubbleable_metadata = new BubbleableMetadata();
           $output = $this->tokenService->replace($input, ['node' => $node], ['langcode' => $this->interfaceLanguage->getId()], $bubbleable_metadata);
    -      $this->assertEqual($output, $expected, format_string('Node token %token replaced.', ['%token' => $input]));
    +      $this->assertEqual($output, $expected);
           $this->assertEqual($bubbleable_metadata, $metadata_tests[$input]);
         }
    

    useful

18. +++ b/core/modules/search/tests/src/Functional/SearchNumberMatchingTest.php
    @@ -89,7 +89,7 @@ public function testNumberSearching() {
             t('Search'));
    -      $this->assertNoText($node->label(), format_string('%number: node title not shown in dummy search', ['%number' => $i]));
    +      $this->assertNoText($node->label());
     
           // Now verify that we can find node i by searching for any of the
           // numbers.
    @@ -102,7 +102,7 @@ public function testNumberSearching() {
    
    @@ -102,7 +102,7 @@ public function testNumberSearching() {
             $this->drupalPostForm('search/node',
               ['keys' => $number],
               t('Search'));
    -        $this->assertText($node->label(), format_string('%i: node title shown (search found the node) in search for number %number', ['%i' => $i, '%number' => $number]));
    +        $this->assertText($node->label());
           }
         }
     
    diff --git a/core/modules/search/tests/src/Functional/SearchNumbersTest.php b/core/modules/search/tests/src/Functional/SearchNumbersTest.php
    
    diff --git a/core/modules/search/tests/src/Functional/SearchNumbersTest.php b/core/modules/search/tests/src/Functional/SearchNumbersTest.php
    index 0901a270e5..82cd66dca4 100644
    
    index 0901a270e5..82cd66dca4 100644
    --- a/core/modules/search/tests/src/Functional/SearchNumbersTest.php
    
    --- a/core/modules/search/tests/src/Functional/SearchNumbersTest.php
    +++ b/core/modules/search/tests/src/Functional/SearchNumbersTest.php
    
    +++ b/core/modules/search/tests/src/Functional/SearchNumbersTest.php
    +++ b/core/modules/search/tests/src/Functional/SearchNumbersTest.php
    @@ -109,7 +109,7 @@ public function testNumberSearching() {
    
    @@ -109,7 +109,7 @@ public function testNumberSearching() {
           $this->drupalPostForm('search/node',
             ['keys' => $number],
             t('Search'));
    -      $this->assertText($node->label(), format_string('%type: node title shown (search found the node) in search for number %number.', ['%type' => $type, '%number' => $number]));
    +      $this->assertText($node->label());
         }
    

    that %numer/i/type thing might be useful?

19. +++ b/core/modules/shortcut/tests/src/Functional/ShortcutTranslationUITest.php
    @@ -120,7 +121,7 @@ protected function doTestTranslationChanged() {
         $this->assertFalse(
           $entity instanceof EntityChangedInterface,
    -      format_string('%entity is not implementing EntityChangedInterface.', ['%entity' => $this->entityTypeId])
    +      new FormattableMarkup('%entity is not implementing EntityChangedInterface.', ['%entity' => $this->entityTypeId])
         );
    

    could use assertNotInstanceOf() and then drop the message?

20. +++ b/core/modules/statistics/tests/src/Functional/StatisticsTokenReplaceTest.php
    @@ -47,7 +47,7 @@ public function testStatisticsTokenReplacement() {
           $output = \Drupal::token()->replace($input, ['node' => $node], ['langcode' => $language_interface->getId()]);
    -      $this->assertEqual($output, $expected, format_string('Statistics token %token replaced.', ['%token' => $input]));
    +      $this->assertEqual($output, $expected);
    

    another one of those token tests, useful.

21. +++ b/core/modules/system/src/Tests/Module/ModuleTestBase.php
    @@ -4,6 +4,7 @@
     @trigger_error(__NAMESPACE__ . '\ModuleTestBase is deprecated for removal before Drupal 9.0.0. Use \Drupal\Tests\system\Functional\Module\ModuleTestBase instead. See https://www.drupal.org/node/2999939', E_USER_DEPRECATED);
     
    +use Drupal\Component\Render\FormattableMarkup;
     use Drupal\Core\Config\InstallStorage;
     use Drupal\Core\Database\Database;
     use Drupal\Core\Config\FileStorage;
    

    deprecated legacy base test class, skip :)

22. +++ b/core/modules/system/src/Tests/System/SystemConfigFormTestBase.php
    @@ -64,7 +65,7 @@ public function testConfigForm() {
           '%errors' => $valid_form ? t('None') : implode(' ', $errors),
         ];
    -    $this->assertTrue($valid_form, format_string('Input values: %values<br/>Validation handler errors: %errors', $args));
    +    $this->assertTrue($valid_form, new FormattableMarkup('Input values: %values<br/>Validation handler errors: %errors', $args));
     
    

    same.

23. +++ b/core/modules/system/tests/src/Functional/Database/SelectPagerDefaultTest.php
    @@ -42,7 +42,7 @@ public function testEvenPagerQuery() {
     
    -      $this->assertCount($correct_number, $data->names, format_string('Correct number of records returned by pager: @number', ['@number' => $correct_number]));
    +      $this->assertCount($correct_number, $data->names);
         }
    
    @@ -76,7 +76,7 @@ public function testOddPagerQuery() {
           }
     
    -      $this->assertCount($correct_number, $data->names, format_string('Correct number of records returned by pager: @number', ['@number' => $correct_number]));
    +      $this->assertCount($correct_number, $data->names);
    

    pager number useful?

24. +++ b/core/modules/system/tests/src/Functional/Database/SelectTableSortDefaultTest.php
    @@ -60,8 +60,8 @@ public function testTableSortQueryFirst() {
     
    -      $this->assertEqual($first->task, $sort['first'], format_string('Items appear in the correct order sorting by @field @sort.', ['@field' => $sort['field'], '@sort' => $sort['sort']]));
    -      $this->assertEqual($last->task, $sort['last'], format_string('Items appear in the correct order sorting by @field @sort.', ['@field' => $sort['field'], '@sort' => $sort['sort']]));
    +      $this->assertEqual($first->task, $sort['first']);
    +      $this->assertEqual($last->task, $sort['last']);
    

    field/sort order useful?

25. +++ b/core/modules/system/tests/src/Functional/Entity/EntityReferenceSelection/EntityReferenceSelectionAccessTest.php
    @@ -86,7 +86,7 @@ protected function assertReferenceable(array $selection_options, $tests, $handle
           foreach ($test['arguments'] as $arguments) {
             $result = call_user_func_array([$handler, 'getReferenceableEntities'], $arguments);
    -        $this->assertEqual($result, $test['result'], format_string('Valid result set returned by @handler.', ['@handler' => $handler_name]));
    +        $this->assertEqual($result, $test['result']);
     
             $result = call_user_func_array([$handler, 'countReferenceableEntities'], $arguments);
    
    @@ -97,7 +97,7 @@ protected function assertReferenceable(array $selection_options, $tests, $handle
             }
     
    -        $this->assertEqual($result, $count, format_string('Valid count returned by @handler.', ['@handler' => $handler_name]));
    +        $this->assertEqual($result, $count);
    

    handler name is kinda useful. also, not using it means the method argument $handler_name would now be unused and cleaning that up would be much more work :)

26. +++ b/core/modules/system/tests/src/Functional/Form/CheckboxTest.php
    @@ -48,8 +48,7 @@ public function testFormCheckbox() {
             }
             $checked_in_html = strpos($form, 'checked') !== FALSE;
    -        $message = format_string('#default_value is %default_value #return_value is %return_value.', ['%default_value' => var_export($default_value, TRUE), '%return_value' => var_export($return_value, TRUE)]);
    -        $this->assertIdentical($checked, $checked_in_html, $message);
    +        $this->assertSame($checked, $checked_in_html);
           }
    
    @@ -78,7 +77,7 @@ public function testFormCheckbox() {
           $name = $checkbox->getAttribute('name');
    -      $this->assertIdentical($checked, $name == 'checkbox_zero_default[0]' || $name == 'checkbox_string_zero_default[0]', format_string('Checkbox %name correctly checked', ['%name' => $name]));
    +      $this->assertSame($checked, $name == 'checkbox_zero_default[0]' || $name == 'checkbox_string_zero_default[0]');
    
    @@ -92,7 +91,7 @@ public function testFormCheckbox() {
           $name = (string) $checkbox->getAttribute('name');
    -      $this->assertIdentical($checked, $name == 'checkbox_off[0]' || $name == 'checkbox_zero_default[0]' || $name == 'checkbox_string_zero_default[0]', format_string('Checkbox %name correctly checked', ['%name' => $name]));
    +      $this->assertSame($checked, $name == 'checkbox_off[0]' || $name == 'checkbox_zero_default[0]' || $name == 'checkbox_string_zero_default[0]');
         }
    

    these are all in loops, so useful? also you still have the switch to assertSame() here but didn't switch the argument order.

27. +++ b/core/modules/system/tests/src/Functional/Form/FormTest.php
    @@ -369,11 +369,7 @@ public function testCheckboxProcessing() {
         foreach ($expected_values as $widget => $expected_value) {
    -      $this->assertSame($values[$widget], $expected_value, format_string('Checkbox %widget returns expected value (expected: %expected, got: %value)', [
    -        '%widget' => var_export($widget, TRUE),
    -        '%expected' => var_export($expected_value, TRUE),
    -        '%value' => var_export($values[$widget], TRUE),
    -      ]));
    +      $this->assertSame($values[$widget], $expected_value);
    
    @@ -446,11 +442,7 @@ public function testSelect() {
         foreach ($expected as $key => $value) {
    -      $this->assertIdentical($values[$key], $value, format_string('@name: @actual is equal to @expected.', [
    -        '@name' => $key,
    -        '@actual' => var_export($values[$key], TRUE),
    -        '@expected' => var_export($value, TRUE),
    -      ]));
    

    more loops that are useful?

28. +++ b/core/modules/system/tests/src/Functional/Form/FormTest.php
    @@ -660,7 +652,7 @@ public function assertFormValuesDefault($values, $form) {
             }
    -        $this->assertIdentical($expected_value, $values[$key], format_string('Default value for %type: expected %expected, returned %returned.', ['%type' => $key, '%expected' => var_export($expected_value, TRUE), '%returned' => var_export($values[$key], TRUE)]));
    +        $this->assertSame($expected_value, $values[$key]);
           }
    

    identical -> same, but here the expected is first already, so might be ok. $type might be useful, maybe remove the expected/returned part?

29. +++ b/core/modules/system/tests/src/Functional/Form/StateValuesCleanTest.php
    @@ -35,16 +35,16 @@ public function testFormStateValuesClean() {
         // Verify that all internal Form API elements were removed.
    -    $this->assertFalse(isset($values['form_id']), format_string('%element was removed.', ['%element' => 'form_id']));
    -    $this->assertFalse(isset($values['form_token']), format_string('%element was removed.', ['%element' => 'form_token']));
    -    $this->assertFalse(isset($values['form_build_id']), format_string('%element was removed.', ['%element' => 'form_build_id']));
    -    $this->assertFalse(isset($values['op']), format_string('%element was removed.', ['%element' => 'op']));
    +    $this->assertFalse(isset($values['form_id']), new FormattableMarkup('%element was removed.', ['%element' => 'form_id']));
    +    $this->assertFalse(isset($values['form_token']), new FormattableMarkup('%element was removed.', ['%element' => 'form_token']));
    +    $this->assertFalse(isset($values['form_build_id']), new FormattableMarkup('%element was removed.', ['%element' => 'form_build_id']));
    +    $this->assertFalse(isset($values['op']), new FormattableMarkup('%element was removed.', ['%element' => 'op']));
    

    these could use assertArrayNotHasKey then we could drop the messages, but I'm not sure how far we want to go with these changes :-/

30. +++ b/core/modules/system/tests/src/Functional/Module/ModuleTestBase.php
    @@ -161,7 +162,7 @@ public function assertModules(array $modules, $enabled) {
             $message = 'Module "@module" is not enabled.';
           }
    -      $this->assertEqual($this->container->get('module_handler')->moduleExists($module), $enabled, format_string($message, ['@module' => $module]));
    +      $this->assertEqual($this->container->get('module_handler')->moduleExists($module), $enabled);
         }
    

    this compares with a boolean, so is technically an asertTrue/False. module is useful information (and message would be unused now.)

31. +++ b/core/modules/system/tests/src/Functional/Path/UrlAlterFunctionalTest.php
    @@ -95,7 +95,7 @@ public function testUrlAlter() {
         $result = $this->container->get('path_processor_manager')->processOutbound($original);
    -    return $this->assertIdentical($result, $final, format_string('Altered outbound URL %original, expected %final, and got %result.', ['%original' => $original, '%final' => $final, '%result' => $result]));
    +    return $this->assertSame($result, $final);
       }
     
       /**
    @@ -112,7 +112,7 @@ protected function assertUrlOutboundAlter($original, $final) {
    
    @@ -112,7 +112,7 @@ protected function assertUrlOutboundAlter($original, $final) {
       protected function assertUrlInboundAlter($original, $final) {
         // Test inbound altering.
         $result = $this->container->get('path.alias_manager')->getPathByAlias($original);
    -    return $this->assertIdentical($result, $final, format_string('Altered inbound URL %original, expected %final, and got %result.', ['%original' => $original, '%final' => $final, '%result' => $result]));
    +    return $this->assertSame($result, $final);
       }
    

    identical/same change.

32. +++ b/core/modules/system/tests/src/FunctionalJavascript/FrameworkTest.php
    @@ -37,9 +38,9 @@ public function testLazyLoad() {
         // be lazy loaded as part of the next requests.
    -    $this->assertTrue(!isset($original_settings[$expected['setting_name']]), format_string('Page originally lacks the %setting, as expected.', ['%setting' => $expected['setting_name']]));
    ...
    -    $this->assertTrue(!in_array($expected['library_2'], $original_libraries), format_string('Page originally lacks the %library library, as expected.', ['%library' => $expected['library_2']]));
    +    $this->assertTrue(!isset($original_settings[$expected['setting_name']]), new FormattableMarkup('Page originally lacks the %setting, as expected.', ['%setting' => $expected['setting_name']]));
    +    $this->assertTrue(!in_array($expected['library_1'], $original_libraries), new FormattableMarkup('Page originally lacks the %library library, as expected.', ['%library' => $expected['library_1']]));
    +    $this->assertTrue(!in_array($expected['library_2'], $original_libraries), new FormattableMarkup('Page originally lacks the %library library, as expected.', ['%library' => $expected['library_2']]));
    
    @@ -48,9 +49,9 @@ public function testLazyLoad() {
         // Verify the setting was not added when not expected.
    -    $this->assertTrue(!isset($new_settings[$expected['setting_name']]), format_string('Page still lacks the %setting, as expected.', ['%setting' => $expected['setting_name']]));
    -    $this->assertTrue(!in_array($expected['library_1'], $new_libraries), format_string('Page still lacks the %library library, as expected.', ['%library' => $expected['library_1']]));
    -    $this->assertTrue(!in_array($expected['library_2'], $new_libraries), format_string('Page still lacks the %library library, as expected.', ['%library' => $expected['library_2']]));
    +    $this->assertTrue(!isset($new_settings[$expected['setting_name']]), new FormattableMarkup('Page still lacks the %setting, as expected.', ['%setting' => $expected['setting_name']]));
    +    $this->assertTrue(!in_array($expected['library_1'], $new_libraries), new FormattableMarkup('Page still lacks the %library library, as expected.', ['%library' => $expected['library_1']]));
    +    $this->assertTrue(!in_array($expected['library_2'], $new_libraries), new FormattableMarkup('Page still lacks the %library library, as expected.', ['%library' => $expected['library_2']]));
     
    

    more things that could use assertArray(NOt)HasKey and assert(Not)Contains if we'd want to, but as mentioned before, likely too much for this issue.

33. +++ b/core/modules/system/tests/src/FunctionalJavascript/FrameworkTest.php
    @@ -61,11 +62,11 @@ public function testLazyLoad() {
         // Verify the expected setting was added, both to drupalSettings, and as
         // the first AJAX command.
    -    $this->assertIdentical($new_settings[$expected['setting_name']], $expected['setting_value'], format_string('Page now has the %setting.', ['%setting' => $expected['setting_name']]));
    +    $this->assertSame($new_settings[$expected['setting_name']], $expected['setting_value']);
     
    

    remove is fine, but identical/same with wrong order.

34. +++ b/core/modules/system/tests/src/Kernel/Block/SystemMenuBlockTest.php
    @@ -227,7 +227,7 @@ public function testConfigLevelDepth() {
           $items = isset($block_build['#items']) ? $block_build['#items'] : [];
    -      $this->assertIdentical($no_active_trail_expectations[$id], $this->convertBuiltMenuToIdTree($items), format_string('Menu block %id with no active trail renders the expected tree.', ['%id' => $id]));
    +      $this->assertSame($no_active_trail_expectations[$id], $this->convertBuiltMenuToIdTree($items));
    
    @@ -279,7 +279,7 @@ public function testConfigLevelDepth() {
           $block_build = $block->build();
           $items = isset($block_build['#items']) ? $block_build['#items'] : [];
    -      $this->assertIdentical($active_trail_expectations[$id], $this->convertBuiltMenuToIdTree($items), format_string('Menu block %id with an active trail renders the expected tree.', ['%id' => $id]));
    +      $this->assertSame($active_trail_expectations[$id], $this->convertBuiltMenuToIdTree($items));
    

    more

35. +++ b/core/modules/system/tests/src/Kernel/Common/AddFeedTest.php
    @@ -68,7 +69,7 @@ public function testBasicFeedAddNoTitle() {
         foreach ($urls as $description => $feed_info) {
    -      $this->assertPattern($this->urlToRSSLinkPattern($feed_info['url'], $feed_info['title']), format_string('Found correct feed header for %description', ['%description' => $description]));
    +      $this->assertPattern($this->urlToRSSLinkPattern($feed_info['url'], $feed_info['title']), new FormattableMarkup('Found correct feed header for %description', ['%description' => $description]));
         }
       }
    

    assertPattern() doesn't have a message argument

36. +++ b/core/modules/system/tests/src/Kernel/Extension/ModuleHandlerTest.php
    @@ -61,7 +61,7 @@ public function testModuleList() {
         $enabled_modules = array_keys($this->container->get('module_handler')->getModuleList());
    -    $this->assertEqual($expected_values, $enabled_modules, format_string('@condition: extension handler returns correct results', ['@condition' => $condition]));
    +    $this->assertEqual($expected_values, $enabled_modules);
    

    hm, $condition specifically exists to be shown in the message, seems useful and if we remove it you need to clean up the argument and calls to it ;)

37. +++ b/core/modules/system/tests/src/Kernel/Form/ProgrammaticTest.php
    @@ -77,14 +78,14 @@ protected function doSubmitForm($values, $valid_input) {
           $stored_values = $form_state->get('programmatic_form_submit');
           foreach ($values as $key => $value) {
    -        $this->assertEqual($stored_values[$key], $value, format_string('Submission handler correctly executed: %stored_key is %stored_value', ['%stored_key' => $key, '%stored_value' => print_r($value, TRUE)]));
    +        $this->assertEqual($stored_values[$key], $value);
           }
    

    $key useful?

38. +++ b/core/modules/system/tests/src/Kernel/Token/TokenReplaceKernelTest.php
    @@ -150,7 +150,7 @@ public function testSystemDateTokenReplacement() {
         foreach ($tests as $input => $expected) {
           $output = $this->tokenService->replace($input, ['date' => $date], ['langcode' => $this->interfaceLanguage->getId()]);
    -      $this->assertEqual($output, $expected, format_string('Date token %token replaced.', ['%token' => $input]));
    +      $this->assertEqual($output, $expected);
    

    token test, useful.

39. +++ b/core/modules/taxonomy/tests/src/Functional/TokenReplaceTest.php
    @@ -105,7 +105,7 @@ public function testTaxonomyTokenReplacement() {
           $bubbleable_metadata = new BubbleableMetadata();
           $output = $token_service->replace($input, ['term' => $term1], ['langcode' => $language_interface->getId()], $bubbleable_metadata);
    -      $this->assertEqual($output, $expected, format_string('Sanitized taxonomy term token %token replaced.', ['%token' => $input]));
    +      $this->assertEqual($output, $expected);
           $this->assertEqual($bubbleable_metadata, $metadata_tests[$input]);
    
    @@ -126,7 +126,7 @@ public function testTaxonomyTokenReplacement() {
           $output = $token_service->replace($input, ['term' => $term2], ['langcode' => $language_interface->getId()]);
    -      $this->assertEqual($output, $expected, format_string('Sanitized taxonomy term token %token replaced.', ['%token' => $input]));
    +      $this->assertEqual($output, $expected);
         }
    
    @@ -142,7 +142,7 @@ public function testTaxonomyTokenReplacement() {
         foreach ($tests as $input => $expected) {
           $output = $token_service->replace($input, ['vocabulary' => $this->vocabulary], ['langcode' => $language_interface->getId()]);
    -      $this->assertEqual($output, $expected, format_string('Sanitized taxonomy vocabulary token %token replaced.', ['%token' => $input]));
    +      $this->assertEqual($output, $expected);
         }
    

    more token tests.

40. +++ b/core/modules/text/tests/src/Kernel/TextFormatterTest.php
    @@ -91,7 +91,7 @@ public function testFormatters() {
           \Drupal::service('renderer')->renderRoot($build[0]);
           $this->assertEqual($build[0]['#markup'], "<p>Hello, world!</p>\n");
    -      $this->assertEqual($build[0]['#cache']['tags'], FilterFormat::load('my_text_format')->getCacheTags(), format_string('The @formatter formatter has the expected cache tags when formatting a formatted text field.', ['@formatter' => $formatter]));
    +      $this->assertEqual($build[0]['#cache']['tags'], FilterFormat::load('my_text_format')->getCacheTags());
    

    $oformatter useful?

41. +++ b/core/modules/text/tests/src/Kernel/TextSummaryTest.php
    @@ -221,10 +221,7 @@ public function testInvalidFilterFormat() {
         $summary = text_summary($text, $format, $size);
    -    $this->assertIdentical($summary, $expected, format_string('<pre style="white-space: pre-wrap">@actual</pre> is identical to <pre style="white-space: pre-wrap">@expected</pre>', [
    -      '@actual' => $summary,
    -      '@expected' => $expected,
    -    ]));
    +    $this->assertSame($summary, $expected);
    

    identical/same switch.

42. +++ b/core/modules/views/tests/src/Functional/DefaultViewsTest.php
    @@ -135,14 +136,14 @@ public function testDefaultViews() {
     
    -        $this->assert(TRUE, format_string('View @view will be executed.', ['@view' => $view->storage->id()]));
    +        $this->assert(TRUE, new FormattableMarkup('View @view will be executed.', ['@view' => $view->storage->id()]));
             $view->execute();
    

    this line is useless with phpunit, it only exists to show debug info in the output table in simpletest but phpunit doesn't show passed assertions. and if one fails then we have it below.

43. +++ b/core/modules/views/tests/src/Functional/Handler/FieldWebTest.php
    @@ -560,14 +560,14 @@ public function testTextRendering() {
         });
    -    $this->assertSubString($output, $trimmed_name, format_string('Make sure the trimmed output (@trimmed) appears in the rendered output (@output).', ['@trimmed' => $trimmed_name, '@output' => $output]));
    -    $this->assertNotSubString($output, $row->views_test_data_name, format_string("Make sure the untrimmed value (@untrimmed) shouldn't appear in the rendered output (@output).", ['@untrimmed' => $row->views_test_data_name, '@output' => $output]));
    +    $this->assertSubString($output, $trimmed_name);
    +    $this->assertNotSubString($output, $row->views_test_data_name);
     
         $name_field->options['alter']['max_length'] = 9;
    

    follow-up to deprecate these in favor of assertContains()?

44. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaEntityTest.php
    @@ -70,9 +71,9 @@ public function testEntityAreaData() {
         // Test that all expected entity types have data.
         foreach (array_keys($expected_entities) as $entity) {
    -      $this->assertTrue(!empty($data['entity_' . $entity]), format_string('Views entity area data found for @entity', ['@entity' => $entity]));
    +      $this->assertTrue(!empty($data['entity_' . $entity]), new FormattableMarkup('Views entity area data found for @entity', ['@entity' => $entity]));
           // Test that entity_type is set correctly in the area data.
    -      $this->assertEqual($entity, $data['entity_' . $entity]['area']['entity_type'], format_string('Correct entity_type set for @entity', ['@entity' => $entity]));
    +      $this->assertEqual($entity, $data['entity_' . $entity]['area']['entity_type']);
    

    $entity might be useful?

45. +++ b/core/modules/views/tests/src/Kernel/Handler/FieldKernelTest.php
    @@ -298,26 +298,17 @@ public function testFieldTokens() {
           });
    -      $this->assertEqual($output, $expected_output_0, format_string('Test token replacement: "@token" gave "@output"', [
    -        '@token' => $name_field_0->options['alter']['text'],
    -        '@output' => $output,
    -      ]));
    +      $this->assertEqual($output, $expected_output_0);
     
    

    @token seems useful, we could drop $ouptut but that will only make the patch bigger..

46. +++ b/core/modules/views/tests/src/Kernel/Plugin/StyleMappingTest.php
    @@ -68,7 +68,7 @@ protected function mappedOutputHelper($view) {
             $actual_result = (string) $field;
    -        $this->assertIdentical($expected_result, $actual_result, format_string('The fields were mapped successfully: %name => %field_id', ['%name' => $name, '%field_id' => $field_id]));
    +        $this->assertSame($expected_result, $actual_result);
           }
    

    identical/same, although order is correct here. you might want to search if I found all of them ;)

47. +++ b/core/modules/views/tests/src/Kernel/QueryGroupByTest.php
    @@ -104,8 +104,8 @@ public function groupByTestHelper($aggregation_function, $values) {
    -    $this->assertEqual($results['name1'], $values[0], format_string('Aggregation with @aggregation_function and groupby name: name1 returned the expected amount of results', ['@aggregation_function' => $aggregation_function]));
    -    $this->assertEqual($results['name2'], $values[1], format_string('Aggregation with @aggregation_function and groupby name: name2 returned the expected amount of results', ['@aggregation_function' => $aggregation_function]));
    +    $this->assertEqual($results['name1'], $values[0]);
    +    $this->assertEqual($results['name2'], $values[1]);
    

    removel is fine I think, aggregation function is obvious from the name of test method calling this helper.that information was not available in simpletest, but is in phpunit.

48. +++ b/core/modules/views/tests/src/Kernel/TokenReplaceTest.php
    @@ -68,7 +68,7 @@ public function testTokenReplacement() {
           $bubbleable_metadata = new BubbleableMetadata();
           $output = $token_handler->replace($token, ['view' => $view], [], $bubbleable_metadata);
    -      $this->assertIdentical($output, $expected_output, format_string('Token %token replaced correctly.', ['%token' => $token]));
    +      $this->assertSame($output, $expected_output);
           $this->assertEqual($bubbleable_metadata, $metadata_tests[$token]);
         }
    
    @@ -125,7 +125,7 @@ public function testTokenReplacementNoResults() {
     
         foreach ($expected as $token => $expected_output) {
           $output = $token_handler->replace($token, ['view' => $view]);
    -      $this->assertIdentical($output, $expected_output, format_string('Token %token replaced correctly.', ['%token' => $token]));
    +      $this->assertSame($output, $expected_output);
    
    @@ -144,7 +144,7 @@ public function testTokenReplacementNoPath() {
           $output = $token_handler->replace($token, ['view' => $view]);
    -      $this->assertIdentical($output, $expected_output, format_string('Token %token replaced correctly.', ['%token' => $token]));
    +      $this->assertSame($output, $expected_output);
    

    more token stuff.

49. +++ b/core/modules/views/tests/src/Kernel/ViewStorageTest.php
    @@ -101,7 +102,7 @@ protected function loadTests() {
           $original_options = $data['display'][$key];
           foreach ($original_options as $orig_key => $value) {
    -        $this->assertIdentical($display[$orig_key], $value, format_string('@key is identical to saved data', ['@key' => $key]));
    +        $this->assertSame($display[$orig_key], $value);
    

    identical/same

50. +++ b/core/modules/views/tests/src/Kernel/ViewStorageTest.php
    @@ -137,8 +138,8 @@ protected function createTests() {
    -      $this->assertTrue($created->get($property) !== NULL, format_string('Property: @property created on View.', ['@property' => $property]));
    -      $this->assertIdentical($values[$property], $created->get($property), format_string('Property value: @property matches configuration value.', ['@property' => $property]));
    +      $this->assertTrue($created->get($property) !== NULL, new FormattableMarkup('Property: @property created on View.', ['@property' => $property]));
    +      $this->assertSame($values[$property], $created->get($property));
    

    same and property seems useful.

51. +++ b/core/modules/views/tests/src/Kernel/ViewStorageTest.php
    @@ -337,14 +338,14 @@ public function testCreateDuplicate() {
     
         foreach ($config_properties as $property) {
    -      $this->assertIdentical($view->storage->get($property), $copy->get($property), format_string('@property property is identical.', ['@property' => $property]));
    +      $this->assertSame($view->storage->get($property), $copy->get($property));
    

    same and same (property)

52. +++ b/core/modules/views_ui/tests/src/Functional/StorageTest.php
    @@ -49,7 +49,7 @@ public function testDetails() {
     
         foreach (['label', 'tag', 'description', 'langcode'] as $property) {
    -      $this->assertEqual($view->storage->get($property), $edit[$property], format_string('Make sure the property @property got probably saved.', ['@property' => $property]));
    +      $this->assertEqual($view->storage->get($property), $edit[$property]);
         }
    

    another useful property

53. +++ b/core/tests/Drupal/KernelTests/Core/Database/ConnectionTest.php
    @@ -168,9 +168,9 @@ public function testPostgresqlReservedWords() {
           $expected = '"' . $word . '"';
    -      $this->assertIdentical($db->escapeTable($word), $expected, format_string('The reserved word %word was correctly escaped when used as a table name.', ['%word' => $word]));
    -      $this->assertIdentical($db->escapeField($word), $expected, format_string('The reserved word %word was correctly escaped when used as a column name.', ['%word' => $word]));
    -      $this->assertIdentical($db->escapeAlias($word), $expected, format_string('The reserved word %word was correctly escaped when used as an alias.', ['%word' => $word]));
    +      $this->assertSame($db->escapeTable($word), $expected);
    +      $this->assertSame($db->escapeField($word), $expected);
    +      $this->assertSame($db->escapeAlias($word), $expected);
         }
    

    identical/same

54. +++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php
    @@ -686,7 +687,7 @@ protected function assertFieldAdditionRemoval($field_spec) {
         $this->schema->createTable($table_name, $table_spec);
    -    $this->pass(format_string('Table %table created.', ['%table' => $table_name]));
    +    $this->pass(new FormattableMarkup('Table %table created.', ['%table' => $table_name]));
    

    these passes are also pretty useless (all calls to pass() are I guess). not sure if you want to remove them here or not.

55. +++ b/core/tests/Drupal/KernelTests/Core/Database/SelectComplexTest.php
    @@ -100,7 +100,7 @@ public function testGroupBy() {
         foreach ($correct_results as $task => $count) {
    -      $this->assertEqual($records[$task], $count, format_string("Correct number of '@task' records found.", ['@task' => $task]));
    +      $this->assertEqual($records[$task], $count);
         }
     
         $this->assertEqual($num_records, 6, 'Returned the correct number of total rows.');
    @@ -134,7 +134,7 @@ public function testGroupByAndHaving() {
    
    @@ -134,7 +134,7 @@ public function testGroupByAndHaving() {
         ];
     
         foreach ($correct_results as $task => $count) {
    -      $this->assertEqual($records[$task], $count, format_string("Correct number of '@task' records found.", ['@task' => $task]));
    +      $this->assertEqual($records[$task], $count);
    

    task useful?

56. +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityAccessControlHandlerTest.php
    @@ -40,12 +40,7 @@ public function setUp() {
        */
       public function assertEntityAccess($ops, AccessibleInterface $object, AccountInterface $account = NULL) {
         foreach ($ops as $op => $result) {
    -      $message = format_string("Entity access returns @result with operation '@op'.", [
    -        '@result' => !isset($result) ? 'null' : ($result ? 'true' : 'false'),
    -        '@op' => $op,
    -      ]);
    -
    -      $this->assertEqual($result, $object->access($op, $account), $message);
    +      $this->assertEqual($result, $object->access($op, $account));
         }
       }
    

    this seems useful as it is a loop and you need to know which operation failed. it's an assertEqual() but again really a boolean comparison.

57. +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityApiTest.php
    @@ -66,32 +67,32 @@ protected function assertCRUD($entity_type, UserInterface $user1) {
         $entities[0]->delete();
         $entities = array_values($storage->loadByProperties(['name' => 'test2']));
    -    $this->assertEqual($entities, [], format_string('%entity_type: Entity deleted.', ['%entity_type' => $entity_type]));
    +    $this->assertEqual($entities, []);
     
    ...
     
         $all = $storage->loadMultiple();
    -    $this->assertTrue(empty($all), format_string('%entity_type: Deleted all entities.', ['%entity_type' => $entity_type]));
    +    $this->assertTrue(empty($all), new FormattableMarkup('%entity_type: Deleted all entities.', ['%entity_type' => $entity_type]));
     
    

    those two checks do exactly the same, a bit arbitrary to remove one message and keep the other because assertEqual()/assertTrue. I'd either remove both or keep both.

58. +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityFieldTest.php
    @@ -115,66 +116,66 @@ protected function doTestReadWrite($entity_type) {
         $entity->user_id->entity = $new_user1;
    -    $this->assertEqual($new_user1->id(), $entity->user_id->target_id, format_string('%entity_type: Updated user id can be read.', ['%entity_type' => $entity_type]));
    -    $this->assertEqual($new_user1->getAccountName(), $entity->user_id->entity->name->value, format_string('%entity_type: Updated username value can be read.', ['%entity_type' => $entity_type]));
    +    $this->assertEqual($new_user1->id(), $entity->user_id->target_id);
    +    $this->assertEqual($new_user1->getAccountName(), $entity->user_id->entity->name->value);
     
         // Change the assigned user by id.
    

    the thing with all these assertions is that the whole method runs with different entity types, so we lose the info on which one it fails with all those removals. not just this assert or even this method but several huge chunks around here.

59. +++ b/core/tests/Drupal/KernelTests/Core/File/MimeTypeTest.php
    @@ -45,12 +45,12 @@ public function testFileMimeTypeDetection() {
           foreach ($prefixes as $prefix) {
             $output = $guesser->guess($prefix . $input);
    -        $this->assertIdentical($output, $expected, format_string('Mimetype for %input is %output (expected: %expected).', ['%input' => $prefix . $input, '%output' => $output, '%expected' => $expected]));
    +        $this->assertSame($output, $expected);
           }
     
           // Test normal path equivalent
           $output = $guesser->guess($input);
    -      $this->assertIdentical($output, $expected, format_string('Mimetype (using default mappings) for %input is %output (expected: %expected).', ['%input' => $input, '%output' => $output, '%expected' => $expected]));
    +      $this->assertSame($output, $expected);
         }
     
         // Now test the extension guesser by passing in a custom mapping.
    @@ -85,7 +85,7 @@ public function testFileMimeTypeDetection() {
    
    @@ -85,7 +85,7 @@ public function testFileMimeTypeDetection() {
     
         foreach ($test_case as $input => $expected) {
           $output = $extension_guesser->guess($input);
    -      $this->assertIdentical($output, $expected, format_string('Mimetype (using passed-in mappings) for %input is %output (expected: %expected).', ['%input' => $input, '%output' => $output, '%expected' => $expected]));
    +      $this->assertSame($output, $expected);
    

    identical/same

The removing/changing of messages seems to introduce a lot of extra scope to this, that, I think, is really not needed. Shouldn't we just do a 1-1 conversion here and worry about the messages later? We need to do that any way since this only addresses the part of the messages that happen to use format_string(), there are a lot of other messages that need removing/updating.

Raised this with core committers as well. @alexpott wrote this and then @larowlan chimed in to agree:

I can see both sides here. I guess for fixing the deprecation the 1-1 replacement is the way to go but at some point we really do need to go through and remove all the messages. So many of them are useless.

It is WAY easier to review a 400k patch when there are no changes to debate one by one, I agree. So let's focus only on replacing the use of deprecated API and making our tests better separately.

So back to around the state at #90 for a much faster commit :)

Just reroll of #108

Fix CS issues.

Please ignore #113 and #114 comments.
Just reroll patch based on the #90. Addressed #112.
Also, the patch which fixes CS issues in deprecation messages.

Reroll
Conflict was in core/modules/filter/tests/filter_test/src/Plugin/Filter/FilterTestPlaceholders.php
Patch is ready for the review

Let's open a follow-up for the changes in #108. Agreed with doing a direct find and replace here just to sort out the deprecation.

#3066933: Cleanup formatted messages in the tests filled. Feel free to update IS of the issue in case if there are not fully highlighted the scope of the issue.

Scanned through the changes and the important bits are there:

• trigger_error() added to format_string()
• deprecation test added for format_string()

So I guess the green tests means we got everything.

Only thing I see remaining is the mention of format_string in \Drupal\Tests\Core\Logger\LogMessageParserTest::providerTestParseMessagePlaceholders which we might want to clean up, but I would propose doing that in a follow up too, just to keep the scope here as small as possible.
RTBC I would say.

Credited @Mile23, @andypost, @ianthomas_uk, @alexpott, @dawehner, @xjm, @Gábor Hojtsy, @Berdir, @Lendude and @catch for general issue management, review, and online discussion.

Committed e39262b and pushed to 8.8.x. Thanks!