Check for int in menu priority

[kirtan95]

Similar to #48249, this ticket focuses on ensuring that devs don't accidentally pass non-integer values in the position argument.

A similar issue was also reported in #40927, where passing a float argument in add_menu_page causes to override menu item.

For backward compatibility, I'd like to suggest informing users with _doing_it_wrong if a non-integer value is passed.

Optionally, we can also typecast float to string to ensure in case someone has passed float value, at least it doesn't override menus.

It will throw error with _doing_it_wrong if the typeof position is not
int.
Additionally, if the type of position is float, it will conver it to
string instead of typecasting it down to int which can cause override.

Fixes #54798, #40927

Trac Link - https://core.trac.wordpress.org/ticket/54798#comment:1

Trac ticket:

[kirtan95]

Fixed a last minute change in unit test 😅

[audrasjb]

Hello and welcome back to Trac @kirtan95 !

This change looks good to me.

Self-assigning for commit consideration.

[audrasjb]

In 52569:

Administration: Ensure an integer is used for menu priority in add_menu_page().

This change adds a verification of the $position parameter in add_menu_page() to ensure an integer is used. If not, the function informs developers of the wrong parameter type via a _doing_it_wrong message. This brings consistency with a similar check used in add_submenu_page().

This change also typecasts any floating number to string to ensure that in case a float value was passed, at least it doesn't override existing menus.

Follow-up to [46570].

Props kirtan95.
Fixes #54798. See #48249.

Committed in https://core.trac.wordpress.org/changeset/52569

[audrasjb]

In 52570:

Tests: Trac ticket number correction after changeset [52569].

Follow-up to [52569].

See #54798.

[audrasjb]

Looking back on the code committed in [52569], it looks like there is a missing indentation in the translators comment. Also, /* translators: %s: add_submenu_page() */ should probably be replaced with /* translators: %s: add_menu_page() */.

Reopening to address those issues.

[audrasjb]

In 52571:

Administration: Fix an erroneous translators comment after changeset [52569].

This change also fixes the indentation of the translators comment.

Fixes #54798.

[SergeyBiryukov]

Reopening, as there are some concerns with the current implementation that does not allow for float values, which are widely used by plugins to prevent "first come first serve" issues where menus go up and down sporadically.

See comment:18:ticket:40927.

[kirtan95]

@SergeyBiryukov So is it fine to add type check for float values? It will at least prevent users from passing float values as strings and plugin authors will be easily able to migrate their code in case they're passing float as a string.

If passing a float value is so widely used, perhaps we can even change the function docstring of both add_menu_page and add_submenu_page to reflect that they accept float values.

The motive of this PR was to add stricter type checks that would help developers spot bugs early during development, so I think the above two suggestions will help achieve this goal.

[peterwilsoncc]

I think this can be closed with the commits on #40927

add_menu_page() position:

• can be either a float or an integer in line with common plugin practices
• the default position, null, adds the menu as the last item
• a _doing_it_wrong() notice is triggered if the position is not numberic
• float types are cast to string to ensure PHP doesn't internally cast the number to an integer
• the position is sorted as a human would expect it, eg: 1, "1.043", "2", 3.

[jrf]

Commit [52569] introduced an new incompatibility with PHP 8.1 Implicit conversion from float 1.5 to int loses precision....

This needs a follow-up to fix that.

[jrf]

Fix newly introduced PHP 8.1 incompatibility

[peterwilsoncc]

@jrf Could you please open a follow up ticket.

This has commits on a closed milestone. Reopening and moving to a new milestone will create confusion during bisects and similar investigation of code history.

[jrf]

Replying to peterwilsoncc:

@jrf Could you please open a follow up ticket.

I will move the patch to #55656 which is already open for PHP 8.x patches.

As a general word of warning though: the fact that PHP 8.1 build does not currently pass, does not mean that it is acceptable to introduce new incompatibilities... Please always check the build manually for newly introduced errors.

[SergeyBiryukov]

In 53555:

Code Modernization: Use the integer portion of an item position in add_submenu_page().

This fixes an Implicit conversion from float to int loses precision PHP 8.1 deprecation notice when adding a new admin menu item with a float value passed as the $position parameter.

This change is covered by existing unit tests and addresses 8 errors when running the test suite on PHP 8.1.

References:

• ​PHP 8.1: Implicit incompatible float to int conversion is deprecated
• ​PHP RFC: Deprecate implicit non-integer-compatible float to int conversions

Follow-up to [52569], [53104].

Props jrf.
See #55656, #54798.