Repository navigation
Code Quality: Improve PHPStan types in l10n.php - #14079
westonruter wants to merge 8 commits into
Conversation
Translatable strings, contexts, and text domains are extracted statically from source, so passing a variable to `__()`, `_x()`, `_n()`, `_n_noop()` and friends yields a string that can never be translated. Add `@phpstan-param literal-string` narrowings for `$text`, `$single`, `$singular`, `$plural`, `$context`, and `$domain` on the gettext wrappers so PHPStan flags such misuse. Add `@phpstan-return` shapes to `_n_noop()` and `_nx_noop()` and a matching `@phpstan-param` shape for `$nooped_plural` on `translate_nooped_plural()`, so the literal types carry through the nooped plural array into `_n()`/`_nx()`. Making the shape keys required also resolves the existing `offsetAccess.notFound` errors in `translate_nooped_plural()`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`WP_Locale::get_word_count_type()` validates the translated value and falls back to `words` for anything other than `characters_excluding_spaces` or `characters_including_spaces`, so the return value is always one of those three strings. Declare that union via `@phpstan-return` on both the method and `wp_get_word_count_type()`, which returns either the method's result or the `words` default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The function returns language data keyed by text domain and then by
locale, with each value being the header data from
`wp_get_pomo_file_data()` or `wp_get_l10n_php_file_data()`. Document it
as `array<string, array<string, string[]>>` instead of a bare `array`.
Add a conditional `@phpstan-return` so that an unsupported `$type`
resolves to `array{}`, matching the early return for anything other
than `plugins`, `themes`, or `core`. `$type` itself stays `string`
since the function deliberately tolerates other values.
This resolves the missing iterable value type on the function as well
as the `mixed` foreach and concatenation errors in `plugin.php` and
`theme.php` where the result is consumed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`wp_get_pomo_file_data()` and `wp_get_l10n_php_file_data()` always return the same four headers, so declare that shape via `@phpstan-return` and use it for the innermost type of `wp_get_installed_translations()`. To make the shape hold: * `wp_get_pomo_file_data()` now builds its result from an explicit array of the four headers rather than returning the `get_file_data()` result, whose keys PHPStan cannot infer. The `preg_replace()` that strips the closing double quote and the preceding contextual `\n` is replaced with equivalent string functions, since `preg_replace()` may return `null`. The only input the two differ on is a value ending in a real newline, which cannot occur because `get_file_data()` trims each value. * `wp_get_l10n_php_file_data()` now ignores non-string header values from the included file, keeping the empty-string default instead of passing through whatever a malformed file contains. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The header arrays returned by `wp_get_pomo_file_data()` and `wp_get_l10n_php_file_data()` are keyed by header name, so document them as `array<string, string>` rather than `string[]`, and likewise for the innermost type in the `@return` of `wp_get_installed_translations()`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since the performant translations library was introduced in WordPress 6.5, `load_textdomain()` stores a `WP_Translations` instance in the `$l10n` global, and `get_translations_for_domain()` returns it as is. `WP_Translations` does not extend `Translations`, so the documented `Translations|NOOP_Translations` return type has been inaccurate since then. Add `WP_Translations` to the union. `Translations` remains for `MO` instances that plugins may still place in `$l10n` directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`$l10n_unloaded` was documented as `MO[]`, but it only ever maps text domains to `true`. Document it as `array<string, true>`. `$l10n` was documented as `MO[]` in some places and as `array<string, WP_Translations|NOOP_Translations>` in others. Since WordPress 6.5, core stores `WP_Translations` instances from `load_textdomain()` and the shared `NOOP_Translations` fallback from `get_translations_for_domain()`, while plugins may still assign `MO` instances directly, which `load_textdomain()` accounts for with its `instanceof MO` check. Document it consistently as `array<string, WP_Translations|NOOP_Translations|MO>` in both the `@global` tags and the inline `@var` tags. The `@global` tags are significant for static analysis here, since the PHPStan `GlobalDocBlockVisitor` applies them to the `global` statements. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each `global` statement in `l10n.php` that carried an inline `@var` tag is in a function whose docblock already documents the same global with an `@global` tag of the same type. The PHPStan `GlobalDocBlockVisitor` applies those `@global` tags to the `global` statements, so the inline tags are redundant. PHPStan results are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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. |
Translatable strings, contexts, and text domains are extracted statically from source, so the gettext functions now require literal strings for these arguments. This lets PHPStan flag values that can never be translated, even when they reach the function indirectly through a variable or a nooped plural array. The return type of these functions remains `string`, since an empty string can still result from the `gettext` filters or from an empty translation in a translation file. Correcting the documented types of the `$l10n` and `$l10n_unloaded` globals revealed that the return type of `get_translations_for_domain()` has been inaccurate since r57337, because `WP_Translations` does not extend `Translations`. The `$l10n` type retains `MO` since plugins may still assign such instances to the global directly, which `load_textdomain()` accounts for. Making the declared header shapes hold for `wp_get_pomo_file_data()` and `wp_get_l10n_php_file_data()` required small runtime changes: the former no longer relies on `preg_replace()`, which may return `null`, and the latter now ignores non-string header values from a malformed translation file. Developed in #14079. Follow-up to r25520, r38198, r55279, r57337, r58061. Props westonruter, swissspidy. See #65817. git-svn-id: https://develop.svn.wordpress.org/trunk@64232 602fd350-edb4-49c9-b593-d223f7449a82
Translatable strings, contexts, and text domains are extracted statically from source, so the gettext functions now require literal strings for these arguments. This lets PHPStan flag values that can never be translated, even when they reach the function indirectly through a variable or a nooped plural array. The return type of these functions remains `string`, since an empty string can still result from the `gettext` filters or from an empty translation in a translation file. Correcting the documented types of the `$l10n` and `$l10n_unloaded` globals revealed that the return type of `get_translations_for_domain()` has been inaccurate since r57337, because `WP_Translations` does not extend `Translations`. The `$l10n` type retains `MO` since plugins may still assign such instances to the global directly, which `load_textdomain()` accounts for. Making the declared header shapes hold for `wp_get_pomo_file_data()` and `wp_get_l10n_php_file_data()` required small runtime changes: the former no longer relies on `preg_replace()`, which may return `null`, and the latter now ignores non-string header values from a malformed translation file. Developed in WordPress/wordpress-develop#14079. Follow-up to r25520, r38198, r55279, r57337, r58061. Props westonruter, swissspidy. See #65817. Built from https://develop.svn.wordpress.org/trunk@64232 git-svn-id: http://core.svn.wordpress.org/trunk@63383 1a063a9b-81f0-0310-95a4-ce76da25c4cd
|
There were a couple PHPStan errors introduced by this which I missed: I'm following up in another PR. |
See #14088 |
✅ Committed in r64232 (c64de70).
Improves the PHPStan types in
src/wp-includes/l10n.php, mostly through PHPDoc, plus two small runtime hardenings needed to make the declared return shapes hold.Literal strings for gettext arguments
Translatable strings, contexts, and text domains are extracted statically from source, so a non-literal argument to
__(),_x(),_n(),_n_noop(), etc. yields a string that can never be translated. These functions now declare@phpstan-param literal-stringfor$text,$single/$singular,$plural,$context, and$domain(literal-string|nullfor the_n_noop()/_nx_noop()domain). This complements theWordPress.WP.I18nPHPCS sniff: PHPStan tracks types through variables and the nooped plural array, so a value from a non-literal source likeget_option()is flagged even when it reaches__()indirectly.To carry the literal types through nooped plurals,
_n_noop()and_nx_noop()gain@phpstan-returnarray shapes, andtranslate_nooped_plural()gets a matching@phpstan-paramshape for$nooped_plural. Making those keys required also resolves the existingoffsetAccess.notFounderrors insidetranslate_nooped_plural().The return type of
__()and friends is intentionally left asstring: an empty string can still be returned via thegettextfilters or an empty translation in a.mo/.l10n.phpfile.Return types
wp_get_word_count_type()andWP_Locale::get_word_count_type()return'characters_excluding_spaces'|'characters_including_spaces'|'words', since the method validates the translated value and falls back towords.wp_get_installed_translations()documents its nested structure (keyed by text domain, then locale) and has a conditional@phpstan-returnresolving toarray{}for an unsupported$type.$typeitself staysstringsince the function deliberately tolerates other values.wp_get_pomo_file_data()andwp_get_l10n_php_file_data()return a fixed shape of the four headers. To make that hold:wp_get_pomo_file_data()builds its result from an explicit array of the four headers instead of returning theget_file_data()result, and replaces thepreg_replace()(which may returnnull) with equivalent string functions. The only input the two differ on is a value ending in a real newline, which cannot occur sinceget_file_data()trims each value.wp_get_l10n_php_file_data()ignores non-string header values from the included file, keeping the empty-string default.get_translations_for_domain()now includesWP_Translationsin its return type. Since the performant translations library landed in r57337,load_textdomain()storesWP_Translationsinstances, which do not extendTranslations, so the documented return type has been inaccurate since 6.5.Globals
$l10n_unloadedwas documented asMO[]but only ever maps text domains totrue; it is nowarray<string, true>.$l10nwas documented inconsistently asMO[]andarray<string, WP_Translations|NOOP_Translations>; it is now consistentlyarray<string, WP_Translations|NOOP_Translations|MO>.MOis retained since plugins may still assignMOinstances directly, whichload_textdomain()accounts for with itsinstanceof MOcheck.@vartags onglobalstatements are removed where the function already has a matching@globaltag, sinceGlobalDocBlockVisitorapplies those to theglobalstatements.PHPStan impact
Comparing full level 10 runs on
src/againsttrunk: 25 fewer errors, with none introduced. Ten remaining errors are pre-existing ones whose messages now cite the narrower types (e.g. callers passingmixedtotranslate_nooped_plural()). One of those is a legitimately non-literal call in core:wp_generate_tag_cloud()passes the deprecatedsingle_text/multiple_textarguments to_n_noop().Testing
Tests_L10n::test_wp_get_pomo_file_dataandTests_L10n::test_wp_get_installed_translations_for_corepass.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
Used for: Implementing the type narrowings and code changes, verifying them with PHPStan, PHPCS, and PHPUnit, and drafting the commit messages and this description, under direction and review by me.
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.
🤖 Generated with Claude Code