Allow opting in to multiline arrays explicitly

[dkarlovi]

Since multiline arrays are fully supported, it seems weird to not allow a feature to enable them when printing except by adding a dummy comment in the array.

An attribute like multiline = true seems like it should be enough to check in maybeMultiline? This could then be set when parsing, preserving the intent.

[ondrejmirtes]

You can do this by yourself by extending the Standard printer, overriding the pCommaSeparated method and insert newlines between the items if the items are PhpParser\Node\ArrayItem.

[dkarlovi]

Why would I do this myself if it's already fully supported, the only issue is you can't opt-in sanely, which this issue is trying to remedy.

[ondrejmirtes]

Because it doesn't make sense to add options for everything. People might have different preferences for different code structures (many things can be formatted in multi-line fashion in PHP).

What about a single item array? Would you still want to have it formatted on multiple lines?

There are many unknowns and many people have different preferences.

That's why (IMHO) it's not worth it to burden the maintainer and all the users with additional complexity.

You can implement exactly what you need already today.

[dkarlovi]

People might have different preferences

Which is why it says "opting in" in the title of the issue.

What about a single item array? Would you still want to have it formatted on multiple lines?

Maybe. I should get a chance to do that since it's literally already supported, the hard part is done.

[pmjones]

@dkarlovi pmjones/php-styler may be useful to you here.

[dkarlovi]

@pmjones That looks like an external package so I'm not sure how it can help allowing to opt in into multiline arrays explicitly in this one?

[dkarlovi]

@ondrejmirtes just realized your argument

it doesn't make sense to add options for everything

doesn't make sense here because there's literally the kind attribute for Array_ which decides how the array stmt should get printed (short or long syntax).

This could in theory be an extension to that: long / short, single line / multiline, do a Cartesian product of those.

[lyrixx]

An attribute like multiline = true seems like it should be enough to check in maybeMultiline? This could then be set when parsing, preserving the intent.

Love it, I did that in my project, but in another place for for arguments.

class MyBetterStandardPrinter extends BetterStandardPrinter
{
    #[\Override]
    protected function pMaybeMultiline(array $nodes, bool $trailingComma = false): string {
        if ($this->onMultiline($nodes)) {  // Note: Only theses two lines are differents from the original implementation
            return $this->pCommaSeparatedMultiline($nodes, true) . $this->nl;
        } elseif (!$this->hasNodeWithComments($nodes)) {
            return $this->pCommaSeparated($nodes);
        } else {
            return $this->pCommaSeparatedMultiline($nodes, $trailingComma) . $this->nl;
        }
    }

    private function onMultiline(array $nodes): bool
    {
        foreach ($nodes as $node) {
            if ($node->getAttribute('multiline')) {
                return true;
            }
        }

        return false;
    }
}

An BTW, this was a bit problematic, because Rector\PhpParser\Printer\BetterStandardPrinter is final. https://github.com/nikic/PHP-Parser/blob/14f9c9df7fab294d33cc4eb02207076cf2eb23ed/lib/PhpParser/PrettyPrinter/Standard.php#L1173-L1180

So having an attribute to control the flow here, would be very convenient

[dkarlovi]

@lyrixx what do you think about KIND_MULTILINE=4 and doing a bitwise? This would preserve BC and the API surface would stay exactly the same.

[lyrixx]

Where should I put this? I don't know very well the printer :/

[dkarlovi]

IMO, this should be a feature in PHP-Parser:

1. adding a KIND_MULTILINE=4 here
2. reacting to this attribute in maybeMultiline and forcing it if the kind attribute contains KIND_MULTILINE
3. when parsing array statements, setting it instead of the current kind attribute

This allows opting in to dumping by just setting the attribute yourself if you're building the AST in memory and it also preserves the multiline-ness of the parsed code.

[lyrixx]

I won't work for my use case since it's not about array, but about method/function parameter.

But from userland point of view, how do you configure the kind? is it an attribute?

[dkarlovi]

@lyrixx yes, you pass it like

new Array_($items, ['kind' => Array_::KIND_SHORT])

this is what the parser does when parsing too, but you can construct the same AST manually and get the same result.

[dkarlovi]

@lyrixx see #1073 for a start on this idea.

[dkarlovi]

@lyrixx not sure if you benefit from this, but #1073 works now, I've realized it's actually a bugfix since prior to it the parser just loses multiline information.