Destructuring arrays

[dingo-d]

Bug Description

I am updating my coding standards and I'm testing out some 'modern' PHP code like destrcturing and came upon an odd thing regarding array indentations.

Minimal Code Snippet

<?php
/**
 * Test file
 *
 * @package test
 */

// Destructuring - inline.
[ 'enabled' => $enabled, 'compression' => $compression ] = $options;

$x = [ 'o' => [ [ 1, 2, 3 ], [ 'what' => 'WHAT' ] ] ];

[ 'o' => [ [ $one, $two, $three ], [ 'what' => $what ] ] ] = $x;

// Destructuring - multiline.
[
	'prop1' => $prop1,
	'prop2' => $prop2,
] = $some_var;

$options = [ 'enabled' => true, 'compression' => 'gzip' ];
[ 'enabled' => $enabled, 'compression' => $compression ] = $options;

Error Code

-------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  9 | ERROR | [x] When a multi-item array uses associative keys, each value should start on a new line.
    |       |     (WordPress.Arrays.ArrayDeclarationSpacing.AssociativeArrayFound)
 21 | ERROR | [x] When a multi-item array uses associative keys, each value should start on a new line.
    |       |     (WordPress.Arrays.ArrayDeclarationSpacing.AssociativeArrayFound)
 22 | ERROR | [x] When a multi-item array uses associative keys, each value should start on a new line.
    |       |     (WordPress.Arrays.ArrayDeclarationSpacing.AssociativeArrayFound)
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Notice how code on line 11 didn't trigger an error, probably because the second item in an array wasn't an associative array. I found the code example in: https://blog.frankdejonge.nl/array-destructuring-in-php/ and was wondering how phpcs didn't pick this up (or should have this been picked up)?

I tested this with:

vendor/bin/phpcs wpcs-test.php --standard=WordPress --no-colors -s

And I installed the latest wpcs using composer.

Environment

Question	Answer
PHP version	7.2.14
PHP_CodeSniffer version	3.4.1
WPCS version	2.1.0
WPCS install type	composer local project
IDE (if relevant)	Terminal (not in IDE)

[dingo-d]

Also this could be linked (not 100% sure) to this issue I reported about destructured arrays inside classes: squizlabs/PHP_CodeSniffer#2479

[jrfnl]

@dingo-d I'll try and have a look at this this week, however, what is the actual question here ?

Just based on a quick look at the code, you're using both short list and short array syntax and I get the impression that you expect sniffs which are intended to examine (short) arrays to also examine short lists, but as that is a different type of construct, they should be exempt and/or have their own rules.

[dingo-d]

Oh I thought that same sniff will catch lists and arrays 🙈

I kinda expected that since

[ 'enabled' => $enabled, 'compression' => $compression ] = $options;

triggered an error, that

$x = [ 'o' => [ [ 1, 2, 3 ], [ 'what' => 'WHAT' ] ] ];

would trigger it as well 🤷‍♂️

Looking at it, I now see that it has only one assignment so maybe that's why it didn't trigger it 🤔

[jrfnl]

Short lists should actually be excluded from all array sniffs, but as WP still supported PHP 5.2 this never really was a problem until now. And even now, with the minimum requirement of PHP 5.6, it's not (yet) a problem I expect many people to run into as short lists were only introduced in PHP 7.1.

I wrote a utility function a while back for PHPCompatibility to distinguish between short lists and short arrays and I expect (hope) it will be added to PHPCS as part of the PHPCS refactor for v 3.5.0.
See: https://github.com/squizlabs/PHP_CodeSniffer/blob/f23ba737d2053f7382c292fa810fff7cb03a1857/src/Util/Sniffs/TokenIs.php#L150-L207

And it is my intention that, as soon as PHPCS 3.5.0 is out, WPCS will upgrade as that will allow us to get rid of a lot of the utility functions here.

With that in mind, I hope you'll find it acceptable that this bug (= false positive from array sniffs on short lists) will remain in the code for a few more months while we wait for PHPCS 3.5.

As for the question about why one snippet triggered the error and the other didn't...

Looking at it, I now see that it has only one assignment so maybe that's why it didn't trigger it

You got it, that's the reason.

[jrfnl]

Related to #764

[dingo-d]

Thanks for the clarification.

I don't think that this bug is going to be a problem (I'm not destructuring arrays in that many cases), but I'm looking forward to updates 🙂

You can close this if you want.

[jrfnl]

If you don't mind, I'll leave it open until it's been fixed ;-)

[jrfnl]

PR #1780 should fix this.

I needed the isShortList() utility function for something else as well now, so I've decided to introduce it ahead of PHPCS 3.5.0. anyway.