From d3b7f7b7dcf131329d966c173c99b05eebb08ceb Mon Sep 17 00:00:00 2001 From: mscherer Date: Sat, 9 May 2026 21:19:25 +0200 Subject: [PATCH 1/2] Reduce PHPStan baseline by addressing actual issues Remove 13 stale baseline entries by fixing the underlying issues: - BakeSeedCommand: drop wrong @var annotation, cast option value; rewrite reference foreach to track removable keys explicitly - AbstractAdapter: tighten generateBulkInsertSql @param type and cast keys to string for array_map - AdapterFactory: declare final constructor (allows safe new static) - MysqlAdapter: drop redundant nested isset() checks - Environment: useTransactions() is on the interface, drop the dead method_exists guard; gate up()/down() dispatch with method_exists - Manager: cast strpos result, fall back to 0 in max() - TableFinder: drop redundant isset checks on always-set list keys - MigrationHelper: rewrite as if/else so PHPStan can narrow the type Two entries remain in the baseline: - Index::setUnique dynamic dispatch (handled via continue) - SqliteAdapter null check kept for test mock compatibility --- phpstan-baseline.neon | 78 -------------------- src/Command/BakeSeedCommand.php | 13 ++-- src/Db/Adapter/AbstractAdapter.php | 6 +- src/Db/Adapter/AdapterFactory.php | 7 ++ src/Db/Adapter/MysqlAdapter.php | 4 +- src/Migration/Environment.php | 7 +- src/Migration/Manager.php | 15 +++- src/Util/TableFinder.php | 4 +- src/View/Helper/MigrationHelper.php | 3 +- tests/TestCase/Migration/EnvironmentTest.php | 3 +- 10 files changed, 36 insertions(+), 104 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 9a720032..0d7ad078 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,35 +1,5 @@ parameters: ignoreErrors: - - - message: '#^PHPDoc tag @var with type string is not subtype of native type non\-falsy\-string\|true\.$#' - identifier: varTag.nativeType - count: 1 - path: src/Command/BakeSeedCommand.php - - - - message: '#^Strict comparison using \!\=\= between string and false will always evaluate to true\.$#' - identifier: notIdentical.alwaysTrue - count: 1 - path: src/Command/BakeSeedCommand.php - - - - message: '#^Parameter \#1 \$callback of function array_map expects \(callable\(int\|string\)\: mixed\)\|null, Closure\(string\)\: string given\.$#' - identifier: argument.type - count: 1 - path: src/Db/Adapter/AbstractAdapter.php - - - - message: '#^Unsafe usage of new static\(\)\.$#' - identifier: new.static - count: 1 - path: src/Db/Adapter/AdapterFactory.php - - - - message: '#^Offset ''id'' on non\-empty\-array\ in isset\(\) always exists and is not nullable\.$#' - identifier: isset.offset - count: 2 - path: src/Db/Adapter/MysqlAdapter.php - - message: '#^Strict comparison using \!\=\= between Cake\\Database\\StatementInterface and null will always evaluate to true\.$#' identifier: notIdentical.alwaysTrue @@ -41,51 +11,3 @@ parameters: identifier: method.notFound count: 1 path: src/Db/Table/Index.php - - - - message: '#^Call to an undefined method Migrations\\MigrationInterface\:\:down\(\)\.$#' - identifier: method.notFound - count: 1 - path: src/Migration/Environment.php - - - - message: '#^Call to an undefined method Migrations\\MigrationInterface\:\:up\(\)\.$#' - identifier: method.notFound - count: 1 - path: src/Migration/Environment.php - - - - message: '#^Call to function method_exists\(\) with Migrations\\MigrationInterface and ''useTransactions'' will always evaluate to true\.$#' - identifier: function.alreadyNarrowedType - count: 1 - path: src/Migration/Environment.php - - - - message: '#^Method Migrations\\Migration\\Manager\:\:getMigrationClassName\(\) should return class\-string\ but returns string\.$#' - identifier: return.type - count: 2 - path: src/Migration/Manager.php - - - - message: '#^Parameter \#1 \.\.\.\$arg1 of function max expects non\-empty\-array, array given\.$#' - identifier: argument.type - count: 1 - path: src/Migration/Manager.php - - - - message: '#^Parameter \#3 \$length of function substr expects int\|null, int\<0, max\>\|false given\.$#' - identifier: argument.type - count: 1 - path: src/Migration/Manager.php - - - - message: '#^Offset 0 on non\-empty\-list\ in isset\(\) always exists and is not nullable\.$#' - identifier: isset.offset - count: 2 - path: src/Util/TableFinder.php - - - - message: '#^Possibly invalid array key type Cake\\Database\\Schema\\TableSchemaInterface\|string\.$#' - identifier: offsetAccess.invalidOffset - count: 2 - path: src/View/Helper/MigrationHelper.php diff --git a/src/Command/BakeSeedCommand.php b/src/Command/BakeSeedCommand.php index 4129df76..cd8a9c1e 100644 --- a/src/Command/BakeSeedCommand.php +++ b/src/Command/BakeSeedCommand.php @@ -116,8 +116,7 @@ public function templateData(Arguments $arguments): array if ($arguments->getOption('data')) { $limit = (int)$arguments->getOption('limit'); - /** @var string $fields */ - $fields = $arguments->getOption('fields') ?: '*'; + $fields = (string)$arguments->getOption('fields') ?: '*'; if ($fields !== '*') { $fields = explode(',', $fields); } @@ -214,6 +213,7 @@ protected function prettifyArray(array $array, int $tabCount = 3, string $indent $lines = explode("\n", $content); $inString = false; + $removeKeys = []; foreach ($lines as $k => &$line) { if ($k === 0) { @@ -237,7 +237,7 @@ protected function prettifyArray(array $array, int $tabCount = 3, string $indent $tabCount--; } elseif (preg_match("/^\d+\s\=\>\s$/", $line)) { // Mark '0 =>' kind of lines to remove - $line = false; + $removeKeys[] = $k; continue; } @@ -264,10 +264,9 @@ protected function prettifyArray(array $array, int $tabCount = 3, string $indent } unset($line); - // Remove marked lines - $lines = array_filter($lines, function ($line): bool { - return $line !== false; - }); + foreach ($removeKeys as $key) { + unset($lines[$key]); + } return implode("\n", $lines); } diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index 052a74e3..39c78bd6 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -841,7 +841,7 @@ public function bulkinsert( * Generates the SQL for a bulk insert. * * @param \Migrations\Db\Table\TableMetadata $table The table to insert into - * @param array $rows The rows to insert + * @param array> $rows The rows to insert * @param \Migrations\Db\InsertMode|null $mode Insert mode * @param array|null $updateColumns Columns to update on upsert conflict * @param array|null $conflictColumns Columns that define uniqueness for upsert (unused in MySQL) @@ -859,8 +859,8 @@ protected function generateBulkInsertSql( $this->getInsertPrefix($mode), $this->quoteTableName($table->getName()), ); - $current = current($rows); - $keys = array_keys($current); + $current = (array)current($rows); + $keys = array_map(strval(...), array_keys($current)); $sql .= '(' . implode(', ', array_map($this->quoteColumnName(...), $keys)) . ') VALUES '; diff --git a/src/Db/Adapter/AdapterFactory.php b/src/Db/Adapter/AdapterFactory.php index b0dafb92..43c5e573 100644 --- a/src/Db/Adapter/AdapterFactory.php +++ b/src/Db/Adapter/AdapterFactory.php @@ -23,6 +23,13 @@ class AdapterFactory */ protected static ?AdapterFactory $instance = null; + /** + * Constructor. + */ + final public function __construct() + { + } + /** * Get the factory singleton instance. * diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index db13ed17..f98148f7 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -300,11 +300,11 @@ public function createTable(TableMetadata $table, array $columns = [], array $in ); // Add the default primary key - if (!isset($options['id']) || (isset($options['id']) && $options['id'] === true)) { + if (!isset($options['id']) || $options['id'] === true) { $options['id'] = 'id'; } - if (isset($options['id']) && is_string($options['id'])) { + if (is_string($options['id'])) { $useUnsigned = (bool)Configure::read('Migrations.unsigned_primary_keys'); // Handle id => "field_name" to support AUTO_INCREMENT $column = new Column(); diff --git a/src/Migration/Environment.php b/src/Migration/Environment.php index 2dd7de09..80d8437a 100644 --- a/src/Migration/Environment.php +++ b/src/Migration/Environment.php @@ -67,10 +67,7 @@ public function executeMigration(MigrationInterface $migration, string $directio $migration->{MigrationInterface::INIT}(); } - $atomic = $adapter->hasTransactions(); - if (method_exists($migration, 'useTransactions')) { - $atomic = $migration->useTransactions(); - } + $atomic = $migration->useTransactions(); // begin the transaction if the adapter supports it if ($atomic) { $adapter->beginTransaction(); @@ -97,7 +94,7 @@ public function executeMigration(MigrationInterface $migration, string $directio } else { $migration->{MigrationInterface::CHANGE}(); } - } else { + } elseif (method_exists($migration, $direction)) { $migration->{$direction}(); } } diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index 8b77bfc4..46fd1efb 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -322,7 +322,8 @@ public function markMigrated(int $version, string $path): bool * Resolves a migration class name based on $path * * @param string $path Path to the migration file of which we want the class name - * @return class-string<\Migrations\MigrationInterface> Migration class name + * @return string Migration class name + * @phpstan-return class-string<\Migrations\MigrationInterface> */ protected function getMigrationClassName(string $path): string { @@ -330,10 +331,15 @@ protected function getMigrationClassName(string $path): string $class = str_replace('_', ' ', $class); $class = ucwords($class); $class = str_replace(' ', '', $class); - if (str_contains($class, '.')) { - return substr($class, 0, strpos($class, '.')); + $dotPos = strpos($class, '.'); + if ($dotPos !== false) { + /** @var class-string<\Migrations\MigrationInterface> $name */ + $name = substr($class, 0, $dotPos); + + return $name; } + /** @var class-string<\Migrations\MigrationInterface> $class */ return $class; } @@ -448,7 +454,8 @@ public function migrate(?int $version = null, bool $fake = false, ?int $count = } if ($version === null) { - $version = max(array_merge($versions, array_keys($migrations))); + $candidates = [...$versions, ...array_keys($migrations)]; + $version = $candidates ? max($candidates) : 0; } elseif ($version !== 0 && !isset($migrations[$version])) { $this->getIo()->out(sprintf( 'warning %s is not a valid version', diff --git a/src/Util/TableFinder.php b/src/Util/TableFinder.php index a15d4917..27da85d9 100644 --- a/src/Util/TableFinder.php +++ b/src/Util/TableFinder.php @@ -81,7 +81,7 @@ public function getTablesToBake(CollectionInterface $collection, array $options $config = (array)ConnectionManager::getConfig($this->connection); $key = isset($config['schema']) ? 'schema' : 'database'; - if (isset($split[0], $split[1]) && $config[$key] === $split[1]) { + if (isset($split[1]) && $config[$key] === $split[1]) { $table = $split[0]; } } @@ -191,7 +191,7 @@ public function fetchTableName(string $className, ?string $pluginName = null): a $config = ConnectionManager::getConfig($this->connection); if (is_array($config)) { $key = isset($config['schema']) ? 'schema' : 'database'; - if (isset($splitted[0]) && $config[$key] === $splitted[1]) { + if ($config[$key] === $splitted[1]) { $tableName = $splitted[0]; } } diff --git a/src/View/Helper/MigrationHelper.php b/src/View/Helper/MigrationHelper.php index e334ec3f..bb2b0dd7 100644 --- a/src/View/Helper/MigrationHelper.php +++ b/src/View/Helper/MigrationHelper.php @@ -708,9 +708,10 @@ public function getCreateTablesElementData(array $tables): array 'tables' => [], ]; foreach ($tables as $table) { - $tableName = $table; if ($table instanceof TableSchemaInterface) { $tableName = $table->name(); + } else { + $tableName = $table; } $data = $this->getCreateTableData($table); $tableConstraintsNoUnique = array_filter( diff --git a/tests/TestCase/Migration/EnvironmentTest.php b/tests/TestCase/Migration/EnvironmentTest.php index 633c20f8..febadbf7 100644 --- a/tests/TestCase/Migration/EnvironmentTest.php +++ b/tests/TestCase/Migration/EnvironmentTest.php @@ -204,8 +204,7 @@ public function testExecutingAMigrationWithUseTransactions(): void $adapterStub->expects($this->never()) ->method('commitTransaction'); - $adapterStub->expects($this->atLeastOnce()) - ->method('hasTransactions') + $adapterStub->method('hasTransactions') ->willReturn(true); $this->environment->setAdapter($adapterStub); From 472fa5c94a239e7639fd84b3f693a8a6941cbc92 Mon Sep 17 00:00:00 2001 From: mscherer Date: Sat, 9 May 2026 21:46:31 +0200 Subject: [PATCH 2/2] Apply rector fixes Match parent visibility on inherited buildOptionParser overrides, type the closure parameter, simplify if/else to ternary, and add the newline rector wants. --- src/Command/BakeSeedCommand.php | 2 +- src/Command/BakeSimpleMigrationCommand.php | 2 +- src/Db/Adapter/AbstractAdapter.php | 2 +- src/Migration/Manager.php | 1 + src/View/Helper/MigrationHelper.php | 6 +----- 5 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/Command/BakeSeedCommand.php b/src/Command/BakeSeedCommand.php index cd8a9c1e..cfc2d2e3 100644 --- a/src/Command/BakeSeedCommand.php +++ b/src/Command/BakeSeedCommand.php @@ -172,7 +172,7 @@ protected function bake(string $name, Arguments $args, ConsoleIo $io): void * @param \Cake\Console\ConsoleOptionParser $parser Option parser to update. * @return \Cake\Console\ConsoleOptionParser */ - public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionParser + protected function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionParser { $parser = parent::buildOptionParser($parser); diff --git a/src/Command/BakeSimpleMigrationCommand.php b/src/Command/BakeSimpleMigrationCommand.php index d99b63a7..b85f0f7e 100644 --- a/src/Command/BakeSimpleMigrationCommand.php +++ b/src/Command/BakeSimpleMigrationCommand.php @@ -236,7 +236,7 @@ protected function getMigrationName(?string $name = null): string * @param \Cake\Console\ConsoleOptionParser $parser Option parser to update. * @return \Cake\Console\ConsoleOptionParser */ - public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionParser + protected function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionParser { $parser = $this->_setCommonOptions($parser); diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index 39c78bd6..ee76ee1a 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -867,7 +867,7 @@ protected function generateBulkInsertSql( $upsertClause = $this->getUpsertClause($mode, $updateColumns, $conflictColumns); if ($this->isDryRunEnabled()) { - $values = array_map(function ($row): string { + $values = array_map(function (array $row): string { return '(' . implode(', ', array_map($this->quoteValue(...), $row)) . ')'; }, $rows); diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index 46fd1efb..8b011c37 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -331,6 +331,7 @@ protected function getMigrationClassName(string $path): string $class = str_replace('_', ' ', $class); $class = ucwords($class); $class = str_replace(' ', '', $class); + $dotPos = strpos($class, '.'); if ($dotPos !== false) { /** @var class-string<\Migrations\MigrationInterface> $name */ diff --git a/src/View/Helper/MigrationHelper.php b/src/View/Helper/MigrationHelper.php index bb2b0dd7..3a6c5256 100644 --- a/src/View/Helper/MigrationHelper.php +++ b/src/View/Helper/MigrationHelper.php @@ -708,11 +708,7 @@ public function getCreateTablesElementData(array $tables): array 'tables' => [], ]; foreach ($tables as $table) { - if ($table instanceof TableSchemaInterface) { - $tableName = $table->name(); - } else { - $tableName = $table; - } + $tableName = $table instanceof TableSchemaInterface ? $table->name() : $table; $data = $this->getCreateTableData($table); $tableConstraintsNoUnique = array_filter( $data['constraints'],