Problem/Motivation

There are still instances of " array(" in the core that are not detected by the sniff. Many of these are in strings. These instances should be replaced as far as possible by the short array syntax [].

Steps to reproduce

(11.x)$ git grep -r "\barray(" *  | awk -F: '{print $1}' | sort -u | grep -v drupal6 | grep -v drupal7 | grep -v 'Component/Annotation/Doctrine' | nl
     1  core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php
     2  core/lib/Drupal/Core/Template/TwigNodeTrans.php
     3  core/modules/system/tests/src/Functional/Form/FormTest.php
     4  core/tests/Drupal/KernelTests/Core/Plugin/Discovery/DiscoveryTestBase.php
     5  core/tests/Drupal/KernelTests/Core/Site/SettingsRewriteTest.php
     6  core/tests/Drupal/Tests/Component/Utility/VariableTest.php
     7  core/tests/Drupal/Tests/ComposerIntegrationTest.php
     8  core/tests/Drupal/Tests/Core/Test/fixtures/phpunit_error.xml

Proposed resolution

  1. core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php. This is deliberately ignored by phpcs to "simplify future synchronizations". A comment says "98% of this code is a literal copy of Symfony's YamlFileLoader " but comparing the two file this doesn't seem to be true anymore. Since the Symfony version doesn't use use long array syntax, so these are changed.
  2. core/lib/Drupal/Core/Template/TwigNodeTrans.php
  3. core/modules/system/tests/src/Functional/Form/FormTest.php. Not sure if it should be changed.
  4. core/tests/Drupal/KernelTests/Core/Plugin/Discovery/DiscoveryTestBase.php
  5. core/tests/Drupal/KernelTests/Core/Site/SettingsRewriteTest.php
  6. core/tests/Drupal/Tests/Component/Utility/VariableTest.php. Not changed because it is used PHP var_export which still uses long array syntax.
  7. core/tests/Drupal/Tests/ComposerIntegrationTest.php. Like YamlFileLoader this is ignored for the same reason but it is also out of sync.
  8. core/tests/Drupal/Tests/Core/Test/fixtures/phpunit_error.xml

Remaining tasks

Review
Commit

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3501866

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

quietone created an issue. See original summary.

quietone’s picture

Issue summary: View changes
Priority: Normal » Minor
Status: Active » Needs review
oily’s picture

@quietone I was going to run the command in the 'steps to reproduce'. I notice the command starts: (11.x)$ git grep. Is that correct? Or should the 'git' be removed?

Anyway, if someone pulls the issue branch to local then switches between the 11.x branch and this branch and the command returns 0 and 8 lines respectively, will that be sufficient as a review?

oily’s picture

StatusFileSize
new264.64 KB

This is running the git grep command on the current 11.x branch

oily’s picture

StatusFileSize
new112.05 KB

Running the git grep command on the MR. It returns 2 lines instead of 8.

oily’s picture

It seems from the 'proposed resolution' that both these files are expected to be returned by the command. There seem to be 1 or 2 of the other lines that should be returned but that are not. But so long as that is not an issue the issue seems ready. Changing to RTBTC.

oily’s picture

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

I'm not trying to be a pedant, but isn't this issue implementing the short array syntax, not removing it? I'm only asking because I opened this issue to see if there was some change in coding standards that I wasn't aware of.

oily’s picture

Title: Remove remaining instances of short array syntax » Remove remaining instances of the array language construct
Issue summary: View changes

nod_ made their first commit to this issue’s fork.

  • nod_ committed 523cbdf6 on 11.x
    Issue #3501866 by quietone, oily: Remove remaining instances of the...
nod_’s picture

Status: Reviewed & tested by the community » Fixed

Committed 523cbdf and pushed to 11.x. Thanks!

nod_’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.