[wp-trac] [WordPress Trac] #46346: Page counts and related calculations typically expect and document ints but are actually floats

WordPress Trac noreply at wordpress.org
Wed Mar 3 13:35:12 UTC 2021


#46346: Page counts and related calculations typically expect and document ints but
are actually floats
----------------------------------------+-------------------------------
 Reporter:  lev0                        |       Owner:  SergeyBiryukov
     Type:  defect (bug)                |      Status:  reviewing
 Priority:  normal                      |   Milestone:  5.8
Component:  General                     |     Version:
 Severity:  minor                       |  Resolution:
 Keywords:  has-patch needs-unit-tests  |     Focuses:  coding-standards
----------------------------------------+-------------------------------

Comment (by jrf):

 @lev0 Thanks for the additional patch.

 I like the idea of having a common function for this, though I'm not sure
 the function should be called `wp_total_pages()` as in not all cases it is
 ''about'' pages, but naming things is hard, so I also don't have a
 alternative name in mind.

 Having said that, the new patch does not contain any unit tests covering
 this change for the existing functions, let alone contain any tests for
 the new function. Which also means that it also still doesn't contain a
 test which would proof the ''need'' for this fix.
 In any case, the patch should probably be pulled to GitHub, so at least
 the results of a run of the existing unit tests would be available for
 this patch via GH Actions.

 The function also, as I pointed out before, still doesn't solve your
 problem of it potentially returning a float.

 Let me try and explain it one more time:
 1. [https://www.php.net/manual/en/function.ceil.php `ceil()`] will return
 an integer as a float by design. As long as the inputs (and by extension
 the result) is below `PHP_INT_MAX`, it should be perfectly safe to cast
 this float to an integer without loss of precision.
 2. If you still fear loss of precision,
 [https://www.php.net/manual/en/function.round.php `round()`] can be used
 around the call to `ceil()`. Again, this will return a float, but as long
 as the inputs (and by extension the result) is below `PHP_INT_MAX`, it
 should be perfectly safe to cast this float to an integer without loss of
 precision.
 3. As per [https://www.php.net/manual/en/language.operators.arithmetic.php
 the manual], the division operator `/`:
 > ... returns a float value unless the two operands are integers (or
 strings that get converted to integers) and the numbers are evenly
 divisible, in which case an integer value will be returned.
 The same remark I made for the previous two about `PHP_INT_MAX` also
 applies.
 4. Next, we have the PHP 7.0+ `intdiv` function. Aside from the fact that
 we can't use it yet unless we would polyfill it (PHP 5.6 minimum), this
 will return an integer, but will also:
     - throw a `DisionByZeroError` exception if the second operant is `0`;
     - throw a `AritmeticError` exception is the first operant is
 `PHP_INT_MIN` and the second is `-1`;
     - throw a `TypeError` if either of the operants passed can't be cast
 to integers;
     - and will cast passed non-integer operants which ''can'' be cast to
 integers to integers ''before'' the operation.
 5. Lastly, there is [https://www.php.net/manual/en/function.bcdiv
 `bcdiv()`] which expects two arbitrary precision numbers passed as strings
 as input and will return the result of the division as a string (or `null`
 if the second operant is `0`). Again, as long as the integer value of the
 returned string is below `PHP_INT_MAX`, it should be perfectly safe to
 cast this string to an integer without loss of precision. The problem with
 using `bcdiv()` is that, even though PHP ships with BCMath by default, it
 is an extension which will only be available if PHP was build with
 `--enable-bcmath`, so it would add a new requirement to WP.

 In the case of the last two (three) options, we still need to calculate
 the modulo as well and add either `0` or `1` to the results, which, in the
 case of `bcdiv()` will recast the result to integer/float.

 You can have a look at
 [https://phpcheatsheets.com/static_results/arithmetic/ these cheatsheets]
 to see what the return value is when using `/`, `intdiv()`or `bcdiv()`
 with a wide variety of input values and types.

 Or have a look at this 3v4l and study the results on various PHP versions
 carefully: https://3v4l.org/V6jdD

 Hints:
 * Without input validation (are both parameters integers ? is the second
 operant not a 0 ?), the logic as currently proposed in `wp_total_pages()`
 is liable to cause more problems than it solves.
 * As soon as the calculations are done with one of the numbers being a
 number above `PHP_INT_MAX`, the results become unreliable, no matter what.
 * `(int) ceil()` would be a perfectly valid solution (unless you can come
 up with a test case which proves differently).

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/46346#comment:18>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list