WordPress-Coding-Standards: Faulty core fix: Array elements aren't aligned at the =>

Given this example:

<?php

array(
	'aaa' => 'b',
	'c' => 'd',
	'eeeeeee' => 'f',
);

The array elements aren’t being aligned at the =>. This behaviour isn’t explicitly noted in the handbook, just recommended. I can make it more explicit in the Hanbook, thought.

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 1
Comments: 19 (19 by maintainers)

Most upvoted comments

Adding or removing an item from the array then means all of the items in the array will need to get re-aligned, resulting in messy diffs and conflicting patches. It’s also a direct contradiction of the subsequent point in the coding standards, about use of a comma after the last item in an array

The array might re-aligned, depending on whether you’re adding or removing the longest key.

I also find myself less and less concerned about the cleanliness of a diff, vs. the cleanliness of the actual code - I look at diffs and revision history that would be affected by this occasionally, I work with the actual code all day, every day. It seems like a reasonable trade off to make the common case nicer.

pento on Sep 5, 2017

I don’t really understand the alternative of removing part of the Arrays.AssignmentOperatorSpacing sniff—how would it still work, what would it still do?

@JDGrimes Having thought about this some more based on the questions you posed, I think removing part of the Arrays.AssignmentOperatorSpacing sniff will in actual fact be the best option. I will also rename the sniff to something like Arrays.MultipleStatementAlignment (better name suggestions welcome).

In the WIP branch, I was checking both spacing before as well as after the => array assignment operator, similar to what the upstream Squiz.Arrays.ArrayDeclaration sniff does and the - now deprecated - WP version of that once did. For single line arrays, this meant: enforcing exactly one space before and after the double arrow. For multi-line arrays, enforcing alignment before the double arrow and one space after. (with some configuration options, but that’s besides the point)

This is basically already covered by the WhiteSpace.OperatorSpacing except for the alignment before the arrow. So, I think it’s best to narrow the focus of the sniff and only target the multi statement alignment for multi-line arrays. That would still yield an overlapping message when no space at all is found between an array index key and the double arrow, but would eliminate the other duplications.

I will adjust the WIP branch and expect to be able to pull this tomorrow.

jrfnl on Sep 8, 2017

Sometimes when the difference is greater than that aligning just doesn’t make sense (depending on the values being assigned), and can actually decrease readability. I don’t think this was configurable, but I might be remembering wrong.

It actually is configurable (and looks like it always has been) using the $maxPadding property: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Customisable-Sniff-Properties#genericformattingmultiplestatementalignment

Of course, for array alignment, I tend to not worry about how many spaces need to be used, since it is all part of a single statement. Aligning in an array is almost always more readable. Though I will admit that I’m sometimes pragmatic about alignment even then.

I intend to implement a similar $maxPadding property for the WP Array assignment alignment sniff, so for those of us who want a little more pragmatism, it will be configurable.

jrfnl on Sep 7, 2017