Simplified improvement for absint() that always returns an int - #12944
Simplified improvement for absint() that always returns an int#12944josephscott wants to merge 5 commits into
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
irozum
left a comment
There was a problem hiding this comment.
This tightens up absint() well for int/string/bool/object/resource inputs, and the return-value discussion with @dmsnell/@westonruter on the ticket (PHP_INT_MIN → PHP_INT_MAX instead of overflowing to float) looks like the right call given the name of the function. I ran the full Tests_Functions_Absint suite (78 tests, all green on PHP 8.3) plus PHPCS and PHPStan against load.php and the test file — no new errors from this diff.
That said, I think this is exactly what's causing the CI failures on every PHP 8.5 job (24/24 red, all in test_absint_returns_non_negative_int_for_unrepresentable_floats): load.php:1470's $maybeint = (int) $maybeint; casts the raw input before any range/finiteness check, so a non-finite or out-of-range float (INF, NAN, 1.0e20, (float) PHP_INT_MAX) hits PHP 8.5's new "float ... is not representable as an int, cast occurred" deprecation right there — the existing PHP_INT_MIN === $maybeint guard only runs after the cast has already happened. I confirmed the cast itself is silently undefined-behavior-but-not-yet-deprecated on PHP 8.3 ((int) 1.0e20 returns a platform-dependent garbage int rather than PHP_INT_MAX), which is why it passes locally but not on 8.5 — this needs a is_finite() / range check on $maybeint while it's still a float, before the cast, not just the post-cast PHP_INT_MIN special case.
One smaller thing: the ticket this PR references (#65826) is already closed via the revert in r63158, so once this lands it'll probably want its own fresh ticket for props/changelog purposes — not blocking, just a heads up for whoever picks this up for 7.2.
With a long list of tests
https://core.trac.wordpress.org/ticket/65826
This will cause return values to change compared to the old
absint()- https://3v4l.org/MssWC - because the old one wasn't actually limited to returning only anintAI assistance: Yes
Tool(s): Claude
Model(s): Fable 5
Used for: Discussion on potential approaches on how to fix
absint()to only return anintin a more simplified way. Claude also wrote all of the new tests.This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.