Skip to content

[Php74] Keep comments above if on IfToNullCoalescingAssignRector - #8388

Merged
TomasVotruba merged 1 commit into
rectorphp:mainfrom
Soean:fix-null-coalescing-assign-lost-comments
Aug 27, 2026
Merged

[Php74] Keep comments above if on IfToNullCoalescingAssignRector#8388
TomasVotruba merged 1 commit into
rectorphp:mainfrom
Soean:fix-null-coalescing-assign-lost-comments

Conversation

@Soean

@Soean Soean commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

IfToNullCoalescingAssignRector builds a fresh Expression node for the ??= statement and returns it. A new node carries no attributes, so the COMMENTS of the replaced If_ are dropped — every docblock or line comment above the guard is lost.

Before (rule fires, comment disappears)

/**
 * Fallback value description
 *
 * @var int $limit
 */
if (! isset($limit)) {
    $limit = 100;
}

// simple comment
if (! isset($name)) {
    $name = 'default';
}

...becomes this — both comments are gone

$limit ??= 100;

$name ??= 'default';

Fix

Mirror the comments from the If_ onto the new Expression before returning it.

After

/**
 * Fallback value description
 *
 * @var int $limit
 */
$limit ??= 100;

// simple comment
$name ??= 'default';

All three guard forms (! isset(), is_null(), === null) share the single return path, so one fixture covers them.

@Soean
Soean marked this pull request as ready for review August 27, 2026 12:35
@TomasVotruba
TomasVotruba merged commit 4d8940b into rectorphp:main Aug 27, 2026
44 checks passed
@TomasVotruba

Copy link
Copy Markdown
Member

Looks good, thank you

@Soean
Soean deleted the fix-null-coalescing-assign-lost-comments branch August 27, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants