Delete the error base's unused render method - #728
Merged
Conversation
ErrorBase::view() rendered the template, echoed it and called die(). Nothing called it: it is final protected, and each of the four error controllers returns ActionResponse::ok($this->view->render()) itself, which is what the dispatch contract requires — a method that echoes and exits could not satisfy it anyway. It was left behind by the move to that contract, in the one place nobody looks. Its imports go with it. All four pages were requested against the running application afterwards: the error index, maintenance, database error and database connection pages each still render.
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
1 participant
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.
What
ErrorBase::view()rendered the template, echoed it, and calleddie().Nothing called it. It is
final protected, and each of the four error controllers returnsActionResponse::ok($this->view->render())itself — which is what the dispatch contract requires. A method that echoes and exits could not satisfy that contract anyway: it would send the page and then die before the action could return anything.So it is a leftover from the move to
ActionResponse, sitting in the one place nobody looks — the error path. Its imports go with it.Found while covering the tail (#727), where it was reported rather than reached through an artificial subclass invented to give it a caller.
Testing
Both suites green: 3081 unit, 850 integration. PHPStan and PHPCS clean.
And because "nothing calls it" is exactly the claim that deserves a live check rather than a grep, all four pages were requested against the running application afterwards — the error index, maintenance, database error and database connection pages each still render: