PHP-CS-Fixer icon indicating copy to clipboard operation
PHP-CS-Fixer copied to clipboard

[Proposal] Array Items on Separate Lines Fixer

Open vinkla opened this issue 9 years ago • 30 comments

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?

vinkla avatar May 26 '15 09:05 vinkla

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 avatar May 26 '15 09:05 GrahamCampbell

@GrahamCampbell That is true. Maybe it should only apply to variables then?

vinkla avatar May 26 '15 09:05 vinkla

+1 for variables

tlenex avatar Jul 25 '15 13:07 tlenex

I'd love to see this in but as mentioned, only for properties and variables. Rest I prefer to do by hand

Anahkiasen avatar Jul 25 '15 13:07 Anahkiasen

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.

TomasVotruba avatar Jul 07 '17 11:07 TomasVotruba

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

TomasVotruba avatar Aug 25 '17 08:08 TomasVotruba

feel free to send a PR

keradus avatar Aug 25 '17 09:08 keradus

Not in my prirority list Now.

TomasVotruba avatar Aug 25 '17 14:08 TomasVotruba

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.

soullivaneuh avatar Oct 21 '17 22:10 soullivaneuh

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.

ikari7789 avatar Nov 21 '18 04:11 ikari7789

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.

taylankasap avatar Feb 11 '20 14:02 taylankasap

has anybody found a solution for this?

goyote avatar Feb 22 '21 05:02 goyote

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.

madebycaliper avatar Sep 11 '21 17:09 madebycaliper

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.

julienfalque avatar Sep 12 '21 10:09 julienfalque

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

charlesdeb avatar Dec 20 '21 17:12 charlesdeb

+1

Haythamasalama avatar Mar 03 '23 18:03 Haythamasalama

Hi folks, you can do better than bumping the topic - helping to have this feature by raising PR with its implementation.

kubawerlos avatar Mar 03 '23 18:03 kubawerlos

  1. a general rule that chops down long lines when possible (parameters list, array items, ...)
  2. 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.

  1. 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,
];
  1. 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 avatar Apr 08 '23 23:04 alexander-zierhut

@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 avatar Apr 08 '23 23:04 Wirone

@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.

alexander-zierhut avatar Apr 09 '23 00:04 alexander-zierhut

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.

github-actions[bot] avatar Jul 08 '23 12:07 github-actions[bot]

I used this: https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/pull/6315

adjenks avatar Aug 17 '23 20:08 adjenks

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.

github-actions[bot] avatar Nov 23 '23 12:11 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.

github-actions[bot] avatar Feb 22 '24 12:02 github-actions[bot]

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 avatar Feb 22 '24 16:02 madebycaliper

@madebycaliper there IS a PR 😉. And the issue wouldn't be closed as I already removed stale label before 😅.

Wirone avatar Feb 22 '24 16:02 Wirone

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

soullivaneuh avatar May 14 '24 10:05 soullivaneuh

Could we take into account the fact that some entries are already on multiple lines, regardless of the length of the array ?

See my original comment

Before :

$tab = [
    'a',
    'b', 'c'
];

After :

$tab = [
    'a',
    'b',
    'c'
];

jlecordier avatar May 24 '24 14:05 jlecordier