refactor: improve database migration scanning (larastan#670)

bepsvpt-fork · Sep 27, 2020 · bc5dab5 · bc5dab5
1 parent aefdb60
commit bc5dab5
Show file tree

Hide file tree

Showing 10 changed files with 318 additions and 58 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Parameter type of the query builder's `where`, `orWhere` and `addArrayOfWheres` ([#651](https://github.com/nunomaduro/larastan/pull/651)).
 - Using Reflection to initiate a model in `ModelPropertyExtension` to avoid errors caused by using Model constructor. ([#666](https://github.com/nunomaduro/larastan/pull/666))
 
+### Changed
+- Made improvements to database migrations scanning. ([#670](https://github.com/nunomaduro/larastan/pull/670))
+
 ## [0.6.4] - 2020-09-02
 
 ### Changed

diff --git a/extension.neon b/extension.neon
@@ -41,11 +41,13 @@ parameters:
     noUnnecessaryCollectionCall: true
     noUnnecessaryCollectionCallOnly: []
     noUnnecessaryCollectionCallExcept: []
+    databaseMigrationsPath: null
 
 parametersSchema:
     noUnnecessaryCollectionCall: bool()
     noUnnecessaryCollectionCallOnly: listOf(string())
     noUnnecessaryCollectionCallExcept: listOf(string())
+    databaseMigrationsPath: schema(anyOf(string()), nullable())
 
 conditionalTags:
     NunoMaduro\Larastan\Rules\NoUnnecessaryCollectionCallRule:
@@ -297,3 +299,6 @@ services:
 
     -
         class: NunoMaduro\Larastan\Types\RelationParserHelper
+
+    -
+        class: NunoMaduro\Larastan\Properties\MigrationHelper
diff --git a/src/Properties/MigrationHelper.php b/src/Properties/MigrationHelper.php
@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace NunoMaduro\Larastan\Properties;
+
+use Iterator;
+use PHPStan\Parser\CachedParser;
+use RecursiveDirectoryIterator;
+use RecursiveIteratorIterator;
+use RegexIterator;
+use SplFileInfo;
+
+class MigrationHelper
+{
+    /** @var CachedParser */
+    private $parser;
+
+    /** @var ?string */
+    private $databaseMigrationPath;
+
+    public function __construct(CachedParser $parser, ?string $databaseMigrationPath)
+    {
+        $this->parser = $parser;
+        $this->databaseMigrationPath = $databaseMigrationPath;
+    }
+
+    /**
+     * @return array<string, SchemaTable>
+     */
+    public function initializeTables(): array
+    {
+        if ($this->databaseMigrationPath === null) {
+            $this->databaseMigrationPath = database_path().'/migrations';
+        }
+
+        if (! is_dir($this->databaseMigrationPath)) {
+            return [];
+        }
+
+        $schemaAggregator = new SchemaAggregator();
+        $files = $this->getMigrationFiles($this->databaseMigrationPath);
+        $filesArray = iterator_to_array($files);
+        ksort($filesArray);
+
+        $this->requireFiles($filesArray);
+
+        foreach ($filesArray as $file) {
+            $schemaAggregator->addStatements($this->parser->parseFile($file->getPathname()));
+        }
+
+        return $schemaAggregator->tables;
+    }
+
+    /**
+     * @param string $path
+     *
+     * @return Iterator<SplFileInfo>
+     */
+    private function getMigrationFiles(string $path): Iterator
+    {
+        return new RegexIterator(new RecursiveIteratorIterator(new RecursiveDirectoryIterator($path)), '/\.php$/i');
+    }
+
+    /**
+     * @param SplFileInfo[] $files
+     */
+    private function requireFiles(array $files): void
+    {
+        foreach ($files as $file) {
+            require_once $file;
+        }
+    }
+}
diff --git a/src/Properties/ModelPropertyExtension.php b/src/Properties/ModelPropertyExtension.php
@@ -6,28 +6,19 @@
 
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Support\Str;
-use Iterator;
-use PHPStan\Parser\CachedParser;
 use PHPStan\PhpDoc\TypeStringResolver;
 use PHPStan\Reflection\Annotations\AnnotationsPropertiesClassReflectionExtension;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\PropertiesClassReflectionExtension;
 use PHPStan\Reflection\PropertyReflection;
 use PHPStan\ShouldNotHappenException;
 use PHPStan\Type\IntegerType;
-use RecursiveDirectoryIterator;
-use RecursiveIteratorIterator;
-use RegexIterator;
-use SplFileInfo;
 
 /**
  * @internal
  */
 final class ModelPropertyExtension implements PropertiesClassReflectionExtension
 {
-    /** @var CachedParser */
-    private $parser;
-
     /** @var SchemaTable[] */
     private $tables = [];
 
@@ -40,11 +31,14 @@ final class ModelPropertyExtension implements PropertiesClassReflectionExtension
     /** @var AnnotationsPropertiesClassReflectionExtension */
     private $annotationExtension;
 
-    public function __construct(CachedParser $parser, TypeStringResolver $stringResolver, AnnotationsPropertiesClassReflectionExtension $annotationExtension)
+    /** @var MigrationHelper */
+    private $migrationHelper;
+
+    public function __construct(TypeStringResolver $stringResolver, AnnotationsPropertiesClassReflectionExtension $annotationExtension, MigrationHelper $migrationHelper)
     {
-        $this->parser = $parser;
         $this->stringResolver = $stringResolver;
         $this->annotationExtension = $annotationExtension;
+        $this->migrationHelper = $migrationHelper;
     }
 
     public function hasProperty(ClassReflection $classReflection, string $propertyName): bool
@@ -66,7 +60,7 @@ public function hasProperty(ClassReflection $classReflection, string $propertyNa
         }
 
         if (count($this->tables) === 0) {
-            $this->initializeTables();
+            $this->tables = $this->migrationHelper->initializeTables();
         }
 
         if ($propertyName === 'id') {
@@ -149,46 +143,6 @@ public function getProperty(
         );
     }
 
-    private function initializeTables(): void
-    {
-        if (! is_dir(database_path().'/migrations')) {
-            return;
-        }
-
-        $schemaAggregator = new SchemaAggregator();
-        $files = $this->getMigrationFiles(database_path().'/migrations');
-        $filesArray = iterator_to_array($files);
-        ksort($filesArray);
-
-        $this->requireFiles($filesArray);
-
-        foreach ($filesArray as $file) {
-            $schemaAggregator->addStatements($this->parser->parseFile($file->getPathname()));
-        }
-
-        $this->tables = $schemaAggregator->tables;
-    }
-
-    /**
-     * @param string $path
-     *
-     * @return Iterator<SplFileInfo>
-     */
-    private function getMigrationFiles(string $path): Iterator
-    {
-        return new RegexIterator(new RecursiveIteratorIterator(new RecursiveDirectoryIterator($path)), '/\.php$/i');
-    }
-
-    /**
-     * @param SplFileInfo[] $files
-     */
-    private function requireFiles(array $files): void
-    {
-        foreach ($files as $file) {
-            require_once $file;
-        }
-    }
-
     private function getDateClass(): string
     {
         if (! $this->dateClass) {

diff --git a/src/Properties/SchemaAggregator.php b/src/Properties/SchemaAggregator.php
@@ -5,6 +5,7 @@
 namespace NunoMaduro\Larastan\Properties;
 
 use PhpParser;
+use PhpParser\NodeFinder;
 
 /**
  * @see https://github.com/psalm/laravel-psalm-plugin/blob/master/src/SchemaAggregator.php
@@ -19,10 +20,13 @@ final class SchemaAggregator
      */
     public function addStatements(array $stmts): void
     {
-        foreach ($stmts as $stmt) {
-            if ($stmt instanceof PhpParser\Node\Stmt\Class_) {
-                $this->addClassStatements($stmt->stmts);
-            }
+        $nodeFinder = new NodeFinder();
+
+        /** @var PhpParser\Node\Stmt\Class_[] $classes */
+        $classes = $nodeFinder->findInstanceOf($stmts, PhpParser\Node\Stmt\Class_::class);
+
+        foreach ($classes as $stmt) {
+            $this->addClassStatements($stmt->stmts);
         }
     }
 
@@ -33,7 +37,7 @@ private function addClassStatements(array $stmts): void
     {
         foreach ($stmts as $stmt) {
             if ($stmt instanceof PhpParser\Node\Stmt\ClassMethod
-                && $stmt->name->name === 'up'
+                && $stmt->name->name !== 'down'
                 && $stmt->stmts
             ) {
                 $this->addUpMethodStatements($stmt->stmts);
@@ -46,7 +50,10 @@ private function addClassStatements(array $stmts): void
      */
     private function addUpMethodStatements(array $stmts): void
     {
-        foreach ($stmts as $stmt) {
+        $nodeFinder = new NodeFinder();
+        $methods = $nodeFinder->findInstanceOf($stmts, PhpParser\Node\Stmt\Expression::class);
+
+        foreach ($methods as $stmt) {
             if ($stmt instanceof PhpParser\Node\Stmt\Expression
                 && $stmt->expr instanceof PhpParser\Node\Expr\StaticCall
                 && ($stmt->expr->class instanceof PhpParser\Node\Name)

diff --git a/tests/Unit/MigrationHelperTest.php b/tests/Unit/MigrationHelperTest.php
@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Unit;
+
+use NunoMaduro\Larastan\Properties\MigrationHelper;
+use NunoMaduro\Larastan\Properties\SchemaTable;
+use PHPStan\Parser\CachedParser;
+use PHPStan\Testing\TestCase;
+
+class MigrationHelperTest extends TestCase
+{
+    /** @var CachedParser */
+    private $cachedParser;
+
+    public function setUp(): void
+    {
+        $this->cachedParser = self::getContainer()->getByType(CachedParser::class);
+    }
+
+    /** @test */
+    public function it_will_return_empty_array_if_migrations_path_is_not_a_directory()
+    {
+        $migrationHelper = new MigrationHelper($this->cachedParser, 'foobar');
+
+        self::assertSame([], $migrationHelper->initializeTables());
+    }
+
+    /** @test */
+    public function it_can_read_basic_migrations_and_create_table_structure()
+    {
+        $migrationHelper = new MigrationHelper($this->cachedParser, __DIR__.'/data/basic_migration');
+
+        $tables = $migrationHelper->initializeTables();
+
+        $this->assertUsersTableSchema($tables);
+    }
+
+    /** @test */
+    public function it_can_read_schema_definitions_from_any_method_in_class()
+    {
+        $migrationHelper = new MigrationHelper($this->cachedParser, __DIR__.'/data/migrations_with_different_methods');
+
+        $tables = $migrationHelper->initializeTables();
+
+        $this->assertUsersTableSchema($tables);
+    }
+
+    /** @test */
+    public function it_can_read_schema_definitions_with_multiple_create_and_drop_methods_for_one_table()
+    {
+        $migrationHelper = new MigrationHelper($this->cachedParser, __DIR__.'/data/complex_migrations');
+
+        $tables = $migrationHelper->initializeTables();
+
+        self::assertCount(1, $tables);
+        self::assertArrayHasKey('users', $tables);
+        self::assertCount(6, $tables['users']->columns);
+        self::assertSame(['id', 'email', 'birthday', 'created_at', 'updated_at', 'active'], array_keys($tables['users']->columns));
+        self::assertSame('int', $tables['users']->columns['id']->readableType);
+        self::assertSame('string', $tables['users']->columns['email']->readableType);
+        self::assertSame('string', $tables['users']->columns['birthday']->readableType);
+        self::assertSame('string', $tables['users']->columns['created_at']->readableType);
+        self::assertSame('string', $tables['users']->columns['updated_at']->readableType);
+        self::assertSame('int', $tables['users']->columns['active']->readableType);
+    }
+
+    /**
+     * @param array<string, SchemaTable> $tables
+     */
+    private function assertUsersTableSchema(array $tables): void
+    {
+        self::assertCount(1, $tables);
+        self::assertArrayHasKey('users', $tables);
+        self::assertCount(5, $tables['users']->columns);
+        self::assertSame(['id', 'name', 'email', 'created_at', 'updated_at'], array_keys($tables['users']->columns));
+        self::assertSame('int', $tables['users']->columns['id']->readableType);
+        self::assertSame('string', $tables['users']->columns['name']->readableType);
+        self::assertSame('string', $tables['users']->columns['email']->readableType);
+        self::assertSame('string', $tables['users']->columns['created_at']->readableType);
+        self::assertSame('string', $tables['users']->columns['updated_at']->readableType);
+    }
+}
diff --git a/tests/Unit/data/basic_migration/2020_01_30_000000_create_users_table.php b/tests/Unit/data/basic_migration/2020_01_30_000000_create_users_table.php
@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Unit\BasicMigrations;
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class CreateUsersTable extends Migration
+{
+    /**
+     * Run the migrations.
+     */
+    public function up(): void
+    {
+        Schema::create('users', static function (Blueprint $table) {
+            $table->bigIncrements('id');
+            $table->string('name')->nullable();
+            $table->string('email')->unique();
+            $table->timestamps();
+        });
+    }
+}
diff --git a/tests/Unit/data/complex_migrations/2020_01_30_000000_create_users_table.php b/tests/Unit/data/complex_migrations/2020_01_30_000000_create_users_table.php
@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Unit\ComplexMigrations;
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class CreateUsersTable extends Migration
+{
+    /**
+     * Run the migrations.
+     */
+    public function up(): void
+    {
+        Schema::create('users', static function (Blueprint $table) {
+            $table->bigIncrements('id');
+            $table->string('name')->nullable();
+            $table->string('email')->unique();
+            $table->timestamps();
+        });
+    }
+}