Modernize PHP test matrix, update PHPStan for 8.2+ compatibility#180
Modernize PHP test matrix, update PHPStan for 8.2+ compatibility#180kadamwhite wants to merge 21 commits intodevelopfrom
Conversation
Removes deprecated PHP versions Updates to latest stable WordPress
If phpstan trusts our annotation, it assumes the variable will always exist and that the null check is therefore unneeded. Since we do not control this global, we do need the check; therefore, we should tell phpstan that it is nullable.
Solves error, function has parameter $additional with no value type specified in iterable type array.
|
Test failure cause: In WP 6.6.1 (current on-dev version used in testing), the prepare_item_for_response method had But after updating in WP 6.9, a role guard was added: This was part of a security patch, changeset 60814. |
… now required to enumerate "roles" property See https://core.trac.wordpress.org/changeset/60814
goldenapples
left a comment
There was a problem hiding this comment.
Fixes the issues with the build, but because this fix uncovered a breaking change in the plugin's functionality in recent versions of WP, I'd like to leave this open for discussion on how to handle that change.
|
Agree we don't need to rush this through. Opened #181 to break out the discussion of the cap check change and how we want to handle it. Looking for design input from longer-tenured contributors. |
| - vendor/szepeviktor/phpstan-wordpress/extension.neon | ||
| parameters: | ||
| level: max | ||
| level: 6 |
There was a problem hiding this comment.
Note: this change was motivated by 50+ warnings which appeared after upgrading PHPStan, relating to PHPStan's assessment that many functions which returned the results of apply_filters now implicitly had the "mixed" return type. (Regardless of function return signature notations or PHPDoc.)
While this analysis is technically correct, the relevant functions had a return type specified on the function which would make malicious or misguided filter return values throw an error. To avoid excessive code changes in this PR I deemed it more prudent to lower the PHPStan level to 6 versus adding copious type coercion code, given that this return apply_filters() pattern is fairly core to how we develop on WordPress.
…rmission now required to enumerate "roles" property" This reverts commit 21e6758.
|
From @johnbillion (emphasis added):
Done — reverted the prior change that switched the test case to use an |
On the branch from #178, PHPStan is failing locally due to outdated versions. This PR updates the PHP and WordPress minimum versions to modern targets and refreshes the PHP version matrices in CI jobs to cover non-EOL PHP.
Minimal code changes are included to resolve PHPStan issues, only one of which is a non-comment functional code change: since the $cache property of a WP_Object_Cache::$cache is initialized to [] it is not documented as nullable and the isset should not be required.