Squiz.Arrays.ArrayDeclaration sniff not very configurable (follow up on Squizlabs#582)

[michalbundyra]

Describe the bug

A clear and concise description of what the bug is.

Code sample

class HelloWorld
{
    public static function dataProvider(): \Generator
    {
        yield 'test case' => [[1.1, 2.5, 3.14], static fn (float $item): float => match ($item) {
            1.1, 2.5 => 1.1,
            default => $item
        }, [1.1, 3.14]];
    }
}

Custom ruleset

<?xml version="1.0"?>
<ruleset name="My Custom Standard">
  <description>PSR-2 base with some additions</description>

    <rule ref="PSR2" />

    <rule ref="Squiz.Arrays.ArrayDeclaration">
        <!-- Disable arrow alignment -->
        <exclude name="Squiz.Arrays.ArrayDeclaration.DoubleArrowNotAligned"/>
        <!-- Uses indentation of only single space -->
        <exclude name="Squiz.Arrays.ArrayDeclaration.KeyNotAligned"/>
        <!-- Allow multiple values on a single line -->
        <exclude name="Squiz.Arrays.ArrayDeclaration.SingleLineNotAllowed"/>
        <!-- Disable alignment of braces -->
        <exclude name="Squiz.Arrays.ArrayDeclaration.CloseBraceNotAligned"/>
        <!-- Disable alignment of values with opening brace -->
        <exclude name="Squiz.Arrays.ArrayDeclaration.ValueNotAligned"/>
        <!-- Checked by SlevomatCodingStandard.Arrays.TrailingArrayComma.MissingTrailingComma -->
        <exclude name="Squiz.Arrays.ArrayDeclaration.NoCommaAfterLast"/>
        <exclude name="Squiz.Arrays.ArrayDeclaration.NoComma"/>
    </rule>

    <rule ref="Generic.Arrays.ArrayIndent"/>
</ruleset>

To reproduce

Steps to reproduce the behavior:

1. Create a file called test.php with the code sample above...
2. Run phpcs test.php ...
3. See error message displayed

  7 | ERROR | [x] The first value in a multi-value array must be on a new line (Squiz.Arrays.ArrayDeclaration.ValueNoNewline)
  7 | ERROR | [x] Each value in a multi-line array must be on a new line (Squiz.Arrays.ArrayDeclaration.ValueNoNewline)
 10 | ERROR | [x] Each value in a multi-line array must be on a new line (Squiz.Arrays.ArrayDeclaration.ValueNoNewline)
 10 | ERROR | [x] Closing brace of array declaration must be on a new line (Generic.Arrays.ArrayIndent.CloseBraceNotNewLine)
 10 | ERROR | [x] Closing parenthesis of array declaration must be on a new line (Squiz.Arrays.ArrayDeclaration.CloseBraceNewLine)

Expected behavior

When I run:

phpcbf ./test.php --sniffs=Squiz.Arrays.ArrayDeclaration,Generic.Arrays.ArrayIndent

the code is fixed as below:

class HelloWorld
{
    public static function dataProvider(): \Generator
    {
        yield 'test case' => [
            [1.1, 2.5, 3.14], static
fn (float $item): float => match ($item) {
            1.1, 2.5 => 1.1,
            default => $item
        },
            [1.1, 3.14]
        ];
    }
}

so it looks like it's breaking the closure in the wrong place, and because of that it causes issue with:

Squiz.WhiteSpace.ScopeKeywordSpacing
Generic.WhiteSpace.ScopeIndent

rules.

Versions (please complete the following information)

Operating System	-
PHP version	8.2/8.3
PHP_CodeSniffer version	3.9.0
Standard	see above
Install type	composer

Additional context

Add any other context about the problem here.

Please confirm:

• I have searched the issue list and am not opening a duplicate issue.
• I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
• I have verified the issue still exists in the master branch of PHP_CodeSniffer.

[jrfnl]

Confirmed. Thanks for reporting this @michalbundyra .

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.

With that in mind, I will not be working on this issue.
If someone would submit a PR with a small/simple fix for this issue, I will accept it, but other than that, this sniff is out of bounds for large fixes/rewrites as it is just not worth the time.

In the mean time, rewriting the code to the below will prevent running into this particular problem with the sniff:

class HelloWorld
    public static function dataProvider(): \Generator
    {
        yield 'test case' => [
            [1.1, 2.5, 3.14],
            static fn (float $item): float => match ($item) {
                1.1, 2.5 => 1.1,
                default => $item
            },
            [1.1, 3.14]
        ];
    }
}

[michalbundyra]

@jrfnl thanks for checking this out. I should have mentioned that this is the issue we actually have in doctrine/coding-standard rules:
https://github.com/doctrine/coding-standard/blob/3e88327e4bb74e5538787642a59a45919376e0a9/lib/Doctrine/ruleset.xml#L517-L531

I'll see if I can provide a quick fix. Thanks

[jrfnl]

@michalbundyra I realize it may be a difficult case to advocate in the context of Doctrine, but if you want a solution in the mean time (aside from the fix you are preparing), you could consider using the NormalizedArrays sniffs in combination with one or two of the WordPressCS array sniffs (which are also customizable).

[michalbundyra]

@jrfnl it turns out to be a quick fix: #369

Thanks