-
-
Notifications
You must be signed in to change notification settings - Fork 21
Handle JSON_ERROR_NON_BACKED_ENUM #46
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
base: master
Are you sure you want to change the base?
Conversation
src/JsonException.php
Outdated
| \JSON_ERROR_UNSUPPORTED_TYPE => 'A value of a type that cannot be encoded was given', | ||
| \JSON_ERROR_INVALID_PROPERTY_NAME => 'A property name that cannot be encoded was given', | ||
| \JSON_ERROR_UTF16 => 'Malformed UTF-16 characters, possibly incorrectly encoded', | ||
| \JSON_ERROR_NON_BACKED_ENUM ?? 11 => 'Value contains a non-backed enum which cannot be serialized.', |
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.
Why the ?? 11? None of the others have a numeric fallback. Since it's a lookup table, it shouldn't get hit even on an older version. (See the getExceptionMessage() method below.)
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.
Huh, well, looks like the tests fail anyway on 8.0, because of the undefined constant, even with that fallback.
Possible solutions:
- According to Packagist, there's ~8% of installs on 7.4(!) right now. Otherwise, nearly everyone is on 8.1 or higher. So we could just bump the version requirement to 8.1 (or, really, 8.2 or 8.3), release a new minor, and move on with life. I have little interest in expending effort to support unsupported PHP versions.
- Change the map from a const to just a protected array, and then conditionally add an extra value to it in the constructor if the constant is defined.
Thoughts?
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.
I agree with 1.
7.4 reached eol 3+ years ago. 8.1 reached eol last month.
8.2 and 8.3 only get security support at this point, but I see no harm in supporting them if they aren't causing issues.
|
|
Handles new constant added since 8.1 (not currently documented in table but [exists here](https://www.php.net/manual/en/json.constants.php#constant.json-error-non-backed-enum)
Thank you.
Done. ✔️ |
Handles new constant added since 8.1 (not currently documented in table but exists here