[PHP 7.2] each() function is deprecated

core/lib/Drupal/Component/Diff/Engine/DiffEngine.php:205:        while (list ($junk, $y) = each($matches)) {
core/lib/Drupal/Component/Diff/Engine/DiffEngine.php:213:        while (list ($junk, $y) = each($matches)) {
core/lib/Drupal/Core/Extension/ModuleInstaller.php:97:      while (list($module) = each($module_list)) {
core/lib/Drupal/Core/Extension/ModuleInstaller.php:334:      while (list($module) = each($module_list)) {
core/lib/Drupal/Core/Extension/ThemeInstaller.php:124:      while (list($theme) = each($theme_list)) {
core/modules/book/src/BookOutline.php:42:    // Assigning the array to $flat resets the array pointer for use with each().
core/modules/book/src/BookOutline.php:47:      list($key, $curr) = each($flat);
core/modules/book/src/BookOutline.php:81:    // Assigning the array to $flat resets the array pointer for use with each().
core/modules/book/src/BookOutline.php:84:      list($key,) = each($flat);
core/modules/language/tests/src/Functional/LanguageNegotiationInfoTest.php:164:        while ($equal && list($id) = each($negotiation)) {
core/modules/language/tests/src/Functional/LanguageNegotiationInfoTest.php:165:          list(, $info_id) = each($info['fixed']);
core/modules/rdf/tests/src/Kernel/Field/FieldRdfaTestBase.php:181:      // to an empty array to allow foreach(...) constructions.
core/modules/simpletest/src/AssertContentTrait.php:231:      // to an empty array to allow foreach(...) constructions.
core/modules/system/src/Form/ModulesListForm.php:389:    while (list($module) = each($modules['install'])) {
core/modules/system/src/Tests/Module/InstallUninstallTest.php:75:    while (list($name, $module) = each($all_modules)) {
core/modules/views/tests/src/Functional/Handler/FieldWebTest.php:184:      // to an empty array to allow foreach(...) constructions.
core/modules/views_ui/src/Form/Ajax/ViewsFormBase.php:107:      list($key, $top) = each($view->stack);

Replacing those each() calls with proper foreach() loops is a good novice task :)

This issue is for drupal 7 or 8??

1. +++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
   @@ -93,8 +93,8 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
   -      // the while loop continues.
   -      while (list($module) = each($module_list)) {
   +      // the foreach loop continues.
   +	  foreach ($module_list as $module => $value) {
   

   indentation error, make sure this is on the same level as the comment before.

2. +++ b/core/modules/system/src/Form/ModulesListForm.php
   @@ -386,7 +386,7 @@ protected function buildModuleList(FormStateInterface $form_state) {
        // Add all dependencies to a list.
   -    while (list($module) = each($modules['install'])) {
   +	foreach ($modules['install'] as $module => $value) {
   

   indentation error.

After applying the patch 3 instances are missing:

modules/system/src/Tests/Module/InstallUninstallTest.php
75:    while (list($name, $module) = each($all_modules)) {

modules/language/tests/src/Functional/LanguageNegotiationInfoTest.php
164:        while ($equal && list($id) = each($negotiation)) {
165:          list(, $info_id) = each($info['fixed']);

includes/bootstrap.inc
1024:    while (list($key, $callback) = each($callbacks)) {

Fixed 1 & 2 of #11. Patch applying cleanly in my system.

Thanks, after applying the patch 3 instances are missing:

modules/system/src/Tests/Module/InstallUninstallTest.php
75:    while (list($name, $module) = each($all_modules)) {

modules/language/tests/src/Functional/LanguageNegotiationInfoTest.php
164:        while ($equal && list($id) = each($negotiation)) {
165:          list(, $info_id) = each($info['fixed']);

includes/bootstrap.inc
1024:    while (list($key, $callback) = each($callbacks)) {

Hi All,

I have rerolled the patch #14 with fixing the comment #15.

@arunkumark, Next time onwards kindly add interdiff file also with patch.
Here is the minor change.
foreach ($callbacks as $key => $callbacks) {
should be
foreach ($callbacks as $key => $callback) {

Fixed #17.

Thanks! looks good. checked on local. it's not present in any other PHP file.
$ grep --include=\*.php -rnw 'core/' -e 'each('

The change to DiffEngine.php will not necessarily work correctly, see https://3v4l.org/DZBS0.

This change will happen in contrib so better to profile & file change record with suggestions

Here are the suggested patches and interdiff files.

@drewklein
Do you think you are able to fix the failures?

indeed iterator is slow(

What's the status of this change?

If you do not feel comfortable digging into the code for this, you could at least create a proper issue summary.

As #26 shows foreach is the most performant

Not sure it needs CR besides suggestion to use foreach

Please open a separate issue for the 7.x backport, the 8.x patch should ideally be the last one on this issue so DrupalCI can retest it easily etc.

@catch d7 patch #30 is hidden in #34, so #28 is lat valid patch

Thanks for working on this issue!

+++ b/core/lib/Drupal/Component/Diff/Engine/DiffEngine.php
@@ -27,6 +27,14 @@
+
+/**
+ * Note on Drupal 8 porting.
+ * The following changes have been done:
+ *  Changed all calls of each() to alternatives for php 7.2 compatibility.
+ */
+
+
 class DiffEngine {

I'd add this to the docblock above it rather than adding a separate docblock. After all, it's adding a diff either way.

I think we should also add/enable a coder rule for this to ensure that we don't reintroduce each() use.

Meant "Needs work", but we can also postpone the issue on adding the coder rule if one is not already available in the Drupal ruleset.

Oh, I forgot to mention, I grepped to confirm this is fixing all instances of each() once the patch is applied:

[ibnsina:drupal | Sun 14:24:49] $ grep -r "\beach(" * | grep -v "vendor" | grep -v ".js"
core/lib/Drupal/Component/Diff/Engine/DiffEngine.php: *  Changed all calls of each() to alternatives for php 7.2 compatibility.

We have Generic.PHP.DeprecatedFunctions sniff but that is unfortunately not picking this up on PHP7.2. I think that is because each is a built-in and therefore the way in which it detects deprecated functions does not work for this. We can add each to the list of functions in \Drupal_Sniffs_Functions_DiscouragedFunctionsSniff.

As noted in #42 I think there is a place to make that change in coder that @xjm has requested but, on second thoughts, I'm not sure we should be adding deprecations of inbuilt functions to our coding standards checks. We don't have checks for all the changes in PHP7 from PHP5 - see http://php.net/manual/en/migration70.incompatible.php#migration70.incomp.... I think what we really need is a testbot for PHP7.2 - if we had that we have a gazillion failing tests and this would be one of the causes.

Wrt to the comment format that @xjm asked for in #38. I think what @vaplas was going for was to copy what is in core/lib/Drupal/Core/Archiver/ArchiveTar.php. That said that file has // @codingStandardsIgnoreFile and this does not. There are a couple of other things to add to the list so let's file a followup to address whether a comment needs to be added and just remove the comment altogether. I think there is a argument the comment is unnecessary and we should just be using git for this.

Also as this is producing warnings in PHP7.2 and that will out in November this issue is critical.

Filed a followup for whether we should add a comment - see #2920903: Discuss whether a list of changes for Drupal 8 is needed in Diff class

Tried this patch, works fine :), RTBC

However, while this removes calls to each() in Drupal's codebase, PHP Unit tests still fail on PHP 7.2, apparently because PHP Unit itself has calls to each() in the release we can use, 4.8.36.

See https://github.com/sebastianbergmann/phpunit/blob/4.8/src/Util/Getopt.ph...

I checked the 5.7 branch and it looks like the calls to each() were prefixed with the error suppression operator @, see https://github.com/sebastianbergmann/phpunit/commit/20c70e0b72eb92367d00...

EDIT:

There will be no more releases for the 4.8 series, anyway :(, https://github.com/sebastianbergmann/phpunit/pull/2773

We really should get a php7.2 testbot to verify this, but it looks great. RTBC++

Committed 7206f5d and pushed to 8.5.x. Thanks!

Shoudn't this be pushed to 8.4.x, too? (assuming we want to support PHP 7.2 for it)

Triggered a test of #43 on 8.4.x.

If we have a 7.2 DrupalCI environment passing tests with 8.5.x, then we could backport these patches then, but that seems pretty far off at the moment so not sure about backports individually.

Created #2925449: [PHP 7] Function each() is deprecated since PHP 7.2 [D7] for D7.

Fresh install of Drupal 8.4.4 showing this issue on PHP 7.2.2 resulting in no styling/themes not working.

-- Install Site --
Warning: is_dir(): open_basedir restriction in effect. File(c:\winnt\temp) is not within the allowed path(s): (C:\Windows\Temp\) in Drupal\Component\FileSystem\FileSystem::getOsTemporaryDirectory() (line 37 of core\lib\Drupal\Component\FileSystem\FileSystem.php).
Drupal\Component\FileSystem\FileSystem::getOsTemporaryDirectory() (Line: 595)
system_requirements('install') (Line: 967)
drupal_check_profile('standard') (Line: 1976)
install_check_requirements(Array) (Line: 1028)
install_verify_requirements(Array) (Line: 671)
install_run_task(Array, Array) (Line: 549)
install_run_tasks(Array) (Line: 117)
install_drupal(Object) (Line: 44)

-- Configure Site --
Deprecated function: The each() function is deprecated. This message will be suppressed on further calls in Drupal\Core\Extension\ThemeInstaller->install() (line 124 of core\lib\Drupal\Core\Extension\ThemeInstaller.php).
Drupal\Core\Extension\ThemeInstaller->install(Array, 1) (Line: 164)
Drupal\Core\Extension\ThemeHandler->install(Array) (Line: 1563)
install_profile_themes(Array) (Line: 671)
install_run_task(Array, Array) (Line: 549)
install_run_tasks(Array) (Line: 117)
install_drupal(Object) (Line: 44)

-- Finish --
Error message
Deprecated function: The each() function is deprecated. This message will be suppressed on further calls in Drupal\Core\Extension\ThemeInstaller->install() (line 124 of core\lib\Drupal\Core\Extension\ThemeInstaller.php).
Drupal\Core\Extension\ThemeInstaller->install(Array, 1) (Line: 164)
Drupal\Core\Extension\ThemeHandler->install(Array) (Line: 1563)
install_profile_themes(Array) (Line: 671)
install_run_task(Array, Array) (Line: 549)
install_run_tasks(Array) (Line: 117)
install_drupal(Object) (Line: 44)
Warning: count(): Parameter must be an array or an object that implements Countable in Drupal\Core\Form\FormValidator->doValidateForm() (line 260 of core\lib\Drupal\Core\Form\FormValidator.php).
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object) (Line: 239)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object) (Line: 239)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object, 'install_configure_form') (Line: 119)
Drupal\Core\Form\FormValidator->validateForm('install_configure_form', Array, Object) (Line: 571)
Drupal\Core\Form\FormBuilder->processForm('install_configure_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm('install_configure_form', Object) (Line: 899)
install_get_form('Drupal\Core\Installer\Form\SiteConfigureForm', Array) (Line: 593)
install_run_task(Array, Array) (Line: 549)
install_run_tasks(Array) (Line: 117)
install_drupal(Object) (Line: 44)
Warning: count(): Parameter must be an array or an object that implements Countable in Drupal\Core\Form\FormValidator->doValidateForm() (line 260 of core\lib\Drupal\Core\Form\FormValidator.php).
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object) (Line: 239)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object) (Line: 239)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object, 'install_configure_form') (Line: 119)
Drupal\Core\Form\FormValidator->validateForm('install_configure_form', Array, Object) (Line: 571)
Drupal\Core\Form\FormBuilder->processForm('install_configure_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm('install_configure_form', Object) (Line: 899)
install_get_form('Drupal\Core\Installer\Form\SiteConfigureForm', Array) (Line: 593)
install_run_task(Array, Array) (Line: 549)
install_run_tasks(Array) (Line: 117)
install_drupal(Object) (Line: 44)
Warning: count(): Parameter must be an array or an object that implements Countable in Drupal\Core\Form\FormValidator->doValidateForm() (line 260 of core\lib\Drupal\Core\Form\FormValidator.php).
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object) (Line: 239)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object) (Line: 239)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object) (Line: 239)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object, 'install_configure_form') (Line: 119)
Drupal\Core\Form\FormValidator->validateForm('install_configure_form', Array, Object) (Line: 571)
Drupal\Core\Form\FormBuilder->processForm('install_configure_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm('install_configure_form', Object) (Line: 899)
install_get_form('Drupal\Core\Installer\Form\SiteConfigureForm', Array) (Line: 593)
install_run_task(Array, Array) (Line: 549)
install_run_tasks(Array) (Line: 117)
install_drupal(Object) (Line: 44)
Warning: count(): Parameter must be an array or an object that implements Countable in Drupal\Core\Form\FormValidator->doValidateForm() (line 260 of core\lib\Drupal\Core\Form\FormValidator.php).
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object) (Line: 239)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object) (Line: 239)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object) (Line: 239)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object, 'install_configure_form') (Line: 119)
Drupal\Core\Form\FormValidator->validateForm('install_configure_form', Array, Object) (Line: 571)
Drupal\Core\Form\FormBuilder->processForm('install_configure_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm('install_configure_form', Object) (Line: 899)
install_get_form('Drupal\Core\Installer\Form\SiteConfigureForm', Array) (Line: 593)
install_run_task(Array, Array) (Line: 549)
install_run_tasks(Array) (Line: 117)
install_drupal(Object) (Line: 44)

All - I'm helping with the D7 version of this issue and I think I found a bug with this implementation.

This patch changed the following code in ModuleInstaller from:

      // Add dependencies to the list. The new modules will be processed as
      // the while loop continues.
      while (list($module) = each($module_list)) {
        foreach (array_keys($module_data[$module]->requires) as $dependency) {
          if (!isset($module_data[$dependency])) {
            // The dependency does not exist.
            throw new MissingDependencyException("Unable to install modules: module '$module' is missing its dependency module $dependency.");
          }

          // Skip already installed modules.
          if (!isset($module_list[$dependency]) && !isset($installed_modules[$dependency])) {
            $module_list[$dependency] = $dependency;
          }
        }
      }

to

      // Add dependencies to the list. The new modules will be processed as
      // the foreach loop continues.
      foreach ($module_list as $module => $value) {
        foreach (array_keys($module_data[$module]->requires) as $dependency) {
          if (!isset($module_data[$dependency])) {
            // The dependency does not exist.
            throw new MissingDependencyException("Unable to install modules: module '$module' is missing its dependency module $dependency.");
          }

          // Skip already installed modules.
          if (!isset($module_list[$dependency]) && !isset($installed_modules[$dependency])) {
            $module_list[$dependency] = $dependency;
          }
        }
      }

By changing the outer loop to a foreach, I believe that means any dependencies that are added to the module list are not checked to see if they exist or not, because the foreach will never get to them. Foreach operates on a copy of the original array. Instead I think what's needed is to manually iterate:

      // Add dependencies to the list.
      // The array is iterated over manually (instead of using a foreach) because
      // modules may be added to the list within the loop and we need to process
      // them.
      while ($module = key($module_list)) {
        next($module_list);
        foreach (array_keys($module_data[$module]->requires) as $dependency) {
          if (!isset($module_data[$dependency])) {
            // The dependency does not exist.
            throw new MissingDependencyException("Unable to install modules: module '$module' is missing its dependency module $dependency.");
          }

          // Skip already installed modules.
          if (!isset($module_list[$dependency]) && !isset($installed_modules[$dependency])) {
            $module_list[$dependency] = $dependency;
          }
        }
      }

We tried using a foreach for this in the D7 version of the patch but tests were failing.

@alexpott found that this is not actually a bug due to how ->requires works on modules, but the code should be cleaned up anyway. Created follow up: #2950400: Clean up confusing code in ModuleInstaller that checks for dependencies

When I installing the drupal 8.3.7 on our local machine that has php 7.2, I got this issue on Configuration Site page. After replacing the each loop with foreach loop and install again then i did not get this issue. I have replaced the file in following manner

- while (list($theme) = each($theme_list)) {
+ foreach ($theme_list as $theme => $value) {