Fixing or baselining most reported psalm errors

Note: many reflection-related suppressions depend on vimeo/psalm#8722 Also, we preserved many assertions on `class-string`, since removing input validation would be a BC break.
tux-rampage · Nov 25, 2022 · 97f864e · 97f864e
1 parent 9578762
commit 97f864e
Show file tree

Hide file tree

Showing 13 changed files with 71 additions and 62 deletions.
diff --git a/src/Container/AutowireFactory.php b/src/Container/AutowireFactory.php
@@ -23,7 +23,6 @@ private function getInjector(ContainerInterface $container): InjectorInterface
     {
         $injector = $container->get(InjectorInterface::class);
 
-        /** @psalm-suppress DocblockTypeContradiction Can we remove all code below and return directly from container get? */
         if (! $injector instanceof InjectorInterface) {
             throw new Exception\RuntimeException(
                 'Could not get a dependency injector form the container implementation'

diff --git a/src/Definition/DefinitionInterface.php b/src/Definition/DefinitionInterface.php
@@ -24,6 +24,7 @@ public function getClasses(): array;
     public function hasClass(string $class): bool;
 
     /**
+     * @param class-string $class
      * @throws ClassNotFoundException
      */
     public function getClassDefinition(string $class): ClassDefinitionInterface;

diff --git a/src/Definition/Reflection/Parameter.php b/src/Definition/Reflection/Parameter.php
@@ -63,17 +63,17 @@ public function getPosition(): int
      */
     public function getType(): ?string
     {
-        if ($this->reflection->hasType()) {
-            $type = $this->reflection->getType();
+        $type = $this->reflection->getType();
 
-            if ($type !== null && ! $type instanceof ReflectionNamedType) {
-                throw UnsupportedReflectionTypeException::fromUnionOrIntersectionType($type);
-            }
+        if (! $type) {
+            return null;
+        }
 
-            return $type->getName();
+        if (! $type instanceof ReflectionNamedType) {
+            throw UnsupportedReflectionTypeException::fromUnionOrIntersectionType($type);
         }
 
-        return null;
+        return $type->getName();
     }
 
     /**
@@ -95,16 +95,16 @@ public function isRequired(): bool
      */
     public function isBuiltin(): bool
     {
-        if ($this->reflection->hasType()) {
-            $type = $this->reflection->getType();
+        $type = $this->reflection->getType();
 
-            if ($type !== null && ! $type instanceof ReflectionNamedType) {
-                throw UnsupportedReflectionTypeException::fromUnionOrIntersectionType($type);
-            }
+        if (! $type) {
+            return false;
+        }
 
-            return $type !== null ? $type->isBuiltin() : false;
+        if (! $type instanceof ReflectionNamedType) {
+            throw UnsupportedReflectionTypeException::fromUnionOrIntersectionType($type);
         }
 
-        return false;
+        return $type->isBuiltin();
     }
 }
diff --git a/src/Definition/RuntimeDefinition.php b/src/Definition/RuntimeDefinition.php
@@ -67,7 +67,8 @@ public function addExplicitClass(string $class): self
     }
 
     /**
-     * @param class-string $class
+     * @psalm-assert class-string $class
+     *
      * @throws Exception\ClassNotFoundException
      */
     private function ensureClassExists(string $class): void
@@ -102,11 +103,7 @@ public function hasClass(string $class): bool
         return class_exists($class);
     }
 
-    /**
-     * @param class-string $class
-     * @return ClassDefinition
-     * @throws Exception\ClassNotFoundException
-     */
+    /** @throws Exception\ClassNotFoundException */
     public function getClassDefinition(string $class): ClassDefinitionInterface
     {
         if (! isset($this->definition[$class])) {

diff --git a/src/Injector.php b/src/Injector.php
@@ -220,12 +220,11 @@ private function getInjectionValue(InjectionInterface $injection)
     private function resolveParameters(string $type, array $params = []): array
     {
         $resolved = $this->resolver->resolveParameters($type, $params);
-        $params   = [];
+        $foundParams   = [];
 
         foreach ($resolved as $injection) {
             try {
-                /** @psalm-var mixed */
-                $params[] = $this->getInjectionValue($injection);
+                $foundParams[] = $this->getInjectionValue($injection);
             } catch (NotFoundExceptionInterface $containerException) {
                 throw new Exception\UndefinedReferenceException(
                     $containerException->getMessage(),
@@ -235,6 +234,6 @@ private function resolveParameters(string $type, array $params = []): array
             }
         }
 
-        return $params;
+        return $foundParams;
     }
 }
diff --git a/test/CodeGenerator/AbstractInjectorTest.php b/test/CodeGenerator/AbstractInjectorTest.php
@@ -85,7 +85,7 @@ public function testImplementsContract(): void
     public function testCanCreateReturnsTrueWhenAFactoryIsAvailable(): void
     {
         $className = uniqid('SomeClass');
-        $provider  = fn() => [$className => 'SomeClassFactory'];
+        $provider  = static fn(): array => [$className => 'SomeClassFactory'];
 
         $this->decoratedInjector
             ->expects(self::never())
@@ -100,7 +100,7 @@ public function testCanCreateUsesDecoratedInjectorWithoutFactory(): void
     {
         $missingClass  = uniqid('SomeClass');
         $existingClass = 'stdClass';
-        $provider      = fn() => [];
+        $provider      = static fn(): array => [];
 
         $this->decoratedInjector
             ->expects(self::exactly(2))
@@ -123,7 +123,7 @@ public function testCreateUsesFactory(): void
         $className = uniqid('SomeClass');
         $params    = ['someArg' => uniqid()];
         $expected  = new stdClass();
-        $provider  = fn() => [$className => $factory];
+        $provider  = static fn(): array => [$className => $factory];
 
         $factory
             ->expects(self::once())
@@ -145,7 +145,7 @@ public function testCreateUsesDecoratedInjectorIfNoFactoryIsAvailable(): void
         $className = uniqid('SomeClass');
         $expected  = new stdClass();
         $params    = ['someArg' => uniqid()];
-        $provider  = fn() => [];
+        $provider  = static fn(): array => [];
 
         $this->decoratedInjector
             ->expects(self::once())
@@ -162,7 +162,7 @@ public function testConstructionWithoutContainerUsesDefaultContainer(): void
         $factory   = $this->createMock(FactoryInterface::class);
         $className = uniqid('SomeClass');
         $expected  = new stdClass();
-        $provider  = fn() => [$className => $factory];
+        $provider  = static fn(): array => [$className => $factory];
 
         $factory
             ->expects(self::once())

diff --git a/test/CodeGenerator/InjectorGeneratorTest.php b/test/CodeGenerator/InjectorGeneratorTest.php
@@ -112,7 +112,7 @@ public function testGeneratorLogsErrorWhenFactoryGenerationFailed(): void
     {
         $config    = new Config();
         $resolver  = new DependencyResolver(new RuntimeDefinition(), $config);
-        $logger    = $this->createStub(LoggerInterface::class);
+        $logger    = $this->createMock(LoggerInterface::class);
         $generator = new InjectorGenerator($config, $resolver, null, $logger);
 
         $generator->setOutputDirectory($this->dir);

diff --git a/test/Container/AutowireFactoryTest.php b/test/Container/AutowireFactoryTest.php
@@ -19,7 +19,7 @@
  */
 class AutowireFactoryTest extends TestCase
 {
-    private ?AutowireFactory $instance = null;
+    private AutowireFactory $instance;
 
     /**
      * Prepares the environment before running a test.
@@ -30,15 +30,6 @@ protected function setUp(): void
         $this->instance = new AutowireFactory();
     }
 
-    /**
-     * Cleans up the environment after running a test.
-     */
-    protected function tearDown(): void
-    {
-        $this->instance = null;
-        parent::tearDown();
-    }
-
     private function createContainerMock(InjectorInterface $injector): ContainerInterface
     {
         $container = $this->getMockBuilder(ContainerInterface::class)->getMockForAbstractClass();

diff --git a/test/Definition/Reflection/ClassDefinitionTest.php b/test/Definition/Reflection/ClassDefinitionTest.php
@@ -12,12 +12,14 @@
 use PHPUnit\Framework\TestCase;
 use ReflectionClass;
 
+use ReflectionParameter;
 use function array_values;
+use function assert;
 use function sort;
 use function uasort;
 
 /**
- * @coversDefaultClass Laminas\Di\Definition\Reflection\ClassDefinition
+ * @covers \Laminas\Di\Definition\Reflection\ClassDefinition
  */
 final class ClassDefinitionTest extends TestCase
 {
@@ -137,26 +139,29 @@ public function testGetParametersReturnsAnArray(string $class): void
 
     public function testRedundantUaSortInClassDefinition(): void
     {
-        $reflectionClass       = new ReflectionClass(ClassDefinitionRedundantUaSortTestDependency::class);
-        $constructor           = $reflectionClass->getConstructor();
+        $reflectionClass = new ReflectionClass(ClassDefinitionRedundantUaSortTestDependency::class);
+        $constructor     = $reflectionClass->getConstructor();
+
+        assert($constructor !== null);
+
         $constructorParameters = $constructor->getParameters();
 
         $parameters = [];
         foreach ($constructorParameters as $parameter) {
             $parameters[$parameter->getName()] = $parameter;
         }
 
-        static::assertEquals(
+        self::assertEquals(
             $constructorParameters,
             array_values($parameters)
         );
 
         uasort(
             $parameters,
-            fn ($a, $b) => $a->getPosition() - $b->getPosition()
+            static fn (ReflectionParameter $a, ReflectionParameter $b) => $a->getPosition() - $b->getPosition()
         );
 
-        static::assertEquals(
+        self::assertEquals(
             $constructorParameters,
             array_values($parameters)
         );

diff --git a/test/Definition/Reflection/ParameterTest.php b/test/Definition/Reflection/ParameterTest.php
@@ -9,7 +9,9 @@
 use LaminasTest\Di\TestAsset;
 use PHPUnit\Framework\TestCase;
 use ReflectionClass;
+use ReflectionMethod;
 use ReflectionParameter;
+use function assert;
 
 /**
  * Parameter test case.
@@ -101,7 +103,7 @@ public function testScalarTypehintedParameters(ReflectionParameter $reflection,
 
     public function testIterablePseudoType()
     {
-        $reflections = (new ReflectionClass(TestAsset\IterableDependency::class))->getConstructor()->getParameters();
+        $reflections = $this->getConstructor(TestAsset\IterableDependency::class)->getParameters();
         $param       = new Parameter($reflections[0]);
 
         $this->assertTrue($param->isBuiltin());
@@ -116,7 +118,7 @@ public function testIsBuiltinGivenUnionTypeExpectedUnsupportedReflectionTypeExce
         $this->expectException(UnsupportedReflectionTypeException::class);
 
         $class      = TestAsset\Constructor\UnionTypeConstructorDependency::class;
-        $parameters = (new ReflectionClass($class))->getConstructor()->getParameters();
+        $parameters = $this->getConstructor($class)->getParameters();
         $param      = new Parameter($parameters[0]);
 
         $param->isBuiltin();
@@ -129,8 +131,9 @@ public function testIsBuiltinGivenIntersectionTypeExpectedUnsupportedReflectionT
     {
         $this->expectException(UnsupportedReflectionTypeException::class);
 
-         $class      = 'LaminasTest\Di\TestAsset\Constructor\IntersectionTypeConstructorDependency';
-        $parameters = (new ReflectionClass($class))->getConstructor()->getParameters();
+        /** @var class-string $class */
+        $class      = 'LaminasTest\Di\TestAsset\Constructor\IntersectionTypeConstructorDependency';
+        $parameters = $this->getConstructor($class)->getParameters();
         $param      = new Parameter($parameters[0]);
 
         $param->isBuiltin();
@@ -144,7 +147,7 @@ public function testGetTypeGivenUnionTypeExpectedUnsupportedReflectionTypeExcept
         $this->expectException(UnsupportedReflectionTypeException::class);
 
         $class      = TestAsset\Constructor\UnionTypeConstructorDependency::class;
-        $parameters = (new ReflectionClass($class))->getConstructor()->getParameters();
+        $parameters = $this->getConstructor($class)->getParameters();
         $param      = new Parameter($parameters[0]);
 
         $param->getType();
@@ -157,10 +160,21 @@ public function testGetTypeGivenIntersectionTypeExpectedUnsupportedReflectionTyp
     {
         $this->expectException(UnsupportedReflectionTypeException::class);
 
+        /** @var class-string $class */
         $class      = 'LaminasTest\Di\TestAsset\Constructor\IntersectionTypeConstructorDependency';
-        $parameters = (new ReflectionClass($class))->getConstructor()->getParameters();
+        $parameters = $this->getConstructor($class)->getParameters();
         $param      = new Parameter($parameters[0]);
 
         $param->getType();
     }
+
+    /** @param class-string $class */
+    private function getConstructor(string $class): ReflectionMethod
+    {
+        $constructor = (new ReflectionClass($class))->getConstructor();
+
+        assert($constructor !== null);
+
+        return $constructor;
+    }
 }
diff --git a/test/Definition/RuntimeDefinitionTest.php b/test/Definition/RuntimeDefinitionTest.php
@@ -54,6 +54,7 @@ public function testSetExplicitClassesReplacesPreviousValues(): void
         $this->assertEquals($expected, $definition->getClasses());
     }
 
+    /** @return non-empty-array<non-empty-string, array{class-string}> */
     public function provideExistingClasses(): array
     {
         return [
@@ -110,10 +111,8 @@ public function testAddInvalidExplicitClassThrowsException(string $class)
         $definition->addExplicitClass($class);
     }
 
-    /**
-     * @dataProvider provideExistingClasses
-     */
-    public function testHasClassReturnsTrueDynamically(string $class)
+    /** @dataProvider provideExistingClasses */
+    public function testHasClassReturnsTrueDynamically(string $class): void
     {
         $this->assertTrue(
             (new RuntimeDefinition())->hasClass($class)
@@ -132,8 +131,10 @@ public function testHasClassReturnsFalseForInvalidClasses(string $class)
 
     /**
      * @dataProvider provideExistingClasses
+     *
+     * @param class-string $class
      */
-    public function testGetClassDefinition(string $class)
+    public function testGetClassDefinition(string $class): void
     {
         $definition = new RuntimeDefinition();
         $result     = $definition->getClassDefinition($class);
@@ -145,8 +146,10 @@ public function testGetClassDefinition(string $class)
 
     /**
      * @dataProvider provideExistingClasses
+     *
+     * @param class-string $class
      */
-    public function testGetClassDefinitionAutoPopulatesClass(string $class)
+    public function testGetClassDefinitionAutoPopulatesClass(string $class): void
     {
         $definition = new RuntimeDefinition();
 

diff --git a/test/GeneratedInjectorDelegatorTest.php b/test/GeneratedInjectorDelegatorTest.php
@@ -42,7 +42,7 @@ public function testProvidedNamespaceIsNotAString(): void
     public function testGeneratedInjectorDoesNotExist(): void
     {
         $injector = $this->createMock(InjectorInterface::class);
-        $callback = fn() => $injector;
+        $callback = static fn(): InjectorInterface => $injector;
 
         $container = $this->createMock(ContainerInterface::class);
         $container
@@ -59,7 +59,7 @@ public function testGeneratedInjectorDoesNotExist(): void
     public function testGeneratedInjectorExists(): void
     {
         $injector = $this->createMock(InjectorInterface::class);
-        $callback = fn() => $injector;
+        $callback = static fn(): InjectorInterface => $injector;
 
         $container = $this->createMock(ContainerInterface::class);
         $container

diff --git a/test/InjectorTest.php b/test/InjectorTest.php
@@ -500,7 +500,7 @@ public function testConstructionWithManyParameters(string $class, array $paramet
         $this->assertEquals($parameters, $result->result);
     }
 
-    public function testCreateGivenExistingInterfaceExpectedClassNotFoundExceptionThrown()
+    public function testCreateGivenExistingInterfaceExpectedClassNotFoundExceptionThrown(): void
     {
         $definition = $this->createMock(DefinitionInterface::class);
         $definition