Deduplicate PHP version branching in RectorConfigBuilder - #8227
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #8225, same class. Two methods mapped a pile of bool arguments to a PHP version through long branch chains, and nine more methods repeated the same three statements.
withPhpSets()Sixteen bool arguments were resolved by nine early-return blocks followed by a seven-branch
elseifchain:Now one map:
This also replaces the
func_get_args()call that the arity checks were built on, so the "pick only one version" and "resolve from composer.json" branches read off the same array. The unreachable'Invalid PHP version set'branch goes away — a non-empty picked list always has exactly one entry by that point.The argument order in the signature is unchanged, including
$php84/$php85/$php86staying last for positional-call BC.withDowngradeSets()Same shape, nine branches:
The nine
withPhpNNSets()methodsEach was the same three statements with one constant swapped:
becomes
withPhpSets()uses the same private helper for both its composer.json branch and its explicit-version branch.Verification
Public API and argument order are unchanged. Since these methods have no direct test coverage, I diffed old against new behavior over 14 cases through reflection on the resulting
$sets: no arguments (composer.json resolution),php53/php74by flag vs. by dedicated method,php80,php86, an explicitly-passedfalse, both downgrade targets, and all four throw paths (two versions at once,withPhpSets()called twice, no downgrade target, two downgrade targets). Output is byte-identical on every one.Net
-83lines.