PHP_CodeSniffer icon indicating copy to clipboard operation
PHP_CodeSniffer copied to clipboard

Squiz ArrayDeclarationSniff process a single-line array containing a multi-line array as a multiline array

Open AntoineRoue opened this issue 4 months ago • 3 comments

Describe the bug

If I use Squiz.Arrays.ArrayDeclaration.NoCommaAfterLast rule with this kind of array below, then the first array will be treated as a multi-line array, and this rule will expect a comma between arrays end tokens.

Code sample

  1. Test code:
$a = [[
    1,
    2,
]];
  1. With Squiz.Arrays.ArrayDeclaration.NoCommaAfterLast rule, this code is expected:
$a = [[
    1,
    2,
],];

Ruleset

<?xml version="1.0"?>
<ruleset name="My Custom Standard">
  <rule ref="Squiz.Arrays.ArrayDeclaration.NoCommaAfterLast"/>
</ruleset>

To reproduce

Steps to reproduce the behavior:

  1. Create a file called test.php with the test code above.
  2. Run phpcs test.php or phpcbf test.php
  3. See that a comma is expected line 4, between both arrays end tokens. PHPCS output : ERROR | [x] Comma required after last value in array declaration

Expected behavior

The first array should not be detected as multi-line.

Versions (please complete the following information)

Operating System Ubuntu 24 with WSL 2 with Windows 11
PHP version 8.3
PHP_CodeSniffer version 4.0.0
Standard Squiz
Install type Composer (local)

Addtitional information

In the code, the condition deciding if an array is single or multi-line is at the end of ArrayDeclarationSniff::process: https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/ad16dfa2d72c28d04a9551e6367e9060716399cb/src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php#L144

Please confirm

  • [x] I have searched the issue list and am not opening a duplicate issue.
  • [x] I have read the Contribution Guidelines and this is not a support question.
  • [x] I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
  • [x] I have verified the issue still exists in the 4.x branch of PHP_CodeSniffer.

AntoineRoue avatar Sep 18 '25 09:09 AntoineRoue

The first array should not be detected as multi-line.

Why ? It clearly is a multi-line array. I.e. the opener is not on the same line as the closer.

jrfnl avatar Sep 18 '25 12:09 jrfnl

Hmm indeed, it is clearly a multi-line array when thinking like that. I thought that since its element is declared on the same line than the array, it would be considered as a single line array.

For me it makes sense to leave a trailing comma for this $b array, but not for the $a array above.

$b = [
    [
        1,
        2,
    ],
];

To conclude my problem is not about the definition of single or multi-line arrays (should I reword my post?), but about this weird trailing comma in the first code sample.

AntoineRoue avatar Sep 18 '25 12:09 AntoineRoue

To conclude my problem is not about the definition of single or multi-line arrays (should I reword my post?), but about this weird trailing comma in the first code sample.

I would think your problem is the definition of single or multi-line arrays.

The trailing comma is only expected in multi-line arrays, not in single line arrays.

Changing what is considered a single-line vs multi-line array for this sniff is out of the question as it would break expectations.

And to quote myself from a comment on another ticket:

The Squiz.Arrays.ArrayDeclaration sniff is known to be problematic and has been for years.

The problem is largely that as soon as something is excluded from that sniff, the sniff is broken. This is a design flaw in the sniff.

I've been building up a (highly configurable) NormalizedArrays standard in PHPCSExtra to replace it, but that's not complete yet. Would be lovely if I could find some time to finish that at some point.

I might consider making the definition of what would be considered single-line vs multi-line configurable for the NormalizedArrays standard (with the default being the same as the Squiz sniff has it), but making any such changes for the Squiz sniff is not on the table.

jrfnl avatar Sep 18 '25 23:09 jrfnl