Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ArgumentDefaultValueReplacer #6795

Merged

Conversation

carlos-granados
Copy link
Contributor

Derived from #6794

@@ -6,6 +6,7 @@ class VersionCompare
{
public function run()
{
version_compare($a, $b, $c);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case where previously the rule would have been applied even though no code was changed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you extract this one a to a new "skip_*" file?

@@ -113,15 +113,17 @@ private function processArgs(
$replaceArgumentDefaultValue->getValueBefore()
) && $argValue === $replaceArgumentDefaultValue->getValueBefore()) {
$expr->args[$position] = $this->normalizeValueToArgument($replaceArgumentDefaultValue->getValueAfter());
return $expr;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only return an Expr if the expr is actually changed, so that the rule is not applied if no change is made

@carlos-granados carlos-granados force-pushed the fix-ArgumentDefaultValueReplacer branch 2 times, most recently from c49d752 to f240f5d Compare March 21, 2025 08:30
@carlos-granados carlos-granados force-pushed the fix-ArgumentDefaultValueReplacer branch from f240f5d to 7b2626f Compare March 21, 2025 08:42
@samsonasik samsonasik merged commit 6b7412b into rectorphp:main Mar 21, 2025
44 checks passed
@samsonasik
Copy link
Member

Thank you @carlos-granados

@carlos-granados carlos-granados deleted the fix-ArgumentDefaultValueReplacer branch March 21, 2025 08:59
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.

3 participants