Generic.WhiteSpace.ScopeIndent false positive for arrow functions inside arrays

[BinaryKitten]

Describe the bug
A clear and concise description of what the bug is.

Code sample

<?php

$overrides = [
    'abcd1234' => [ // name
        'title' => function ($product) {
            return trim(substr($product['title'], 0, 100));
        }
    ],
    'abcd1255' => [ //company - Remove '2019/19' from the beginning of their title
        'title' => fn ($product) => trim(preg_replace('/^[0-9\/]{4,7}/', '', $product['title']))
    ],
    'abcd9982' => [ // another company
        'type' => 'Product Delivery', // Product Delivery
    ]
];

To reproduce
Steps to reproduce the behavior:

1. Create a file called test.php with the code sample above...
2. Run phpcs --standard=Generic --sniffs=Generic.WhiteSpace.ScopeIndent test.php
3. See error message displayed

---------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------
 14 | ERROR | [x] Line indented incorrectly; expected at least 8 spaces, found 4
---------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------

Expected behavior
no error - spaces = 4 should be correct?

Versions (please complete the following information):

• OS: Ubunutu 18.04
• PHP: 7.4.16
• PHPCS: 3.6

[jrfnl]

Bug confirmed.

For clarification: line 14 is the line with the ] close bracket for the last array item.

I suspect this has something to do with the arrow function in the array item before it.
Removing the array item with the arrow function removes the error.

[jrfnl]

@BinaryKitten and me spend some time this afternoon looking at this issue.

We used a slightly smaller code sample to reproduce the issue:

$overrides = [
    'abcd1255' => [ //company - Remove '2019/19' from the beginning of their title
        'title' => fn ($product) => trim(preg_replace('/^[0-9\/]{4,7}/', '', $product['title']))
    ],
    'abcd9982' => [ // another company
        'type' => 'Product Delivery', // Product Delivery
    ]
];

The ScopeIndent debug output shows us this:

Start with token 0 on line 1 with indent 0                                                                           
Opening short array bracket found on line 3                                                                          
        => disabling exact indent checking until 69 (T_CLOSE_SHORT_ARRAY) on line 10                                 
Opening short array bracket found on line 4                                                                          
        => continuing to disable exact indent checking until 69 (T_CLOSE_SHORT_ARRAY) on line 10                     
Opening parenthesis found on line 5                                                                                  
        => disabling exact indent checking until 69 (T_CLOSE_SHORT_ARRAY)                                            
Closing parenthesis found on line 5                                                                                  
         * ignoring single-line definition *                                                                         
* ignoring arrow function on line 5 *                                                                                
Opening short array bracket found on line 7                                                                          
        => continuing to disable exact indent checking until 69 (T_CLOSE_SHORT_ARRAY) on line 10                     
Closing short array bracket found on line 9                                                                          
        * first token on line 7 is 50 (T_CONSTANT_ENCAPSED_STRING) *                                                 
        * previous is T_COMMA on line 6 *                                                                            
        * amended first token is 17 (T_CONSTANT_ENCAPSED_STRING) on line 5 *                                         
        => checking indent of 8; main indent set to 8 by token 17 (T_CONSTANT_ENCAPSED_STRING)                       
[Line 9] Line indented incorrectly; expected at least 8 spaces, found 4                                              
        Generic.WhiteSpace.ScopeIndent:1527 replaced token 66 (T_WHITESPACE on line 9) "    ]" => "        ]"        
        => add adjustment of 4 for token 67 (T_CLOSE_SHORT_ARRAY) on line 9                                          
Closing short array bracket found on line 10                                                                         
        * first token on line 3 is 2 (T_VARIABLE) *                                                                  
        => checking indent of 0; main indent set to 0 by token 2 (T_VARIABLE)                                        

Basically, what it comes down to is this:

• The array opener on line 4 sets the indent to +4, but...
• When the scope closer of an arrow function is a T_CLOSE_SHORT_ARRAY token, like it is in this case (on line 6), it is only recognized as a scope closer and not handled anymore for the array closer which it also is.
• This means that the close bracket of the first nested array (line 4-6) is never found, and the opener of the second nested array in line 7, increased the expected indent by another +4, making it 8, instead of the desired 4.

This problem, again, boils down to an issue with the scope setting of arrow functions. Between the two of us, we may have come up with a more structural solution for that, which I will write up in a separate issue, however, @gsherwood would you be able to have a closer look at this issue for an interim solution ?

[gsherwood]

I've taken a look into this and it looks like a problem with findStartOfStatement. It tries to find the start of the statement beginning with the string 'abcd9982' (ignoring commas and double arrows) but it hits the closing short array bracket on the previous line and returns that because it is a scope closer.

It already excludes a number of tokens to stop this happening, and adding the close short array bracket to that list solves the problem with no failed tests, but I'm not convinced that's the right fix. I think it might be better to make an exception for the closer of an array function when these type of closers are found.

Either way, this looks fixable without changing the sniff in any way.

[gsherwood]

That comment wasn't quite right. The problem was with the arrow function end token logic.

In this sample code:

$a = [
    'a' => [
        'a' => fn () => foo()
    ],
    'a' => [
        'a' => 'a',
    ]
];

The end of the arrow function is set to the closing bracket of the short array, which is not right. When arrow functions are nested inside function calls, the code knows to not use the closing parenthesis of the outer function call as the closer, but it wasn't doing the same thing for square brackets. I added in that case and the sniff no longer complains about the code structures in this issue.

[gsherwood]

I've committed a fix for the problem I described above. Would love some more testing, so please let me know if you come across any other code that causes this problem. Thanks a lot for the bug report.