Cleanup unused code, fix a range of minor psalm issues

Signed-off-by: George Steel <george@net-glue.co.uk>
laminas · Jan 3, 2024 · 9a3675b · 9a3675b
1 parent 76c6d05
commit 9a3675b
Show file tree

Hide file tree

Showing 16 changed files with 123 additions and 220 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.15.0@5c774aca4746caf3d239d9c8cadb9f882ca29352">
+<files psalm-version="5.18.0@b113f3ed0259fd6e212d87c3df80eec95a6abf19">
   <file src="src/EventManager.php">
     <DocblockTypeContradiction>
       <code>is_string($eventName)</code>
@@ -25,22 +25,45 @@
       <code>(int) $priority</code>
     </RedundantCastGivenDocblockType>
   </file>
+  <file src="src/EventManagerAwareInterface.php">
+    <UnusedClass>
+      <code>EventManagerAwareInterface</code>
+    </UnusedClass>
+  </file>
   <file src="src/EventManagerAwareTrait.php">
     <MissingConstructor>
       <code>$events</code>
     </MissingConstructor>
     <MissingReturnType>
       <code>setEventManager</code>
     </MissingReturnType>
-    <MixedArgumentTypeCoercion>
-      <code>$identifiers</code>
-    </MixedArgumentTypeCoercion>
-    <RedundantConditionGivenDocblockType>
+    <PossiblyUnusedMethod>
+      <code>getEventManager</code>
+    </PossiblyUnusedMethod>
+    <RedundantCondition>
       <code><![CDATA[(is_string($this->eventIdentifier))
                 || (is_array($this->eventIdentifier))]]></code>
       <code><![CDATA[is_string($this->eventIdentifier)]]></code>
       <code><![CDATA[isset($this->eventIdentifier)]]></code>
-    </RedundantConditionGivenDocblockType>
+    </RedundantCondition>
+    <TypeDoesNotContainType>
+      <code><![CDATA[$this->eventIdentifier instanceof Traversable]]></code>
+      <code><![CDATA[(is_string($this->eventIdentifier))
+                || (is_array($this->eventIdentifier))]]></code>
+      <code><![CDATA[(is_string($this->eventIdentifier))
+                || (is_array($this->eventIdentifier))
+                || $this->eventIdentifier instanceof Traversable]]></code>
+      <code><![CDATA[(is_string($this->eventIdentifier))
+                || (is_array($this->eventIdentifier))
+                || $this->eventIdentifier instanceof Traversable]]></code>
+      <code><![CDATA[is_array($this->eventIdentifier)]]></code>
+      <code><![CDATA[is_object($this->eventIdentifier)]]></code>
+    </TypeDoesNotContainType>
+  </file>
+  <file src="src/EventsCapableInterface.php">
+    <PossiblyUnusedMethod>
+      <code>getEventManager</code>
+    </PossiblyUnusedMethod>
   </file>
   <file src="src/Filter/FilterInterface.php">
     <MissingReturnType>
@@ -113,10 +136,19 @@
       <code><![CDATA[$this->listener]]></code>
       <code><![CDATA[$this->listener]]></code>
     </MixedReturnStatement>
+    <PossiblyUnusedReturnValue>
+      <code>callable</code>
+    </PossiblyUnusedReturnValue>
     <PropertyNotSetInConstructor>
       <code>$listener</code>
     </PropertyNotSetInConstructor>
   </file>
+  <file src="src/LazyListenerAggregate.php">
+    <UnusedProperty>
+      <code>$container</code>
+      <code>$env</code>
+    </UnusedProperty>
+  </file>
   <file src="src/ResponseCollection.php">
     <MissingReturnType>
       <code>setStopped</code>
@@ -209,9 +241,6 @@
     </RedundantCastGivenDocblockType>
   </file>
   <file src="src/SharedEventManagerInterface.php">
-    <InvalidParamDefault>
-      <code>string</code>
-    </InvalidParamDefault>
     <MissingReturnType>
       <code>attach</code>
       <code>clearListeners</code>
@@ -438,12 +467,6 @@
       <code>LazyListener</code>
     </MoreSpecificReturnType>
   </file>
-  <file src="test/ListenerAggregateTraitTest.php">
-    <UndefinedInterfaceMethod>
-      <code>getCallbacks</code>
-      <code>getCallbacks</code>
-    </UndefinedInterfaceMethod>
-  </file>
   <file src="test/SharedEventManagerTest.php">
     <MissingReturnType>
       <code>testAttachRaisesExceptionForInvalidEvent</code>
@@ -489,6 +512,11 @@
       <code>$callback</code>
     </UnusedVariable>
   </file>
+  <file src="test/SharedListenerIntegrationTest.php">
+    <InvalidPropertyFetch>
+      <code><![CDATA[$listener->count]]></code>
+    </InvalidPropertyFetch>
+  </file>
   <file src="test/Test/EventListenerIntrospectionTraitTest.php">
     <MissingClosureParamType>
       <code>$e</code>
@@ -573,34 +601,23 @@
     <MissingReturnType>
       <code>build</code>
     </MissingReturnType>
+    <PossiblyUnusedMethod>
+      <code>build</code>
+    </PossiblyUnusedMethod>
+  </file>
+  <file src="test/TestAsset/CountingListener.php">
+    <PossiblyUnusedProperty>
+      <code>$index</code>
+    </PossiblyUnusedProperty>
   </file>
   <file src="test/TestAsset/MockAbstractListenerAggregate.php">
     <MissingReturnType>
       <code>doFoo</code>
     </MissingReturnType>
   </file>
-  <file src="test/TestAsset/MockAggregate.php">
-    <ImplementedReturnTypeMismatch>
-      <code>string</code>
-    </ImplementedReturnTypeMismatch>
-    <InvalidPropertyAssignmentValue>
-      <code><![CDATA[$this->listeners]]></code>
-    </InvalidPropertyAssignmentValue>
-    <MixedArgument>
-      <code>$listener</code>
-    </MixedArgument>
-    <MixedAssignment>
-      <code>$listener</code>
-    </MixedAssignment>
-  </file>
   <file src="test/TestAsset/MockListenerAggregateTrait.php">
     <MissingReturnType>
       <code>doFoo</code>
     </MissingReturnType>
   </file>
-  <file src="test/TestAsset/StaticEventsMock.php">
-    <MissingReturnType>
-      <code>attach</code>
-    </MissingReturnType>
-  </file>
 </files>
diff --git a/psalm.xml b/psalm.xml
@@ -5,6 +5,9 @@
     xmlns="https://getpsalm.org/schema/config"
     xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
     errorBaseline="psalm-baseline.xml"
+    findUnusedCode="true"
+    findUnusedBaselineEntry="true"
+    findUnusedPsalmSuppress="true"
 >
     <projectFiles>
         <directory name="src"/>
@@ -16,19 +19,11 @@
     </projectFiles>
 
     <issueHandlers>
-        <InternalMethod>
+        <UnusedClass>
             <errorLevel type="suppress">
-                <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::method"/>
+                <directory name="psalm" />
             </errorLevel>
-
-            <errorLevel type="suppress">
-                <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::willReturn"/>
-            </errorLevel>
-
-            <errorLevel type="suppress">
-                <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::with"/>
-            </errorLevel>
-        </InternalMethod>
+        </UnusedClass>
     </issueHandlers>
 
     <plugins>

diff --git a/src/Event.php b/src/Event.php
@@ -93,7 +93,7 @@ public function getTarget()
      */
     public function setParams($params)
     {
-        /** @psalm-suppress DocblockTypeContradiction Sanity check to actually enforce docblock. */
+        /** @psalm-suppress DocblockTypeContradiction, RedundantCondition 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))
@@ -128,7 +128,6 @@ public function getParam($name, $default = null)
         }
 
         // Check in normal objects
-        /** @psalm-suppress MixedPropertyFetch Only object is left over from union. */
         if (! isset($this->params->{$name})) {
             return $default;
         }

diff --git a/src/EventManager.php b/src/EventManager.php
@@ -7,8 +7,7 @@
 use function array_keys;
 use function array_merge;
 use function array_unique;
-use function gettype;
-use function is_object;
+use function get_debug_type;
 use function is_string;
 use function krsort;
 use function sprintf;
@@ -187,7 +186,7 @@ public function attach($eventName, callable $listener, $priority = 1)
             throw new Exception\InvalidArgumentException(sprintf(
                 '%s expects a string for the event; received %s',
                 __METHOD__,
-                is_object($eventName) ? $eventName::class : gettype($eventName)
+                get_debug_type($eventName),
             ));
         }
 
@@ -213,7 +212,7 @@ public function detach(callable $listener, $eventName = null, $force = false)
             throw new Exception\InvalidArgumentException(sprintf(
                 '%s expects a string for the event; received %s',
                 __METHOD__,
-                is_object($eventName) ? $eventName::class : gettype($eventName)
+                get_debug_type($eventName),
             ));
         }
 

diff --git a/src/LazyListenerAggregate.php b/src/LazyListenerAggregate.php
@@ -4,9 +4,8 @@
 
 use Psr\Container\ContainerInterface;
 
-use function gettype;
+use function get_debug_type;
 use function is_array;
-use function is_object;
 use function sprintf;
 
 /**
@@ -78,7 +77,7 @@ public function __construct(array $listeners, ContainerInterface $container, arr
             if (! $listener instanceof LazyEventListener) {
                 throw new Exception\InvalidArgumentException(sprintf(
                     'All listeners must be LazyEventListener instances or definitions; received %s',
-                    is_object($listener) ? $listener::class : gettype($listener)
+                    get_debug_type($listener),
                 ));
             }
 

diff --git a/src/SharedEventManager.php b/src/SharedEventManager.php
@@ -4,8 +4,7 @@
 
 use function array_keys;
 use function array_merge;
-use function gettype;
-use function is_object;
+use function get_debug_type;
 use function is_string;
 use function sprintf;
 
@@ -63,14 +62,14 @@ public function attach($identifier, $event, callable $listener, $priority = 1)
         if (! is_string($identifier) || empty($identifier)) {
             throw new Exception\InvalidArgumentException(sprintf(
                 'Invalid identifier provided; must be a string; received "%s"',
-                is_object($identifier) ? $identifier::class : gettype($identifier)
+                get_debug_type($identifier),
             ));
         }
 
         if (! is_string($event) || empty($event)) {
             throw new Exception\InvalidArgumentException(sprintf(
                 'Invalid event provided; must be a non-empty string; received "%s"',
-                is_object($event) ? $event::class : gettype($event)
+                get_debug_type($event),
             ));
         }
 
@@ -93,7 +92,7 @@ public function detach(callable $listener, $identifier = null, $eventName = null
         if (! is_string($identifier) || empty($identifier)) {
             throw new Exception\InvalidArgumentException(sprintf(
                 'Invalid identifier provided; must be a string, received %s',
-                is_object($identifier) ? $identifier::class : gettype($identifier)
+                get_debug_type($identifier),
             ));
         }
 
@@ -112,7 +111,7 @@ public function detach(callable $listener, $identifier = null, $eventName = null
         if (! is_string($eventName) || empty($eventName)) {
             throw new Exception\InvalidArgumentException(sprintf(
                 'Invalid event name provided; must be a string, received %s',
-                is_object($eventName) ? $eventName::class : gettype($eventName)
+                get_debug_type($eventName),
             ));
         }
 

diff --git a/test/AbstractListenerAggregateTest.php b/test/AbstractListenerAggregateTest.php
@@ -4,10 +4,50 @@
 
 namespace LaminasTest\EventManager;
 
-use Laminas\EventManager\ListenerAggregateInterface;
+use Laminas\EventManager\EventManagerInterface;
+use LaminasTest\EventManager\TestAsset\MockAbstractListenerAggregate;
+use PHPUnit\Framework\TestCase;
 
-class AbstractListenerAggregateTest extends ListenerAggregateTraitTest
+use function in_array;
+
+class AbstractListenerAggregateTest extends TestCase
 {
-    /** @var class-string<ListenerAggregateInterface> */
-    public $aggregateClass = TestAsset\MockAbstractListenerAggregate::class;
+    public function testDetachRemovesAttachedListeners(): void
+    {
+        $aggregate = new MockAbstractListenerAggregate();
+
+        $events = $this->createMock(EventManagerInterface::class);
+        $events->expects(self::atLeast(2))
+            ->method('attach')
+            ->with(
+                self::callback(static function (string $value): bool {
+                    self::assertTrue(in_array($value, ['foo.bar', 'foo.baz'], true));
+
+                    return true;
+                }),
+                self::callback(static function (array $value) use ($aggregate): bool {
+                    self::assertSame($aggregate, $value[0] ?? null);
+                    self::assertSame('doFoo', $value[1] ?? null);
+
+                    return true;
+                }),
+            )->willReturnArgument(1);
+
+        $events->expects(self::exactly(2))
+            ->method('detach')
+            ->with([$aggregate, 'doFoo']);
+
+        $aggregate->attach($events);
+
+        $listeners = $aggregate->getCallbacks();
+        self::assertCount(2, $listeners);
+
+        foreach ($listeners as $listener) {
+            self::assertSame([$aggregate, 'doFoo'], $listener);
+        }
+
+        $aggregate->detach($events);
+
+        self::assertSame([], $aggregate->getCallbacks());
+    }
 }
diff --git a/test/FilterChainTest.php b/test/FilterChainTest.php
@@ -98,14 +98,12 @@ public function testFilterIsPassedContextAndArguments(): void
 
     public function testInterceptingFilterShouldReceiveChain(): void
     {
-        /** @psalm-suppress UnusedClosureParam */
         $this->filterchain->attach(function (self $context, array $params, mixed $chain) {
             self::assertInstanceOf(FilterIterator::class, $chain);
         });
         $this->filterchain->run($this);
     }
 
-    /** @psalm-suppress UnusedClosureParam */
     public function testFilteringStopsAsSoonAsAFilterFailsToCallNext(): void
     {
         $this->filterchain->attach(function ($context, $params, $chain) {

diff --git a/test/ListenerAggregateTraitTest.php b/test/ListenerAggregateTraitTest.php
@@ -5,21 +5,16 @@
 namespace LaminasTest\EventManager;
 
 use Laminas\EventManager\EventManagerInterface;
-use Laminas\EventManager\ListenerAggregateInterface;
 use LaminasTest\EventManager\TestAsset\MockListenerAggregateTrait;
 use PHPUnit\Framework\TestCase;
 
 use function in_array;
 
 class ListenerAggregateTraitTest extends TestCase
 {
-    /** @var class-string<ListenerAggregateInterface> */
-    public $aggregateClass = MockListenerAggregateTrait::class;
-
     public function testDetachRemovesAttachedListeners(): void
     {
-        $class     = $this->aggregateClass;
-        $aggregate = new $class();
+        $aggregate = new MockListenerAggregateTrait();
 
         $events = $this->createMock(EventManagerInterface::class);
         $events->expects(self::atLeast(2))
@@ -45,7 +40,6 @@ public function testDetachRemovesAttachedListeners(): void
         $aggregate->attach($events);
 
         $listeners = $aggregate->getCallbacks();
-        self::assertIsArray($listeners);
         self::assertCount(2, $listeners);
 
         foreach ($listeners as $listener) {