It seems like this would have to be added manually. ArrayDeclarationSniff in PHPCS has trailing comma checks (NoCommaAfterLast / NoComma) but squizlabs/PHP_CodeSniffer#582 suggests it's not really usable unless we enable all the other Squiz Labs array style checks as well.

This behavior by default is ignored by PHP itself (I think it's a Hack syntax?) but I've read about it and there is one big advantage of allowing a trailing comma in a multiline array;

• It speeds up or enables duplicating or adding more lines in the array easily. So, one doesn't have to care about comma for lines being added (after the last line previously) as once the last line in the array is copied, we focus on pasting (in the next lines) and don't worry on breaking the code (as there is already a comma after each addition), hence reduces the possibility of introducing errors by omitting a comma. For example, consider the array;

$array = [
    'a',
    'a',
    'a',
];

If we want more values of 'a' in the array, we just copy the last line and keep pasting and hitting return.

But for a case like;

$array = [
    'a',
    'a',
    'a'
];

We need to be very careful while adding new entries and not to forget adding a comma and this can actually break code :)

In T222042#5142599, @D3r1ck01 wrote:

We need to be very careful while adding new entries and not to forget adding a comma and this can actually break code :)

That's what linters are for ;)

This is one of these cases where I wonder if the effort is worth it. How often do we run into a situation where we think "dang, I wish a sniff would have reported this before". I can't recall having this problem. In cases where I have to add an array element, but there is no trailing comma, I can clearly see this in my IDE, because the code is literally broken otherwise and the IDE immediately highlights this.

I think I run into a merge-conflict because of a missing comma a single time in like five years.

Let me put it this way: This sniff absolutely must be auto-fixable, in 100% of the cases it reports.

That said, I prefer having these extra commas, and always typically add them to code I write:

$array = [
    'a' => 1,
    'b' => 2,
];

But not when the array is formatted like this:

$array = [ 'a' => 1, 'b' => 2 ];

But not when the array is formatted like this:

$array = [ 'a' => 1, 'b' => 2 ];

You're absolutely right per above, that is not a multiline array, it's uniline. :)

I just found a relevant use-case where I intentionally do not want an array to have a dangling comma, not even when it spans multiple lines:

/**
 * @return Revision[] An array with exactly two revisions, the previous and the current one.
 */
public function getRevisions() {
    return [
        $previousRevision,
        $currentRevision
    ];
}

Or:

public function provideTestCases() {
    return [
        'first test case' => [
            'first argument',
            'second argument'
        ],
        'second test case' => [
            'first argument',
            'second argument'
        ],
    ];
}

/**
 * @dataProvider provideTestCases
 */
public function test( $firstArgument, $secondArgument ) {
}

In both cases the arrays must contain exactly two elements. Not less. Not more. I do not want to make it "easier" for people to add a third array element. Not having the dangling comma marks the end of the array as not being expandable.

I would say about 5 to 10% of the code I write is like that. The problem is: I do not want to mark 5 to 10% of my code with // phpcs:… exceptions just to be allowed to keep the benefit of not having dangling commas in such situations. I will probably disable the sniff entirely. Or in other words: I feel this is an argument to not turn this into a strict rule.

Hm, I think this might be down to personal preference… I know that when I write data providers for tests, I always use a trailing comma in the arguments arrays. Adding another parameter to a test function isn’t unheard of, and even if I think it will always only have the current parameters, I still prefer to have the trailing comma everywhere.

In the getRevisions() example, I’m less sure, but I would probably still include a trailing comma. I don’t think omitting trailing commas should be used to discourage adding a third array element, it’s not a strong enough signal for that anyways (without a // phpcs:… exception comment, nothing tells me if the omission is intentional or not). Instead, the fact that the array is not expandable should be clear from the context.

In T222042#5142935, @thiemowmde wrote:

This is one of these cases where I wonder if the effort is worth it. How often do we run into a situation where we think "dang, I wish a sniff would have reported this before". I can't recall having this problem. In cases where I have to add an array element, but there is no trailing comma, I can clearly see this in my IDE, because the code is literally broken otherwise and the IDE immediately highlights this.

I think I run into a merge-conflict because of a missing comma a single time in like five years.

Let me put it this way: This sniff absolutely must be auto-fixable, in 100% of the cases it reports.

That said, I prefer having these extra commas, and always typically add them to code I write:

$array = [
    'a' => 1,
    'b' => 2,
];

But not when the array is formatted like this:

$array = [ 'a' => 1, 'b' => 2 ];

Can do - if a sniff does indeed auto-fix 100% of the cases it reports, would it be accepted / is this proposal approved?

No, as argued a little lower. There are reasons to not have a dangling comma in multi-line arrays. It should be a human that makes the decisions about this.

In T222042#7822308, @Lucas_Werkmeister_WMDE wrote:

Reopening, since trailing commas keep coming up in places where they’re uncontroversial (e.g. resources.php file or message lists), but get forgotten because there’s no automatic sniff/fix for them. I did an informal poll among Wikibase developers, and it seems like we have consensus to enable such a rule there. I think the implementation of the sniff should live in the MediaWiki codesniffer; it doesn’t have to be included in the default ruleset, but it should be available for individual repositories if the developers there decide on it. Whether we want to enable it on our largest shared code base (MediaWiki core) is a different question, and if that’s more controversial then I’m happy to leave that alone for now.

@DannyS712 do you still have the code that you wrote at the time? The built-in ArrayDeclarationSniff still seems to be as unsuitable for this purpose as it was three years ago (T222042#5142556), as far as I can tell (the issue to make it more configurable remains open with little progress).

I don't recall writing code for this, but I can look around or try from scratch

Also, perhaps instead of just not including it by default, do something like the sorted array rule, and require that a comment be added before the array to indicate that trailing commas should be enforce? (Or do that in addition to having it be available as a rule without requiring comments).

Change 785872 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/tools/codesniffer@master] Introduce TrailingCommaSniff

https://gerrit.wikimedia.org/r/785872

Change 786286 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/tools/codesniffer@master] Move AlphabeticArraySort tests into own directory

https://gerrit.wikimedia.org/r/786286

Change 786286 merged by jenkins-bot:

[mediawiki/tools/codesniffer@master] Move AlphabeticArraySort tests into own directory

https://gerrit.wikimedia.org/r/786286

Change 785872 merged by jenkins-bot:

[mediawiki/tools/codesniffer@master] Introduce TrailingCommaSniff

https://gerrit.wikimedia.org/r/785872

Is there anything left here, other than making the sniff default?

In T222042#8476252, @Tgr wrote:

Is there anything left here, other than making the sniff default?

I don't think we should make it enabled by default, eg per T222042#5155779, unless we first establish as a coding convention that all (single line / multiline / both) arrays should have (or not have) trailing commas. This can be enabled on a per-repo basis with the properties. As such, I propose to close this as resolved

I also made T325532 to enable the sniff in Wikibase – we can remove the explicit phpcs config there again if we decide to enable the sniff by default after all.