Automatic static analysis with phpstan (#11)

Author: yann-eugone

.github/workflows/tests.yml

@@ -43,6 +43,40 @@ jobs:
       - name: "Run tests with phpunit/phpunit"
         run: vendor/bin/phpunit
 
+  phpstan:
+    name: "PhpStan"
+    runs-on: ubuntu-latest
+
+    strategy:
+      matrix:
+        include:
+          - php-version: 7.4
+            symfony-version: 5.1.*
+
+    steps:
+      - name: "Checkout"
+        uses: actions/checkout@v2
+
+      - name: "Setup PHP"
+        uses: shivammathur/setup-php@v2
+        with:
+          coverage: none
+          php-version: ${{ matrix.php-version }}
+
+      - name: "Install dependencies with composer"
+        run: |
+          composer require --no-update "symfony/console:${{ matrix.symfony-version }}"
+          composer require --no-update "symfony/filesystem:${{ matrix.symfony-version }}"
+          composer require --no-update "symfony/framework-bundle:${{ matrix.symfony-version }}"
+          composer require --no-update "symfony/messenger:${{ matrix.symfony-version }}"
+          composer require --no-update "symfony/process:${{ matrix.symfony-version }}"
+          composer require --no-update "symfony/serializer:${{ matrix.symfony-version }}"
+          composer require --no-update "symfony/validator:${{ matrix.symfony-version }}"
+          composer update --no-interaction --no-progress --no-suggest
+
+      - name: "Run static analyzis with phpstan/phpstan"
+        run: vendor/bin/phpstan analyze
+
   checkstyke:
     name: "Checkstyle"
     runs-on: ubuntu-latest

.gitignore

@@ -1,3 +1,4 @@
+/.idea/
 /.phpcs-cache
 /.phpunit.result.cache
 /tests/.artifacts/

composer.json

@@ -25,6 +25,7 @@
     "symfony/validator": "^4.4|^5.0"
   },
   "require-dev": {
+    "phpstan/phpstan": "^0.12",
     "phpspec/prophecy-phpunit": "^2.0",
     "phpunit/phpunit": "^9.4",
     "squizlabs/php_codesniffer": "^3.5",

phpstan.neon

@@ -0,0 +1,20 @@
+parameters:
+  level: max
+  checkMissingIterableValueType: false
+  checkGenericClassInNonGenericObjectType: false
+  paths:
+    - src/batch/src/
+    - src/batch-box-spout/src/
+    - src/batch-doctrine-dbal/src/
+    - src/batch-doctrine-orm/src/
+    - src/batch-doctrine-persistence/src/
+    - src/batch-symfony-console/src/
+    - src/batch-symfony-framework/src/
+    - src/batch-symfony-messenger/src/
+    - src/batch-symfony-serializer/src/
+    - src/batch-symfony-validator/src/
+
+  ignoreErrors:
+    # The DependencyInjection returns are complex to deal with
+    - message: '#.*NodeParentInterface\|null.*#'
+      path: ./src/batch-symfony-framework/src/DependencyInjection

src/batch-box-spout/src/FlatFileWriter.php

@@ -8,6 +8,7 @@
 use Box\Spout\Writer\Common\Creator\WriterFactory;
 use Box\Spout\Writer\CSV\Writer as CsvWriter;
 use Box\Spout\Writer\WriterInterface;
+use Yokai\Batch\Exception\BadMethodCallException;
 use Yokai\Batch\Exception\RuntimeException;
 use Yokai\Batch\Exception\UnexpectedValueException;
 use Yokai\Batch\Job\Item\FlushableInterface;
@@ -90,6 +91,10 @@ public function initialize(): void
      */
     public function write(iterable $items): void
     {
+        if ($this->writer === null) {
+            throw BadMethodCallException::itemComponentNotInitialized($this);
+        }
+
         if (!$this->headersAdded) {
             $this->headersAdded = true;
             if ($this->headers !== null) {
@@ -110,6 +115,10 @@ public function write(iterable $items): void
      */
     public function flush(): void
     {
+        if ($this->writer === null) {
+            throw BadMethodCallException::itemComponentNotInitialized($this);
+        }
+
         $this->writer->close();
         $this->writer = null;
         $this->headersAdded = false;

src/batch-box-spout/tests/FlatFileWriterTest.php

@@ -8,6 +8,7 @@
 use Box\Spout\Reader\Wrapper\XMLReader;
 use PHPUnit\Framework\TestCase;
 use Yokai\Batch\Bridge\Box\Spout\FlatFileWriter;
+use Yokai\Batch\Exception\BadMethodCallException;
 use Yokai\Batch\Exception\UnexpectedValueException;
 use Yokai\Batch\JobExecution;
 use Yokai\Batch\JobParameters;
@@ -76,6 +77,28 @@ public function testWrite(
         $this->assertFileContents($type, $file, $expectedContent);
     }
 
+    /**
+     * @dataProvider types
+     */
+    public function testShouldInitializeBeforeWrite(string $type): void
+    {
+        $this->expectException(BadMethodCallException::class);
+
+        $writer = new FlatFileWriter($type);
+        $writer->write([true]);
+    }
+
+    /**
+     * @dataProvider types
+     */
+    public function testShouldInitializeBeforeFlush(string $type): void
+    {
+        $this->expectException(BadMethodCallException::class);
+
+        $writer = new FlatFileWriter($type);
+        $writer->flush();
+    }
+
     public function types(): \Generator
     {
         foreach ([Type::CSV, Type::XLSX, Type::ODS] as $type) {

src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php

@@ -5,6 +5,7 @@
 namespace Yokai\Batch\Bridge\Doctrine\DBAL;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Driver\Result;
 use Doctrine\DBAL\Exception as DBALException;
 use Doctrine\DBAL\Schema\AbstractAsset;
 use Doctrine\DBAL\Schema\Schema;
@@ -359,6 +360,7 @@ private function identity(JobExecution $execution): array
 
     private function fetchRow(string $jobName, string $id): array
     {
+        /** @var Result $statement */
         $statement = $this->connection->executeQuery(
             "SELECT * FROM {$this->table} WHERE {$this->jobNameCol} = :jobName AND {$this->idCol} = :id",
             ['jobName' => $jobName, 'id' => $id],
@@ -367,7 +369,12 @@ private function fetchRow(string $jobName, string $id): array
         $rows = $statement->fetchAllAssociative();
         switch (count($rows)) {
             case 1:
-                return array_shift($rows);
+                $row = array_shift($rows);
+                if (!\is_array($row)) {
+                    throw UnexpectedValueException::type('array', $row);
+                }
+
+                return $row;
             case 0:
                 throw new RuntimeException(
                     \sprintf('No row found for job %s#%s.', $jobName, $id)
@@ -381,6 +388,7 @@ private function fetchRow(string $jobName, string $id): array
 
     private function queryList(string $query, array $parameters, array $types): iterable
     {
+        /** @var Result $statement */
         $statement = $this->connection->executeQuery($query, $parameters, $types);
 
         while ($row = $statement->fetchAssociative()) {

src/batch-doctrine-dbal/src/JobExecutionRowNormalizer.php

@@ -185,6 +185,11 @@ public function toChildRow(JobExecution $jobExecution): array
         ];
     }
 
+    /**
+     * @param array|string $value
+     *
+     * @return array
+     */
     private function jsonFromString($value): array
     {
         if (is_string($value)) {

src/batch-doctrine-persistence/src/ObjectWriter.php

@@ -42,6 +42,10 @@ public function __construct(ManagerRegistry $doctrine)
     public function write(iterable $items): void
     {
         foreach ($items as $item) {
+            if (!is_object($item)) {
+                throw $this->createInvalidItemException($item);
+            }
+
             $this->getManagerForClass($item)->persist($item);
         }
 
@@ -57,12 +61,8 @@ public function write(iterable $items): void
         $this->encounteredClasses = [];
     }
 
-    private function getManagerForClass($item): ObjectManager
+    private function getManagerForClass(object $item): ObjectManager
     {
-        if (!is_object($item)) {
-            throw $this->createInvalidItemException($item);
-        }
-
         $class = get_class($item);
 
         $manager = $this->managerForClass[$class] ?? null;
@@ -81,6 +81,11 @@ private function getManagerForClass($item): ObjectManager
         return $manager;
     }
 
+    /**
+     * @param mixed $item
+     *
+     * @return InvalidArgumentException
+     */
     private function createInvalidItemException($item): InvalidArgumentException
     {
         return new InvalidArgumentException(

src/batch-doctrine-persistence/tests/ObjectWriterTest.php

@@ -9,7 +9,7 @@
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
-use Prophecy\Prophecy\ObjectProphecny;
+use Prophecy\Prophecy\ObjectProphecy;
 use Yokai\Batch\Bridge\Doctrine\Persistence\ObjectWriter;
 use Yokai\Batch\Exception\InvalidArgumentException;