fix: preserve multiline style of the modified returned array

[artengin]

refs: #71 (comment)

[coveralls]

Coverage Report for CI Build 34157484442

Coverage increased (+0.5%) to 98.929%

Details

• Coverage increased (+0.5%) from the base build.
• Patch coverage: 25 of 25 lines across 2 files are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	467
Covered Lines:	462
Line Coverage:	98.93%
Coverage Strength:	11.39 hits per line

---

💛 - Coveralls

[chatgpt-codex-connector]

💡 Codex Review

larabuilder/src/Visitors/MethodVisitors/AddReturnedArrayItem.php

Lines 52 to 56 in f4b5384

	foreach ($returnNode->expr->items as $item) {
	if ($item instanceof ArrayItem && $this->isSameKey($item->key)) {
	$item->value = $this->value;
	return;

Mark replaced array items as modified

When the requested key already exists, this branch replaces its value without setting the new Modified attribute. Therefore an inline returned array containing more than two items never enters the new pArray() formatting path and remains inline, contrary to the documented rule for modified arrays; mark the containing item or array before returning.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Preserve long array syntax when reprinting

When an item is appended to a returned array written with array(...), pArray() now forces the entire expression through this override, which always emits square brackets. Consequently, a formatting-only update unexpectedly converts long array syntax to short syntax; select delimiters based on the array's kind attribute or delegate that part to the parent printer.

Useful? React with 👍 / 👎.

[artengin]

Short array syntax is the standard in Laravel projects, so the linter rewrites array(...) to [...] on any file it touches anyway. Preserving long syntax here would only postpone an edit the linter makes regardless, so not supporting it is a deliberate choice.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ffa6c9921

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Inspect the whole original array for multiline formatting

When a one-item array places its first item on the opening line but its closing bracket on a later line, such as return ['first',\n];, this stops scanning at the first item's start and misses the newline after it. Appending a second item does not satisfy count($nodes) > 2, so the array is marked non-multiline and collapsed onto one line despite the documented rule that an already multiline array stays multiline.

Useful? React with 👍 / 👎.

[vitgrams]

@artengin I noticed this case as well. I don't think it's critical, though. Looks good for me

    protected function getSortableFields(): array
    {
        return ['id',
        ];
    }

VS

    protected function getSortableFields(): array
    {
        return ['id', 'name'];
    }

[vitgrams]

@artengin It might be worth adding some of the test cases below to explicitly confirm/document the existing behavior. Please take a look.

I added these locally, and everything worked as expected for me.

1. Comments associated with array elements are preserved

Before:

protected function getValidationRules(): array
{
    return [
        // profile
        'name' => 'required|string',
        'email' => 'required|email',

        // address
        'city' => 'nullable|string'
    ];
}

After:

protected function getValidationRules(): array
{
    return [
        // profile
        'name' => 'required|string',
        'email' => 'required|email',
        // address
        'city' => 'nullable|string',
        'zip' => 'nullable|string',
    ];
}

2. Nested arrays preserve their original formatting

Before:

protected function getUserData(): array
{
    return [
        'name' => 'John',
        'settings' => [
            'locale' => 'en',
            'timezone' => 'UTC',
        ],
    ];
}

After:

protected function getUserData(): array
{
    return [
        'name' => 'John',
        'settings' => [
            'locale' => 'en',
            'timezone' => 'UTC',
        ],
        'roles' => ['admin', 'editor'],
    ];
}

3. Empty multiline array → append

This is the only case I found where the wasMultiline fallback branch is triggered (findRight(']')), preserving the original multiline array syntax even when adding the first element.

Before:

protected function getExtraFilters(): array
{
    return [
    ];
}

After:

protected function getExtraFilters(): array
{
    return [
        'is_hidden',
    ];
}

[vitgrams]

@artengin The solution in its current form works well for me. I have just one suggestion:

Perhaps we could replace Multiline with OriginNode, which is supported by the parent printer out of the box. Please take a look at this alternative approach and let me know what you think. The implementation could then look something like this:

    protected function pArray(
        array $nodes,
        array $origNodes,
        int &$pos,
        int $indentAdjustment,
        string $parentNodeClass,
        string $subNodeName,
        ?int $fixup,
    ): ?string {
        $hasModifiedItem = ($parentNodeClass === Array_::class && $subNodeName === 'items')
            && $this->hasModifiedItem($nodes);

        return ($hasModifiedItem && !$this->isMultiline($origNodes))
            ? null
            : parent::pArray($nodes, $origNodes, $pos, $indentAdjustment, $parentNodeClass, $subNodeName, $fixup);
    }


    protected function pExpr_Array(Array_ $node): string
    {
        if ($this->hasParentOfType($node, PropertyItem::class) || $this->shouldBeMultiline($node)) {
            return '[' . $this->pCommaSeparatedMultiline($node->items, true) . $this->nl . ']';
        }

        return parent::pExpr_Array($node);
    }

    protected function shouldBeMultiline(Array_ $node): bool
    {
        return !empty($this->origTokens)
            && $this->hasModifiedItem($node->items)
            && (count($node->items) > 2 || $this->wasMultiline($node->getAttribute(StatementAttributeEnum::OrigNode->value)));
    }
    
    protected function wasMultiline(?Array_ $origNode): bool
    {
        if (empty($origNode)) {
            return false;
        }

        $startPos = $origNode->getStartTokenPos();
        $endPos = Arr::get($origNode->items, 0)?->getStartTokenPos() ?? $origNode->getEndTokenPos();

        return str_contains($this->origTokens->getTokenCode($startPos, $endPos, 0), "\n");
    }

The main differences are:

• pArray no longer writes to the AST or decides the style — it only selects the path. If the original elements are already separated by line breaks (isMultiline($origNodes) from php-parser), it delegates to parent::pArray, which handles writing each item on a new line while preserving the interior formatting (blank lines, standalone comments). The interception (return null → fallback) remains only for edge cases: 0–1 items and single-line arrays.
• pExpr_Array now determines the style itself via shouldBeMultiline: the Modified marker on items + (count > 2 or wasMultiline(origNode)). The === false branch with pCommaSeparated was removed — single-line arrays are now printed by parent::pExpr_Array, which preserves item comments and respects kind (array() vs []).
• wasMultiline now works from origNode with the exact boundary from getEndTokenPos(). This fixes the findRight(']') issue that could go beyond the array boundary for legacy array().
• findModifiedArray, which previously hopped through Parent, was replaced with the simpler boolean hasModifiedItem (array_any, as in AbstractNodeVisitor).