Ensure that attribute and action methods are deduplicated (#447)

* Ensure that attribute and action methods are deduplicated

* Remove a legacy attribute version check

* Add light tests for typehinting events to ease my mind

src/mantle/framework/events/class-discover-events.php

@@ -20,8 +20,6 @@
 
 /**
  * Discover events within a specific directory.
- *
- * @todo Add support for WordPress hooks using attributes.
  */
 class Discover_Events {
 	/**
@@ -80,23 +78,22 @@ protected static function get_listener_events( $listeners, string $base_path ):
 			}
 
 			foreach ( $listener->getMethods( ReflectionMethod::IS_PUBLIC ) as $method ) {
-				// Check for attribute support with PHP 8.
-				if ( version_compare( phpversion(), '8.0.0', '>=' ) ) {
-					// Check if the method has an attribute action.
-					$action_attributes = $method->getAttributes( Action::class );
-
-					if ( ! empty( $action_attributes ) ) {
-						foreach ( $action_attributes as $attribute ) {
-							$instance = $attribute->newInstance();
-
-							$listener_events[ $listener->name . '@' . $method->name ] = [
-								[ $instance->action ],
-								$instance->priority,
-							];
-						}
-
-						continue;
+				// Check if the method has an attribute action.
+				$action_attributes = $method->getAttributes( Action::class );
+
+				if ( ! empty( $action_attributes ) ) {
+					foreach ( $action_attributes as $attribute ) {
+						$instance = $attribute->newInstance();
+
+						$listener_events[ $listener->name . '@' . $method->name ] = [
+							[
+								$instance->hook_name,
+							],
+							$instance->priority,
+						];
 					}
+
+					continue;
 				}
 
 				// Handle WordPress hooks being registered with a listener.

src/mantle/support/attributes/class-action.php

@@ -12,15 +12,15 @@
 /**
  * Hook Action Attribute
  *
- * Used to hook a method to an WordPress action at a specific priority.
+ * Used to hook a method to an WordPress hook at a specific priority.
  */
 #[Attribute]
 class Action {
 	/**
 	 * Constructor.
 	 *
-	 * @param string $action Action name.
+	 * @param string $hook_name Hook name.
 	 * @param int    $priority Priority, defaults to 10.
 	 */
-	public function __construct( public string $action, public int $priority = 10 ) {}
+	public function __construct( public string $hook_name, public int $priority = 10 ) {}
 }

src/mantle/support/class-service-provider.php

@@ -80,24 +80,36 @@ public function boot_provider() {
 		}
 
 		$this->boot_action_hooks();
-		$this->boot_attribute_hooks();
 		$this->boot();
 	}
 
 	/**
-	 * Boot all actions on the service provider.
+	 * Boot all actions and attribute methods on the service provider.
 	 *
-	 * Allow methods in the 'on_{hook}_at_priority' and 'on_{hook}' format
-	 * to automatically register WordPress hooks.
+	 * Collects all of the `on_{hook}` and `on_{hook}_at_{priority}` methods as
+	 * well as the attribute based `#[Action]` methods and registers them with
+	 * the respective WordPress hooks.
 	 */
-	protected function boot_action_hooks() {
-		collect( get_class_methods( static::class ) )
+	protected function boot_action_hooks(): void {
+		$this->collect_action_methods()
+			->merge( $this->collect_attribute_hooks() )
+			->unique()
+			->each(
+				fn ( array $item ) => add_action( $item['hook'], [ $this, $item['method'] ], $item['priority'] ),
+			);
+	}
+
+	/**
+	 * Collect all action methods from the service provider.
+	 *
+	 * @return Collection<int, array{hook: string, method: string, priority: int}>
+	 */
+	protected function collect_action_methods(): Collection {
+		return collect( get_class_methods( static::class ) )
 			->filter(
-				function( string $method ) {
-					return Str::starts_with( $method, 'on_' );
-				}
+				fn ( string $method ) => Str::starts_with( $method, 'on_' )
 			)
-			->each(
+			->map(
 				function( string $method ) {
 					$hook     = Str::after( $method, 'on_' );
 					$priority = 10;
@@ -108,30 +120,42 @@ function( string $method ) {
 						$hook     = Str::before_last( $hook, '_at_' );
 					}
 
-					add_action( $hook, [ $this, $method ], $priority );
+					return [
+						'hook'     => $hook,
+						'method'   => $method,
+						'priority' => $priority,
+					];
 				}
 			);
 	}
 
 	/**
-	 * Boot all attribute actions on the service provider.
+	 * Collect all attribute actions on the service provider.
+	 *
+	 * Allow methods with the `#[Action]` attribute to automatically register
+	 * WordPress hooks.
+	 *
+	 * @return Collection<int, array{hook: string, method: string, priority: int}>
 	 */
-	protected function boot_attribute_hooks() {
+	protected function collect_attribute_hooks(): Collection {
+		$items = new Collection();
 		$class = new ReflectionClass( static::class );
 
 		foreach ( $class->getMethods() as $method ) {
-			$action_attributes = $method->getAttributes( Action::class );
-
-			if ( empty( $action_attributes ) ) {
-				continue;
-			}
-
-			foreach ( $action_attributes as $attribute ) {
+			foreach ( $method->getAttributes( Action::class ) as $attribute ) {
 				$instance = $attribute->newInstance();
 
-				add_action( $instance->action, [ $this, $method->name ], $instance->priority );
+				$items->push(
+					[
+						'hook'     => $instance->hook_name,
+						'method'   => $method->getName(),
+						'priority' => $instance->priority,
+					]
+				);
 			}
 		}
+
+		return $items;
 	}
 
 	/**

tests/events/test-wordpress-action-dispatcher.php

@@ -170,4 +170,24 @@ function() {
 		do_action( 'test_action_to_fire' );
 		$this->assertTrue( $_SERVER['__test'] );
 	}
+
+	public function test_typehint_action_argument() {
+		$_SERVER['__test'] = false;
+
+		add_action(
+			Example_Typehint_Event::class,
+			function ( Example_Typehint_Event $event ): void {
+				$_SERVER['__test'] = $event;
+			}
+		);
+
+		$this->app['events']->dispatch( new Example_Typehint_Event( 'test' ) );
+
+		$this->assertInstanceOf( Example_Typehint_Event::class, $_SERVER['__test'] );
+		$this->assertEquals( 'test', $_SERVER['__test']->example );
+	}
+}
+
+class Example_Typehint_Event {
+	public function __construct( public string $example ) {}
 }

tests/support/test-service-provider.php

@@ -4,6 +4,7 @@
 use Mantle\Application\Application;
 use Mantle\Console\Command;
 use Mantle\Contracts\Providers as ProviderContracts;
+use Mantle\Events\Dispatcher;
 use Mantle\Support\Service_Provider;
 use Mantle\Support\Attributes\Action;
 use Mockery as m;
@@ -14,6 +15,7 @@ protected function setUp(): void {
 
 		remove_all_actions( 'init' );
 		remove_all_filters( 'custom_filter' );
+		remove_all_filters( 'custom_filter_dedupe' );
 
 		Service_Provider::$publishes = [];
 		Service_Provider::$publish_tags = [];
@@ -66,12 +68,6 @@ public function test_hook_method_filter() {
 	}
 
 	public function test_hook_attribute() {
-		// Abandon if we're not running PHP 8.
-		if ( version_compare( phpversion(), '8.0.0', '<' ) ) {
-			$this->markTestSkipped( 'Requires PHP 8.0.0 or greater.' );
-			return;
-		}
-
 		$app = m::mock( Application::class )->makePartial();
 		$app->register( Provider_Test_Hook::class );
 		$app->boot();
@@ -81,6 +77,30 @@ public function test_hook_attribute() {
 		$this->assertTrue( $_SERVER['__custom_hook_fired'] ?? false );
 	}
 
+	public function test_hook_attribute_deduplicate() {
+		$app = m::mock( Application::class )->makePartial();
+		$app->register( Provider_Test_Hook::class );
+		$app->boot();
+
+		$value = apply_filters( 'custom_filter_dedupe', 0 );
+
+		$this->assertEquals( 10, $value );
+	}
+
+	public function test_typehint_event() {
+		$_SERVER['__custom_event_fired'] = false;
+
+		$app = m::mock( Application::class )->makePartial();
+		$app->register( Provider_Test_Hook::class );
+		$app->boot();
+
+		$app['events'] = new Dispatcher( $app );
+
+		$app['events']->dispatch( new Example_Service_Provider_Event() );
+
+		$this->assertInstanceOf( Example_Service_Provider_Event::class, $_SERVER['__custom_event_fired'] );
+	}
+
 	public function test_publishable_service_providers() {
 		$app = m::mock( Application::class )->makePartial();
 		$app->register( ServiceProviderForTestingOne::class );
@@ -248,8 +268,15 @@ public function handle_custom_hook() {
 		$_SERVER['__custom_hook_fired'] = true;
 	}
 
-	public function handle_custom_filter( $value ) {
-		return $value + 100;
+	// Assert that only a single action is registered for this hook.
+	#[Action('custom_filter_dedupe')]
+	public function on_custom_filter_dedupe( $value ) {
+		return $value + 10;
+	}
+
+	#[Action(Example_Service_Provider_Event::class)]
+	public function handle_custom_event( Example_Service_Provider_Event $event ) {
+		$_SERVER['__custom_event_fired'] = $event;
 	}
 }
 
@@ -270,3 +297,7 @@ public function boot() {
 		$this->publishes( [ 'source/tagged/two/b' => 'destination/tagged/two/b' ], 'some_tag' );
 	}
 }
+
+class Example_Service_Provider_Event {
+
+}