diff --git a/.gitattributes b/.gitattributes index 803524df..31602891 100644 --- a/.gitattributes +++ b/.gitattributes @@ -14,3 +14,4 @@ /phpbench.json export-ignore /phpunit.xml.dist export-ignore /test/ export-ignore +/psalm/ export-ignore diff --git a/composer.json b/composer.json index 69e02b4f..bf3f7c48 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,8 @@ "php": "8.0.99" }, "allow-plugins": { - "dealerdirect/phpcodesniffer-composer-installer": true + "dealerdirect/phpcodesniffer-composer-installer": true, + "composer/package-versions-deprecated": true } }, "extra": { @@ -52,7 +53,8 @@ "autoload-dev": { "psr-4": { "LaminasTest\\EventManager\\": "test/", - "LaminasBench\\EventManager\\": "benchmarks/" + "LaminasBench\\EventManager\\": "benchmarks/", + "LaminasPsalm\\EventManager\\": "psalm/" } }, "scripts": { diff --git a/phpcs.xml b/phpcs.xml index db62861b..ba27e653 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -14,6 +14,7 @@ benchmarks + psalm src test diff --git a/psalm.xml b/psalm.xml index 62df690a..2e18e492 100644 --- a/psalm.xml +++ b/psalm.xml @@ -9,6 +9,7 @@ + diff --git a/psalm/EventChecks.php b/psalm/EventChecks.php new file mode 100644 index 00000000..07fb33ee --- /dev/null +++ b/psalm/EventChecks.php @@ -0,0 +1,91 @@ +>, + * EventInterface>, + * } + */ + public function checkEmptyCtorInference(): array + { + $event = new Event(); + return [ + $event, + $event, + $event, + $event, + ]; + } + + /** + * @return array{ + * Event<'target-string', array{foo: 'bar', baz: true}>, + * 'target-string', + * array{foo: 'bar', baz: true}, + * } + */ + public function checkCtorInference(): array + { + $event = new Event(null, 'target-string', [ + 'foo' => 'bar', + 'baz' => true, + ]); + return [ + $event, + $event->getTarget(), + $event->getParams(), + ]; + } + + /** + * Verifies that the psalm-this-out annotations are applied correctly to {@see Event}. + * + * @return Event + */ + public function checkThisOut(): Event + { + $event = new Event(); + $event->setTarget(new CheckObject()); + $event->setParams(['foo' => 'bar']); + return $event; + } + + /** + * Verifies that the inherited psalm-this-out annotations do not change the class back to one of the inherited + * classes. Note: This assumes child classes have no template variables. + * + * @return array{ + * CheckEvent&Event, + * CheckEvent&EventInterface, + * Event, + * CheckObject, + * array{foo: 'bar'}, + * } + */ + public function checkThisOutInheritance(): array + { + $event = new CheckEvent(); + $event->setTarget(new CheckObject()); + $event->setParams(['foo' => 'bar']); + return [ + $event, + $event, + $event, + $event->getTarget(), + $event->getParams(), + ]; + } +} diff --git a/psalm/EventInterfaceChecks.php b/psalm/EventInterfaceChecks.php new file mode 100644 index 00000000..f0243dcb --- /dev/null +++ b/psalm/EventInterfaceChecks.php @@ -0,0 +1,85 @@ + $e + * @return array{ + * CheckObject, + * array{foo: int, bar: CheckObject}, + * int, + * CheckObject + * } + */ + public function checkTargetAndParamsMatchTemplate(EventInterface $e): array + { + return [ + $e->getTarget(), + $e->getParams(), + $e->getParams()['foo'], + $e->getParams()['bar'], + ]; + } + + /** + * @param EventInterface> $e + * @return EventInterface> + */ + public function checkSetTargetChangesTemplate(EventInterface $e): EventInterface + { + $e->setTarget(new CheckObject()); + return $e; + } + + /** + * @param EventInterface $e + * @return EventInterface + */ + public function checkSetParamsChangesTemplate(EventInterface $e): EventInterface + { + $e->setParams([ + 'foo' => new CheckObject(), + 'bar' => 'baz', + ]); + return $e; + } + + /** + * Individual params obtained via `getParam()` can't be inferred because their keys/values can't be selected from + * the template type. + * + * @param EventInterface $e + * @return array{ + * mixed, + * mixed + * } + */ + public function checkIndividualParamsNotInferred(EventInterface $e): array + { + return [ + $e->getParam('foo'), + $e->getParam('bar'), + ]; + } + + /** + * Changing the template and statically checking individual values is not possible with Psalm because + * key-of and value-of do not work on objects. + * + * @param EventInterface $e + * @return EventInterface + */ + public function checkIndividualParamDoesNotChangeTemplate(EventInterface $e): EventInterface + { + $e->setParam('foo', 'notAnInt'); + $e->setParam('bar', 'keyDidNotExist'); + return $e; + } +} diff --git a/psalm/Model/CheckEvent.php b/psalm/Model/CheckEvent.php new file mode 100644 index 00000000..1f421a4c --- /dev/null +++ b/psalm/Model/CheckEvent.php @@ -0,0 +1,14 @@ + + */ +class CheckEvent extends Event +{ +} diff --git a/psalm/Model/CheckObject.php b/psalm/Model/CheckObject.php new file mode 100644 index 00000000..0dcb9923 --- /dev/null +++ b/psalm/Model/CheckObject.php @@ -0,0 +1,9 @@ + */ class Event implements EventInterface { - /** @var string Event name */ + /** @var string|null Event name */ protected $name; - /** @var string|object The event target */ + /** + * @var object|string|null The event target + * @psalm-var TTarget + */ protected $target; - /** @var array|ArrayAccess|object The event parameters */ + /** + * @var array|ArrayAccess|object The event parameters + * @psalm-var TParams + * @psalm-suppress InvalidPropertyAssignmentValue Empty array _can_ be assigned, but there is no "template type + * default" functionality in Psalm (https://github.com/vimeo/psalm/issues/3048). + */ protected $params = []; /** @var bool Whether or not to stop propagation */ @@ -34,11 +46,13 @@ class Event implements EventInterface * * Accept a target and its parameters. * - * @param string $name Event name - * @param string|object $target - * @param array|ArrayAccess $params + * @param string|null $name Event name + * @param string|object|null $target + * @psalm-param TTarget $target + * @param array|ArrayAccess|object|null $params + * @psalm-param TParams|array|null $params */ - public function __construct($name = null, $target = null, $params = null) + public function __construct($name = null, $target = null, $params = []) { if (null !== $name) { $this->setName($name); @@ -48,15 +62,13 @@ public function __construct($name = null, $target = null, $params = null) $this->setTarget($target); } - if (null !== $params) { + if ($params !== null && $params !== []) { $this->setParams($params); } } /** - * Get event name - * - * @return string + * {@inheritDoc} */ public function getName() { @@ -64,11 +76,7 @@ public function getName() } /** - * Get the event target - * - * This may be either an object, or the name of a static method. - * - * @return string|object + * {@inheritDoc} */ public function getTarget() { @@ -76,28 +84,28 @@ public function getTarget() } /** - * Set parameters + * {@inheritDoc} * - * Overwrites parameters - * - * @param array|ArrayAccess|object $params + * @template NewTParams of array|ArrayAccess|object + * @psalm-param NewTParams $params + * @psalm-this-out static&self * @throws Exception\InvalidArgumentException */ public function setParams($params) { + /** @psalm-suppress DocblockTypeContradiction Sanity check to actually enforce docblock. */ if (! is_array($params) && ! is_object($params)) { throw new Exception\InvalidArgumentException( sprintf('Event parameters must be an array or object; received "%s"', gettype($params)) ); } + /** @psalm-suppress InvalidPropertyAssignmentValue Pretty sure this is correct after this-out. */ $this->params = $params; } /** - * Get all parameters - * - * @return array|object|ArrayAccess + * {@inheritDoc} */ public function getParams() { @@ -105,13 +113,7 @@ public function getParams() } /** - * Get an individual parameter - * - * If the parameter does not exist, the $default value will be returned. - * - * @param string|int $name - * @param mixed $default - * @return mixed + * {@inheritDoc} */ public function getParam($name, $default = null) { @@ -121,46 +123,49 @@ public function getParam($name, $default = null) return $default; } + /** @psalm-suppress MixedArrayAccess We've just verified `$this->params` is array-like... */ return $this->params[$name]; } // Check in normal objects + /** @psalm-suppress MixedPropertyFetch Only object is left over from union. */ if (! isset($this->params->{$name})) { return $default; } + /** @psalm-suppress MixedPropertyFetch Only object is left over from union. */ return $this->params->{$name}; } /** - * Set the event name - * - * @param string $name + * {@inheritDoc} */ public function setName($name) { + /** @psalm-suppress RedundantCastGivenDocblockType Cast is safety measure in case caller passes junk. */ $this->name = (string) $name; } /** - * Set the event target/context + * {@inheritDoc} * - * @param null|string|object $target + * @template NewTTarget of object|string|null + * @psalm-param NewTTarget $target + * @psalm-this-out static&self */ public function setTarget($target) { + /** @psalm-suppress InvalidPropertyAssignmentValue Pretty sure this is correct after this-out. */ $this->target = $target; } /** - * Set an individual parameter to a value - * - * @param string|int $name - * @param mixed $value + * {@inheritDoc} */ public function setParam($name, $value) { if (is_array($this->params) || $this->params instanceof ArrayAccess) { // Arrays or objects implementing array access + /** @psalm-suppress MixedArrayAssignment No way to extend existing array template. */ $this->params[$name] = $value; return; } @@ -170,19 +175,16 @@ public function setParam($name, $value) } /** - * Stop further event propagation - * - * @param bool $flag + * {@inheritDoc} */ public function stopPropagation($flag = true) { + /** @psalm-suppress RedundantCastGivenDocblockType Cast is safety measure in case caller passes junk. */ $this->stopPropagation = (bool) $flag; } /** - * Is propagation stopped? - * - * @return bool + * {@inheritDoc} */ public function propagationIsStopped() { diff --git a/src/EventInterface.php b/src/EventInterface.php index 2f4e578e..b11b6995 100644 --- a/src/EventInterface.php +++ b/src/EventInterface.php @@ -6,34 +6,39 @@ /** * Representation of an event + * + * @template-covariant TTarget of object|string|null + * @template-covariant TParams of array|ArrayAccess|object */ interface EventInterface { /** * Get event name * - * @return string + * @return string|null */ public function getName(); /** * Get target/context from which event was triggered * - * @return null|string|object + * @return object|string|null + * @psalm-return TTarget */ public function getTarget(); /** * Get parameters passed to the event * - * @return array|ArrayAccess + * @return array|ArrayAccess|object + * @psalm-return TParams */ public function getParams(); /** * Get a single parameter by name * - * @param string $name + * @param string|int $name * @param mixed $default Default value to return if parameter does not exist * @return mixed */ @@ -50,15 +55,21 @@ public function setName($name); /** * Set the event target/context * - * @param null|string|object $target + * @param object|string|null $target + * @template NewTTarget of object|string|null + * @psalm-param NewTTarget $target + * @psalm-this-out static&self * @return void */ public function setTarget($target); /** - * Set event parameters + * Set event parameters. Overwrites parameters. * - * @param array|ArrayAccess $params + * @param array|ArrayAccess|object $params + * @template NewTParams of array|ArrayAccess|object + * @psalm-param NewTParams $params + * @psalm-this-out static&self * @return void */ public function setParams($params); @@ -66,7 +77,7 @@ public function setParams($params); /** * Set a single parameter by key * - * @param string $name + * @param string|int $name * @param mixed $value * @return void */