Fixing Psalm issues, adding Psalm action

cycle · Sep 18, 2023 · 79ff979 · 79ff979
2 parents 2395c99 + 25b26a6
commit 79ff979
Show file tree

Hide file tree

Showing 15 changed files with 198 additions and 73 deletions.
diff --git a/.github/workflows/psalm.yml b/.github/workflows/psalm.yml
@@ -0,0 +1,16 @@
+on:
+  pull_request:
+  push:
+    branches:
+      - '*.*'
+
+name: static analysis
+
+jobs:
+  psalm:
+    uses: spiral/gh-actions/.github/workflows/psalm.yml@master
+    with:
+      os: >-
+        ['ubuntu-latest']
+      php: >-
+        ['8.2']
diff --git a/composer.json b/composer.json
@@ -8,7 +8,7 @@
     "require": {
         "php": ">=8.1",
         "cycle/orm": "^2.2.0",
-        "cycle/schema-builder": "^2.4",
+        "cycle/schema-builder": "^2.5.1",
         "spiral/attributes": "^2.8|^3.0",
         "spiral/tokenizer": "^2.8|^3.0",
         "doctrine/inflector": "^2.0"

diff --git a/psalm.xml b/psalm.xml
@@ -4,11 +4,20 @@
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xmlns="https://getpsalm.org/schema/config"
     xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
+    findUnusedCode="false"
+    findUnusedBaselineEntry="true"
 >
     <projectFiles>
         <directory name="src" />
         <ignoreFiles>
             <directory name="vendor" />
         </ignoreFiles>
     </projectFiles>
+    <issueHandlers>
+        <UndefinedAttributeClass>
+            <errorLevel type="suppress">
+                <referencedClass name="JetBrains\PhpStorm\ExpectedValues" />
+            </errorLevel>
+        </UndefinedAttributeClass>
+    </issueHandlers>
 </psalm>
diff --git a/src/Configurator.php b/src/Configurator.php
@@ -42,22 +42,27 @@ public function initEntity(Entity $ann, \ReflectionClass $class): EntitySchema
         $e = new EntitySchema();
         $e->setClass($class->getName());
 
-        $e->setRole($ann->getRole() ?? $this->inflector->camelize($class->getShortName()));
+        $role = $ann->getRole() ?? $this->inflector->camelize($class->getShortName());
+        \assert(!empty($role));
+        $e->setRole($role);
 
         // representing classes
         $e->setMapper($this->resolveName($ann->getMapper(), $class));
         $e->setRepository($this->resolveName($ann->getRepository(), $class));
         $e->setSource($this->resolveName($ann->getSource(), $class));
         $e->setScope($this->resolveName($ann->getScope(), $class));
         $e->setDatabase($ann->getDatabase());
-        $e->setTableName(
-            $ann->getTable() ?? $this->utils->tableName($e->getRole(), $this->tableNamingStrategy)
-        );
+
+        $tableName = $ann->getTable() ?? $this->utils->tableName($role, $this->tableNamingStrategy);
+        \assert(!empty($tableName));
+        $e->setTableName($tableName);
 
         $typecast = $ann->getTypecast();
         if (\is_array($typecast)) {
+            /** @var non-empty-string[] $typecast */
             $typecast = \array_map(fn (string $value): string => $this->resolveName($value, $class), $typecast);
         } else {
+            /** @var non-empty-string|null $typecast */
             $typecast = $this->resolveName($typecast, $class);
         }
 
@@ -75,7 +80,9 @@ public function initEmbedding(Embeddable $emb, \ReflectionClass $class): EntityS
         $e = new EntitySchema();
         $e->setClass($class->getName());
 
-        $e->setRole($emb->getRole() ?? $this->inflector->camelize($class->getShortName()));
+        $role = $emb->getRole() ?? $this->inflector->camelize($class->getShortName());
+        \assert(!empty($role));
+        $e->setRole($role);
 
         // representing classes
         $e->setMapper($this->resolveName($emb->getMapper(), $class));
@@ -89,7 +96,7 @@ public function initFields(EntitySchema $entity, \ReflectionClass $class, string
             try {
                 $column = $this->reader->firstPropertyMetadata($property, Column::class);
             } catch (\Exception $e) {
-                throw new AnnotationException($e->getMessage(), $e->getCode(), $e);
+                throw new AnnotationException($e->getMessage(), (int) $e->getCode(), $e);
             } catch (\ArgumentCountError $e) {
                 throw AnnotationRequiredArgumentsException::createFor($property, Column::class, $e);
             } catch (\TypeError $e) {
@@ -112,20 +119,25 @@ public function initRelations(EntitySchema $entity, \ReflectionClass $class): vo
             try {
                 $metadata = $this->reader->getPropertyMetadata($property, RelationAnnotation\RelationInterface::class);
             } catch (\Exception $e) {
-                throw new AnnotationException($e->getMessage(), $e->getCode(), $e);
+                throw new AnnotationException($e->getMessage(), (int) $e->getCode(), $e);
             }
 
             foreach ($metadata as $meta) {
                 \assert($meta instanceof RelationAnnotation\RelationInterface);
 
-                if ($meta->getTarget() === null) {
+                $target = $meta->getTarget();
+                if ($target === null) {
                     throw new AnnotationException(
                         "Relation target definition is required on `{$entity->getClass()}`.`{$property->getName()}`"
                     );
                 }
 
                 $relation = new Relation();
-                $relation->setTarget($this->resolveName($meta->getTarget(), $class));
+
+                $resolvedTarget = $this->resolveName($target, $class);
+                \assert(!empty($resolvedTarget));
+                $relation->setTarget($resolvedTarget);
+
                 $relation->setType($meta->getType());
 
                 $inverse = $meta->getInverse() ?? $this->reader->firstPropertyMetadata(
@@ -141,12 +153,13 @@ public function initRelations(EntitySchema $entity, \ReflectionClass $class): vo
                 }
 
                 if ($meta instanceof RelationAnnotation\Embedded && $meta->getPrefix() === null) {
+                    /** @var class-string $target */
+                    $target = $relation->getTarget();
                     /** @var Embeddable|null $embeddable */
-                    $embeddable = $this->reader->firstClassMetadata(
-                        new \ReflectionClass($relation->getTarget()),
-                        Embeddable::class
-                    );
-                    $meta->setPrefix($embeddable->getColumnPrefix());
+                    $embeddable = $this->reader->firstClassMetadata(new \ReflectionClass($target), Embeddable::class);
+                    if ($embeddable !== null) {
+                        $meta->setPrefix($embeddable->getColumnPrefix());
+                    }
                 }
 
                 foreach ($meta->getOptions() as $option => $value) {
@@ -170,7 +183,7 @@ public function initModifiers(EntitySchema $entity, \ReflectionClass $class): vo
         try {
             $metadata = $this->reader->getClassMetadata($class, SchemaModifierInterface::class);
         } catch (\Exception $e) {
-            throw new AnnotationException($e->getMessage(), $e->getCode(), $e);
+            throw new AnnotationException($e->getMessage(), (int) $e->getCode(), $e);
         }
 
         foreach ($metadata as $meta) {
@@ -200,19 +213,14 @@ public function initColumns(EntitySchema $entity, array $columns, \ReflectionCla
             $propertyName ??= $isNumericKey ? null : $key;
             $columnName = $column->getColumn() ?? $propertyName;
             $propertyName ??= $columnName;
+            \assert(!empty($propertyName));
 
             if ($columnName === null) {
                 throw new AnnotationException(
                     "Column name definition is required on `{$entity->getClass()}`"
                 );
             }
 
-            if ($column->getType() === null) {
-                throw new AnnotationException(
-                    "Column type definition is required on `{$entity->getClass()}`.`{$columnName}`"
-                );
-            }
-
             $field = $this->initField($columnName, $column, $class, '');
             $field->setEntityClass($entity->getClass());
             $entity->getFields()->set($propertyName, $field);
@@ -224,7 +232,11 @@ public function initField(string $name, Column $column, \ReflectionClass $class,
         $field = new Field();
 
         $field->setType($column->getType());
-        $field->setColumn($columnPrefix . ($column->getColumn() ?? $this->inflector->tableize($name)));
+
+        $columnName = $columnPrefix . ($column->getColumn() ?? $this->inflector->tableize($name));
+        \assert(!empty($columnName));
+        $field->setColumn($columnName);
+
         $field->setPrimary($column->isPrimary());
 
         $field->setTypecast($this->resolveTypecast($column->getTypecast(), $class));
@@ -255,6 +267,13 @@ public function initField(string $name, Column $column, \ReflectionClass $class,
 
     /**
      * Resolve class or role name relative to the current class.
+     *
+     * @template TEntity of object
+     *
+     * @psalm-return($name is class-string<TEntity>
+     *     ? class-string<TEntity>
+     *     : ($name is string ? string : null)
+     * )
      */
     public function resolveName(?string $name, \ReflectionClass $class): ?string
     {

diff --git a/src/Embeddings.php b/src/Embeddings.php
@@ -50,9 +50,10 @@ public function verifyNoRelations(EntitySchema $entity, \ReflectionClass $class)
     {
         foreach ($class->getProperties() as $property) {
             try {
+                /** @var object[] $ann */
                 $ann = $this->reader->getPropertyMetadata($property);
             } catch (\Exception $e) {
-                throw new AnnotationException($e->getMessage(), $e->getCode(), $e);
+                throw new AnnotationException($e->getMessage(), (int) $e->getCode(), $e);
             }
 
             foreach ($ann as $ra) {

diff --git a/src/Entities.php b/src/Entities.php
@@ -58,11 +58,15 @@ public function run(Registry $registry): Registry
             // additional columns (mapped to local fields automatically)
             $this->generator->initColumns($e, $entity->attribute->getColumns(), $entity->class);
 
-            if ($this->utils->hasParent($e->getClass())) {
-                foreach ($this->utils->findParents($e->getClass()) as $parent) {
+            /** @var class-string $class */
+            $class = $e->getClass();
+            if ($this->utils->hasParent($class)) {
+                foreach ($this->utils->findParents($class) as $parent) {
                     // additional columns from parent class
                     $ann = $this->reader->firstClassMetadata($parent, Entity::class);
-                    $this->generator->initColumns($e, $ann->getColumns(), $parent);
+                    if ($ann !== null) {
+                        $this->generator->initColumns($e, $ann->getColumns(), $parent);
+                    }
                 }
 
                 $children[] = $e;
@@ -71,11 +75,17 @@ public function run(Registry $registry): Registry
 
             // register entity (OR find parent)
             $registry->register($e);
-            $registry->linkTable($e, $e->getDatabase(), $e->getTableName());
+
+            $tableName = $e->getTableName();
+            \assert(!empty($tableName));
+            $registry->linkTable($e, $e->getDatabase(), $tableName);
         }
 
         foreach ($children as $e) {
-            $registry->registerChildWithoutMerge($registry->getEntity($this->utils->findParent($e->getClass())), $e);
+            $class = $e->getClass();
+            \assert($class !== null);
+
+            $registry->registerChildWithoutMerge($registry->getEntity($this->utils->getParent($class)), $e);
         }
 
         return $this->normalizeNames($registry);
@@ -118,12 +128,11 @@ private function normalizeNames(Registry $registry): Registry
                         }
                     }
                 } catch (RegistryException $ex) {
+                    /** @var non-empty-string $role */
+                    $role = $e->getRole();
+
                     throw new RelationException(
-                        sprintf(
-                            'Unable to resolve `%s`.`%s` relation target (not found or invalid)',
-                            $e->getRole(),
-                            $name
-                        ),
+                        \sprintf('Unable to resolve `%s`.`%s` relation target (not found or invalid)', $role, $name),
                         $ex->getCode(),
                         $ex
                     );
@@ -134,24 +143,34 @@ private function normalizeNames(Registry $registry): Registry
         return $registry;
     }
 
-    private function resolveTarget(Registry $registry, string $name): ?string
+    /**
+     * @return non-empty-string
+     */
+    private function resolveTarget(Registry $registry, string $name): string
     {
         if (\interface_exists($name, true)) {
             // do not resolve interfaces
             return $name;
         }
 
+        $target = static function (EntitySchema $entity): string {
+            $role = $entity->getRole();
+            \assert($role !== null);
+
+            return $role;
+        };
+
         if (!$registry->hasEntity($name)) {
             // point all relations to the parent
             foreach ($registry as $entity) {
                 foreach ($registry->getChildren($entity) as $child) {
                     if ($child->getClass() === $name || $child->getRole() === $name) {
-                        return $entity->getRole();
+                        return $target($entity);
                     }
                 }
             }
         }
 
-        return $registry->getEntity($name)->getRole();
+        return $target($registry->getEntity($name));
     }
 }
diff --git a/src/Exception/AnnotationRequiredArgumentsException.php b/src/Exception/AnnotationRequiredArgumentsException.php
@@ -6,12 +6,15 @@
 
 class AnnotationRequiredArgumentsException extends AnnotationException
 {
+    /**
+     * @param class-string $annotationClass
+     */
     public static function createFor(\ReflectionProperty $property, string $annotationClass, \Throwable $e): static
     {
         $column = new \ReflectionClass($annotationClass);
 
         $requiredArguments = [];
-        foreach ($column->getConstructor()->getParameters() as $parameter) {
+        foreach ($column->getConstructor()?->getParameters() ?? [] as $parameter) {
             if (! $parameter->isOptional()) {
                 $requiredArguments[] = $parameter->getName();
             }
@@ -24,7 +27,7 @@ public static function createFor(\ReflectionProperty $property, string $annotati
                 $property->getDeclaringClass()->getName(),
                 $property->getName()
             ),
-            $e->getCode(),
+            (int) $e->getCode(),
             $e
         );
     }

diff --git a/src/Exception/AnnotationWrongTypeArgumentException.php b/src/Exception/AnnotationWrongTypeArgumentException.php
@@ -15,7 +15,7 @@ public static function createFor(\ReflectionProperty $property, \Throwable $e):
                 $property->getName(),
                 $e->getMessage()
             ),
-            $e->getCode(),
+            (int) $e->getCode(),
             $e
         );
     }

diff --git a/src/Locator/TokenizerEmbeddingLocator.php b/src/Locator/TokenizerEmbeddingLocator.php
@@ -29,7 +29,7 @@ public function getEmbeddings(): array
             try {
                 $attribute = $this->reader->firstClassMetadata($class, Embeddable::class);
             } catch (\Exception $e) {
-                throw new AnnotationException($e->getMessage(), $e->getCode(), $e);
+                throw new AnnotationException($e->getMessage(), (int) $e->getCode(), $e);
             }
 
             if ($attribute !== null) {

diff --git a/src/Locator/TokenizerEntityLocator.php b/src/Locator/TokenizerEntityLocator.php
@@ -30,7 +30,7 @@ public function getEntities(): array
                 /** @var Attribute $attribute */
                 $attribute = $this->reader->firstClassMetadata($class, Attribute::class);
             } catch (\Exception $e) {
-                throw new AnnotationException($e->getMessage(), $e->getCode(), $e);
+                throw new AnnotationException($e->getMessage(), (int) $e->getCode(), $e);
             }
 
             if ($attribute !== null) {

diff --git a/src/MergeColumns.php b/src/MergeColumns.php
@@ -76,7 +76,7 @@ private function copy(EntitySchema $e, ?string $class): void
             }
             $columns = \array_merge($columns, $this->getColumns($class));
         } catch (\Exception $e) {
-            throw new AnnotationException($e->getMessage(), $e->getCode(), $e);
+            throw new AnnotationException($e->getMessage(), (int) $e->getCode(), $e);
         }
 
         $columns = \array_filter(
@@ -97,17 +97,24 @@ private function getColumns(\ReflectionClass $class): array
     {
         $columns = [];
 
+        $columnName = static function (Column $column): string {
+            $name = $column->getProperty() ?? $column->getColumn();
+            \assert(!empty($name));
+
+            return $name;
+        };
+
         /** @var Table|null $table */
         $table = $this->reader->firstClassMetadata($class, Table::class);
         foreach ($table === null ? [] : $table->getColumns() as $name => $column) {
             if (\is_numeric($name)) {
-                $name = $column->getProperty() ?? $column->getColumn();
+                $name = $columnName($column);
             }
             $columns[$name] = $column;
         }
 
         foreach ($this->reader->getClassMetadata($class, Column::class) as $column) {
-            $columns[$column->getProperty() ?? $column->getColumn()] = $column;
+            $columns[$columnName($column)] = $column;
         }
 
         return $columns;