Make WordPress Core

Opened 16 months ago

Closed 9 months ago

Last modified 9 months ago

#58683 closed defect (bug) (fixed)

Cast return value to int in functions that use ceil()

Reported by: crstauf's profile crstauf Owned by: audrasjb's profile audrasjb
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

The use-case expects an integer to be returned, but the count() function returns a float; cast the returned value to an integer.

  • get_comment_pages_count()
  • wp_embed_defaults()
  • get_oembed_response_data()

This was mentioned by Sergey in https://core.trac.wordpress.org/ticket/55654#comment:9, and subsequent ticket #56800 aimed to update the tests, but the functions do not appear to have been fixed.

Change History (10)

This ticket was mentioned in PR #4768 on WordPress/wordpress-develop by @crstauf.


16 months ago
#1

  • Keywords has-patch added

Handful of functions use ceil(), which returns a float, but the functions expect an integer. Change those functions to cast ceil() to an integer.

Trac ticket: https://core.trac.wordpress.org/ticket/58683

#2 @crstauf
16 months ago

Reported by @wileycoyote78 in User Contributed Notes.

#3 @audrasjb
10 months ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 6.5
  • Owner set to audrasjb
  • Status changed from new to accepted

Looks good to me, thanks!
Moving for 6.5 consideration.

@audrasjb commented on PR #4768:


9 months ago
#4

Looks like Performance tests are failing with this change 👀

#6 @swissspidy
9 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 57648:

General: Consistently cast return value to int in functions that use ceil().

The return value of ceil() is still of type float as the value range of float is usually bigger than that of int.

Props crstauf, audrasjb.
Fixes #58683.

#9 @SergeyBiryukov
9 months ago

In 57650:

Tests: Use assertSame() in get_comment_pages_count() tests.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [27055], [48937], [54402], [57244], [57648].

Props costdev.
See #58683, #59655.

#10 @SergeyBiryukov
9 months ago

In 57653:

Tests: Use assertSame() in WP_Query tests involving ::$max_num_pages property.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [48937], [54402], [54768], [57648].

Props costdev.
See #58683, #59655.

Note: See TracTickets for help on using tickets.