[Poll] Zend Coding Standard 2: Aligning statements, arrays and comment tags

zend-coding-standard

#1

If you haven’t seen it yet, work is being done on Zend Coding Standard 2. Most rules we have so far are based on the coding style used in the newer zend-expressive* packages. However there is a big question left and we can’t agree on it:

To align or not to align?

Should we align equal statements, arrays and phpdoc comment tags or not? There is no right or wrong here, it’s personal preference, but we do like to know your opinion. Examples are below the poll.

  • Yes, align equal statements, arrays and phpdoc comment tags.
  • No, do not align anything.

0 voters

Arguments I’ve seen so far:

The readability benefits are typically not huge, and enforcing it means a lot of extra changes for what are often trivial additions.

Aligning brings back extra line changes in commits and that’s exactly what the trailing comma prevents.

Readability is king, and git diff , git blame/annotate as well as the github and gitlab UI have flags for ignoring whitespace changes and so do modern IDEs like PhpStorm.

Examples:

Statement not aligned:

$shortVar = (1 + 2);
$veryLongVarName = 'string';

Statement aligned:

$shortVar        = (1 + 2);
$veryLongVarName = 'string';

Array not aligned:

$serializedRequest = [
    'method' => 'POST',
    'request_target' => '/foo/bar?baz=bat',
    'uri' => 'http://example.com/foo/bar?baz=bat',
    'protocol_version' => '1.1',
    'headers' => [
        'Host' => ['example.com'],
        'Accept' => ['application/json'],
        'X-Foo-Bar' => [
            'Baz',
            'Bat',
        ],
    ],
    'body' => '{"test":"value"}',
];

Array aligned:

$serializedRequest = [
    'method'           => 'POST',
    'request_target'   => '/foo/bar?baz=bat',
    'uri'              => 'http://example.com/foo/bar?baz=bat',
    'protocol_version' => '1.1',
    'headers'          => [
        'Host'      => ['example.com'],
        'Accept'    => ['application/json'],
        'X-Foo-Bar' => [
            'Baz',
            'Bat',
        ],
    ],
    'body'             => '{"test":"value"}',
];

Function comment not aligned:

/**
 * @param array $serverParams Server parameters, typically from $_SERVER
 * @param array $uploadedFiles Upload file information, a tree of UploadedFiles
 * @param null|string|UriInterface $uri URI for the request, if any.
 * @param null|string $method HTTP method for the request, if any.
 * @param string|resource|StreamInterface $body Message body, if any.
 * @param array $headers Headers for the message, if any.
 * @param array $cookies Cookies for the message, if any.
 * @param array $queryParams Query params for the message, if any.
 * @param null|array|object $parsedBody The deserialized body parameters, if any.
 * @param string $protocol HTTP protocol version.
 * @throws Exception\InvalidArgumentException for any invalid value.
 */

Function comment aligned:

/**
 * @param array                           $serverParams  Server parameters, typically from $_SERVER
 * @param array                           $uploadedFiles Upload file information, a tree of UploadedFiles
 * @param null|string|UriInterface        $uri           URI for the request, if any.
 * @param null|string                     $method        HTTP method for the request, if any.
 * @param string|resource|StreamInterface $body          Message body, if any.
 * @param array                           $headers       Headers for the message, if any.
 * @param array                           $cookies       Cookies for the message, if any.
 * @param array                           $queryParams   Query params for the message, if any.
 * @param null|array|object               $parsedBody    The deserialized body parameters, if any.
 * @param string                          $protocol      HTTP protocol version.
 * @throws Exception\InvalidArgumentException for any invalid value.
 */

If you have additional thoughts or comments, please let us know.


#2

I used to align but changed because some long variable names and array configs can make the even alignment hard to read. It is easier to read without alignment in my opinion.


#3

As somebody that reads a dew thousands LOC a day, alignment is welcome and useful: code that is not aligned in logical blocks takes a lot of time to read and follow.

Logical assignment grouping is also unclear when code is nor aligned, since a non-assignment operation may sneak in the block of non-aligned statements, making things even harder to scan.

Overall, given the amount of time that I spend reading vs writing code, any improvement that makes it easier to group together declarations and operations is welcome.


#4

My eyes bleed when I see non-aligned code. Having the ability to visually group similar variable assignments and array properties is more important than not having extra lines in the commit.

As you can tell from the example with the aligned array above, there is a slight drawback, related to multi-level nested arrays. Sometimes the deepest level could have less indentation than a higher level element. However, such arrays are not so likely to exist, as arrays are mostly used in module configuration, which now lives inside the almost flat ConfigProvider. What I’m trying to say is that we can disregard this downside and take advantage of the other stuff the alignment provides.

That’s why I strongly vote to align!


#5

When it comes to writing code I’ve better things to do than aligning all code. IMHO it’s a waste of time for a very small benefit. The key question for me is: are there tools that can do this automatically? Can this be automated with cs-fix via the coding-standard-packge? If not then I’d argue that we’re wasting valuable time of the contributors. And honestly I don’t find the aligned Arrays more readable. Aligning phpdoc and variable assignments may improve readability a bit. My vote for no, as long as this can’t be automated.


#6

@tux-rampage
PhpStorm provides this in their formatting tool (code style).
PHP-CS has it already - you can check it here: https://github.com/FriendsOfPHP/PHP-CS-Fixer
(search for binary_operator_spaces)


#7

And also, the linked patch (from the OP) already does the alignment part.


#8

Aligning statements, docblocks and arrays can be done automatically with phpcbf or composer cs-fix in ZF components. However reverting aligned docblocks and arrays cannot be done automatically AFAIK.

The whole reason for that PR is to automate checking and fixing code. Only some rules are not automated (yet).


#9

Good to know, then forget about my concerns. Since this can be automated, I don’t see a reason why not.


#10

If we can not automatically reverting the alignment, how should this be fixed in all existing components if the poll result is “no”?


#11

Let’s see what the result is first. After that we’ll find a way to enforce it. I don’t think we should decide for a code style because the way we actually wanted has no existing rule.


#12

My question is based on your statements:

However reverting aligned docblocks and arrays cannot be done automatically AFAIK.

The whole reason for that PR is to automate checking and fixing code.

Right, we want automatism and this with a minimum of extra code and custom rules. But you say this cannot be done automatically?!

After that we’ll find a way to enforce it.

So we can only hope that it is easy. :wink:


#13

I don’t think so but I’m not 100% sure. It takes a lot of time tweaking rules like this and I stopped specifically for this rule until the voting is closed.

I’m pretty sure that you can call 0 new classes to enforce about 120 rules “a minimum of extra code and custom rules” :stuck_out_tongue: