Rector improvements

CHANGELOG.md

@@ -9,6 +9,13 @@ See [GitHub releases](https://github.com/mll-lab/php-utils/releases).
 
 ## Unreleased
 
+## v5.6.0
+
+### Added
+
+- Improve code quality with automatic type declarations via Rector
+- Improve code quality with automatic rector rules for PHPUnit
+
 ## v5.5.2
 
 ### Fixed

composer.json

@@ -26,8 +26,10 @@
   "require-dev": {
     "ergebnis/composer-normalize": "^2",
     "jangregor/phpstan-prophecy": "^1",
+    "larastan/larastan": "^1 || ^2",
     "mll-lab/graphql-php-scalars": "^6.3",
     "mll-lab/php-cs-fixer-config": "^5",
+    "orchestra/testbench": "^6 || ^7 || ^8 || ^9",
     "phpstan/extension-installer": "^1",
     "phpstan/phpstan": "^1",
     "phpstan/phpstan-deprecation-rules": "^1",

rector.php

@@ -2,25 +2,37 @@
 
 use Rector\CodeQuality\Rector\Concat\JoinStringConcatRector;
 use Rector\Config\RectorConfig;
-use Rector\PHPUnit\Rector\Class_\PreferPHPUnitSelfCallRector;
+use Rector\PHPUnit\CodeQuality\Rector\Class_\PreferPHPUnitSelfCallRector;
+use Rector\PHPUnit\CodeQuality\Rector\Class_\PreferPHPUnitThisCallRector;
+use Rector\PHPUnit\Set\PHPUnitSetList;
 use Rector\Set\ValueObject\SetList;
 
-return static function (RectorConfig $rectorConfig): void {
-    $rectorConfig->sets([
-        SetList::PHP_71,
-        SetList::PHP_72,
-        SetList::PHP_73,
-        SetList::PHP_74,
+return RectorConfig::configure()
+    ->withSets([
         SetList::CODE_QUALITY,
-    ]);
-    $rectorConfig->skip([
+        SetList::TYPE_DECLARATION,
+        SetList::RECTOR_PRESET,
+        PHPUnitSetList::PHPUNIT_40,
+        PHPUnitSetList::PHPUNIT_50,
+        PHPUnitSetList::PHPUNIT_60,
+        PHPUnitSetList::PHPUNIT_70,
+        PHPUnitSetList::PHPUNIT_80,
+        PHPUnitSetList::PHPUNIT_90,
+        PHPUnitSetList::PHPUNIT_100,
+        PHPUnitSetList::PHPUNIT_110,
+        PHPUnitSetList::PHPUNIT_CODE_QUALITY,
+    ])
+    ->withPhpSets()
+    ->withRules([PreferPHPUnitSelfCallRector::class])
+    ->withSkip([
+        PreferPHPUnitThisCallRector::class, // breaks tests
         JoinStringConcatRector::class => [
             __DIR__ . '/tests/CSVArrayTest.php', // keep `\r\n` for readability
         ],
-    ]);
-
-    $rectorConfig->rule(PreferPHPUnitSelfCallRector::class);
-
-    $rectorConfig->paths([__DIR__ . '/src', __DIR__ . '/tests']);
-    $rectorConfig->phpstanConfig(__DIR__ . '/phpstan.neon');
-};
+    ])
+    ->withPaths([__DIR__ . '/src', __DIR__ . '/tests'])
+    ->withBootstrapFiles([
+        // Rector uses PHPStan internally, which in turn requires Larastan to be set up correctly
+        __DIR__ . '/vendor/larastan/larastan/bootstrap.php',
+    ])
+    ->withPHPStanConfigs([__DIR__ . '/phpstan.neon']);

src/IlluminaSampleSheet/V1/DataSection.php

@@ -53,10 +53,10 @@ public function convertSectionToString(): string
     protected function validateDuplicatedSampleIDs(): void
     {
         $groups = $this->rows
-            ->groupBy(fn (Row $row) => $row->sampleID);
+            ->groupBy(fn (Row $row): string => $row->sampleID);
 
         $duplicates = $groups
-            ->filter(fn ($group) => count($group) > 1)
+            ->filter(fn ($group): bool => count($group) > 1)
             ->keys();
         $duplicateIDsAsString = $duplicates->implode(', ');
 

src/IlluminaSampleSheet/V2/BclConvert/DataSection.php

@@ -26,8 +26,12 @@ public function convertSectionToString(): string
     {
         $this->assertNotEmpty();
 
+        $object = $this->dataRows[0];
+        if (! is_object($object)) {
+            throw new IlluminaSampleSheetException('Trying to convert empty data section to string.');
+        }
         /** @var array<string> $samplePropertiesOfFirstSample */
-        $samplePropertiesOfFirstSample = array_keys(get_object_vars($this->dataRows[0]));
+        $samplePropertiesOfFirstSample = array_keys(get_object_vars($object));
         foreach ($this->dataRows as $sample) {
             $actualProperties = array_keys(get_object_vars($sample));
             if ($samplePropertiesOfFirstSample !== $actualProperties) {
@@ -52,10 +56,10 @@ public function convertSectionToString(): string
     /** @param array<string> $samplePropertiesOfFirstSample */
     protected function generateDataHeaderByProperties(array $samplePropertiesOfFirstSample): string
     {
-        $samplePropertiesOfFirstSample = array_filter($samplePropertiesOfFirstSample, fn (string $value) // @phpstan-ignore-next-line Variable property access on a non-object required here
+        $samplePropertiesOfFirstSample = array_filter($samplePropertiesOfFirstSample, fn (string $value): bool // @phpstan-ignore-next-line Variable property access on a non-object required here
         => $this->dataRows[0]->$value !== null);
 
-        $samplePropertiesOfFirstSample = array_map(fn (string $value) => ucfirst($value), $samplePropertiesOfFirstSample);
+        $samplePropertiesOfFirstSample = array_map(fn (string $value): string => ucfirst($value), $samplePropertiesOfFirstSample);
 
         return implode(',', $samplePropertiesOfFirstSample);
     }

src/IlluminaSampleSheet/V2/BclConvert/OverrideCycles.php

@@ -53,7 +53,7 @@ public function makeOverrideCycle(string $cycleString): OverrideCycle
 
         return new OverrideCycle(
             array_map(
-                fn (array $match) => new CycleTypeWithCount(new CycleType($match[1]), (int) $match[2]),
+                fn (array $match): CycleTypeWithCount => new CycleTypeWithCount(new CycleType($match[1]), (int) $match[2]),
                 $matches
             )
         );

src/Microplate/AbstractMicroplate.php

@@ -70,7 +70,6 @@ function ($content, string $key) use ($flowDirection): string {
     /** @return Collection<string, null> */
     public function freeWells(): Collection
     {
-        // @phpstan-ignore-next-line Only recognized to be correct with larastan
         return $this->wells()->filter(
             /** @param TWell $content */
             static fn ($content): bool => $content === self::EMPTY_WELL
@@ -80,7 +79,6 @@ public function freeWells(): Collection
     /** @return Collection<string, TWell> */
     public function filledWells(): Collection
     {
-        // @phpstan-ignore-next-line Only recognized to be correct with larastan
         return $this->wells()->filter(
             /** @param TWell $content */
             static fn ($content): bool => $content !== self::EMPTY_WELL

src/Microplate/Coordinates.php

@@ -52,7 +52,7 @@ public function __construct(string $row, int $column, CoordinateSystem $coordina
      *
      * @return static<TCoord>
      */
-    public static function fromArray(array $coordinates, CoordinateSystem $coordinateSystem)
+    public static function fromArray(array $coordinates, CoordinateSystem $coordinateSystem): self
     {
         return new self($coordinates['row'], $coordinates['column'], $coordinateSystem);
     }

src/Tecan/Rack/BaseRack.php

@@ -18,6 +18,7 @@ abstract class BaseRack implements Rack
     public function __construct(CoordinateSystem $coordinateSystem)
     {
         $this->coordinateSystem = $coordinateSystem;
+        /** @phpstan-ignore-next-line types are correct, but phpstan doesn't understand it */
         $this->positions = Collection::times($this->positionCount(), fn () => self::EMPTY_POSITION)
             ->mapWithKeys(fn ($content, int $position): array => [$position + 1 => $content]);
     }

tests/DnaSequenceTest.php

@@ -10,19 +10,19 @@ final class DnaSequenceTest extends TestCase
     public function testReverse(): void
     {
         $dnaSequence = new DnaSequence('ATGC');
-        self::assertEquals('CGTA', $dnaSequence->reverse());
+        self::assertSame('CGTA', $dnaSequence->reverse());
     }
 
     public function testComplement(): void
     {
         $dnaSequence = new DnaSequence('ATGC');
-        self::assertEquals('TACG', $dnaSequence->complement());
+        self::assertSame('TACG', $dnaSequence->complement());
     }
 
     public function testReverseComplement(): void
     {
         $dnaSequence = new DnaSequence('ATGC');
-        self::assertEquals('GCAT', $dnaSequence->reverseComplement());
+        self::assertSame('GCAT', $dnaSequence->reverseComplement());
     }
 
     public function testInvalidSequenceThrowsException(): void
@@ -34,8 +34,8 @@ public function testInvalidSequenceThrowsException(): void
     public function testEmptySequence(): void
     {
         $dnaSequence = new DnaSequence('');
-        self::assertEquals('', $dnaSequence->reverse());
-        self::assertEquals('', $dnaSequence->complement());
-        self::assertEquals('', $dnaSequence->reverseComplement());
+        self::assertSame('', $dnaSequence->reverse());
+        self::assertSame('', $dnaSequence->complement());
+        self::assertSame('', $dnaSequence->reverseComplement());
     }
 }

tests/IlluminaSampleSheet/V1/DualIndexTest.php

@@ -33,17 +33,15 @@ public function testValidate(
         new DualIndex($i7IndexID, $index, $i5IndexID, $index2);
     }
 
-    /** @return array<string, array<int, string|bool|null>> */
-    public static function provideValidDualIndexes(): array
+    /** @return iterable<string, array<int, string|bool|null>> */
+    public static function provideValidDualIndexes(): iterable
     {
-        return [
-            'Both indices valid long' => ['someIndexID', 'ATCGNGTANGT', 'someOtherIndexID', 'ATGNAAATTTTAC', true],
-            'Both indices valid short' => ['someIndexID', 'A', 'someOtherIndexID', 'G', true],
-            'Both indices invalid' => ['i7IndexID', 'invalidValue', 'i5IndexID', 'invalidValue2', false, 'invalidValue'],
-            'First index invalid' => ['i7IndexID', 'invalidValue', 'i5IndexID', 'ATCGNGT', false, 'invalidValue'],
-            'Second index invalid' => ['i7IndexID', 'ATCGNGT', 'i5IndexID', 'invalidValue2', false, 'invalidValue2'],
-            'First index empty' => ['someIndexID', '', 'someOtherIndexID', 'ATGNAAAC', false, ''],
-            'Second index empty' => ['someIndexID', 'ATCGNGT', 'someOtherIndexID', '', false, ''],
-        ];
+        yield 'Both indices valid long' => ['someIndexID', 'ATCGNGTANGT', 'someOtherIndexID', 'ATGNAAATTTTAC', true];
+        yield 'Both indices valid short' => ['someIndexID', 'A', 'someOtherIndexID', 'G', true];
+        yield 'Both indices invalid' => ['i7IndexID', 'invalidValue', 'i5IndexID', 'invalidValue2', false, 'invalidValue'];
+        yield 'First index invalid' => ['i7IndexID', 'invalidValue', 'i5IndexID', 'ATCGNGT', false, 'invalidValue'];
+        yield 'Second index invalid' => ['i7IndexID', 'ATCGNGT', 'i5IndexID', 'invalidValue2', false, 'invalidValue2'];
+        yield 'First index empty' => ['someIndexID', '', 'someOtherIndexID', 'ATGNAAAC', false, ''];
+        yield 'Second index empty' => ['someIndexID', 'ATCGNGT', 'someOtherIndexID', '', false, ''];
     }
 }

tests/Microplate/MicroplateTest.php

@@ -29,8 +29,8 @@ public function testCanAddAndRetrieveWellBasedOnCoordinateSystem(): void
         $wellContent2 = 'bar';
         $microplate->addWell($microplateCoordinate2, $wellContent2);
 
-        self::assertEquals($wellContent1, $microplate->well($microplateCoordinate1));
-        self::assertEquals($wellContent2, $microplate->well($microplateCoordinate2));
+        self::assertSame($wellContent1, $microplate->well($microplateCoordinate1));
+        self::assertSame($wellContent2, $microplate->well($microplateCoordinate2));
 
         $coordinateWithOtherCoordinateSystem = new Coordinates('B', 2, new CoordinateSystem4x3());
         // @phpstan-ignore-next-line expecting a type error due to mismatching coordinates
@@ -145,7 +145,7 @@ private function preparePlate(): Microplate
         foreach ($data12x8 as $well) {
             $microplateCoordinates = Coordinates::fromArray($well, new CoordinateSystem12x8());
 
-            $randomNumber = rand(1, 100);
+            $randomNumber = random_int(1, 100);
             $randomNumberOrNull = $randomNumber > 50 ? $randomNumber : null;
 
             $microplate->addWell($microplateCoordinates, $randomNumberOrNull);
@@ -200,7 +200,7 @@ public function testThrowsPlateFullException(): void
             $microplateCoordinates = Coordinates::fromArray($wellData, $coordinateSystem);
             // check that it does not throw before the plate is full
             self::assertEquals($microplateCoordinates, $microplate->nextFreeWellCoordinates(FlowDirection::ROW()));
-            $microplate->addWell($microplateCoordinates, rand(1, 100));
+            $microplate->addWell($microplateCoordinates, random_int(1, 100));
         }
 
         $this->expectException(MicroplateIsFullException::class);

tests/Tecan/Rack/RackTest.php

@@ -25,8 +25,8 @@ public function testAssignsFirstEmptyPosition(): void
         self::assertSame(32, $rack->positionCount());
 
         $position = $rack->assignFirstEmptyPosition('Sample');
-        self::assertEquals(1, $position);
-        self::assertEquals('Sample', $rack->positions[1]);
+        self::assertSame(1, $position);
+        self::assertSame('Sample', $rack->positions[1]);
     }
 
     public function testAssignsLastEmptyPosition(): void
@@ -36,8 +36,8 @@ public function testAssignsLastEmptyPosition(): void
         self::assertSame($lastPosition, $rack->positionCount());
 
         $position = $rack->assignLastEmptyPosition('Sample');
-        self::assertEquals($lastPosition, $position);
-        self::assertEquals('Sample', $rack->positions[$lastPosition]);
+        self::assertSame($lastPosition, $position);
+        self::assertSame('Sample', $rack->positions[$lastPosition]);
     }
 
     public function testThrowsExceptionWhenNoEmptyPosition(): void