Allow array unpacking in ArrayDeclaration multiline Sniff

[edorian]

See #3557

Description

Squiz.Arrays.ArrayDeclaration.NoKeySpecified doesn't accept the following snippet:

$x = [
  'foo' => 'bar',
  ...$baz,
];

It triggers an error as phpcs doesn't detect that the second entry is array unpacking.

Key specified for array entry; first entry has no key (Squiz.Arrays.ArrayDeclaration.KeySpecified)

The following (single line) array validates:

$x = ['foo' => 'bar',  ...$baz];

Suggested changelog entry

Squiz.Arrays.ArrayDeclaration.KeySpecified: Allow array unpacking (...$array) in multi-line key-value array definitions.

Related issues/external references

Fixes #3557

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] This change is only breaking for integrators, not for external standards or end-users.
• [ ] Documentation improvement

PR checklist

• [x] I have checked there is no other PR open for the same change.
• [x] I have read the Contribution Guidelines.
• [x] I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• [x] I have added tests to cover my changes.
  • I did not. I'd be happy to, if you give me a pointer on how I can do this without breaking earlier PHP versions, given it's new syntax.
• [x] I have verified that the code complies with the projects coding standards.
• [ ] [Required for new sniffs] I have added XML documentation for the sniff.
• [ ] [Required for new files] I have added any new files to the package.xml file.

Notes:

The one test failure for PHP 8.2 seems unrelated as it was already present in https://github.com/squizlabs/PHP_CodeSniffer/commit/2e59060b93b3720ff3aab2b57efda0c3ada069b0

[jrfnl]

I did not. I'd be happy to, if you give me a pointer on how I can do this without breaking earlier PHP versions, given it's new syntax.

@edorian Please do add some new tests. The fact that it is new syntax is irrelevant as the PHPCS tokenizer has backported the T_ELLIPSIS token to PHP 5.4, so the test code should tokenize the same in all PHP versions.

(if that is not the case, we have another issue, but better to know about it than not)

[edorian]

• [x] Test

Thanks @jrfnl

Without my fix:

PHP_CodeSniffer\Standards\Squiz\Tests\Arrays\ArrayDeclarationUnitTest::testSniff
[LINE 475] Expected 0 error(s) in ArrayDeclarationUnitTest.2.inc but found 1 error(s). The error(s) found were:
 -> No key specified for array entry; first entry specifies key (Squiz.Arrays.ArrayDeclaration.NoKeySpecified)

With my fix: :heavy_check_mark:

[edorian]

@jrfnl I've implemented your suggestions, fixed the KeySpecified error and added test cases.

I've done so by basically saying "if it's array unpacking line don't decide if it's a 'list' or a 'dict' shaped array".

Doesn't look very clean looking at the diff but it passes all tests I could come up with :)

[edorian]

Added your suggested tests and tried to align the two test files, but the diff of that grew to big for this PR.

Happy to do that in a follow up when this is merged :)

@jrfnl Thank you for the fast response an the clear directions!

[jrfnl]

@edorian Uh oh.. the tests are failing now. Could it be that the test case comment was only applied to the one test case file, not to all four ?

[edorian]

@jrfnl Sorry my bad. Didn't commit/push all the changes. Tests are passing again (failing test on master excluded) and I've applied your copy suggestion everywhere

[jrfnl]

FYI: this fix is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).