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

[Debug] Mark ErrorHandler and ExceptionHandler classes as final #28954

Merged
merged 1 commit into from Dec 1, 2018

Conversation

Projects
None yet
6 participants
@fancyweb
Contributor

fancyweb commented Oct 22, 2018

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? not yet
Deprecations? yes
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

The goal of marking this method final is to be able to change the argument signature to \Throwable in Symfony 5.0

We will then be able to convert the incoming \Throwable to \ErrorException thanks to the FatalThrowableError class.

The use case is when you use the ExceptionHandler::register() method of the Debug component with a custom set_error_handler() that don't handle this conversion. This is for example the case of the Drupal one.

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Oct 22, 2018

What about making the whole class final? That'd be even better to me :)

@nicolas-grekas nicolas-grekas added this to the next milestone Oct 22, 2018

@fancyweb

This comment has been minimized.

Contributor

fancyweb commented Oct 23, 2018

What about making the whole class final? That'd be even better to me :)

Making the whole class final is more protective and might indeed be better for future similar problems.

In this case we should mark the ErrorHandler class final too I guess (you suggested it to me previously).

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Oct 23, 2018

Would work for me.

@fancyweb fancyweb force-pushed the fancyweb:master-make-exception-handler-handle-final branch from 32e2320 to 2a4e2e6 Oct 23, 2018

@fancyweb fancyweb changed the title from [Debug] Mark ExceptionHandler->handle() method as final to [Debug] Mark ErrorHandler and ExceptionHandler classes as final Oct 23, 2018

4.3.0
-----
* made the `ErrorHandler` and `ExceptionHandler` classes final

This comment has been minimized.

@iltar

iltar Oct 23, 2018

Contributor

They aren't actually final, just marked as such. It would make more sense in my opinion, to write "marked the ..."

This comment has been minimized.

@fancyweb

fancyweb Oct 23, 2018

Contributor

Yeah I thought about this but I actually searched how these kind of deprecations were handled in the past in the changelogs, and they were done like that 😕

Should I add also add an entry for the 5.0.0 with the fact that these classes will really be final at this point ?

This comment has been minimized.

@ro0NL

ro0NL Oct 26, 2018

Contributor

@final since Symfony 4.3 so the changelog is correct. Both final class and @final are final :)

@ro0NL

ro0NL approved these changes Oct 26, 2018

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Dec 1, 2018

Thank you @fancyweb.

@nicolas-grekas nicolas-grekas merged commit 2a4e2e6 into symfony:master Dec 1, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Dec 1, 2018

feature #28954 [Debug] Mark ErrorHandler and ExceptionHandler classes…
… as final (fancyweb)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Debug] Mark ErrorHandler and ExceptionHandler classes as final

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | not yet
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

The goal of marking this method final is to be able to change the argument signature to `\Throwable` in Symfony 5.0

We will then be able to convert the incoming `\Throwable` to `\ErrorException` thanks to the `FatalThrowableError` class.

The use case is when you use the `ExceptionHandler::register()` method of the `Debug` component with a custom `set_error_handler()` that don't handle this conversion. This is for example the case of the `Drupal` one.

Commits
-------

2a4e2e6 [Debug] Mark the ErrorHandler and ExceptionHandler classes as final

@fancyweb fancyweb deleted the fancyweb:master-make-exception-handler-handle-final branch Dec 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment