From 5a1d51e09a126811ec21e824dd12d4efc859341d Mon Sep 17 00:00:00 2001 From: Dovid Levine Date: Sat, 16 Aug 2025 17:48:17 +0300 Subject: [PATCH 1/7] dev: require `string $name` for registration --- includes/abilities-api.php | 16 ++++----- .../class-wp-abilities-registry.php | 24 ++++--------- .../abilities-api/wpAbilitiesRegistry.php | 36 +++---------------- 3 files changed, 19 insertions(+), 57 deletions(-) diff --git a/includes/abilities-api.php b/includes/abilities-api.php index ae538a2a..9772f688 100644 --- a/includes/abilities-api.php +++ b/includes/abilities-api.php @@ -20,15 +20,15 @@ * * @since 0.1.0 * - * @param string|\WP_Ability $name The name of the ability, or WP_Ability instance. - * The name must be a string containing a namespace prefix, i.e. `my-plugin/my-ability`. It can only - * contain lowercase alphanumeric characters, dashes and the forward slash. - * @param array $properties Optional. An associative array of properties for the ability. This should - * include `label`, `description`, `input_schema`, `output_schema`, - * `execute_callback`, `permission_callback`, and `meta`. + * @param string $name The name of the ability. The name must be a string containing a namespace + * prefix, i.e. `my-plugin/my-ability`. It can only contain lowercase + * alphanumeric characters, dashes and the forward slash. + * @param array $properties An associative array of properties for the ability. This should include + * `label`, `description`, `input_schema`, `output_schema`, `execute_callback`, + * `permission_callback`, and `meta`. * @return ?\WP_Ability An instance of registered ability on success, null on failure. */ -function wp_register_ability( $name, array $properties = array() ): ?WP_Ability { +function wp_register_ability( string $name, array $properties = array() ): ?WP_Ability { if ( ! did_action( 'abilities_api_init' ) ) { _doing_it_wrong( __FUNCTION__, @@ -36,7 +36,7 @@ function wp_register_ability( $name, array $properties = array() ): ?WP_Ability /* translators: 1: abilities_api_init, 2: string value of the ability name. */ esc_html__( 'Abilities must be registered on the %1$s action. The ability %2$s was not registered.' ), 'abilities_api_init', - '' . esc_html( $name instanceof WP_Ability ? $name->get_name() : $name ) . '' + '' . esc_html( $name ) . '' ), '0.1.0' ); diff --git a/includes/abilities-api/class-wp-abilities-registry.php b/includes/abilities-api/class-wp-abilities-registry.php index 0e72925a..4e6d6eb1 100644 --- a/includes/abilities-api/class-wp-abilities-registry.php +++ b/includes/abilities-api/class-wp-abilities-registry.php @@ -35,21 +35,15 @@ final class WP_Abilities_Registry { * * @since 0.1.0 * - * @param string|\WP_Ability $name The name of the ability, or WP_Ability instance. The name must be a string - * containing a namespace prefix, i.e. `my-plugin/my-ability`. It can only - * contain lowercase alphanumeric characters, dashes and the forward slash. - * @param array $properties Optional. An associative array of properties for the ability. This should - * include `label`, `description`, `input_schema`, `output_schema`, + * @param string $name The name of the ability. The name must be a string containing a namespace + * prefix, i.e. `my-plugin/my-ability`. It can only contain lowercase + * alphanumeric characters, dashes and the forward slash. + * @param array $properties An associative array of properties for the ability. This should include + * `label`, `description`, `input_schema`, `output_schema`, * `execute_callback`, `permission_callback`, and `meta`. * @return ?\WP_Ability The registered ability instance on success, null on failure. */ - public function register( $name, array $properties = array() ): ?WP_Ability { - $ability = null; - if ( $name instanceof WP_Ability ) { - $ability = $name; - $name = $ability->get_name(); - } - + public function register( string $name, array $properties = array() ): ?WP_Ability { if ( ! preg_match( '/^[a-z0-9-]+\/[a-z0-9-]+$/', $name ) ) { _doing_it_wrong( __METHOD__, @@ -71,12 +65,6 @@ public function register( $name, array $properties = array() ): ?WP_Ability { return null; } - // If the ability is already an instance, we can skip the rest of the validation. - if ( null !== $ability ) { - $this->registered_abilities[ $name ] = $ability; - return $ability; - } - if ( empty( $properties['label'] ) || ! is_string( $properties['label'] ) ) { _doing_it_wrong( __METHOD__, diff --git a/tests/unit/abilities-api/wpAbilitiesRegistry.php b/tests/unit/abilities-api/wpAbilitiesRegistry.php index 523f5398..8de3a29b 100644 --- a/tests/unit/abilities-api/wpAbilitiesRegistry.php +++ b/tests/unit/abilities-api/wpAbilitiesRegistry.php @@ -113,10 +113,12 @@ public function test_register_invalid_uppercase_characters_in_name() { * * @expectedIncorrectUsage WP_Abilities_Registry::register */ - public function test_register_invalid_name_using_instance() { + public function test_register_using_instance() { $ability = new WP_Ability( 'invalid_name', array() ); - $result = $this->registry->register( $ability ); - $this->assertNull( $result ); + // Expect error + $this->expectException( \TypeError::class ); + + $this->registry->register( $ability ); } /** @@ -277,21 +279,6 @@ public function test_register_incorrect_already_registered_ability() { $this->assertNull( $result ); } - /** - * Should reject registration for already registered ability when passing an ability instance. - * - * @covers WP_Abilities_Registry::register - * - * @expectedIncorrectUsage WP_Abilities_Registry::register - */ - public function test_register_incorrect_already_registered_ability_using_instance() { - $ability = $this->registry->register( self::$test_ability_name, self::$test_ability_properties ); - - $result = $this->registry->register( $ability ); - - $this->assertNull( $result ); - } - /** * Should successfully register a new ability. * @@ -306,19 +293,6 @@ public function test_register_new_ability() { ); } - /** - * Should successfully register a new ability using an instance. - * - * @covers WP_Abilities_Registry::register - * @covers WP_Ability::construct - */ - public function test_register_new_ability_using_instance() { - $ability = new WP_Ability( self::$test_ability_name, self::$test_ability_properties ); - $result = $this->registry->register( $ability ); - - $this->assertSame( $ability, $result ); - } - /** * Should return false for ability that's not registered. * From 9c361b986c8b19f9e048c563645e710b79b8151b Mon Sep 17 00:00:00 2001 From: Dovid Levine Date: Sat, 16 Aug 2025 19:31:36 +0300 Subject: [PATCH 2/7] tests: remove annotation --- tests/unit/abilities-api/wpAbilitiesRegistry.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/abilities-api/wpAbilitiesRegistry.php b/tests/unit/abilities-api/wpAbilitiesRegistry.php index 8de3a29b..15fe3a63 100644 --- a/tests/unit/abilities-api/wpAbilitiesRegistry.php +++ b/tests/unit/abilities-api/wpAbilitiesRegistry.php @@ -110,8 +110,6 @@ public function test_register_invalid_uppercase_characters_in_name() { * Should reject ability instance with invalid name. * * @covers WP_Abilities_Registry::register - * - * @expectedIncorrectUsage WP_Abilities_Registry::register */ public function test_register_using_instance() { $ability = new WP_Ability( 'invalid_name', array() ); From 39bf7c9cefce2f1c21821ec5eb96591b97fd74af Mon Sep 17 00:00:00 2001 From: Dovid Levine Date: Wed, 20 Aug 2025 21:14:23 +0300 Subject: [PATCH 3/7] dev: add `wp_ability_class` filter and pass-through `$properties` --- .../class-wp-abilities-registry.php | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/includes/abilities-api/class-wp-abilities-registry.php b/includes/abilities-api/class-wp-abilities-registry.php index db4526aa..6e8d4342 100644 --- a/includes/abilities-api/class-wp-abilities-registry.php +++ b/includes/abilities-api/class-wp-abilities-registry.php @@ -147,17 +147,23 @@ public function register( string $name, array $properties = array() ): ?WP_Abili return null; } - $ability = new WP_Ability( + /** + * Filters the class used to instantiate the ability. + * + * This is helpful for creating complex ability types that are not well expressed by array values and callbacks. + * + * @param string $ability_class The class name to instantiate the ability. Must extend \WP_Ability. + * @param string $name The name of the ability being registered. + * @param array $properties The properties of the ability being registered. See + * `wp_register_ability()` for the expected structure. + * + * @phpstan-param class-string<\WP_Ability> $ability_class + */ + $ability_class = apply_filters( 'wp_ability_class', WP_Ability::class, $name, $properties ); + + $ability = new $ability_class( $name, - array( - 'label' => $properties['label'], - 'description' => $properties['description'], - 'input_schema' => $properties['input_schema'] ?? array(), - 'output_schema' => $properties['output_schema'] ?? array(), - 'execute_callback' => $properties['execute_callback'], - 'permission_callback' => $properties['permission_callback'] ?? null, - 'meta' => $properties['meta'] ?? array(), - ) + $properties ); $this->registered_abilities[ $name ] = $ability; From fd99434ef74f2f666a127d77f40b15177e097940 Mon Sep 17 00:00:00 2001 From: Dovid Levine Date: Thu, 21 Aug 2025 00:28:44 +0300 Subject: [PATCH 4/7] docs: limit `WP_Ability`'s `@private` to constructor --- includes/abilities-api/class-wp-ability.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/includes/abilities-api/class-wp-ability.php b/includes/abilities-api/class-wp-ability.php index 2f368552..3bf9178f 100644 --- a/includes/abilities-api/class-wp-ability.php +++ b/includes/abilities-api/class-wp-ability.php @@ -15,7 +15,6 @@ * Encapsulates the properties and methods related to a specific ability in the registry. * * @since 0.1.0 - * @access private * * @see WP_Abilities_Registry */ @@ -91,6 +90,8 @@ class WP_Ability { * * Do not use this constructor directly. Instead, use the `wp_register_ability()` function. * + * @access private + * * @see wp_register_ability() * * @since 0.1.0 From ddd5786688124e07474bc38de36aeea75c8767c3 Mon Sep 17 00:00:00 2001 From: Dovid Levine Date: Thu, 21 Aug 2025 15:11:51 +0300 Subject: [PATCH 5/7] dev: use `_doing_it_wrong()` for invalid WP_Ability props. --- includes/abilities-api/class-wp-ability.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/includes/abilities-api/class-wp-ability.php b/includes/abilities-api/class-wp-ability.php index 3bf9178f..64208351 100644 --- a/includes/abilities-api/class-wp-ability.php +++ b/includes/abilities-api/class-wp-ability.php @@ -109,11 +109,28 @@ class WP_Ability { * execute_callback: callable( array $input): (mixed|\WP_Error), * permission_callback?: ?callable( ?array $input ): bool, * meta?: array, + * ..., * } $properties */ public function __construct( string $name, array $properties ) { $this->name = $name; + foreach ( $properties as $property_name => $property_value ) { + if ( ! property_exists( $this, $property_name ) ) { + _doing_it_wrong( + __METHOD__, + sprintf( + /* translators: %s: Property name. */ + esc_html__( 'Property "%1$s" is not a valid property for ability "%2$s". Please check the %3$s class for allowed properties.' ), + '' . esc_html( $property_name ) . '', + '' . esc_html( $this->name ) . '', + '' . esc_html( self::class ) . '' + ), + '0.1.0' + ); + continue; + } + $this->$property_name = $property_value; } } From 65dc4c98c7656bd13d1a2923dc820f0060efcb80 Mon Sep 17 00:00:00 2001 From: Dovid Levine Date: Thu, 21 Aug 2025 15:14:46 +0300 Subject: [PATCH 6/7] tests: remove now-unnecessary `test_register_using_instance` --- tests/unit/abilities-api/wpAbilitiesRegistry.php | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tests/unit/abilities-api/wpAbilitiesRegistry.php b/tests/unit/abilities-api/wpAbilitiesRegistry.php index 15fe3a63..77610e2b 100644 --- a/tests/unit/abilities-api/wpAbilitiesRegistry.php +++ b/tests/unit/abilities-api/wpAbilitiesRegistry.php @@ -106,19 +106,6 @@ public function test_register_invalid_uppercase_characters_in_name() { $this->assertNull( $result ); } - /** - * Should reject ability instance with invalid name. - * - * @covers WP_Abilities_Registry::register - */ - public function test_register_using_instance() { - $ability = new WP_Ability( 'invalid_name', array() ); - // Expect error - $this->expectException( \TypeError::class ); - - $this->registry->register( $ability ); - } - /** * Should reject ability registration without a label. * From d40a6f5f91c0f5f4a039d96b5e85049c4138e3e5 Mon Sep 17 00:00:00 2001 From: Dovid Levine Date: Mon, 25 Aug 2025 21:02:37 +0300 Subject: [PATCH 7/7] dev: use `$properties['ability_class']` instead of `wp_ability_class` filter --- includes/abilities-api.php | 3 +- .../class-wp-abilities-registry.php | 28 +++++------ .../unit/abilities-api/wpRegisterAbility.php | 50 +++++++++++++++++++ 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/includes/abilities-api.php b/includes/abilities-api.php index 9a2832ac..c37cd5a6 100644 --- a/includes/abilities-api.php +++ b/includes/abilities-api.php @@ -25,7 +25,7 @@ * alphanumeric characters, dashes and the forward slash. * @param array $properties An associative array of properties for the ability. This should include * `label`, `description`, `input_schema`, `output_schema`, `execute_callback`, - * `permission_callback`, and `meta`. + * `permission_callback`, `meta`, and `ability_class`. * @return ?\WP_Ability An instance of registered ability on success, null on failure. * * @phpstan-param array{ @@ -36,6 +36,7 @@ * execute_callback?: callable( array $input): (mixed|\WP_Error), * permission_callback?: callable( ?array $input ): bool, * meta?: array, + * ability_class?: class-string<\WP_Ability>, * ... * } $properties */ diff --git a/includes/abilities-api/class-wp-abilities-registry.php b/includes/abilities-api/class-wp-abilities-registry.php index 6e8d4342..39f1b930 100644 --- a/includes/abilities-api/class-wp-abilities-registry.php +++ b/includes/abilities-api/class-wp-abilities-registry.php @@ -48,7 +48,7 @@ final class WP_Abilities_Registry { * alphanumeric characters, dashes and the forward slash. * @param array $properties An associative array of properties for the ability. This should include * `label`, `description`, `input_schema`, `output_schema`, - * `execute_callback`, `permission_callback`, and `meta`. + * `execute_callback`, `permission_callback`, `meta`, and ability_class. * @return ?\WP_Ability The registered ability instance on success, null on failure. * * @phpstan-param array{ @@ -59,6 +59,7 @@ final class WP_Abilities_Registry { * execute_callback?: callable( array $input): (mixed|\WP_Error), * permission_callback?: ?callable( ?array $input ): bool, * meta?: array, + * ability_class?: class-string<\WP_Ability>, * ... * } $properties */ @@ -147,19 +148,18 @@ public function register( string $name, array $properties = array() ): ?WP_Abili return null; } - /** - * Filters the class used to instantiate the ability. - * - * This is helpful for creating complex ability types that are not well expressed by array values and callbacks. - * - * @param string $ability_class The class name to instantiate the ability. Must extend \WP_Ability. - * @param string $name The name of the ability being registered. - * @param array $properties The properties of the ability being registered. See - * `wp_register_ability()` for the expected structure. - * - * @phpstan-param class-string<\WP_Ability> $ability_class - */ - $ability_class = apply_filters( 'wp_ability_class', WP_Ability::class, $name, $properties ); + if ( isset( $properties['ability_class'] ) && ! is_a( $properties['ability_class'], WP_Ability::class, true ) ) { + _doing_it_wrong( + __METHOD__, + esc_html__( 'The ability properties should provide a valid `ability_class` that extends WP_Ability.' ), + '0.1.0' + ); + return null; + } + + // The class is only used to instantiate the ability, and is not a property of the ability itself. + $ability_class = $properties['ability_class'] ?? WP_Ability::class; + unset( $properties['ability_class'] ); $ability = new $ability_class( $name, diff --git a/tests/unit/abilities-api/wpRegisterAbility.php b/tests/unit/abilities-api/wpRegisterAbility.php index 1668d55d..2a5d03a0 100644 --- a/tests/unit/abilities-api/wpRegisterAbility.php +++ b/tests/unit/abilities-api/wpRegisterAbility.php @@ -1,5 +1,14 @@ assertEquals( 'ability_invalid_permissions', $actual->get_error_code() ); } + /** + * Tests registering an ability with a custom ability class. + */ + public function test_register_ability_custom_ability_class(): void { + do_action( 'abilities_api_init' ); + + $result = wp_register_ability( + self::$test_ability_name, + array_merge( + self::$test_ability_properties, + array( + 'ability_class' => Mock_Custom_Ability::class, + ) + ) + ); + + $this->assertInstanceOf( Mock_Custom_Ability::class, $result ); + $this->assertSame( + 9999, + $result->execute( + array( + 'a' => 2, + 'b' => 3, + ) + ) + ); + + // Try again with an invalid class throws a doing it wrong. + $this->setExpectedIncorrectUsage( WP_Abilities_Registry::class . '::register' ); + wp_register_ability( + self::$test_ability_name, + array_merge( + self::$test_ability_properties, + array( + 'ability_class' => 'Non_Existent_Class', + ) + ) + ); + } + + /** * Tests executing an ability with input not matching schema. */