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

Don't use null as exception message #4736

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

derrabus
Copy link
Member

Q A
Type bug
BC Break no
Fixed issues N/A

Summary

null is not a valid exception message. Before PHP 8.0, null was silently cast to an empty string. On PHP 8.1 however, passing null as $message to Exception__construct() will trigger a deprecation warning.

This PR proposes to change the tests so that the empty string is used instead of null.

Signed-off-by: Alexander M. Turek <me@derrabus.de>
@SenseException
Copy link
Member

The values for $message are all the same. Is it necessary to keep them in the data provider?

@derrabus
Copy link
Member Author

There is also one data provider that emits actual error messages.

@morozov morozov added the PHP label Jul 29, 2021
@morozov
Copy link
Member

morozov commented Jul 29, 2021

Note, these changes are likely already implemented in newer branches. But as I said earlier, I do not think we should be implementing the support for PHP 8.1 in DBAL 2.x.

@derrabus
Copy link
Member Author

See my comment #4734 (comment)

Let's continue the discussion there.

@greg0ire greg0ire merged commit a55eb48 into doctrine:2.13.x Aug 2, 2021
@greg0ire greg0ire added this to the 2.13.3 milestone Aug 2, 2021
@greg0ire
Copy link
Member

greg0ire commented Aug 2, 2021

Thanks @derrabus !

@derrabus derrabus deleted the bugfix/null-exception-message branch August 4, 2021 22:47
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants