Reduce code duplication

src/event/AsyncHandlerList.php

@@ -23,119 +23,13 @@ declare(strict_types=1);
 
 namespace pocketmine\event;
 
-use pocketmine\plugin\Plugin;
-use function array_merge;
-use function krsort;
-use function spl_object_id;
 use function uasort;
-use const SORT_NUMERIC;
-
-class AsyncHandlerList{
-	//TODO: we can probably deduplicate most of this code with the sync side if we throw in some generics
-
-	/** @var AsyncRegisteredListener[][] */
-	private array $handlerSlots = [];
 
 /**
-	 * @var RegisteredListenerCache[]
+ * @phpstan-extends BaseHandlerList<AsyncRegisteredListener, AsyncEvent>
-	 * @phpstan-var array<int, RegisteredListenerCache<AsyncRegisteredListener>>
  */
-	private array $affectedHandlerCaches = [];
+class AsyncHandlerList extends BaseHandlerList{
-
+	protected function sortSamePriorityListeners(array $listeners) : array{
-	/**
-	 * @phpstan-param class-string<covariant AsyncEvent> $class
-	 * @phpstan-param RegisteredListenerCache<AsyncRegisteredListener> $handlerCache
-	 */
-	public function __construct(
-		private string $class,
-		private ?AsyncHandlerList $parentList,
-		private RegisteredListenerCache $handlerCache = new RegisteredListenerCache()
-	){
-		for($list = $this; $list !== null; $list = $list->parentList){
-			$list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache;
-		}
-	}
-
-	/**
-	 * @throws \Exception
-	 */
-	public function register(AsyncRegisteredListener $listener) : void{
-		if(isset($this->handlerSlots[$listener->getPriority()][spl_object_id($listener)])){
-			throw new \InvalidArgumentException("This listener is already registered to priority {$listener->getPriority()} of event {$this->class}");
-		}
-		$this->handlerSlots[$listener->getPriority()][spl_object_id($listener)] = $listener;
-		$this->invalidateAffectedCaches();
-	}
-
-	/**
-	 * @param AsyncRegisteredListener[] $listeners
-	 */
-	public function registerAll(array $listeners) : void{
-		foreach($listeners as $listener){
-			$this->register($listener);
-		}
-		$this->invalidateAffectedCaches();
-	}
-
-	public function unregister(AsyncRegisteredListener|Plugin|Listener $object) : void{
-		if($object instanceof Plugin || $object instanceof Listener){
-			foreach($this->handlerSlots as $priority => $list){
-				foreach($list as $hash => $listener){
-					if(($object instanceof Plugin && $listener->getPlugin() === $object)
-						|| ($object instanceof Listener && (new \ReflectionFunction($listener->getHandler()))->getClosureThis() === $object) //this doesn't even need to be a listener :D
-					){
-						unset($this->handlerSlots[$priority][$hash]);
-					}
-				}
-			}
-		}else{
-			unset($this->handlerSlots[$object->getPriority()][spl_object_id($object)]);
-		}
-		$this->invalidateAffectedCaches();
-	}
-
-	public function clear() : void{
-		$this->handlerSlots = [];
-		$this->invalidateAffectedCaches();
-	}
-
-	/**
-	 * @return AsyncRegisteredListener[]
-	 */
-	public function getListenersByPriority(int $priority) : array{
-		return $this->handlerSlots[$priority] ?? [];
-	}
-
-	public function getParent() : ?AsyncHandlerList{
-		return $this->parentList;
-	}
-
-	/**
-	 * Invalidates all known caches which might be affected by this list's contents.
-	 */
-	private function invalidateAffectedCaches() : void{
-		foreach($this->affectedHandlerCaches as $cache){
-			$cache->list = null;
-		}
-	}
-
-	/**
-	 * @return AsyncRegisteredListener[]
-	 * @phpstan-return list<AsyncRegisteredListener>
-	 */
-	public function getListenerList() : array{
-		if($this->handlerCache->list !== null){
-			return $this->handlerCache->list;
-		}
-
-		$handlerLists = [];
-		for($currentList = $this; $currentList !== null; $currentList = $currentList->parentList){
-			$handlerLists[] = $currentList;
-		}
-
-		$listenersByPriority = [];
-		foreach($handlerLists as $currentList){
-			foreach($currentList->handlerSlots as $priority => $listeners){
 		uasort($listeners, function(AsyncRegisteredListener $left, AsyncRegisteredListener $right) : int{
 			//While the system can handle these in any order, it's better for latency if concurrent handlers
 			//are processed together. It doesn't matter whether they are processed before or after exclusive handlers.
@@ -144,13 +38,6 @@ class AsyncHandlerList{
 			}
 			return -1;
 		});
-				$listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $listeners);
+		return $listeners;
-			}
-		}
-
-		//TODO: why on earth do the priorities have higher values for lower priority?
-		krsort($listenersByPriority, SORT_NUMERIC);
-
-		return $this->handlerCache->list = array_merge(...$listenersByPriority);
 	}
 }

src/event/AsyncRegisteredListener.php

@@ -26,32 +26,17 @@ namespace pocketmine\event;
 use pocketmine\plugin\Plugin;
 use pocketmine\promise\Promise;
 use pocketmine\timings\TimingsHandler;
-use function in_array;
 
-class AsyncRegisteredListener{
+class AsyncRegisteredListener extends BaseRegisteredListener{
 	public function __construct(
-		private \Closure $handler,
+		\Closure $handler,
-		private int $priority,
+		int $priority,
-		private Plugin $plugin,
+		Plugin $plugin,
-		private bool $handleCancelled,
+		bool $handleCancelled,
 		private bool $exclusiveCall,
-		private TimingsHandler $timings
+		TimingsHandler $timings
 	){
-		if(!in_array($priority, EventPriority::ALL, true)){
+		parent::__construct($handler, $priority, $plugin, $handleCancelled, $timings);
-			throw new \InvalidArgumentException("Invalid priority number $priority");
-		}
-	}
-
-	public function getHandler() : \Closure{
-		return $this->handler;
-	}
-
-	public function getPlugin() : Plugin{
-		return $this->plugin;
-	}
-
-	public function getPriority() : int{
-		return $this->priority;
 	}
 
 	/**
@@ -69,9 +54,6 @@ class AsyncRegisteredListener{
 		}
 	}
 
-	public function isHandlingCancelled() : bool{
-		return $this->handleCancelled;
-	}
 	public function canBeCalledConcurrently() : bool{
 		return !$this->exclusiveCall;
 	}

src/event/BaseHandlerList.php

@@ -0,0 +1,170 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\event;
+
+use pocketmine\plugin\Plugin;
+use function array_merge;
+use function krsort;
+use function spl_object_id;
+use const SORT_NUMERIC;
+
+/**
+ * @phpstan-template TListener of BaseRegisteredListener
+ * @phpstan-template TEvent
+ */
+abstract class BaseHandlerList{
+	/**
+	 * @var BaseRegisteredListener[][]
+	 * @phpstan-var array<int, array<int, TListener>>
+	 */
+	private array $handlerSlots = [];
+
+	/**
+	 * @var RegisteredListenerCache[]
+	 * @phpstan-var array<int, RegisteredListenerCache<TListener>>
+	 */
+	private array $affectedHandlerCaches = [];
+
+	/**
+	 * @phpstan-param class-string<covariant TEvent> $class
+	 * @phpstan-param ?static<TListener, TEvent> $parentList
+	 * @phpstan-param RegisteredListenerCache<TListener> $handlerCache
+	 */
+	public function __construct(
+		private string $class,
+		private ?BaseHandlerList $parentList,
+		private RegisteredListenerCache $handlerCache = new RegisteredListenerCache()
+	){
+		for($list = $this; $list !== null; $list = $list->parentList){
+			$list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache;
+		}
+	}
+
+	/**
+	 * @phpstan-param TListener $listener
+	 */
+	public function register(BaseRegisteredListener $listener) : void{
+		if(isset($this->handlerSlots[$listener->getPriority()][spl_object_id($listener)])){
+			throw new \InvalidArgumentException("This listener is already registered to priority {$listener->getPriority()} of event {$this->class}");
+		}
+		$this->handlerSlots[$listener->getPriority()][spl_object_id($listener)] = $listener;
+		$this->invalidateAffectedCaches();
+	}
+
+	/**
+	 * @param BaseRegisteredListener[] $listeners
+	 * @phpstan-param array<TListener> $listeners
+	 */
+	public function registerAll(array $listeners) : void{
+		foreach($listeners as $listener){
+			$this->register($listener);
+		}
+		$this->invalidateAffectedCaches();
+	}
+
+	/**
+	 * @phpstan-param TListener|Plugin|Listener $object
+	 */
+	public function unregister(BaseRegisteredListener|Plugin|Listener $object) : void{
+		if($object instanceof Plugin || $object instanceof Listener){
+			foreach($this->handlerSlots as $priority => $list){
+				foreach($list as $hash => $listener){
+					if(($object instanceof Plugin && $listener->getPlugin() === $object)
+						|| ($object instanceof Listener && (new \ReflectionFunction($listener->getHandler()))->getClosureThis() === $object) //this doesn't even need to be a listener :D
+					){
+						unset($this->handlerSlots[$priority][$hash]);
+					}
+				}
+			}
+		}else{
+			unset($this->handlerSlots[$object->getPriority()][spl_object_id($object)]);
+		}
+		$this->invalidateAffectedCaches();
+	}
+
+	public function clear() : void{
+		$this->handlerSlots = [];
+		$this->invalidateAffectedCaches();
+	}
+
+	/**
+	 * @return BaseRegisteredListener[]
+	 * @phpstan-return array<int, TListener>
+	 */
+	public function getListenersByPriority(int $priority) : array{
+		return $this->handlerSlots[$priority] ?? [];
+	}
+
+	/**
+	 * @phpstan-return static<TListener, TEvent>
+	 */
+	public function getParent() : ?BaseHandlerList{
+		return $this->parentList;
+	}
+
+	/**
+	 * Invalidates all known caches which might be affected by this list's contents.
+	 */
+	private function invalidateAffectedCaches() : void{
+		foreach($this->affectedHandlerCaches as $cache){
+			$cache->list = null;
+		}
+	}
+
+	/**
+	 * @param BaseRegisteredListener[] $listeners
+	 * @phpstan-param array<int, TListener> $listeners
+	 *
+	 * @return BaseRegisteredListener[]
+	 * @phpstan-return array<int, TListener>
+	 */
+	abstract protected function sortSamePriorityListeners(array $listeners) : array;
+
+	/**
+	 * @return BaseRegisteredListener[]
+	 * @phpstan-return list<TListener>
+	 */
+	public function getListenerList() : array{
+		if($this->handlerCache->list !== null){
+			return $this->handlerCache->list;
+		}
+
+		$handlerLists = [];
+		for($currentList = $this; $currentList !== null; $currentList = $currentList->parentList){
+			$handlerLists[] = $currentList;
+		}
+
+		$listenersByPriority = [];
+		foreach($handlerLists as $currentList){
+			foreach($currentList->handlerSlots as $priority => $listeners){
+				$listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $this->sortSamePriorityListeners($listeners));
+			}
+		}
+
+		//TODO: why on earth do the priorities have higher values for lower priority?
+		krsort($listenersByPriority, SORT_NUMERIC);
+
+		return $this->handlerCache->list = array_merge(...$listenersByPriority);
+	}
+}

src/event/BaseRegisteredListener.php

@@ -0,0 +1,58 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\event;
+
+use pocketmine\plugin\Plugin;
+use pocketmine\timings\TimingsHandler;
+use function in_array;
+
+abstract class BaseRegisteredListener{
+	public function __construct(
+		protected \Closure $handler,
+		private int $priority,
+		private Plugin $plugin,
+		private bool $handleCancelled,
+		protected TimingsHandler $timings
+	){
+		if(!in_array($priority, EventPriority::ALL, true)){
+			throw new \InvalidArgumentException("Invalid priority number $priority");
+		}
+	}
+
+	public function getHandler() : \Closure{
+		return $this->handler;
+	}
+
+	public function getPlugin() : Plugin{
+		return $this->plugin;
+	}
+
+	public function getPriority() : int{
+		return $this->priority;
+	}
+
+	public function isHandlingCancelled() : bool{
+		return $this->handleCancelled;
+	}
+}

src/event/HandlerList.php

@@ -23,123 +23,11 @@ declare(strict_types=1);
 
 namespace pocketmine\event;
 
-use pocketmine\plugin\Plugin;
-use function array_merge;
-use function krsort;
-use function spl_object_id;
-use const SORT_NUMERIC;
-
-class HandlerList{
-	/** @var RegisteredListener[][] */
-	private array $handlerSlots = [];
-
 /**
-	 * @var RegisteredListenerCache[]
+ * @phpstan-extends BaseHandlerList<RegisteredListener, Event>
-	 * @phpstan-var array<int, RegisteredListenerCache<RegisteredListener>>
  */
-	private array $affectedHandlerCaches = [];
+class HandlerList extends BaseHandlerList{
-
+	protected function sortSamePriorityListeners(array $listeners) : array{
-	/**
+		return $listeners;
-	 * @phpstan-param class-string<covariant Event> $class
-	 * @phpstan-param RegisteredListenerCache<RegisteredListener> $handlerCache
-	 */
-	public function __construct(
-		private string $class,
-		private ?HandlerList $parentList,
-		private RegisteredListenerCache $handlerCache = new RegisteredListenerCache()
-	){
-		for($list = $this; $list !== null; $list = $list->parentList){
-			$list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache;
-		}
-	}
-
-	/**
-	 * @throws \Exception
-	 */
-	public function register(RegisteredListener $listener) : void{
-		if(isset($this->handlerSlots[$listener->getPriority()][spl_object_id($listener)])){
-			throw new \InvalidArgumentException("This listener is already registered to priority {$listener->getPriority()} of event {$this->class}");
-		}
-		$this->handlerSlots[$listener->getPriority()][spl_object_id($listener)] = $listener;
-		$this->invalidateAffectedCaches();
-	}
-
-	/**
-	 * @param RegisteredListener[] $listeners
-	 */
-	public function registerAll(array $listeners) : void{
-		foreach($listeners as $listener){
-			$this->register($listener);
-		}
-		$this->invalidateAffectedCaches();
-	}
-
-	public function unregister(RegisteredListener|Plugin|Listener $object) : void{
-		if($object instanceof Plugin || $object instanceof Listener){
-			foreach($this->handlerSlots as $priority => $list){
-				foreach($list as $hash => $listener){
-					if(($object instanceof Plugin && $listener->getPlugin() === $object)
-						|| ($object instanceof Listener && (new \ReflectionFunction($listener->getHandler()))->getClosureThis() === $object) //this doesn't even need to be a listener :D
-					){
-						unset($this->handlerSlots[$priority][$hash]);
-					}
-				}
-			}
-		}else{
-			unset($this->handlerSlots[$object->getPriority()][spl_object_id($object)]);
-		}
-		$this->invalidateAffectedCaches();
-	}
-
-	public function clear() : void{
-		$this->handlerSlots = [];
-		$this->invalidateAffectedCaches();
-	}
-
-	/**
-	 * @return RegisteredListener[]
-	 */
-	public function getListenersByPriority(int $priority) : array{
-		return $this->handlerSlots[$priority] ?? [];
-	}
-
-	public function getParent() : ?HandlerList{
-		return $this->parentList;
-	}
-
-	/**
-	 * Invalidates all known caches which might be affected by this list's contents.
-	 */
-	private function invalidateAffectedCaches() : void{
-		foreach($this->affectedHandlerCaches as $cache){
-			$cache->list = null;
-		}
-	}
-
-	/**
-	 * @return RegisteredListener[]
-	 * @phpstan-return list<RegisteredListener>
-	 */
-	public function getListenerList() : array{
-		if($this->handlerCache->list !== null){
-			return $this->handlerCache->list;
-		}
-
-		$handlerLists = [];
-		for($currentList = $this; $currentList !== null; $currentList = $currentList->parentList){
-			$handlerLists[] = $currentList;
-		}
-
-		$listenersByPriority = [];
-		foreach($handlerLists as $currentList){
-			foreach($currentList->handlerSlots as $priority => $listeners){
-				$listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $listeners);
-			}
-		}
-
-		//TODO: why on earth do the priorities have higher values for lower priority?
-		krsort($listenersByPriority, SORT_NUMERIC);
-
-		return $this->handlerCache->list = array_merge(...$listenersByPriority);
 	}
 }

src/event/RegisteredListener.php

@@ -23,34 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\event;
 
-use pocketmine\plugin\Plugin;
+class RegisteredListener extends BaseRegisteredListener{
-use pocketmine\timings\TimingsHandler;
-use function in_array;
-
-class RegisteredListener{
-	public function __construct(
-		private \Closure $handler,
-		private int $priority,
-		private Plugin $plugin,
-		private bool $handleCancelled,
-		private TimingsHandler $timings
-	){
-		if(!in_array($priority, EventPriority::ALL, true)){
-			throw new \InvalidArgumentException("Invalid priority number $priority");
-		}
-	}
-
-	public function getHandler() : \Closure{
-		return $this->handler;
-	}
-
-	public function getPlugin() : Plugin{
-		return $this->plugin;
-	}
-
-	public function getPriority() : int{
-		return $this->priority;
-	}
 
 	public function callEvent(Event $event) : void{
 		if($event instanceof Cancellable && $event->isCancelled() && !$this->isHandlingCancelled()){
@@ -63,8 +36,4 @@ class RegisteredListener{
 			$this->timings->stopTiming();
 		}
 	}
-
-	public function isHandlingCancelled() : bool{
-		return $this->handleCancelled;
-	}
 }

tests/phpstan/configs/impossible-generics.neon

@@ -6,17 +6,12 @@ parameters:
 			path: ../../../src/event/AsyncRegisteredListener.php
 
 		-
-			message: "#^Method pocketmine\\\\event\\\\AsyncRegisteredListener\\:\\:getHandler\\(\\) return type has no signature specified for Closure\\.$#"
+			message: "#^Method pocketmine\\\\event\\\\BaseRegisteredListener\\:\\:__construct\\(\\) has parameter \\$handler with no signature specified for Closure\\.$#"
 			count: 1
-			path: ../../../src/event/AsyncRegisteredListener.php
+			path: ../../../src/event/BaseRegisteredListener.php
 
 		-
-			message: "#^Method pocketmine\\\\event\\\\RegisteredListener\\:\\:__construct\\(\\) has parameter \\$handler with no signature specified for Closure\\.$#"
+			message: "#^Method pocketmine\\\\event\\\\BaseRegisteredListener\\:\\:getHandler\\(\\) return type has no signature specified for Closure\\.$#"
 			count: 1
-			path: ../../../src/event/RegisteredListener.php
+			path: ../../../src/event/BaseRegisteredListener.php
-
-		-
-			message: "#^Method pocketmine\\\\event\\\\RegisteredListener\\:\\:getHandler\\(\\) return type has no signature specified for Closure\\.$#"
-			count: 1
-			path: ../../../src/event/RegisteredListener.php