Behavior change: Add trailing commas to array items

[gouch]

I propose a behavior change to array formatting: Prettier should add a trailing comma to the last item in a multi-line array.

Trailing commas are beneficial in that they make editing easier and make diffs smaller. But more importantly, the coding standards for several significant PHP projects (e.g. Drupal, Symfony and WordPress) require/recommend trailing commas.

PSR-2 doesn’t address this topic, so I think changing this behavior to match common practice is consistent with Prettier’s goals.

Current behavior:

$array = [
    'item-in-array-example-1',
    'item-in-array-example-2',
    'item-in-array-example-3'
];

Desired behavior:

$array = [
    'item-in-array-example-1',
    'item-in-array-example-2',
    'item-in-array-example-3',
];

I hope this is easy to implement since the main Prettier project already has a config option for trailing comma.

For reference, these project coding standards all recommend trailing commas:

• Drupal Coding Standards: Arrays
• Symfony Coding Standards: Structure
• WordPress Coding Standards: Indentation

[virgofx]

I think it does trailing commas now by default?

[alexander-akait]

We already support trailing commas, we don't do this by default, because prettier doesn't do this by default

[virgofx]

Just to clarify/confirm --- Prettier supports trailing commas and so does the PHP plugin ... which can be configured by using the trailing commas option? So the above use case should be sufficiently handled for the user ...

[gouch]

@evilebottnawi I think trailing commas is a better default for PHP. Prettier also does 2 tab width by default, but this PHP plugin changes that to 4 spaces, because it's a better match for PHP (PSR-2).

[alexander-akait]

@gouch PSR-1/PSR-2/PSR-12 Do not make trailing comma required, unlike 4 spaces

[gouch]

@evilebottnawi I acknowledge that; PSR-2 is silent about trailing commas. But I think it's valuable to consider common practice and the coding standards of significant PHP projects. PHP is a different language after all. That's why this plugin and all its extra code exists.

AFAIK This won't require writing new code that strays from main Prettier (and has to be maintained). Trailing commas are a built-in ability that will surely be maintained by the main Prettier project. The default for JavaScript happens to be no trailing commas (because of bugs in early versions of IE), but, for PHP, trailing commas make a better setting.

[czosel]

Since PSR-2 doesn't say anything about trailing commas, I agree that we should opt for the style that is most popular within the PHP community. What Prettier for JS is doing, doesn't really matter in that regard.
Last time we were discussing brace style I went through the most popular PHP projects on GitHub to make a comparison, maybe this would be a good idea in this case as well?

[alexander-akait]

Anyway, trailing comma is not high priority, we have a lot of ugly code in current version, we should release stable version before add/change options

[virgofx]

@evilebottnawi Can you comment in regards to my question above? I believe that trailing commas can be handled now correct? Just enable the option?

OR is the option not passed through in this plugin?

I agree, we don't need to modify prettier defaults; however, we should support the few options they have (5-10 .. not many) One of which is trailing commas.

[alexander-akait]

@virgofx we already support trailing commas using https://prettier.io/docs/en/options.html#trailing-commas (you should use all value)

[virgofx]

Again ... so this should be a non-issue correct? User can enable proper option for trailing-commas if they want enabled?

[alexander-akait]

@virgofx I can't understand what you mean, if you want trailing commas you should enable this using option, we can't change this behavior now. First we should release stable version, then discussion.

[gouch]

Thanks for clarifying that the PHP plugin supports prettierrc files. For anyone following this, you can target PHP-only in the .prettierrc config (so it doesn't affect your JS):

{
  "overrides": [
    {
      "files": ["*.php"],
      "options": {
        "trailingComma": "all"
      }
    }
  ]
}

(I stand by my argument that this should be the default setting for the PHP plugin.)

[virgofx]

There is no behavior to change. I was just trying to clarify that there is nothing to support as it's already supported. The only decision would be to change the behavior of default options; however, I don't believe that's possible @gouch since we should respect the main prettierrc options. So ... this can be closed as it's already supported.

[czosel]

We are able to specify defaults which are specific to the PHP plugin; we're currently using that for the tab width (see here). If the PHP community leans towards having trailing commas, I agree that our defaults should match community conventions.

[glen-84]

I'm not sure if it makes sense to change the default – what would you change it to? Wouldn't that require knowledge of the PHP version in use?

Single quotes are also far more common than double quotes – are you suggesting that this is changed as well?

Edit: See also prettier/prettier#68 and prettier/prettier#4102.

[czosel]

Over at #1274 we discussed adding a phpVersion option and simplifying the trailingCommaPHP option to only support none and all. Do you think changing the default to all and adding commas whereever the configured php version allows it would reflect current community best practices?

[glen-84]

I don't use trailing commas myself, but as for the community, I guess you could either scan thousands of PHP repositories on GitHub for statistics, or simply wait for some more reactions to this issue.

[czosel]

Similar to how we did before, here's an overview over community practices:

Community style guides:

• Trailing commas in multiline arrays: Wordpress, Laravel, Symfony

Projects:

• Trailing commas in multiline arrays: Phabricator, Faker, composer, PHPMailer, Yii, Matomo, PHPUnit, cphalcon, CakePHP, grav, Carbon, Cachet
• no trailing commas: CodeIgniter, Guzzle, mobiledetect

I didn't find any information on trailing comma usage in use statements or function calls.

[czosel]

We just posted a little RFC-style issue that proposes a way to move forward with this over at #1280. Your feedback would be appreciated!

[joemaller]

I like the idea of following community best practices. And how often does WordPress align with Symfony and Laravel? That agreement should count for a lot.

These little idiosyncrasies help me change contexts faster when jumping between languages. I've already got "trailingCommaPHP": "all" in my prettier PHP overrides.

[mblarsen]

The trailingCommaPHP solution seems to be broken with 2.0.2 of Prettier and 0.14 of this plugin:

➜ ./node_modules/.bin/prettier --config ./package.json --write app/**/*.php
[error] Invalid trailingCommaPHP value. Expected true or false, but received "all".

➜ ./node_modules/.bin/prettier --version
2.0.2

Anyone else experiencing this?

UPDATE: I missed that the prop was updated to a boolean. So the error is correct, but the true value doesn't add comma to arrays, which was the old all behaviour.

[alexander-akait]

/cc @czosel let's implement support of string values

[czosel]

@mblarsen Since 0.14.0, trailingCommaPHP expects a Boolean instead of a string (see #1373). We're gonna try implementing additional support for the string values ("none" and "all") though, as suggested by @evilebottnawi.

[czosel]

I'll close this issue since it has landed with 0.14.0.