Squiz.Arrays.ArrayDeclaration throws NoComma error when array value is a shorthand IF statement

[Aliance]

I'm trying to check this file:

<?php
declare(strict_types=1);

namespace Test;

class SomeClass
{
    protected function transformRow(array $row)
    {
        return [
            'key1' => 'value1',
            'key2' => (int) $row['value2'],
            'status_verify'            => $row['status'] === 'rejected'
                ? self::REJECTED_CODE
                : self::VERIFIED_CODE,
            'user_verification_status' => in_array($row['status'], ['notverified', 'unverified'], true)
                ? self::STATUS_PENDING
                : self::STATUS_VERIFIED,
            'created_at' => strtotime($row['date']),
        ];
    }
}

With this command:

docker run --rm -v $(pwd):/scripts/ texthtml/phpcs:3.3.0 phpcs --standard=/scripts/phpcs.xml -n -p -s /scripts/test.php

And I'm getting these unexpected errors:

13 | ERROR | [x] Each line in an array declaration must end in a
| | comma (Squiz.Arrays.ArrayDeclaration.NoComma)
16 | ERROR | [x] Each line in an array declaration must end in a
| | comma (Squiz.Arrays.ArrayDeclaration.NoComma)

---

Interesting, that if I replace status_verify with key3 f.e., the error on the line 13 will disappear. And if I replace user_verification_status with key4 f.e., the all errors will disappear 🤔

[Aliance]

@gsherwood please pay attention to this bug!

[jrfnl]

@Aliance Already shouting for attention less than 24 hours after reporting something ? You may want to check your attitude.

[gsherwood]

@Aliance I've got the report. I'm probably in a different timezone to you and have been asleep half the time.

[gsherwood]

This can be replicated using this code:

<?php
$foo = [
    '3' => $foo
        ? 1
        : 0,
    '5' => '3',
];

It is caused by the sniff not understanding how to skip shorthand IF statements being used as array values. If you put parenthesis around it, it does skip it. But without, it will need to try and determine the end of the statement itself.

So this, for example, doesn't generate that error:

<?php
$foo = [
    '3' => ($foo
        ? 1
        : 0),
    '5' => '3',
];

[gsherwood]

Forgot to address the fact that the error is muted when changing the keys:

This is just due to the sniff bailing out early when it finds the double arrow alignment error. If you change the keys and align the double arrows, you'll get the errors back:

<?php
declare(strict_types=1);

namespace Test;

class SomeClass
{
    protected function transformRow(array $row)
    {
        return [
            'key1' => 'value1',
            'key2' => (int) $row['value2'],
            'key3' => $row['status'] === 'rejected'
                ? self::REJECTED_CODE
                : self::VERIFIED_CODE,
            'key4' => in_array($row['status'], ['notverified', 'unverified'], true)
                ? self::STATUS_PENDING
                : self::STATUS_VERIFIED,
            'key5' => strtotime($row['date']),
        ];
    }
}

[gsherwood]

The only way to find the end of a shorthand IF is to begin at the very start, so I had to use findEndOfStatement to see where the array values ended. I tried discovering the shorthand IF when I hit the T_INLINE_THEN but that's a bit too late to find the ending because I'm in a slightly different context.

So I ended up changing the way that part of the sniff works to always use findEndOfStatement instead of looping over the value tokens manually and making exceptions for things like closures and multi-line strings.

@Aliance All the tests are passing, but it would be great if you could test out the patch manually before I release it.

[Aliance]

@jrfnl I've just thought that maybe the repo owner does not have the indirect notifications, so I've decided to mention him explicitly. Is it illegal? Where does the 24 hours delay rule is located?

[Aliance]

@gsherwood thanks for the patch!

it would be great if you could test out the patch manually before I release it.

I've tested it, seems like it really works fine!

Are you planning to create a new release?

[gsherwood]

@Aliance Thanks for testing that. This was the last scheduled item for the 3.3.1 release, so a new version should come out this week if nothing major gets reported.