Enforce new line for each array item if there's already at least one new line

[VincentLanglet]

ℹ️ Note: related to #1217, but it looks like it should be implemented as configurable strategy.

Rule request

I just encountered the issue when I was surprised that

[$foo,
$bar]

was fixed to

[$foo,
$bar, ]

by the trailing_comma_in_multiline rule.

1. We definitely need a rule to change this code to

[
    $foo,
    $bar,
]

Any idea about the name ?

2. Even without this rule, shouldn't trailing_comma_in_multiline have a special behavior when the ] is on the same line than the last element ? I would expect

[$foo,
$bar]

to be untouched by this rule.

WDYT ?

[julienfalque]

trailing_comma_in_multiline adding a comma here is definitely a bug to me. The purpose of the comma is to reduce noise in diffs when appending a new item is the array. In this snippet, the ] defeats this purpose so the comma is superfluous.

And I'm :+1: to add a rule that enforces newlines around every entry of an array as soon as there is at least one newline.

[kubawerlos]

@VincentLanglet @julienfalque this ☝🏼 and maybe this then?

[VincentLanglet]

The PR will fix the bug indeed.

Then we could add a rule to improve the display of multiline array (one item by line).

[github-actions[bot]]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

I will close it if no further activity occurs within the next 30 days.

[VincentLanglet]

The rule is still not implemented

[Wirone]

@VincentLanglet isn't it the same feature request as in #1217?

[VincentLanglet]

I would say yes and no.

The referenced issues ask to always split multi items array.

Mine ask to fully split multi items arrays IF you started to write it multi line.

[winiarekk]

The new rule should be a part of @PER-CS ruleset as stated in https://www.php-fig.org/per/coding-style/

Array declarations MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first value in the array MUST be on the next line, and there MUST be only one value per line.

[Wirone]

@winiarekk it's only MAY, so enforcing it would be a little bit too opinionated IMHO. Regardless, it's not required to make such decision here, we rather modify rulesets in separate PRs.

[VincentLanglet]

@winiarekk it's only MAY, so enforcing it would be a little bit too opinionated IMHO. Regardless, it's not required to make such decision here, we rather modify rulesets in separate PRs.

It MAY be split across multiple lines, but when it's split, it MUST be one value per line.

That's exactly the feature request.

I want

[$foo, $bar]

to be untouched, but

[$foo,
$bar]

fixed to

[
    $foo,
    $bar,
]

[Wirone]

You're both right, I wasn't focused enough. Tough time in private life, that's why I don't do much here recently 😥. Good to see fresh heads around 👍.

[github-actions[bot]]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

[keithbrink]

Still relevant.

[github-actions[bot]]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

[VincentLanglet]

Still WIp https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/pull/7813

It requires https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/pull/7901 first

[github-actions[bot]]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.