In #2776975: March 3, 2017: Convert core to array syntax coding standards for Drupal 8.3.x RC phase array syntax in core was converted to be consistent. It seems like at least default.settings.php was still left into an inconsistent state.

Proposed resolution

Convert all usages of the old array syntax to new. This issue only deals with files outside the core/ directory (per #17).

Remaining tasks

  • Convert all remaining usages of the old array syntax
  • Ensure that there are no old array syntax usages left

Find remaining instances of old array syntax.
grep -ril --exclude-dir=core --exclude-dir=.git '[^_]array(' .

User interface changes


API changes


Data model changes


Members fund testing for the Drupal project. Drupal Association Learn more


lauriii created an issue. See original summary.

alexpott’s picture

default.settings.php is not covered by phpcs automated testing - this is how it slips through the net. Same with Perhaps this issue could work out how to scan those files too.

claudiu.cristea’s picture

Status: Active » Needs review
13.6 KB

Let's see

claudiu.cristea’s picture

+++ b/core/phpcs.xml.dist
@@ -2,6 +2,8 @@
+  <file>../sites/default/default.settings.php</file>

Probably this tells us to move phpcs.xml.dist and phpcs.xml files one level up because now they are covering not only core/

alexpott’s picture

@claudiu.cristea I don't think we should be move phpcs.xml.dist up a level - those are core's standards we don't want to people to have to merge them or do whatever thay have to do to root files when they change.

+++ b/sites/default/default.settings.php
@@ -464,14 +464,14 @@
 if ($settings['hash_salt']) {
-  $prefix = 'drupal.' . hash('sha256', 'drupal.' . $settings['hash_salt']);
-  $apc_loader = new \Symfony\Component\ClassLoader\ApcClassLoader($prefix, $class_loader);
-  unset($prefix);
-  $class_loader->unregister();
-  $apc_loader->register();
-  $class_loader = $apc_loader;
+$prefix = 'drupal.' . hash('sha256', 'drupal.' . $settings['hash_salt']);
+$apc_loader = new \Symfony\Component\ClassLoader\ApcClassLoader($prefix, $class_loader);
+$class_loader = $apc_loader;
+ */

This change isn't right. Should maybe be in a coding standards ignore section. Or perhaps we just shouldn't bother with automated tests on this file. It's really nice to bring in though.

claudiu.cristea’s picture

This change isn't right. Should maybe be in a coding standards ignore section

I agree. I ran phpcbf to see all changes. But I think also that this should be fixed in coder. I reopened #2635594: Line indentation is checked incorrectly for commented code for this reason.

claudiu.cristea’s picture

1023 bytes
13.76 KB

@alexpott what about this?

It seems that coder doesn't check the hash comments. This would also align with the other commented code chunks in the file.

pfrenssen’s picture

Title: Ensure that core array syntax is conistent » Ensure that core array syntax is consistent

I wouldn't prefix it with hashes just for the sake of making the coding standards check pass. This makes it harder to uncomment this section.

This is not something that should be fixed in coder since committing commented out code is bad practice. This is like the only valid case we will ever have for this in core. This warrants an exception in my opinion.

I would surround this section with @codingStandardsIgnoreStart and @codingStandardsIgnoreEnd.

Edit: I just discussed this with @claudiu.cristea on chat, actually the use of hashes to comment out sections of code is used everywhere in settings.php, so any system administrator working on a copy of the file is already uncommenting by removing hashes.

I changed my mind, this is a good solution after all.

pfrenssen’s picture

Status: Needs review » Reviewed & tested by the community
pfrenssen’s picture

Edit: wrong issue, sorry.

lauriii’s picture

Status: Reviewed & tested by the community » Needs work

There are more PHP files under the core/scripts that are not cleaned up yet

Manuel Garcia’s picture

Status: Needs work » Needs review
17.92 KB
4.44 KB

I was just preparing a patch updating default.settings.php comments to short array syntax and found this issue. Adding my changes to previous patch, hope this is the right place to do this =)

joelpittet’s picture

Did you script this? If so could you share that in the IS?

Manuel Garcia’s picture

I didn't @joelpittet, did it by hand...

joelpittet’s picture

Issue summary: View changes

Ok here's maybe a helper script to find others that are outside tests. Feel free to iterate on this:

grep -r --exclude-dir='[Tt]ests' --exclude='*Test.php' '[^_]array(' core

Manuel Garcia’s picture

OK, I'll bite... this is quite painful but writing a script to handle this would be tricky for the closing parenthesis... I guess Rome was not built in one day right? :D

Cleaned up:

  • core/modules/contextual/src/Element/ContextualLinks.php
  • core/modules/field_ui/src/Form/EntityDisplayFormBase.php
  • core/modules/locale/
  • core/modules/migrate/src/Plugin/Migration.php
  • core/modules/node/node.api.php
  • core/modules/node/src/Plugin/Search/NodeSearch.php
  • core/modules/rest/src/Plugin/ResourceInterface.php
  • core/modules/simpletest/simpletest.module
  • core/modules/simpletest/src/BlockCreationTrait.php
  • core/modules/simpletest/src/NodeCreationTrait.php
  • core/modules/simpletest/src/TestBase.php
  • core/modules/simpletest/src/TestDiscovery.php
  • core/modules/simpletest/src/WebTestBase.php
  • core/modules/user/user.module
  • core/modules/views/src/Annotation/ViewsDisplay.php
  • core/modules/views/src/Plugin/Derivative/ViewsLocalTask.php
  • core/modules/views/src/Plugin/views/join/JoinPluginBase.php
  • core/modules/views/views.api.php
  • core/modules/views_ui/
  • core/modules/views_ui/src/Form/Ajax/RearrangeFilter.php

PLENTY left to do, just run the command grep -r --exclude-dir='[Tt]ests' --exclude='*Test.php' '[^_]array(' core > arrays.txt, open up arrays.txt and start working on the files listed.

Note that some are legitimate, so mindless search & replace wont work - for example these three I've encountered so far:

core/modules/hal/src/Normalizer/ContentEntityNormalizer.php:   *   The type array(s) (value of the 'type' attribute of the incoming data).
core/modules/views/src/ViewExecutable.php:   * @var array()
core/modules/views/src/Plugin/views/wizard/WizardPluginBase.php:   * @var array()
alexpott’s picture

Status: Needs review » Needs work

Hmmm... think the direction this patch has taken is getting out-of-scope. Let's wind this back to being about just applying the already agreed coding standards to PHP files outside /core.

The question of coding standards of code inside @code docblocks is another issue entirely.

faline’s picture

I've change the array() -> [] in some more files.
I didn't change on @code docblocks per comment #17

benjifisher’s picture

Status: Needs work » Needs review

@faline, I am guessing that you just forgot to move the issue back to "Needs Review". If you had a reason not to do that, then I apologize.

Is the goal to get 100% of the remaining array()s out of core, or if we get 80% to 90% there, can we commit it and then do the rest in a follow-up? This seems like the sort of patch that will lead to a lot of re-rolls if it stays open too long.

benjifisher’s picture

Issue tags: +Baltimore2017
joelpittet’s picture

Can @benjifisher or @faline move the @code block changes to a follow-up as mentioned in #17?

benjifisher’s picture

Let's see, I think I can do this. The interdiff for this should be the same as the first patch on the follow-up issue!

I am going through the patch and reverting changes to @code blocks. I am leaving in changes to @param lines (and other lines) in doc blocks. I am not sure whether we want to update those (as part of this issue): for now, I am just leaving in such changes from the previous patch.

Then there is this, which I am also leaving in:

--- a/core/modules/views_ui/src/Form/Ajax/RearrangeFilter.php
+++ b/core/modules/views_ui/src/Form/Ajax/RearrangeFilter.php
@@ -77,12 +77,12 @@ public function buildForm(array $form, FormStateInterface $form_state) {
      * Filter groups is an array that contains:
-     * array(
+     * [
      *   'operator' => 'and' || 'or',
-     *   'groups' => array(
+     *   'groups' => [
      *     $group_id => 'and' || 'or',
-     *   ),
-     * );
+     *   ],
+     * ];
     $grouping = count(array_keys($groups['groups'])) > 1;

I am also leaving in this, which may be out of scope:

--- a/core/modules/locale/
+++ b/core/modules/locale/
@@ -205,7 +205,7 @@ function locale_translate_batch_import($file, array $options, &$context) {
           'seek' => 0,
-      // Update the seek and the number of items in the $options array().
+      // Update the seek and the number of items in the $options array.
       $options['seek'] = $context['sandbox']['parse_state']['seek'];
       $options['items'] = $context['sandbox']['parse_state']['chunk_size'];
       $report = Gettext::fileToDatabase($file, $options);

There are also some changes in core/scripts/ that look like unrelated fixes.

Why are we commenting out this section of settings.php?

@@ -462,16 +462,14 @@
  * example, to use Symfony's APC class loader without automatic detection,
  * uncomment the code below.
-if ($settings['hash_salt']) {
-  $prefix = 'drupal.' . hash('sha256', 'drupal.' . $settings['hash_salt']);
-  $apc_loader = new \Symfony\Component\ClassLoader\ApcClassLoader($prefix, $class_loader);
-  unset($prefix);
-  $class_loader->unregister();
-  $apc_loader->register();
-  $class_loader = $apc_loader;
+# if ($settings['hash_salt']) {
+#   $prefix = 'drupal.' . hash('sha256', 'drupal.' . $settings['hash_salt']);
+#   $apc_loader = new \Symfony\Component\ClassLoader\ApcClassLoader($prefix, $class_loader);
+#   unset($prefix);
+#   $class_loader->unregister();
+#   $apc_loader->register();
+#   $class_loader = $apc_loader;
+# }

I am feeling a little bleary-eyed, so these patch files could use another look!

benjifisher’s picture

Issue summary: View changes

I am updating the issue summary to reflect the scope stated in #17: this issue only deals with files outside the core/ directory.

I am also updating the grep command in the issue summary to reflect the scope.

John Cook’s picture

Status: Needs review » Needs work

I've had a check of the patch in comment #22 and it does do what the issue intends to do.

But the patch does touch files inside the core directory. New issues need to be created as necessary (one per module / sub-system) and the changes in this patch need to be moved there. Maybe separate documentation tickets also need to be created where needed.

When running the check before I get 16 occurrences of "array(" and 11 afterwards. These 11 remaining occurrences are all in @code sections of comments.

For reference, the command I used was:

$ grep -rn --exclude-dir='core' --exclude-dir='.git' --exclude-dir='vendor' --exclude="./sites/default/files/*" --exclude="./modules/*" '[^_]array(' .

Running the search over core as well gave 10700(approx) occurrences of "array(" over 180 files spread over about 50 modules and subsystems.

cilefen’s picture

vegantriathlete’s picture

Issue tags: +dcco2017
Mile23’s picture

If you find that you're correcting an array in a file that has @codingStandardsIgnoreStart or @codingStandardsIgnoreFile annotation, and you can remove that annotation, you should.

You would not remove that annotation if it leads to out-of-scope phpcs errors.

I doubt it will be possible, but it's worth mentioning.

vegantriathlete’s picture

Issue tags: -dcco2017

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

gi.ab’s picture

As per comments #17, #23 and #24, I did a new patch that affects only files outside /core directory.

Thank you guys for the useful hints about using the grep command: @joelpittet, @Manuel Garcia, @benjifisher and @John Cook

Status: Needs review » Needs work

The last submitted patch, 30: array-syntax-2868078-30.patch, failed testing. View results

Ivan Berezhnov’s picture

Issue tags: +CSKyiv18
MaskyS’s picture

Status: Needs work » Needs review
Maheshwaran.j’s picture

Assigned: Unassigned » Maheshwaran.j
Maheshwaran.j’s picture

Assigned: Maheshwaran.j » Unassigned

Patch is good to go.

My doubt is when after installed databases creates array, is there any separate issue for that?or do we need to work on this.

$databases['default']['default'] = array ();
$settings['install_profile'] = 'standard';
$config_directories['sync'] = '';
benjifisher’s picture

@Maheshwaran.j, I am not sure what you are asking. The file sites/default/default.settings.php currently has the code

$databases = array();

and the current patch changes that to

$databases = [];

I do not see the line

$databases['default']['default'] = array ();

at all, although I do see

 * @code
 * $databases['default']['default'] = array (
 * // ...
 * );
 * @endcode

which should be handled in the related issue #2874067: Use short array syntax inside @code blocks.

I do not see how the other lines you quote relate to this issue.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Maheshwaran.j’s picture


When we freshly install a new site, settings.php will create the database array, will that change too?
I might not have asked the question properly but I got the answer thanks :)