From 6adb458e2021e89f5d19cb4e3c88e27cca0895d4 Mon Sep 17 00:00:00 2001 From: Alain Schlesser Date: Fri, 23 Oct 2026 21:34:40 +0200 Subject: [PATCH 1/2] Require Delayed for conditions that read the admin screen --- docs/architecture-rules.md | 26 ++++--- docs/patterns.md | 5 ++ phpstan.neon.dist | 3 + tests/php/Architecture/RuleManifest.php | 4 +- .../Feature/DelayedScreenConditional.php | 19 ++++++ .../Hooks/Feature/ScreenConditional.php | 14 ++++ .../Rule/QueryContextNeedsDelayRule.php | 39 ++++++++--- .../Rule/QueryContextNeedsDelayRuleTest.php | 68 ++++++++++++++++++- tests/php/PHPStan/rules.neon | 2 + 9 files changed, 160 insertions(+), 20 deletions(-) create mode 100644 tests/php/PHPStan/Fixture/Hooks/Feature/DelayedScreenConditional.php create mode 100644 tests/php/PHPStan/Fixture/Hooks/Feature/ScreenConditional.php diff --git a/docs/architecture-rules.md b/docs/architecture-rules.md index 92aa949..b190b16 100644 --- a/docs/architecture-rules.md +++ b/docs/architecture-rules.md @@ -1071,8 +1071,8 @@ and the capability interfaces of [the core idea](design-principles.md#how-servic the plugin waits until a service's condition can answer before building it. **Checks:** when `is_needed()` of a class implementing `Conditional` calls a -configured query-dependent function directly, the class also implements -`Delayed`. +configured query- or screen-dependent function directly, the class also +implements `Delayed`. **Allows:** a `Conditional` that only calls request checks such as `is_admin()`, classes that do not implement `Conditional`, and calls in other @@ -1084,13 +1084,21 @@ Neither proves that the action runs late enough for the condition. **Configuration:** `queryContextNeedsDelay.queryFunctions` lists the query-dependent functions: `is_singular`, `is_single`, `is_page`, `is_front_page`, `is_home`, `is_archive`, `is_search`, `is_404`, -`is_attachment`, `is_feed` and `is_main_query`. The capability interfaces' -namespace comes from `dependencyDirection.infrastructure`. - -**Fix:** implement `Delayed` and return `wp` or a later action from -`get_registration_action()`. The plugin waits for that action before asking -`is_needed()`, so the query has run when the condition reads it. See -[I need to hook in later](patterns.md#i-need-to-hook-in-later) and +`is_attachment`, `is_feed` and `is_main_query`. +`queryContextNeedsDelay.screenFunctions` lists the screen-dependent functions: +`get_current_screen`. Set `screenFunctions: []` to disable the screen check. +The capability interfaces' namespace comes from +`dependencyDirection.infrastructure`. + +**Fix:** for query-dependent conditions, implement `Delayed` and return `wp` +or a later action from `get_registration_action()`. The plugin waits for that +action before asking `is_needed()`, so the query has run when the condition +reads it. + +For screen-dependent conditions, return `current_screen` or a later admin action: +WordPress has not set the admin screen when services are normally checked at +`plugins_loaded`. The two checks have separate messages under the same +identifier. See [I need to hook in later](patterns.md#i-need-to-hook-in-later) and [I need a service only in some requests](patterns.md#i-need-a-service-only-in-some-requests). ### `architecture.registrationActionKnown` diff --git a/docs/patterns.md b/docs/patterns.md index 0da349c..90b5f59 100644 --- a/docs/patterns.md +++ b/docs/patterns.md @@ -150,6 +150,11 @@ When the answer depends on the query, such as `is_singular()`, also implement `Delayed` and wait for `wp` or a later action as shown in [I need to hook in later](#i-need-to-hook-in-later). +When the answer depends on the admin screen, such as `get_current_screen()`, +also implement `Delayed` and wait for `current_screen` or a later admin +action. WordPress has not set the screen at `plugins_loaded`, when the +plugin normally asks `is_needed()`. + **Rules:** `architecture.noContextChecksInRegister`, `architecture.queryContextNeedsDelay`. diff --git a/phpstan.neon.dist b/phpstan.neon.dist index e502803..942aecc 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -227,6 +227,9 @@ parameters: - is_attachment - is_feed - is_main_query + # The admin screen is available from current_screen onward. + screenFunctions: + - get_current_screen # The prefix of each kind of name the plugin creates, and the class # constants that hold it (design principle 13). Rename the prefixes # together with the plugin. diff --git a/tests/php/Architecture/RuleManifest.php b/tests/php/Architecture/RuleManifest.php index 2d0d484..14c72a1 100644 --- a/tests/php/Architecture/RuleManifest.php +++ b/tests/php/Architecture/RuleManifest.php @@ -450,12 +450,14 @@ final class RuleManifest { 'principle' => 'Principle 7: Show the alternative to WordPress anti-patterns', 'principle_anchor' => '7-show-the-alternative-to-wordpress-anti-patterns', 'pattern' => 'i-need-to-hook-in-later', - 'configuration' => 'parameters.architecture.queryContextNeedsDelay.queryFunctions', + 'configuration' => 'parameters.architecture.queryContextNeedsDelay.queryFunctions, ' + . 'parameters.architecture.queryContextNeedsDelay.screenFunctions', 'checked_by' => [ 'MWPD\\BasicScaffold\\Tests\\PHPStan\\Rule\\QueryContextNeedsDelayRule', ], 'signatures' => [ 'asks about the query before it is available', + 'asks about the admin screen before WordPress has set it', ], ], self::REGISTRATION_ACTION_KNOWN => [ diff --git a/tests/php/PHPStan/Fixture/Hooks/Feature/DelayedScreenConditional.php b/tests/php/PHPStan/Fixture/Hooks/Feature/DelayedScreenConditional.php new file mode 100644 index 0000000..b45fdef --- /dev/null +++ b/tests/php/PHPStan/Fixture/Hooks/Feature/DelayedScreenConditional.php @@ -0,0 +1,19 @@ + */ @@ -51,6 +51,13 @@ final class QueryContextNeedsDelayRule implements Rule { */ private array $query_functions; + /** + * Screen-dependent functions, in lowercase. + * + * @var array + */ + private array $screen_functions; + /** * Reflection, to tell which function a call refers to. */ @@ -63,6 +70,7 @@ final class QueryContextNeedsDelayRule implements Rule { * @param string $infrastructure Namespace of the capability * interfaces. * @param array $query_functions Query-dependent functions. + * @param array $screen_functions Screen-dependent functions. * @param ReflectionProvider $reflection Reflection, to tell which * function a call refers to. */ @@ -70,11 +78,13 @@ public function __construct( PluginNamespace $plugin_namespace, string $infrastructure, array $query_functions, + array $screen_functions, ReflectionProvider $reflection ) { $this->plugin_namespace = $plugin_namespace; $this->infrastructure = trim( $infrastructure, '\\' ); $this->query_functions = array_map( 'strtolower', $query_functions ); + $this->screen_functions = array_map( 'strtolower', $screen_functions ); $this->reflection = $reflection; } @@ -88,7 +98,7 @@ public function getNodeType(): string { } /** - * Report a query-dependent function called directly in is_needed(). + * Report a query- or screen-dependent function called directly in is_needed(). * * @param FuncCall $node Function call to inspect. * @param Scope $scope Scope of the call. @@ -110,16 +120,27 @@ public function processNode( Node $node, Scope $scope ): array { $called = CalledFunction::name_of( $node, $scope, $this->reflection ); - if ( $called === null || ! in_array( $called, $this->query_functions, true ) ) { + if ( $called === null ) { + return []; + } + + if ( in_array( $called, $this->query_functions, true ) ) { + $message = '%1$s() in %2$s asks about the query before it is available. Implement Delayed and return' + . ' "wp" or a later action from get_registration_action(), so the plugin waits for the query' + . ' before checking is_needed().'; + } elseif ( in_array( $called, $this->screen_functions, true ) ) { + $message = '%1$s() in %2$s asks about the admin screen before WordPress has set it.' + . ' Implement Delayed and return "current_screen" or a later admin action' + . ' from get_registration_action(),' + . ' so the plugin waits for the screen before checking is_needed().'; + } else { return []; } return [ RuleErrorBuilder::message( sprintf( - '%1$s() in %2$s asks about the query before it is available. Implement Delayed and return' - . ' "wp" or a later action from get_registration_action(), so the plugin waits for the query' - . ' before checking is_needed().', + $message, $called, CalledFunction::describe_location( $scope ) ) diff --git a/tests/php/PHPStan/Rule/QueryContextNeedsDelayRuleTest.php b/tests/php/PHPStan/Rule/QueryContextNeedsDelayRuleTest.php index f825bdd..b547f96 100644 --- a/tests/php/PHPStan/Rule/QueryContextNeedsDelayRuleTest.php +++ b/tests/php/PHPStan/Rule/QueryContextNeedsDelayRuleTest.php @@ -34,6 +34,13 @@ final class QueryContextNeedsDelayRuleTest extends RuleTestCase { */ private array $query_functions = [ 'IS_SINGULAR' ]; + /** + * Screen-dependent functions, including a mixed-case configuration entry. + * + * @var array + */ + private array $screen_functions = [ 'GET_CURRENT_SCREEN' ]; + /** * Namespaces outside the rule's scope. * @@ -46,6 +53,7 @@ protected function getRule(): Rule { new PluginNamespace( $this->root, $this->excluded ), $this->root . '\Infrastructure', $this->query_functions, + $this->screen_functions, $this->createReflectionProvider() ); } @@ -75,10 +83,52 @@ public function test_only_configured_functions_require_delayed(): void { ); } + public function test_screen_conditions_require_delayed(): void { + $this->query_functions = []; + + $this->analyse( + [ + self::FIXTURES . 'Feature/ScreenConditional.php', + self::FIXTURES . 'Feature/DelayedScreenConditional.php', + ], + [ $this->screen_error( 'get_current_screen', 'ScreenConditional' ) ] + ); + } + + public function test_only_configured_screen_functions_require_delayed(): void { + $this->screen_functions = [ 'IS_ADMIN' ]; + + $this->analyse( + [ + self::FIXTURES . 'Feature/ScreenConditional.php', + self::FIXTURES . 'Feature/AdminConditional.php', + ], + [ $this->screen_error( 'is_admin', 'AdminConditional' ) ] + ); + } + + public function test_an_empty_screen_functions_list_disables_the_screen_check(): void { + $this->screen_functions = []; + + $this->analyse( + [ + self::FIXTURES . 'Feature/ScreenConditional.php', + self::FIXTURES . 'Feature/QueryConditional.php', + ], + [ $this->error( 'is_singular', 'QueryConditional' ) ] + ); + } + public function test_excluded_namespaces_are_not_checked(): void { $this->excluded = [ self::HOOKS . '\Feature' ]; - $this->analyse( [ self::FIXTURES . 'Feature/QueryConditional.php' ], [] ); + $this->analyse( + [ + self::FIXTURES . 'Feature/QueryConditional.php', + self::FIXTURES . 'Feature/ScreenConditional.php', + ], + [] + ); } public function test_the_sample_loop_service_passes(): void { @@ -104,4 +154,20 @@ private function error( string $called, string $class_name ): array { . ' Rule: docs/architecture-rules.md#architecturequerycontextneedsdelay', ]; } + + /** + * @return array{string, int, string} + */ + private function screen_error( string $called, string $class_name ): array { + return [ + $called . '() in ' . self::HOOKS . '\\Feature\\' . $class_name . '::is_needed() asks about the admin screen' + . ' before WordPress has set it. Implement Delayed and return "current_screen" or a later admin action' + . ' from get_registration_action(), so the plugin waits for the screen before checking is_needed().', + 12, + 'Principle 7: Show the alternative to WordPress anti-patterns, see' + . ' docs/design-principles.md#7-show-the-alternative-to-wordpress-anti-patterns.' + . ' Pattern: docs/patterns.md#i-need-to-hook-in-later.' + . ' Rule: docs/architecture-rules.md#architecturequerycontextneedsdelay', + ]; + } } diff --git a/tests/php/PHPStan/rules.neon b/tests/php/PHPStan/rules.neon index abbd3ba..67673de 100644 --- a/tests/php/PHPStan/rules.neon +++ b/tests/php/PHPStan/rules.neon @@ -72,6 +72,7 @@ parametersSchema: ]) queryContextNeedsDelay: structure([ queryFunctions: listOf(string()) + screenFunctions: listOf(string()) ]) # An open map keeps every kind optional, including postTypes, taxonomies, # shortcodes, restRoutes, cron, adminPages, settings and ajax. @@ -390,6 +391,7 @@ services: plugin_namespace: @architecturePluginNamespace infrastructure: %architecture.dependencyDirection.infrastructure% query_functions: %architecture.queryContextNeedsDelay.queryFunctions% + screen_functions: %architecture.queryContextNeedsDelay.screenFunctions% reflection: @PHPStan\Reflection\ReflectionProvider tags: - phpstan.rules.rule From 1e4cd952e2d5b5394aab211117f4c2286c7aeb88 Mon Sep 17 00:00:00 2001 From: Alain Schlesser Date: Fri, 23 Oct 2026 21:36:16 +0200 Subject: [PATCH 2/2] Name the rule's configuration key like the other multi-key rules --- tests/php/Architecture/RuleManifest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/php/Architecture/RuleManifest.php b/tests/php/Architecture/RuleManifest.php index 14c72a1..3b79991 100644 --- a/tests/php/Architecture/RuleManifest.php +++ b/tests/php/Architecture/RuleManifest.php @@ -450,8 +450,7 @@ final class RuleManifest { 'principle' => 'Principle 7: Show the alternative to WordPress anti-patterns', 'principle_anchor' => '7-show-the-alternative-to-wordpress-anti-patterns', 'pattern' => 'i-need-to-hook-in-later', - 'configuration' => 'parameters.architecture.queryContextNeedsDelay.queryFunctions, ' - . 'parameters.architecture.queryContextNeedsDelay.screenFunctions', + 'configuration' => 'parameters.architecture.queryContextNeedsDelay', 'checked_by' => [ 'MWPD\\BasicScaffold\\Tests\\PHPStan\\Rule\\QueryContextNeedsDelayRule', ],