Repository navigation
Conversation
…by_mid()`. Both functions validate their numeric arguments with `is_numeric()` and cast them to integers, and every core caller passes strings: date parts cut from a date string with `substr()` or a regular expression, and meta IDs read with `$wpdb->get_col()`. Their docblocks still documented `int` only. This widens `$month`, `$day`, and `$year` in `wp_checkdate()`, and `$meta_id` in `delete_metadata_by_mid()`, to `int|string`. That resolves 17 PHPStan `argument.type` errors across 8 files. Follow-up to r18494, r20435, r38699, r61172. See #65817.
… and `update_metadata_by_mid()`. Both functions share the `is_numeric()` and `floor()` validation and integer cast used by `delete_metadata_by_mid()`, so they accept numeric strings in the same way. See #65817.
# Conflicts: # tests/phpstan/baselines/argument.type.neon
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. 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. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
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. |
…id()` meta IDs. Narrows the date parts in `wp_checkdate()` to the ranges accepted by PHP's `checkdate()`, and the meta IDs in `update_metadata_by_mid()` and `delete_metadata_by_mid()` to numeric values, using `@phpstan-param` so that the documented `int|string` types stay readable. See #65817.
Matches `update_metadata_by_mid()` and `delete_metadata_by_mid()`, which share its validation. See #65817.
There was a problem hiding this comment.
🟡 Changes recommended
Active critical and moderate findings show the proposed PHPStan contracts reject actual caller inputs, including string|false, out-of-range integers, and get_col() values that may be null or nonnumeric.
3 open issues
What changed in this PR
This pull request updates PHPDoc and PHPStan annotations for wp_checkdate() and metadata-by-ID functions, and removes related PHPStan baseline suppressions.
Changes:
- Widens documented date-component and metadata-ID parameter types.
- Adds PHPStan-specific annotations for string inputs.
- Removes 17 corresponding
argument.typebaseline entries.
| File | Review |
|---|---|
tests/phpstan/baselines/argument.type.neon |
Removes the affected baseline entries, but the underlying type errors remain unresolved with the proposed contracts. |
src/wp-includes/meta.php |
The proposed metadata-ID type does not account for get_col() values that may be null or unproven numeric strings. |
src/wp-includes/functions.php |
The proposed wp_checkdate() types do not account for `string |
This pull request requires changes.
Review effort: Lite
| * @phpstan-param int<1, 12>|numeric-string $month | ||
| * @phpstan-param int<1, 31>|numeric-string $day | ||
| * @phpstan-param int<1, 32767>|numeric-string $year |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5 (medium effort)
This does not apply at the project's rule level: at level 5, PHPStan does not report an argument whose union type is only partly accepted, so the string|false and string date parts pass and the removed baseline entries are genuinely resolved. Widening the type to int|string|false would discard the contract that level 7 is meant to enforce.
Details
vendor/bin/phpstan analyse -c phpstan.neon.dist (level 5, with the baselines) was run on ms-site.php, class-wp-customize-manager.php, post.php, and the other five caller files, and it reported no errors, including no unmatched baseline entries. The partially wrong union types are only reported from level 7, where flagging a date part that may be false or non-numeric is the intended result.
| * @param int|string $meta_id ID for a specific meta row. | ||
| * @return bool True on successful delete, false on failure. | ||
| * | ||
| * @phpstan-param int|numeric-string $meta_id |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5 (medium effort)
This does not apply at the project's rule level: at level 5, PHPStan does not report an argument whose union type is only partly accepted, so the non-empty-string|null values from $wpdb->get_col() pass and the removed baseline entries are genuinely resolved. From level 7 the callers will be flagged, which is intended, since a meta ID should be provably numeric.
Details
vendor/bin/phpstan analyse -c phpstan.neon.dist (level 5, with the baselines) was run on comment.php, functions.php, ms-site.php, post.php, taxonomy.php, wp-admin/includes/ms.php, wp-admin/includes/user.php, and class-wp-customize-manager.php, and it reported no errors, including no unmatched baseline entries.
As a validator, `wp_checkdate()` must accept out-of-range and unvalidated input and return `false` for it, as its tests do with month `99` and year `0`. The ranges also rejected the `substr()` results its callers pass from level 7. See #65817.
|
…n types. Reverts 9b69b56. The PHPStan contract follows what `checkdate()` requires. Tests that deliberately pass out-of-range values can use `@phpstan-ignore` if they are ever analyzed. See #65817.
…into phpstan/ms-site
There was a problem hiding this comment.
🟡 Changes recommended
Active critical type errors remain at the updated call sites, and the proposed contracts are not bounded to the documented valid ranges.
5 open issues
1 resolved since last review
Review effort: Lite
| * @phpstan-param int|numeric-string $month | ||
| * @phpstan-param int|numeric-string $day | ||
| * @phpstan-param int|numeric-string $year |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5 (medium effort)
This does not apply at the project's rule level: at level 5, PHPStan does not report an argument whose union type is only partly accepted, so the string|false and string date parts pass and the removed baseline entries are genuinely resolved. Widening the type to int|string|false would discard the contract that level 7 is meant to enforce.
Details
vendor/bin/phpstan analyse -c phpstan.neon.dist (level 5, with the baselines) was run on ms-site.php, class-wp-customize-manager.php, post.php, and the other five caller files, and it reported no errors, including no unmatched baseline entries. The partially wrong union types are only reported from level 7, where flagging a date part that may be false or non-numeric is the intended result.
| * @type string $user_id Optional. The object ID when the meta type is 'user'. | ||
| * } | ||
| * | ||
| * @phpstan-param int|numeric-string $meta_id |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5 (medium effort)
This does not apply at the project's rule level: at level 5, PHPStan does not report an argument whose union type is only partly accepted, so the non-empty-string|null values from $wpdb->get_col() pass and the removed baseline entries are genuinely resolved. From level 7 the callers will be flagged, which is intended, since a meta ID should be provably numeric.
Details
vendor/bin/phpstan analyse -c phpstan.neon.dist (level 5, with the baselines) was run on comment.php, functions.php, ms-site.php, post.php, taxonomy.php, wp-admin/includes/ms.php, wp-admin/includes/user.php, and class-wp-customize-manager.php, and it reported no errors, including no unmatched baseline entries.
pwtyler
changed the title
wp_checkdate() and the *_metadata_by_mid() functionswp_checkdate() and the *_metadata_by_mid() functions
The `wp_checkdate()` and `*_metadata_by_mid()` functions already validate their numeric arguments with `is_numeric()` and cast them to integers, and core callers pass strings: date parts cut from a date string, and meta IDs read with `$wpdb->get_col()`. Their docblocks still documented `int` only, which caused 17 PHPStan `argument.type` errors. The arguments are now documented as `int|string`, with PHPStan narrowing them to `int|numeric-string`. The `checkdate()` ranges are deliberately not applied to `wp_checkdate()`, since the function's purpose is to validate input that may be out of range. Developed in #15472. Follow-up to r18494, r20435, r38699, r61172. Props pwtyler, westonruter. See #65817. git-svn-id: https://develop.svn.wordpress.org/trunk@65269 602fd350-edb4-49c9-b593-d223f7449a82
The `wp_checkdate()` and `*_metadata_by_mid()` functions already validate their numeric arguments with `is_numeric()` and cast them to integers, and core callers pass strings: date parts cut from a date string, and meta IDs read with `$wpdb->get_col()`. Their docblocks still documented `int` only, which caused 17 PHPStan `argument.type` errors. The arguments are now documented as `int|string`, with PHPStan narrowing them to `int|numeric-string`. The `checkdate()` ranges are deliberately not applied to `wp_checkdate()`, since the function's purpose is to validate input that may be out of range. Developed in WordPress/wordpress-develop#15472. Follow-up to r18494, r20435, r38699, r61172. Props pwtyler, westonruter. See #65817. Built from https://develop.svn.wordpress.org/trunk@65269 git-svn-id: http://core.svn.wordpress.org/trunk@64418 1a063a9b-81f0-0310-95a4-ce76da25c4cd
✅ Committed in r65269 (4e77779).
This fixes 17 PHPStan
argument.typeerrors across 8 files. They come from function docblocks that documentintwhile the functions accept numeric strings, and every core caller passes numeric strings.Background
wp_checkdate()(9 errors): callers pass date parts cut from a date string, so they arrive as strings. r61172 (Core-26798) addedis_numeric()checks and(int)casts for exactly that case, but left the@param intdocblocks unchanged. A call-site(int)cast would be wrong here, because it would turn a malformed part like'1a'into a valid1.delete_metadata_by_mid()(8 errors): callers loop over meta IDs from$wpdb->get_col(), which are strings, as they have since r20435. Since r38699 (Core-37746), the function validates withis_numeric()andfloor()and casts to an integer.get_metadata_by_mid()andupdate_metadata_by_mid()share that validation.Changes
wp_checkdate():$month,$dayand$yearareint|string, plus@phpstan-param int|numeric-string. The ranges thatcheckdate()requires are deliberately not applied, because the function's purpose is to validate input that may be out of range.get_metadata_by_mid(),update_metadata_by_mid()anddelete_metadata_by_mid():$meta_idisint|string, plus@phpstan-param int|numeric-string.tests/phpstan/baselines/argument.type.neon.Testing Instructions
npm run typecheck:phpreports no errors.npm run test:php -- --group meta,date,datequerypasses, single-site and with-c tests/phpunit/multisite.xml.Trac ticket: https://core.trac.wordpress.org/ticket/65817
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5, Claude Sonnet 5.5
Used for: Surveying open PRs to avoid overlap, code archeology (tracing changesets), testing type variants against PHPStan, and drafting the change and this description. I reviewed and verified all output.
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.