Skip to content

Fix MailableMethodsInBuild destroying constructor formatting#378

Merged
driftingly merged 1 commit intotighten:mainfrom
superbiche:fix/mailable-formatter-preserves-constructor-formatting
Feb 27, 2026
Merged

Fix MailableMethodsInBuild destroying constructor formatting#378
driftingly merged 1 commit intotighten:mainfrom
superbiche:fix/mailable-formatter-preserves-constructor-formatting

Conversation

@superbiche
Copy link
Copy Markdown
Contributor

Summary

  • Fix MailableMethodsInBuild formatter collapsing multi-line constructor parameters (e.g. promoted properties) into a single line
  • Both constructorVisitor and buildVisitor now mutate $node->stmts in-place instead of creating new ClassMethod nodes, preserving PhpParser's printFormatPreserving identity check
  • Early-return null when no statements need moving, so untouched constructors aren't re-printed at all

Example

Given a Mailable with multi-line promoted constructor properties:

public function __construct(
    public readonly ?AlertErrorTypeEnum $type,
    public readonly string $hour,
    public readonly int $number,
    public readonly Carbon $now
) {}

Before (broken) — the formatter collapses everything to one line:

public function __construct(public readonly ?AlertErrorTypeEnum $type, public readonly string $hour, public readonly int $number, public readonly Carbon $now)
{
}

After (fixed) — constructor formatting is preserved as-is.

Test plan

  • Existing 3 tests pass (no regression)
  • New test: does_not_alter_constructor_parameter_formatting — verifies a Mailable with multi-line promoted constructor properties is not reformatted

The formatter was creating new ClassMethod nodes unconditionally, which
breaks PhpParser's printFormatPreserving identity check and causes the
entire constructor to be re-printed from scratch — collapsing multi-line
promoted properties into a single line.

Mutate $node->stmts in-place instead, and return null early when there
are no statements to move.
@driftingly driftingly merged commit d67dcbf into tighten:main Feb 27, 2026
1 check passed
@driftingly
Copy link
Copy Markdown
Member

Thanks @superbiche 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants