From d25f5087befa576ef1236c2cc8b1062b4a811a58 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin Date: Wed, 25 Jan 2023 11:22:00 +0100 Subject: [PATCH 1/4] Add Column type consts, cleanup code --- src/Fakes/FakeModelsCommand.php | 13 +++++++------ .../Eloquent/ModelPropertyAccessorHandler.php | 1 - .../Eloquent/Schema/SchemaAggregator.php | 18 ++++++++---------- src/Handlers/Eloquent/Schema/SchemaColumn.php | 7 +++++++ 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/Fakes/FakeModelsCommand.php b/src/Fakes/FakeModelsCommand.php index ce4c5181..edcf2b12 100644 --- a/src/Fakes/FakeModelsCommand.php +++ b/src/Fakes/FakeModelsCommand.php @@ -7,6 +7,7 @@ use Illuminate\Filesystem\Filesystem; use Illuminate\Support\Str; use Psalm\LaravelPlugin\Handlers\Eloquent\Schema\SchemaAggregator; +use Psalm\LaravelPlugin\Handlers\Eloquent\Schema\SchemaColumn; use function config; use function get_class; @@ -71,13 +72,13 @@ public function getPropertiesFromTable($model): void $get_type = $set_type = '\Illuminate\Support\Carbon'; } else { switch ($column->type) { - case 'string': - case 'int': - case 'float': + case SchemaColumn::TYPE_STRING: + case SchemaColumn::TYPE_INT: + case SchemaColumn::TYPE_FLOAT: $get_type = $set_type = $column->type; break; - case 'bool': + case SchemaColumn::TYPE_BOOL: switch (config('database.default')) { case 'sqlite': case 'mysql': @@ -91,7 +92,7 @@ public function getPropertiesFromTable($model): void break; - case 'enum': + case SchemaColumn::TYPE_ENUM: if (!$column->options) { $get_type = $set_type = 'string'; } else { @@ -101,7 +102,7 @@ public function getPropertiesFromTable($model): void break; default: - $get_type = $set_type = 'mixed'; + $get_type = $set_type = SchemaColumn::TYPE_MIXED; break; } } diff --git a/src/Handlers/Eloquent/ModelPropertyAccessorHandler.php b/src/Handlers/Eloquent/ModelPropertyAccessorHandler.php index ef88660b..069d987c 100644 --- a/src/Handlers/Eloquent/ModelPropertyAccessorHandler.php +++ b/src/Handlers/Eloquent/ModelPropertyAccessorHandler.php @@ -24,7 +24,6 @@ public static function getClassLikeNames(): array return ModelStubProvider::getModelClasses(); } - public static function doesPropertyExist(PropertyExistenceProviderEvent $event): ?bool { $source = $event->getSource(); diff --git a/src/Handlers/Eloquent/Schema/SchemaAggregator.php b/src/Handlers/Eloquent/Schema/SchemaAggregator.php index 395b9341..a0928ce5 100644 --- a/src/Handlers/Eloquent/Schema/SchemaAggregator.php +++ b/src/Handlers/Eloquent/Schema/SchemaAggregator.php @@ -4,6 +4,7 @@ use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Schema; +use PhpParser\NodeFinder; use PhpParser; use function array_key_exists; @@ -53,16 +54,13 @@ class SchemaAggregator */ public function addStatements(array $stmts): void { - foreach ($stmts as $stmt) { - if ($stmt instanceof PhpParser\Node\Stmt\Class_) { - $this->addClassStatements($stmt->stmts); - } elseif ( - $stmt instanceof PhpParser\Node\Stmt\Return_ && - $stmt->expr instanceof PhpParser\Node\Expr\New_ && - $stmt->expr->class instanceof PhpParser\Node\Stmt\Class_ - ) { - $this->addClassStatements($stmt->expr->class->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); } } diff --git a/src/Handlers/Eloquent/Schema/SchemaColumn.php b/src/Handlers/Eloquent/Schema/SchemaColumn.php index 6a0fbaba..5e87a7b3 100644 --- a/src/Handlers/Eloquent/Schema/SchemaColumn.php +++ b/src/Handlers/Eloquent/Schema/SchemaColumn.php @@ -4,6 +4,13 @@ class SchemaColumn { + public const TYPE_STRING = 'string'; + public const TYPE_INT = 'int'; + public const TYPE_FLOAT = 'float'; + public const TYPE_BOOL = 'bool'; + public const TYPE_ENUM = 'enum'; + public const TYPE_MIXED = 'mixed'; + /** @var string */ public $name; From e1f6acafb6e5b39fe1eee7c2cb89cd9ec83647d4 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin Date: Wed, 25 Jan 2023 11:22:34 +0100 Subject: [PATCH 2/4] Add a test for old migration --- .../Eloquent/Schema/DefaultUserTableTest.php | 19 ++++++++-- .../2014_10_12_000000_create_users_table.php | 0 .../2014_10_12_000000_create_users_table.php | 35 +++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) rename tests/Unit/Handlers/Eloquent/Schema/migrations/{simple => default_users_table_anon}/2014_10_12_000000_create_users_table.php (100%) create mode 100644 tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_named/2014_10_12_000000_create_users_table.php diff --git a/tests/Unit/Handlers/Eloquent/Schema/DefaultUserTableTest.php b/tests/Unit/Handlers/Eloquent/Schema/DefaultUserTableTest.php index 83068643..deeb3b3c 100644 --- a/tests/Unit/Handlers/Eloquent/Schema/DefaultUserTableTest.php +++ b/tests/Unit/Handlers/Eloquent/Schema/DefaultUserTableTest.php @@ -8,9 +8,24 @@ final class DefaultUserTableTest extends AbstractSchemaAggregatorTest { /** @test */ - public function it_detects_all_columns(): void + public function it_detects_all_columns_from_anonymous_class_migration(): void { - $schemaAggregator = $this->instantiateSchemaAggregator(__DIR__.'/migrations/simple'); + $schemaAggregator = $this->instantiateSchemaAggregator(__DIR__ . '/migrations/default_users_table_anon'); + + $this->assertSchemaHasTableAndNotNullableColumnOfType('users.id', 'int', $schemaAggregator); + $this->assertSchemaHasTableAndNotNullableColumnOfType('users.email', 'string', $schemaAggregator); + $this->assertSchemaHasTableAndNotNullableColumnOfType('users.password', 'string', $schemaAggregator); + $this->assertSchemaHasTableAndNotNullableColumnOfType('users.password', 'string', $schemaAggregator); + $this->assertSchemaHasTableAndNotNullableColumnOfType('users.remember_token', 'string', $schemaAggregator); + $this->assertSchemaHasTableAndNullableColumnOfType('users.email_verified_at', 'string', $schemaAggregator); + $this->assertSchemaHasTableAndNullableColumnOfType('users.created_at', 'string', $schemaAggregator); + $this->assertSchemaHasTableAndNullableColumnOfType('users.updated_at', 'string', $schemaAggregator); + } + + /** @test */ + public function it_detects_all_columns_from_named_class_migration(): void + { + $schemaAggregator = $this->instantiateSchemaAggregator(__DIR__ . '/migrations/default_users_table_named'); $this->assertSchemaHasTableAndNotNullableColumnOfType('users.id', 'int', $schemaAggregator); $this->assertSchemaHasTableAndNotNullableColumnOfType('users.email', 'string', $schemaAggregator); diff --git a/tests/Unit/Handlers/Eloquent/Schema/migrations/simple/2014_10_12_000000_create_users_table.php b/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_anon/2014_10_12_000000_create_users_table.php similarity index 100% rename from tests/Unit/Handlers/Eloquent/Schema/migrations/simple/2014_10_12_000000_create_users_table.php rename to tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_anon/2014_10_12_000000_create_users_table.php diff --git a/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_named/2014_10_12_000000_create_users_table.php b/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_named/2014_10_12_000000_create_users_table.php new file mode 100644 index 00000000..16e0768e --- /dev/null +++ b/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_named/2014_10_12_000000_create_users_table.php @@ -0,0 +1,35 @@ +id(); + $table->string('name'); + $table->string('email')->unique(); + $table->timestamp('email_verified_at')->nullable(); + $table->string('password'); + $table->rememberToken(); + $table->timestamps(); + }); + } + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::dropIfExists('users'); + } +} From e57f04bfc95c24afba67c7a6fda9acd312e9929e Mon Sep 17 00:00:00 2001 From: Alies Lapatsin Date: Wed, 25 Jan 2023 11:41:13 +0100 Subject: [PATCH 3/4] Support \Schema alias while parsing migrations --- .../Eloquent/Schema/SchemaAggregator.php | 44 ++++++++++--------- .../Eloquent/Schema/DefaultUserTableTest.php | 22 ++++++---- .../2014_10_12_000000_create_users_table.php | 34 ++++++++++++++ 3 files changed, 71 insertions(+), 29 deletions(-) create mode 100644 tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_root_ns_facade/2014_10_12_000000_create_users_table.php diff --git a/src/Handlers/Eloquent/Schema/SchemaAggregator.php b/src/Handlers/Eloquent/Schema/SchemaAggregator.php index a0928ce5..8887ddfb 100644 --- a/src/Handlers/Eloquent/Schema/SchemaAggregator.php +++ b/src/Handlers/Eloquent/Schema/SchemaAggregator.php @@ -86,30 +86,32 @@ private function addClassStatements(array $stmts): void private function addUpMethodStatements(array $stmts): void { foreach ($stmts as $stmt) { - if ( - $stmt instanceof PhpParser\Node\Stmt\Expression + $is_schema_method_call = $stmt instanceof PhpParser\Node\Stmt\Expression && $stmt->expr instanceof PhpParser\Node\Expr\StaticCall && $stmt->expr->class instanceof PhpParser\Node\Name && $stmt->expr->name instanceof PhpParser\Node\Identifier - && $stmt->expr->class->getAttribute('resolvedName') === Schema::class - ) { - switch ($stmt->expr->name->name) { - case 'create': - $this->alterTable($stmt->expr, true); - break; - - case 'table': - $this->alterTable($stmt->expr, false); - break; - - case 'drop': - case 'dropIfExists': - $this->dropTable($stmt->expr); - break; - - case 'rename': - $this->renameTable($stmt->expr); - } + && in_array($stmt->expr->class->getAttribute('resolvedName'), [Schema::class, 'Schema'], true); + + if (! $is_schema_method_call) { + continue; + } + + switch ($stmt->expr->name->name) { + case 'create': + $this->alterTable($stmt->expr, true); + break; + + case 'table': + $this->alterTable($stmt->expr, false); + break; + + case 'drop': + case 'dropIfExists': + $this->dropTable($stmt->expr); + break; + + case 'rename': + $this->renameTable($stmt->expr); } } } diff --git a/tests/Unit/Handlers/Eloquent/Schema/DefaultUserTableTest.php b/tests/Unit/Handlers/Eloquent/Schema/DefaultUserTableTest.php index deeb3b3c..db8ad57c 100644 --- a/tests/Unit/Handlers/Eloquent/Schema/DefaultUserTableTest.php +++ b/tests/Unit/Handlers/Eloquent/Schema/DefaultUserTableTest.php @@ -12,14 +12,7 @@ public function it_detects_all_columns_from_anonymous_class_migration(): void { $schemaAggregator = $this->instantiateSchemaAggregator(__DIR__ . '/migrations/default_users_table_anon'); - $this->assertSchemaHasTableAndNotNullableColumnOfType('users.id', 'int', $schemaAggregator); - $this->assertSchemaHasTableAndNotNullableColumnOfType('users.email', 'string', $schemaAggregator); - $this->assertSchemaHasTableAndNotNullableColumnOfType('users.password', 'string', $schemaAggregator); - $this->assertSchemaHasTableAndNotNullableColumnOfType('users.password', 'string', $schemaAggregator); - $this->assertSchemaHasTableAndNotNullableColumnOfType('users.remember_token', 'string', $schemaAggregator); - $this->assertSchemaHasTableAndNullableColumnOfType('users.email_verified_at', 'string', $schemaAggregator); - $this->assertSchemaHasTableAndNullableColumnOfType('users.created_at', 'string', $schemaAggregator); - $this->assertSchemaHasTableAndNullableColumnOfType('users.updated_at', 'string', $schemaAggregator); + $this->assertAllDefaultUsersTableColumnsDetectedProperly($schemaAggregator); } /** @test */ @@ -27,6 +20,19 @@ public function it_detects_all_columns_from_named_class_migration(): void { $schemaAggregator = $this->instantiateSchemaAggregator(__DIR__ . '/migrations/default_users_table_named'); + $this->assertAllDefaultUsersTableColumnsDetectedProperly($schemaAggregator); + } + + /** @test */ + public function it_detects_all_columns_from_migration_that_uses_root_namespace_facades(): void + { + $schemaAggregator = $this->instantiateSchemaAggregator(__DIR__ . '/migrations/default_users_table_root_ns_facade'); + + $this->assertAllDefaultUsersTableColumnsDetectedProperly($schemaAggregator); + } + + private function assertAllDefaultUsersTableColumnsDetectedProperly(SchemaAggregator $schemaAggregator): void + { $this->assertSchemaHasTableAndNotNullableColumnOfType('users.id', 'int', $schemaAggregator); $this->assertSchemaHasTableAndNotNullableColumnOfType('users.email', 'string', $schemaAggregator); $this->assertSchemaHasTableAndNotNullableColumnOfType('users.password', 'string', $schemaAggregator); diff --git a/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_root_ns_facade/2014_10_12_000000_create_users_table.php b/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_root_ns_facade/2014_10_12_000000_create_users_table.php new file mode 100644 index 00000000..f3558b79 --- /dev/null +++ b/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_root_ns_facade/2014_10_12_000000_create_users_table.php @@ -0,0 +1,34 @@ +id(); + $table->string('name'); + $table->string('email')->unique(); + $table->timestamp('email_verified_at')->nullable(); + $table->string('password'); + $table->rememberToken(); + $table->timestamps(); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + \Schema::dropIfExists('users'); + } +}; From 6ab372cc1babfa55d8498eadce837df11ec605b5 Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Wed, 25 Jan 2023 12:21:31 +0000 Subject: [PATCH 4/4] =?UTF-8?q?Fix=20PHP=20coding=20style=20issues=20?= =?UTF-8?q?=F0=9F=AA=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2014_10_12_000000_create_users_table.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_root_ns_facade/2014_10_12_000000_create_users_table.php b/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_root_ns_facade/2014_10_12_000000_create_users_table.php index f3558b79..f1110f10 100644 --- a/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_root_ns_facade/2014_10_12_000000_create_users_table.php +++ b/tests/Unit/Handlers/Eloquent/Schema/migrations/default_users_table_root_ns_facade/2014_10_12_000000_create_users_table.php @@ -11,7 +11,7 @@ */ public function up() { - \Schema::create('users', function (Blueprint $table) { + Schema::create('users', function (Blueprint $table) { $table->id(); $table->string('name'); $table->string('email')->unique(); @@ -29,6 +29,6 @@ public function up() */ public function down() { - \Schema::dropIfExists('users'); + Schema::dropIfExists('users'); } };