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
[Config] Add merging of rules configuration, call configure() just once #4838
Conversation
… from previous calls
if ($isBound) { | ||
return; | ||
} | ||
|
||
$this->singleton($rectorClass); | ||
$this->tagRectorService($rectorClass); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this check, the same Rector rule would be registered twice. That could lead to unexpected changes and waste of resources.
): void { | ||
$ruleConfiguration = $this->ruleConfigurations[$rectorClass]; | ||
$configurableRector->configure($ruleConfiguration); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configure()
is now called exactly once with the full configuration.
We might check for the configuration override and have custom property to add this after resolving too. In case of non-configurable rules, but it might be too defensive. Let's see how it performs :)
Thanks for the solution @samsonasik 👍 |
@TomasVotruba this cause a bug in rector-doctrine, see I will remove the |
Fixes rectorphp/rector#8150
Closes #4831