Make Drupal 8 & 9 compatible with PHP 7.4

Also see #3085735: Support PHP 7.4 in Drupal 8

c/p from IS of #3085735: Support PHP 7.4 in Drupal 8 to close that as duplicate.

A patch to test how many failures we get after upgrading some dependencies and quickfixing some of the other code as per the IS.

Done #8, thanks.

No longer necessary to upgrade other packages than phar-stream-wrapper, apparently.

Added some more tweaks.

More.

Too many fails, let's restrict test runs to Unit and Kernel tests and focus on that.

This may help - touches Element so reopening Functional and FunctonalJavascript tests

#13 is down to 107 fails in total!

#14 because otherwise the entire element render is an empty string, hence the thousand of failures. If the $key is numeric we still need to render the element. Or am I missing somethig?

On it.

Changing from plan to task, too - this seems closer now.

Done #16, added more tweaks.

#20 did that because of

PHPUnit 7.5.16 by Sebastian Bergmann and contributors.

Testing Drupal\FunctionalTests\Installer\InstallerExistingConfigNoConfigTest
E                                                                   1 / 1 (100%)

Time: 5.02 seconds, Memory: 4.00 MB

There was 1 error:

1) Drupal\FunctionalTests\Installer\InstallerExistingConfigNoConfigTest::testConfigSync
Exception: Notice: Trying to access array offset on value of type bool
install_config_import_batch()() (Line: 2326)

Done #22 and trying something different with RequestSanitizer

1. +++ b/core/includes/theme.inc
   @@ -1128,7 +1128,7 @@ function template_preprocess_item_list(&$variables) {
   +              if (is_integer($child_key) || $child_key[0] !== '#') {
   

   You wanted ctype_digit

2. +++ b/core/lib/Drupal/Component/Annotation/Doctrine/DocParser.php
   @@ -987,7 +990,7 @@ private function Value()
   +        if (!empty($peek) && DocLexer::T_EQUALS === $peek['type']) {
   

   if ($peek &&) We know it's defined, no need for the !empty wrapper.

3. +++ b/core/lib/Drupal/Core/Asset/CssCollectionRenderer.php
   @@ -49,6 +49,9 @@ public function render(array $css_assets) {
   +      if (empty($css_asset)) {
   

   Same. if (!$css_asset).

4. +++ b/core/lib/Drupal/Core/Form/FormSubmitter.php
   @@ -61,7 +61,7 @@ public function doSubmitForm(&$form, FormStateInterface &$form_state) {
   +      if ($batch['progressive'] ?? NULL) {
   

   Since I am nitpicking anyways why don't we ?? FALSE. You are type coercing to boolean right after anyways.

5. +++ b/core/lib/Drupal/Core/Security/RequestSanitizer.php
   @@ -153,7 +153,7 @@ protected static function checkDestination($destination, array $whitelist) {
   +        if (is_integer($key) || ($key !== '' && $key[0] === '#' && !in_array($key, $whitelist, TRUE))) {
   

   ctype_digit

6. +++ b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php
   @@ -188,7 +188,7 @@ public function setValue($values, $notify = TRUE) {
   +    if (isset($values['options']) && is_string($values['options'])) {
   

   After using ?? so much, why is it not used here? is_string($values['options'] ?? NULL)

7. +++ b/core/modules/locale/tests/src/Functional/LocaleJavascriptTranslationTest.php
   @@ -152,7 +152,7 @@ public function testLocaleTranslationJsDependencies() {
   +    $js_filename = $prefix . '_' . ($js_translation_files[$prefix] ?? NULL) . '.js';
   

   ?? ''

8. +++ b/core/modules/views/src/Plugin/views/display/PathPluginBase.php
   @@ -101,7 +101,7 @@ public function getPath() {
   +    return !empty($menu) && $menu['type'] == 'default tab' && !empty($tab_options['type']) && $tab_options['type'] != 'none';
   

   No need for !empty again. Just $menu && is enough. And since we are touching this ancient code could we please change the == to === and the != to !== please while at it?

9. +++ b/core/modules/views/src/Plugin/views/filter/StringFilter.php
   @@ -341,7 +341,7 @@ protected function opContainsWord($field) {
   +      if ($match[2][0] == '"') {
   

   Yeah triple equals here would be nice too.

10. +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityViewBuilderTest.php
    @@ -194,7 +194,7 @@ public function testEntityViewBuilderWeight() {
    +    $this->assertEqual($view['label']['#weight'] ?? NULL, 20, 'The weight of a display component is respected.');
    

    Same ?? ''

11. +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityTranslationTest.php
    @@ -664,7 +664,7 @@ protected function doTestLanguageFallback($entity_type) {
    +    $this->assertEqual($build['label']['#markup'] ?? NULL, $values[$current_langcode]['name'], 'By default the entity is rendered in the current language.');
    

    ?? ''. assertEquals IMO is a wrapper around the dreaded== and NULL == 0 among other things. No need for that.

12. +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityViewBuilderTest.php
    @@ -194,7 +194,7 @@ public function testEntityViewBuilderWeight() {
    +    $this->assertEqual($view['label']['#weight'] ?? NULL, 20, 'The weight of a display component is respected.');
    

    Same ?? ''.

AFK from now. If anyone wants to take this on feel free :)

Looks like also Composer yet needs to release a PHP 7.4 compatibile version including https://github.com/composer/composer/pull/8296

In the meantime dev-master could do, apparently.

Patch for #33

#30 I can't remember... Let's revert at this stage and see what breaks.

Just a note that mikey179/vfsstream 2.0.x-dev requires PHP 7.2 , so D8.8. cannot upgrade to it - a 1.x version upstream would be needed?

We also need a new release of pear/Archive_Tar fixing https://github.com/pear/Archive_Tar/issues/24

Better solution for #30?

Working on the views test failures.

Addressing #50.

@alexpott you're right, obvioulsy... let's do this one more, then have a general look and start spinning off specific issues? My concern is that tests will fail if we do not use combined patches, though.

Added @todos as per #56.

I was trying to look into the annotation parsing test failures, very obscure to me... it looks like something is wrong with the deprecation error handlers, but cannot figure out what.

PHPUnit 7.5.16 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\Component\Annotation\Doctrine\DocParserTest
EEEEEE.....................EE...E.E.E....E.....................  63 / 227 ( 27%)
........E.................E....................EE...E.E.E....E. 126 / 227 ( 55%)
............................E.................E................ 189 / 227 ( 83%)
.........EE..EEEEE.EEE..EEEESE.EEEEEEE                          227 / 227 (100%)

Time: 359 ms, Memory: 6.00 MB

There were 44 errors:

1) Drupal\Tests\Component\Annotation\Doctrine\DocParserTest::testNestedArraysWithNestedAnnotation
Trying to access array offset on value of type null

/var/www/html/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php:165
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:995
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:687
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:663
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:354
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:518
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:744
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:663
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:354
/var/www/html/core/tests/Drupal/Tests/Component/Annotation/Doctrine/DocParserTest.php:35

[...]

PHPUnit 7.5.16 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\Component\Annotation\Doctrine\Ticket\DCOM58Test
EEEEEE                                                              6 / 6 (100%)

Time: 2.06 seconds, Memory: 4.00 MB

There were 6 errors:

1) Drupal\Tests\Component\Annotation\Doctrine\Ticket\DCOM58Test::testIssue
Trying to access array offset on value of type null

/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:995
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:687
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:663
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:354
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationReader.php:401
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationReader.php:334
/var/www/html/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationReader.php:214
/var/www/html/core/tests/Drupal/Tests/Component/Annotation/Doctrine/Ticket/DCOM58Test.php:34

[...]

Would be great to see green soon, @Taran2L single handedly did the D7 compatibility and all tests are passing for D7. There might be some synergies with his work and the work that needs to happen in the D8 branch.

For this reason I've linked the D7 php 7.4.x compatibility issue here as related.

Fix for YamlFileLoaderTest

typo3 depedency

I looked at the various fails and the test code, nothing obvious at the moment. Would have to put a backtrace right before the failure and dump it to file.
My advice, crank up your memory_limit to 1024M in your php.ini
Then put in a backtrace like this somewhere right in the failed line of code core/lib/Drupal/Core/StreamWrapper/LocalStream.php on line 211

if ($fp = fopen('backtrace.log', 'w'))
{
   $bt = debug_backtrace();
   fwrite($fp, 'debug='.print_r($bt, true));
   fclose($fp);
}

the backtrace.log file should go into the root of the D8 folder.

***BEGIN NEW NOTE***
Ah, I see from the issue description, looks like a bit of followup needed upstream still, explains most of the issues. Looks like the plan is almost complete.
***END NEW NOTE***

Keep up the great work! Almost there.

@joseph.olstad thanks - re #65 please note a separate issue to upgrade phar-stream-wrapper exists already and is RTBC, #3088853: Require typo3/phar-stream-wrapper ^3.1.3 and pear/archive_tar ^1.4.8 in order to support PHP 7.4.

@alexpott re #31: it looks like some PHP 7.4 compatibility tweaks are backported already to v.1.x, https://github.com/bovigo/vfsStream/commit/b4dd94960a680d02dd78fb4c282fe...

Let's see this

Reroll after commit of #3088853: Require typo3/phar-stream-wrapper ^3.1.3 and pear/archive_tar ^1.4.8 in order to support PHP 7.4

@alexpott yep :(

Also, it looks like Composer master is still not full PHP 7.4 compliant, see the failing test

PHPUnit 7.5.16 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\Composer\Plugin\Scaffold\Functional\ComposerHookTest
E                                                                   1 / 1 (100%)

Time: 663 ms, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\Composer\Plugin\Scaffold\Functional\ComposerHookTest::testComposerHooks
RuntimeException: Exit code: 1

Loading composer repositories with package information

                                                       
  [ErrorException]                                     
  Trying to access array offset on value of type null  
                                                       

install [--prefer-source] [--prefer-dist] [--dry-run] [--dev] [--no-dev] [--no-custom-installers] [--no-autoloader] [--no-scripts] [--no-progress] [--no-suggest] [-v|vv|vvv|--verbose] [-o|--optimize-autoloader] [-a|--classmap-authoritative] [--apcu-autoloader] [--ignore-platform-reqs] [--] []...





/var/www/html/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/ExecTrait.php:31
/var/www/html/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Functional/ComposerHookTest.php:89

ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

There's a waiting PR https://github.com/composer/composer/pull/8386, but I don't know if this is going to fix it

Adding a fix for ArgumentValidatorTermTest. However I think this is not correct - the validator property should be initialised beforehand. Added a @todo for a better solution.

The views related failures (see the @todos) require someone knowing the ins and outs of the module...

Sorry ignore the patch in #73

Upping doctrine/annotations to at least 1.7 seems to solve the annotation test failures. If this is OK then we should spin off an issue to update it in D8.8.x

Unfortunately, doctrine/annotation 1.7.0 requires PHP ^7.1 :(

@alexpott, nice work, only 2 fails! I believe you forgot to create an interdiff, however no worries, just easier to follow with an interdiff but not necessary, it is getting closer!

something weird with the test results, I don'T see the failures, the console log is all green but there is 2 fails flagged on the results page.

#86 yes I noticed that too - looks like the testing infrastructure is not reporting all the fails. If you click on 'View results on dispatcher' link in the rsults page, though, the test failures are there.

#87 aha so in this case the failure is not related to composer as 'the library installed in vendor', but to composer as 'the instance used by the testbot to build the codebase'

Can't remember them all, so I reverted #90.1, 4, 5, 6, 7, 8, 9, 10, 11 and 13 in #3065023-90: Ignore, testing issue, and will report back on the basis of the results there.

#90.1 It probably already is an Attributes object in some cases?

See https://3v4l.org/B4ZhS, that apparently was deprecated in PHP 7.4 (only in beta1+ in fact..)

#90.1: yes, sometimes $variables['attributes'] is an object, and array functions do not operate on ArrayObjects (see #3087602-8: DeprecatedArray implements \ArrayAccess, traversals not possible and 9. The current fix is not secure, either. I would suggest to do this, instead

index d2d54979b7..f1bfe697c4 100644
--- a/core/includes/theme.inc
+++ b/core/includes/theme.inc
@@ -817,9 +817,9 @@ function template_preprocess_image(&$variables) {
 
   foreach (['width', 'height', 'alt', 'title', 'sizes'] as $key) {
     if (isset($variables[$key])) {
-      // If the property has already been defined in the attributes,
-      // do not override, including NULL.
-      if (array_key_exists($key, $variables['attributes'])) {
+      // If the property has already been defined in the attributes, do not
+      // override, including NULL. Attributes can be an array or an object.
+      if ((is_array($variables['attributes']) && array_key_exists($key, $variables['attributes'])) || (is_object($variables['attributes']) && property_exists($variables['attributes'], $key))) {
         continue;
       }
       $variables['attributes'][$key] = $variables[$key];

#90.8: this is basically breaking ALL the Functional and FunctionalJavascript tests with something like this:

7) Drupal\Tests\file\Functional\FileFieldAnonymousSubmissionTest::testAnonymousNodeWithFile
Trying to access array offset on value of type int

/home/travis/drupal8/vendor/symfony/validator/Util/PropertyPath.php:40
/home/travis/drupal8/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:130
/home/travis/drupal8/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:159
/home/travis/drupal8/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:100
/home/travis/drupal8/core/lib/Drupal/Core/TypedData/Validation/RecursiveValidator.php:90
/home/travis/drupal8/core/lib/Drupal/Core/TypedData/TypedData.php:132
/home/travis/drupal8/core/modules/user/user.module:291
/home/travis/drupal8/core/lib/Drupal/Core/Installer/Form/SiteConfigureForm.php:267
/home/travis/drupal8/core/lib/Drupal/Core/Form/FormValidator.php:82
/home/travis/drupal8/core/lib/Drupal/Core/Form/FormValidator.php:275
/home/travis/drupal8/core/lib/Drupal/Core/Form/FormValidator.php:118
/home/travis/drupal8/core/lib/Drupal/Core/Form/FormBuilder.php:577
/home/travis/drupal8/core/lib/Drupal/Core/Form/FormBuilder.php:487
/home/travis/drupal8/core/includes/install.core.inc:971
/home/travis/drupal8/core/includes/install.core.inc:625
/home/travis/drupal8/core/includes/install.core.inc:578
/home/travis/drupal8/core/includes/install.core.inc:117
/home/travis/drupal8/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:296
/home/travis/drupal8/core/tests/Drupal/Tests/BrowserTestBase.php:569
/home/travis/drupal8/core/tests/Drupal/Tests/BrowserTestBase.php:402
/home/travis/drupal8/core/modules/file/tests/src/Functional/FileFieldTestBase.php:48
/home/travis/drupal8/core/modules/file/tests/src/Functional/FileFieldAnonymousSubmissionTest.php:20

More to come...

What about adding

if (!$variables['attributes'] instanceof Attribute) {
  if (!is_array($variables['attributes'])) {
    $variables['attributes'] = [$variables['attributes']];
  }
  $variables['attributes'] = new Attribute($variables['attributes']);
}

No more guessing whether it's array or object.

Full details in #3065023-92: Ignore, testing issue.

Maybe some of the other fixes were transient and are no longer needed.
I am working on an updated patch with findings so far, will test there and then post here once green.

#90.4: this and several other migrate_drupal_ui tests

PHPUnit 7.5.16 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\migrate_drupal_ui\Functional\d6\MultilingualReviewPageTest
E                                                                   1 / 1 (100%)

Time: 55 seconds, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\migrate_drupal_ui\Functional\d6\MultilingualReviewPageTest::testMigrateUpgradeReviewPage
Trying to access array offset on value of type null

/var/www/html/core/lib/Drupal/Core/Database/Install/Tasks.php:247
/var/www/html/core/lib/Drupal/Core/Database/Driver/mysql/Install/Tasks.php:146
/var/www/html/core/modules/migrate_drupal_ui/tests/src/Functional/MultilingualReviewPageTestBase.php:108
/var/www/html/core/modules/migrate_drupal_ui/tests/src/Functional/MultilingualReviewPageTestBase.php:51

ERRORS!
Tests: 1, Assertions: 4, Errors: 1.

#90.5:

PHPUnit 7.5.16 by Sebastian Bergmann and contributors.

Testing Drupal\KernelTests\Core\Field\FieldMissingTypeTest
EE                                                                  2 / 2 (100%)

Time: 3.51 seconds, Memory: 4.00 MB

There were 2 errors:

1) Drupal\KernelTests\Core\Field\FieldMissingTypeTest::testFieldStorageMissingType
Trying to access array offset on value of type null

/var/www/html/core/lib/Drupal/Core/Field/FieldTypePluginManager.php:172
/var/www/html/core/modules/field/src/FieldStorageConfigStorage.php:168
/var/www/html/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php:193
/var/www/html/core/lib/Drupal/Core/Entity/EntityStorageBase.php:300
/var/www/html/core/lib/Drupal/Core/Entity/EntityBase.php:542
/var/www/html/core/modules/field/field.module:182
/var/www/html/core/lib/Drupal/Core/Extension/ModuleHandler.php:392
/var/www/html/core/lib/Drupal/Core/Entity/EntityFieldManager.php:572
/var/www/html/core/lib/Drupal/Core/Entity/EntityFieldManager.php:448
/var/www/html/core/modules/field/src/Entity/FieldConfig.php:296
/var/www/html/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php:1241
/var/www/html/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php:524
/var/www/html/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php:449
/var/www/html/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php:415
/var/www/html/core/lib/Drupal/Core/Entity/EntityStorageBase.php:300
/var/www/html/core/lib/Drupal/Core/Entity/EntityStorageBase.php:250
/var/www/html/core/lib/Drupal/Core/Entity/EntityBase.php:532
/var/www/html/core/tests/Drupal/KernelTests/Core/Field/FieldMissingTypeTest.php:74

#90.7:

PHPUnit 7.5.16 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\system\Functional\Batch\ProcessingTest
.....E..                                                            8 / 8 (100%)

Time: 1.7 minutes, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\system\Functional\Batch\ProcessingTest::testBatchFormProgrammatic
Exception: Notice: Trying to access array offset on value of type null
Drupal\Core\Form\FormSubmitter->doSubmitForm()() (Line: 65)


/var/www/html/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:51
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:203
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:156
/var/www/html/vendor/guzzlehttp/promises/src/TaskQueue.php:47
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:246
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:223
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:267
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:225
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:62
/var/www/html/vendor/guzzlehttp/guzzle/src/Client.php:131
/var/www/html/vendor/fabpot/goutte/Goutte/Client.php:180
/var/www/html/vendor/symfony/browser-kit/Client.php:318
/var/www/html/vendor/behat/mink-browserkit-driver/src/BrowserKitDriver.php:144
/var/www/html/vendor/behat/mink/src/Session.php:148
/var/www/html/core/tests/Drupal/Tests/UiHelperTrait.php:326
/var/www/html/core/modules/system/tests/src/Functional/Batch/ProcessingTest.php:145

ERRORS!
Tests: 8, Assertions: 70, Errors: 1.

#90.11: this and other locale tests

PHPUnit 7.5.16 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\locale\Functional\LocaleImportFunctionalTest
EEEEEE                                                              6 / 6 (100%)

Time: 1.12 minutes, Memory: 4.00 MB

There were 6 errors:

1) Drupal\Tests\locale\Functional\LocaleImportFunctionalTest::testStandalonePoFile
Exception: Notice: Trying to access array offset on value of type int
locale_translate_batch_finished()() (Line: 388)


/var/www/html/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:51
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:203
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:156
/var/www/html/vendor/guzzlehttp/promises/src/TaskQueue.php:47
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:246
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:223
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:267
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:225
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:62
/var/www/html/vendor/guzzlehttp/guzzle/src/Client.php:131
/var/www/html/vendor/fabpot/goutte/Goutte/Client.php:180
/var/www/html/vendor/symfony/browser-kit/Client.php:318
/var/www/html/vendor/behat/mink-browserkit-driver/src/BrowserKitDriver.php:144
/var/www/html/vendor/behat/mink/src/Session.php:148
/var/www/html/core/tests/Drupal/Tests/UiHelperTrait.php:326
/var/www/html/core/tests/Drupal/Tests/UiHelperTrait.php:532
/var/www/html/core/tests/Drupal/Tests/UiHelperTrait.php:333
/var/www/html/core/tests/Drupal/Tests/UiHelperTrait.php:532
/var/www/html/core/tests/Drupal/Tests/UiHelperTrait.php:106
/var/www/html/core/tests/Drupal/Tests/UiHelperTrait.php:209
/var/www/html/core/modules/locale/tests/src/Functional/LocaleImportFunctionalTest.php:384
/var/www/html/core/modules/locale/tests/src/Functional/LocaleImportFunctionalTest.php:71

#96

if (!is_array($variables['attributes'])) {
    $variables['attributes'] = [$variables['attributes']];
  }

can $variables['attributes'] be a scalar here?

#101 ok will put in an incoming patch in the testing issue

So here's an updated patch with more todos referrring to latest comments.

EDIT - ouch, the todos are in the interdiff but not in the patch... will correct in next one.

#90.2 if $css_asset is empty, which is the case in some render tests, this fails. Throwing the exception because this is what happens a few lines below if the asset type is not recognised. Yes, it should be split in a separate issue. Added a @todo.

#90.3 I went for an inline check that the version is a string. Casting generally to string before entering in the checks may be fragile: a version number could be interpreted as a float and we never know what the casted string may become (think 1.1 and 1.10, same 'number' but totally different 'version').

Added todos that went missing in #103.

Sorry I lost some bits in #103 and #104. Will fix now.

Re-added the coalesce operators in core/modules/field/src/Plugin/migrate/source/d7/FieldInstance.php and core/modules/taxonomy/src/Plugin/migrate/source/d6/TermLocalizedTranslation.php, and re #105 reverted to the hack in #89, adding a @todo to signal the need of a separate issue.