Skip to content

Deduplicate PHP version branching in RectorConfigBuilder - #8229

Merged
TomasVotruba merged 1 commit into
mainfrom
dedup-php-sets
Jul 30, 2026
Merged

Deduplicate PHP version branching in RectorConfigBuilder#8229
TomasVotruba merged 1 commit into
mainfrom
dedup-php-sets

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

Follow-up to #8225, same class, same kind of copy-paste.

withPhpSets()

16 bool flags were dispatched through nine early-return blocks followed by a seven-arm elseif chain:

if ($php53) {
    $this->withPhp53Sets();
    return $this;
}

// ... php54, php55, php56, php70, php71, php72, php73, php74 ...

if ($php80) {
    $targetPhpVersion = PhpVersion::PHP_80;
} elseif ($php81) {
    $targetPhpVersion = PhpVersion::PHP_81;
} elseif ($php82) {
// ... through php86 ...
} else {
    throw new InvalidConfigurationException('Invalid PHP version set');
}

Now one version-keyed table:

$pickedPhpVersions = array_keys(array_filter([
    PhpVersion::PHP_53 => $php53,
    // ...
    PhpVersion::PHP_86 => $php86,
]));

which also drops the func_get_args() indirection previously needed to count how many flags were set. The unreachable 'Invalid PHP version set' branch goes with it — by that point a non-empty pick list always has exactly one entry.

withDowngradeSets()

Same shape, nine arms:

-if ($php84) {
-    $this->sets[] = DowngradeLevelSetList::DOWN_TO_PHP_84;
-} elseif ($php83) {
-    $this->sets[] = DowngradeLevelSetList::DOWN_TO_PHP_83;
-} elseif ($php82) {
-// ... through php71 ...
-}
+$pickedDowngradeSets = array_keys(array_filter([
+    DowngradeLevelSetList::DOWN_TO_PHP_84 => $php84,
+    // ...
+    DowngradeLevelSetList::DOWN_TO_PHP_71 => $php71,
+]));

The nine withPhpNNSets() methods

Each had the same three-line body:

public function withPhp70Sets(): self
{
    $this->isWithPhpSetsUsed = true;

    $this->sets = array_merge($this->sets, PhpLevelSetResolver::resolveFromPhpVersion(PhpVersion::PHP_70));

    return $this;
}

becomes

public function withPhp70Sets(): self
{
    return $this->addPhpLevelSets(PhpVersion::PHP_70);
}

withPhpSets() reuses the same private helper for both the picked-version and the composer.json-resolved path.

Behavior

Public API unchanged, and unlike #8225 no message or exception type changes here.

These paths have no direct test coverage, so I checked parity by driving the builder through 14 cases and reading back the resolved set list by reflection on both old and new code: no args, each of php53/php74/php80/php86, explicit php80: false, the withPhpNNSets() equivalents, both downgrade targets, and the four throwing cases (two versions picked, withPhpSets() called twice, no downgrade target, two downgrade targets). Output identical across all 14.

Net -83 lines.

withPhpSets() mapped 16 bool arguments to a target version with nine
"if ($phpNN) { $this->withPhpNNSets(); return $this; }" blocks followed by a
seven-branch elseif chain. withDowngradeSets() had the same shape with nine
branches. Both become an array_filter() over a version-keyed map.

The nine withPhpNNSets() methods repeated the same three statements each.
Extract them into a private addPhpLevelSets() helper, which withPhpSets()
now shares.
@TomasVotruba
TomasVotruba merged commit 8d12301 into main Jul 30, 2026
130 checks passed
@TomasVotruba
TomasVotruba deleted the dedup-php-sets branch July 30, 2026 20:06
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.

1 participant