PHP-CS-Fixer
PHP-CS-Fixer copied to clipboard
[Proposal] Array Items on Separate Lines Fixer
I often find myself putting array items on separate lines. Specially when there are a lot of items in the arrays. Maybe we could add a fixer for that? Maybe it exists already? Example below:
// Before
protected $dates = ['deleted_at', 'ended_at', 'started_at'];
// After
protected $dates = [
'deleted_at',
'ended_at',
'started_at'
];
What do you think?
Do people always want this everywhere in there code though? This fixer would be very annoying for the following:
$factory->render('foo', ['bar'], false);
as it would end up:
$factory->render('foo', [
'bar'
], false);
@GrahamCampbell That is true. Maybe it should only apply to variables then?
+1 for variables
I'd love to see this in but as mentioned, only for properties and variables. Rest I prefer to do by hand
I'm looking for similar fixer, probably configurable.
// no keys, one line
$elements = [$path, $name];
// keys
$elements = [
'file' => $path,
'label' => $name
];
I'd appreciate starting points on fixer architecture with arrays, so I could make this happen.
If anyone is interested in this fixer, I made similar prototype: https://github.com/Symplify/CodingStandard/blob/master/README.md#indexed-php-arrays-should-have-1-item-per-line
feel free to send a PR
Not in my prirority list Now.
Do people always want this everywhere in there code though?
@GrahamCampbell Well, don't use it. :trollface:
More seriously, we should add a min_items
option, like for this eslint rule: https://eslint.org/docs/rules/array-element-newline
BTW, we may done the same fixer for function signature parameters.
Do people always want this everywhere in there code though?
I think just simply adding it as an option adds no harm. Can always improve it with configurable settings after the fact. Maybe you don't want it everywhere in the code, but maybe some people do. This proposal seems to have been sitting here now for three years without any movement, but I'd love to see something come of this.
Is adding a configuration to WhitespaceAfterCommaInArrayFixer
instead of creating a new fixer good idea? This way two fixers won't conflict. Even the name fits, since newline is also a whitespace.
has anybody found a solution for this?
Would also love a configurable version of this rule. Maybe based on max line length? (I'm not a CS Fixer expert, sorry if its a dumb idea)
// Remains Untouched
$foo = ['bar' => $bar, 'biz' => $biz];
// Long Assoc Arrays are wrapped and stacked
$foo = ['bar' => $bar, 'biz' => $biz, 'anotherSweetKey' => $totallRadVar, 'longEnoughToBreakLineLengthMax' => $something ];
$foo = [
'bar' => $bar,
'biz' => $biz,
'anotherSweetKey' => $totallRadVar,
'longEnoughToBreakLineLengthMax' => $something
];
If anyone knows whether this exists already, I'd love a clue on which rule to use. Thanks.
IMO this should be a combination of two distinct rules:
- a general rule that chops down long lines when possible (parameters list, array items, ...)
- one that forces arrays to have all items on the same line or one item per line (like we already do for function arguments).
Both rules don't exist at the moment.
This would be handy for WP developers since for more than 1 item in an array, the WP PHP coding standard calls for new elements on a new line.
See https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/#indentation
+1
Hi folks, you can do better than bumping the topic - helping to have this feature by raising PR with its implementation.
- a general rule that chops down long lines when possible (parameters list, array items, ...)
- one that forces arrays to have all items on the same line or one item per line (like we already do for function arguments).
After some digging around for examples I came up with two more possible rules, but I am not sure how to properly define them yet.
- Sometimes you want to use a stacked array, but want to have multiple items in the same row for some rows as you might want to group them semantically. This could be a configuration option to i.e. use stacked arrays, but don't push items into the next row if two or more are already in a given row.
$array = [
$variable1,
$r, $g, $b,
$variable2,
];
- Always force values into a stacked array when inside of another array or when are used in the context of a function call as a single parameter.
$array = [
"value1" => $variable1,
"value2" => [
"inner1" => 1,
"inner2" => 2,
],
"value3" => "string3",
];
$obj->doSomething([
"option1" => "value1",
"option2" => "value2",
]);
@alexander-zierhut regarding scenario no. 3: how tool would determine if elements in the same row are there on purpose or because someone did not hit enter? I think it would be impossible to handle it nicely and would only bring maintenance burden.
@Wirone I agree, maybe someone else can come up with a better idea, but I think it's still worth being an option, which is off by default.
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.
I used this: https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/pull/6315
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.
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.
Adding a comment here to prevent this from closing. I think an implementation of this rule would still be very helpful, despite this issue being almost 10 years old without a PR 🤣
@madebycaliper there IS a PR 😉. And the issue wouldn't be closed as I already removed stale label before 😅.
Do people always want this everywhere in there code though? This fixer would be very annoying for the following:
$factory->render('foo', ['bar'], false);
as it would end up:
$factory->render('foo', [ 'bar' ], false);
Since latest PHP versions, we can also multi-line function parameters with a coma (,
) at each line, reducing diff noise and possible conflict.
We MIGHT consider php-cs-fixer managing both, producing something like (for this case):
$factory->render(
'foo',
[
'bar',
],
false,
);
What do you think? :thinking:
Update: We already have an issue for that: https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/issues/3168
Could we take into account the fact that some entries are already on multiple lines, regardless of the length of the array ?
Before :
$tab = [
'a',
'b', 'c'
];
After :
$tab = [
'a',
'b',
'c'
];