-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Make Controller helpers final #24407
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
Conversation
Thank you @nicolas-grekas. |
…-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle] Make Controller helpers final | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - I propose to make all ControllerTrait methods final so we can add type hints. I also propose to add ControllerTrait::has/get so that AbstractController also has the methods. This will help move from Controller to AbstractController. Commits ------- bbc52a1 [FrameworkBundle] Make Controller helpers final
@nicolas-grekas Could you please explain what is the sense of making this methods final? |
@ossinkine see PR description. Why would you need to extend them? |
@nicolas-grekas PR description just says you propose to make them final but does not say why. abstract class MyController extends Controller
{
public function render($view, array $parameters = [], Response $response = null)
{
$parameters = array_merge(['foo' => 'bar'], $parameters);
return parent::render($view, $parameters, $response);
}
} So children controllers continue to use By the same logic, other methods can be overridden |
@ossinkine after #24722, the method will have a new signature (type-hints, as stated in the above description), so that if your code extends the methods, it will break. That's why. |
@nicolas-grekas I understand the new signature breaks BC and I've have to fix my code. But why I can't override the methods in future when my method signature is fixed? |
I'm surprised after try 3.4 beta 2 and discover this like ossinkine. |
I propose to make all ControllerTrait methods final so we can add type hints.
I also propose to add ControllerTrait::has/get so that AbstractController also has the methods.
This will help move from Controller to AbstractController.