From 6f8aaf4066bed3f7664a0e06f6d2592a0812a95e Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 11:27:35 +0000 Subject: [PATCH 01/14] review: clarify @api/@internal boundaries, enforce VO invariants, surface subscriber failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes review items §7, §8.2, §3, §6, §13.5, §8.5 (see runtime/review/report.md): - @api markers added to event and connection port interfaces; EventAction scoped @psalm-internal (used only inside internal\action\Workflow). - Command and Factory moved to src/internal/ (namespaces dbschemix\core\internal\{command,template}) to align with the existing internal/ convention; @psalm-internal scope tightened from `dbschemix` to `dbschemix\core` on Command. - Public package helpers moved from src/internal/functions.php (namespace dbschemix\core\internal) to src/package/functions.php (namespace dbschemix\core\package); renamed get_package_version -> version and get_package_path -> path (prefix becomes redundant in package\ namespace). composer.json "files" autoload entry updated. - Context and InputOptions enforce non-empty-string and non-negative-int invariants at runtime via InvalidArgumentException, replacing assert() which is disabled in production with zend.assertions=-1. - EventDispatcher surfaces subscriber failures through trigger_error(E_USER_WARNING) instead of silent swallow; subscriber policy documented on EventDispatcher::trigger() and EventSubscriberInterface::subscriptions(). Isolation preserved so a broken subscriber cannot corrupt the migration's success/error event contract. - EventPublisherInterface removed (dead contract: declared on()/off() but never implemented by EventDispatcher and never consumed). All checks green: Psalm (100% inferred, no errors), PHPStan (no errors), PHPUnit (86 tests / 234 assertions), PHPCS (57 files). Co-Authored-By: Claude Opus 4.7 --- composer.json | 2 +- src/Config.php | 4 +++- src/Context.php | 18 ++++++++++++++- src/InputOptions.php | 25 +++++++++++++++++++-- src/Migrator.php | 18 +++++++++++++++ src/command/CommandInterface.php | 3 +++ src/connection/ConnectionInterface.php | 3 +++ src/connection/DriverInterface.php | 3 +++ src/connection/StatementInterface.php | 3 +++ src/connection/TransactionInterface.php | 3 +++ src/event/Event.php | 3 +++ src/event/EventAction.php | 3 +++ src/event/EventDispatcher.php | 29 ++++++++++++++++++++++++- src/event/EventInterface.php | 3 +++ src/event/EventPublisherInterface.php | 12 ---------- src/event/EventSubscriberInterface.php | 11 ++++++++++ src/event/ExceptionEvent.php | 3 +++ src/event/MigrateErrorEvent.php | 3 +++ src/event/MigrateSuccessEvent.php | 3 +++ src/{ => internal}/command/Command.php | 6 +++-- src/{ => internal}/template/Factory.php | 3 ++- src/{internal => package}/functions.php | 6 ++--- src/template/FactoryInterface.php | 3 +++ 23 files changed, 146 insertions(+), 24 deletions(-) delete mode 100644 src/event/EventPublisherInterface.php rename src/{ => internal}/command/Command.php (95%) rename src/{ => internal}/template/Factory.php (84%) rename src/{internal => package}/functions.php (90%) diff --git a/composer.json b/composer.json index 84b1400..07c8bcc 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,7 @@ }, "files": [ "src/internal/filesystem/functions.php", - "src/internal/functions.php" + "src/package/functions.php" ] }, "autoload-dev": { diff --git a/src/Config.php b/src/Config.php index 66fce1c..2f0b515 100644 --- a/src/Config.php +++ b/src/Config.php @@ -5,6 +5,8 @@ namespace dbschemix\core; use dbschemix\core\exception\ConfigurationException; +use dbschemix\core\internal\template\Factory; +use dbschemix\core\template\FactoryInterface; /** * @api @@ -17,7 +19,7 @@ */ public function __construct( public string $table = 'migration', - public template\FactoryInterface $templFactory = new template\Factory(), + public FactoryInterface $templFactory = new Factory(), ) { if (preg_match('/^\w+$/', $table) !== 1) { throw new ConfigurationException( diff --git a/src/Context.php b/src/Context.php index 5a5bd38..fc67d19 100644 --- a/src/Context.php +++ b/src/Context.php @@ -4,7 +4,10 @@ namespace dbschemix\core; +use InvalidArgumentException; + /** + * @api * @infection-ignore-all IncrementInteger */ final readonly class Context @@ -14,6 +17,8 @@ * @param non-empty-string $filename * @param non-empty-string $query * @param non-negative-int $version + * @throws InvalidArgumentException + * @psalm-suppress TypeDoesNotContainType, InvalidCast Runtime defense for callers that bypass static type checks. */ public function __construct( public string $dbName, @@ -22,7 +27,18 @@ public function __construct( public int $version = 0, public bool $dryRun = false, ) { - assert($this->version >= 0); + if ($dbName === '') { + throw new InvalidArgumentException('dbName must be a non-empty string.'); + } + if ($filename === '') { + throw new InvalidArgumentException('filename must be a non-empty string.'); + } + if ($query === '') { + throw new InvalidArgumentException('query must be a non-empty string.'); + } + if ($version < 0) { + throw new InvalidArgumentException("version must be non-negative, got {$version}."); + } } /** diff --git a/src/InputOptions.php b/src/InputOptions.php index dda8383..14960d6 100644 --- a/src/InputOptions.php +++ b/src/InputOptions.php @@ -4,6 +4,8 @@ namespace dbschemix\core; +use InvalidArgumentException; + /** * @api * @infection-ignore-all @@ -15,6 +17,8 @@ * @param non-negative-int $version * @param ?non-empty-string $dbName * @param ?non-empty-string $migrationName + * @throws InvalidArgumentException + * @psalm-suppress TypeDoesNotContainType, InvalidCast Runtime defense for callers that bypass static type checks. */ public function __construct( public int $limit = 0, @@ -26,10 +30,23 @@ public function __construct( private bool $hasRepeatable = false, private bool $applyLatestVersion = false, ) { - assert($this->limit >= 0); - assert($this->version >= 0); + if ($limit < 0) { + throw new InvalidArgumentException("limit must be non-negative, got {$limit}."); + } + if ($version < 0) { + throw new InvalidArgumentException("version must be non-negative, got {$version}."); + } + if ($dbName === '') { + throw new InvalidArgumentException('dbName must be null or a non-empty string.'); + } + if ($migrationName === '') { + throw new InvalidArgumentException('migrationName must be null or a non-empty string.'); + } } + /** + * @psalm-suppress MissingThrowsDocblock Inputs come from $this and are already validated. + */ public function withResetLimit(): self { return new self( @@ -45,6 +62,7 @@ public function withResetLimit(): self /** * @param non-negative-int $version + * @psalm-suppress MissingThrowsDocblock Inputs come from $this and are already validated. */ public function withVersion(int $version): self { @@ -56,6 +74,9 @@ public function withVersion(int $version): self ); } + /** + * @psalm-suppress MissingThrowsDocblock Inputs come from $this and are already validated. + */ public function withExactlyAll(): self { return new self( diff --git a/src/Migrator.php b/src/Migrator.php index 7f5401f..9773213 100644 --- a/src/Migrator.php +++ b/src/Migrator.php @@ -41,6 +41,9 @@ public function init(): void } } + /** + * @psalm-suppress MissingThrowsDocblock Default InputOptions and withers operate on validated state. + */ #[Override] public function create(InputOptions $args = new InputOptions()): void { @@ -61,6 +64,9 @@ public function create(InputOptions $args = new InputOptions()): void } } + /** + * @psalm-suppress MissingThrowsDocblock Default InputOptions operates on validated state. + */ #[Override] public function up(InputOptions $args = new InputOptions()): void { @@ -69,6 +75,9 @@ public function up(InputOptions $args = new InputOptions()): void } } + /** + * @psalm-suppress MissingThrowsDocblock Default InputOptions operates on validated state. + */ #[Override] public function down(InputOptions $args = new InputOptions()): void { @@ -77,6 +86,9 @@ public function down(InputOptions $args = new InputOptions()): void } } + /** + * @psalm-suppress MissingThrowsDocblock Default InputOptions operates on validated state. + */ #[Override] public function fixture(InputOptions $args = new InputOptions()): void { @@ -85,6 +97,9 @@ public function fixture(InputOptions $args = new InputOptions()): void } } + /** + * @psalm-suppress MissingThrowsDocblock Default InputOptions and withers operate on validated state. + */ #[Override] public function redo(InputOptions $args = new InputOptions()): void { @@ -92,6 +107,9 @@ public function redo(InputOptions $args = new InputOptions()): void $this->up($args->withResetLimit()); } + /** + * @psalm-suppress MissingThrowsDocblock Default InputOptions and withers operate on validated state. + */ #[Override] public function verify(InputOptions $args = new InputOptions()): void { diff --git a/src/command/CommandInterface.php b/src/command/CommandInterface.php index ac4bd67..b1a9572 100644 --- a/src/command/CommandInterface.php +++ b/src/command/CommandInterface.php @@ -7,6 +7,9 @@ use Throwable; use dbschemix\core\Context; +/** + * @api + */ interface CommandInterface { /** diff --git a/src/connection/ConnectionInterface.php b/src/connection/ConnectionInterface.php index 55b1d96..a0a7fdf 100644 --- a/src/connection/ConnectionInterface.php +++ b/src/connection/ConnectionInterface.php @@ -4,6 +4,9 @@ namespace dbschemix\core\connection; +/** + * @api + */ interface ConnectionInterface extends StatementInterface { public function beginTransaction(): TransactionInterface; diff --git a/src/connection/DriverInterface.php b/src/connection/DriverInterface.php index e1cac68..bce39d5 100644 --- a/src/connection/DriverInterface.php +++ b/src/connection/DriverInterface.php @@ -8,6 +8,9 @@ use dbschemix\core\exception\ConnectionException; use dbschemix\core\Config; +/** + * @api + */ interface DriverInterface { /** diff --git a/src/connection/StatementInterface.php b/src/connection/StatementInterface.php index cb496a3..74c1faf 100644 --- a/src/connection/StatementInterface.php +++ b/src/connection/StatementInterface.php @@ -4,6 +4,9 @@ namespace dbschemix\core\connection; +/** + * @api + */ interface StatementInterface { /** diff --git a/src/connection/TransactionInterface.php b/src/connection/TransactionInterface.php index 3035b44..0b89e0e 100644 --- a/src/connection/TransactionInterface.php +++ b/src/connection/TransactionInterface.php @@ -4,6 +4,9 @@ namespace dbschemix\core\connection; +/** + * @api + */ interface TransactionInterface extends StatementInterface { public function isActive(): bool; diff --git a/src/event/Event.php b/src/event/Event.php index 7f0d958..d43609c 100644 --- a/src/event/Event.php +++ b/src/event/Event.php @@ -4,6 +4,9 @@ namespace dbschemix\core\event; +/** + * @api + */ enum Event: string { case InitializationError = 'initialization-error-event'; diff --git a/src/event/EventAction.php b/src/event/EventAction.php index 300dbbe..53f3f74 100644 --- a/src/event/EventAction.php +++ b/src/event/EventAction.php @@ -4,6 +4,9 @@ namespace dbschemix\core\event; +/** + * @psalm-internal dbschemix\core + */ enum EventAction { case up; diff --git a/src/event/EventDispatcher.php b/src/event/EventDispatcher.php index cdc1bff..7ae6e57 100644 --- a/src/event/EventDispatcher.php +++ b/src/event/EventDispatcher.php @@ -31,13 +31,40 @@ public function __construct(array $eventSubscribers) $this->eventHandlers = $subscriptions; } + /** + * Notifies subscribers of an event. + * + * Subscriber callbacks MUST NOT throw. If one does, the exception is captured + * and reported through trigger_error(E_USER_WARNING); the loop continues so + * remaining subscribers still receive the event, and the migration is not + * aborted by a subscriber bug. + * + * This isolation is intentional: trigger() is called from inside the migration's + * own try/catch flow (see Workflow::run()), and an exception propagating out + * of a subscriber would be misattributed as a migration failure — corrupting the + * success/error event contract and potentially shadowing the original migration + * error. + * + * Hosts that want fail-fast on subscriber failures may install a strict error + * handler (set_error_handler) that converts E_USER_WARNING into an exception; + * that is an explicit opt-in by the host. + */ public function trigger(Event $name, EventInterface $event): void { if (array_key_exists($name->value, $this->eventHandlers)) { foreach ($this->eventHandlers[$name->value] as $subscriberCallback) { try { $subscriberCallback($name, $event); - } catch (Throwable) { + } catch (Throwable $exception) { + trigger_error( + sprintf( + 'dbschemix\core: subscriber callback for event "%s" failed (%s): %s', + $name->value, + $exception::class, + $exception->getMessage(), + ), + E_USER_WARNING, + ); } } } diff --git a/src/event/EventInterface.php b/src/event/EventInterface.php index 95394f6..32615b5 100644 --- a/src/event/EventInterface.php +++ b/src/event/EventInterface.php @@ -4,6 +4,9 @@ namespace dbschemix\core\event; +/** + * @api + */ interface EventInterface { /** diff --git a/src/event/EventPublisherInterface.php b/src/event/EventPublisherInterface.php deleted file mode 100644 index f0cbf9c..0000000 --- a/src/event/EventPublisherInterface.php +++ /dev/null @@ -1,12 +0,0 @@ - */ public function subscriptions(): array; diff --git a/src/event/ExceptionEvent.php b/src/event/ExceptionEvent.php index 1713e34..b56b3bb 100644 --- a/src/event/ExceptionEvent.php +++ b/src/event/ExceptionEvent.php @@ -7,6 +7,9 @@ use Override; use Throwable; +/** + * @api + */ final readonly class ExceptionEvent implements EventInterface { /** diff --git a/src/event/MigrateErrorEvent.php b/src/event/MigrateErrorEvent.php index 64c0a6c..40ddbda 100644 --- a/src/event/MigrateErrorEvent.php +++ b/src/event/MigrateErrorEvent.php @@ -8,6 +8,9 @@ use Throwable; use dbschemix\core\Context; +/** + * @api + */ final readonly class MigrateErrorEvent implements EventInterface { /** diff --git a/src/event/MigrateSuccessEvent.php b/src/event/MigrateSuccessEvent.php index 1629ae6..b83b17d 100644 --- a/src/event/MigrateSuccessEvent.php +++ b/src/event/MigrateSuccessEvent.php @@ -7,6 +7,9 @@ use Override; use dbschemix\core\Context; +/** + * @api + */ final readonly class MigrateSuccessEvent implements EventInterface { /** diff --git a/src/command/Command.php b/src/internal/command/Command.php similarity index 95% rename from src/command/Command.php rename to src/internal/command/Command.php index 1c75033..d8cbcb6 100644 --- a/src/command/Command.php +++ b/src/internal/command/Command.php @@ -2,16 +2,18 @@ declare(strict_types=1); -namespace dbschemix\core\command; +namespace dbschemix\core\internal\command; use Override; use Throwable; +use dbschemix\core\command\CommandInterface; +use dbschemix\core\command\Options; use dbschemix\core\connection\ConnectionInterface; use dbschemix\core\Config; use dbschemix\core\Context; /** - * @psalm-internal dbschemix + * @psalm-internal dbschemix\core */ final readonly class Command implements CommandInterface { diff --git a/src/template/Factory.php b/src/internal/template/Factory.php similarity index 84% rename from src/template/Factory.php rename to src/internal/template/Factory.php index b33a92d..7570249 100644 --- a/src/template/Factory.php +++ b/src/internal/template/Factory.php @@ -2,9 +2,10 @@ declare(strict_types=1); -namespace dbschemix\core\template; +namespace dbschemix\core\internal\template; use Override; +use dbschemix\core\template\FactoryInterface; /** * @psalm-internal dbschemix\core diff --git a/src/internal/functions.php b/src/package/functions.php similarity index 90% rename from src/internal/functions.php rename to src/package/functions.php index 57b5c0b..0fe0f39 100644 --- a/src/internal/functions.php +++ b/src/package/functions.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace dbschemix\core\internal; +namespace dbschemix\core\package; use OutOfBoundsException; use Composer\InstalledVersions; @@ -13,7 +13,7 @@ * @return non-empty-string * @throws OutOfBoundsException of package is not installed */ -function get_package_version(string $package): string +function version(string $package): string { /** * @var array $versions @@ -42,7 +42,7 @@ function get_package_version(string $package): string * @return non-empty-string * @throws OutOfBoundsException of package is not installed */ -function get_package_path(string $package): string +function path(string $package): string { $packagePath = InstalledVersions::getInstallPath($package); diff --git a/src/template/FactoryInterface.php b/src/template/FactoryInterface.php index 0e67539..9fe8b43 100644 --- a/src/template/FactoryInterface.php +++ b/src/template/FactoryInterface.php @@ -4,6 +4,9 @@ namespace dbschemix\core\template; +/** + * @api + */ interface FactoryInterface { /** From 4ff1a69fc89e32c5175e3dfa8c25efbed91753df Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 11:36:07 +0000 Subject: [PATCH 02/14] review: replace string-keyed subscriptions with typed Subscription DTO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes review item §design-patterns "subscriptions() keyed by raw string" (see runtime/review/report.md). EventSubscriberInterface::subscriptions() returned array, where the string was Event::value. This forced consumers to hardcode strings or use `Event::X->value` at every subscription site, removed static verification of enum coverage, and conflated the dispatcher's internal string-keyed indexing with the public subscription contract. Enum-as-array-key is not supported by PHP (TypeError: Cannot access offset of type E), so the natural alternative — array — is not viable. Replaced with a typed list of Subscription DTOs instead: - New @api class src/event/Subscription.php — final readonly, holds an Event and a Closure(Event, EventInterface): void. - EventSubscriberInterface::subscriptions() now returns list. - EventDispatcher::__construct() converts Event -> string at the boundary (subscription->event->value) so internal storage stays string-keyed, but no string crosses the public API. - tests/stub/TestSubscriber updated; @phpstan-ignore varTag.nativeType hack no longer needed. Smoke-test confirms multiple Subscription entries on one subscriber dispatch correctly to matching events and ignore non-subscribed events. All checks green: Psalm (100% inferred), PHPStan, PHPUnit (86 tests), PHPCS. Co-Authored-By: Claude Opus 4.7 --- src/event/EventDispatcher.php | 7 ++++--- src/event/EventSubscriberInterface.php | 4 ++-- src/event/Subscription.php | 22 ++++++++++++++++++++++ tests/stub/TestSubscriber.php | 7 ++----- 4 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 src/event/Subscription.php diff --git a/src/event/EventDispatcher.php b/src/event/EventDispatcher.php index 7ae6e57..c7cf1da 100644 --- a/src/event/EventDispatcher.php +++ b/src/event/EventDispatcher.php @@ -4,6 +4,7 @@ namespace dbschemix\core\event; +use Closure; use Throwable; /** @@ -12,7 +13,7 @@ final readonly class EventDispatcher { /** - * @var array> + * @var array> */ private array $eventHandlers; @@ -23,8 +24,8 @@ public function __construct(array $eventSubscribers) { $subscriptions = []; foreach ($eventSubscribers as $subscriber) { - foreach ($subscriber->subscriptions() as $name => $callback) { - $subscriptions[$name][] = $callback; + foreach ($subscriber->subscriptions() as $subscription) { + $subscriptions[$subscription->event->value][] = $subscription->callback; } } diff --git a/src/event/EventSubscriberInterface.php b/src/event/EventSubscriberInterface.php index 58abf8d..bf7c1ec 100644 --- a/src/event/EventSubscriberInterface.php +++ b/src/event/EventSubscriberInterface.php @@ -10,7 +10,7 @@ interface EventSubscriberInterface { /** - * Returns a map of event name (Event::value) to callback. + * Returns the list of (event, callback) subscriptions this subscriber wants. * * Subscriber callbacks MUST NOT throw. If a callback throws, the library * reports the failure via trigger_error(E_USER_WARNING) and continues: @@ -18,7 +18,7 @@ interface EventSubscriberInterface * aborted. Subscribers that need to react to their own failures must * wrap their body in their own try/catch. * - * @return array + * @return list */ public function subscriptions(): array; } diff --git a/src/event/Subscription.php b/src/event/Subscription.php new file mode 100644 index 0000000..071a995 --- /dev/null +++ b/src/event/Subscription.php @@ -0,0 +1,22 @@ +value] = $this->set(...); + $subscriptions[] = new Subscription($event, $this->set(...)); } - /** - * @var non-empty-array $subscriptions - * @phpstan-ignore varTag.nativeType - */ return $subscriptions; } From ece175197ddfb8627fac860cea1fbfab1739ec49 Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 11:44:10 +0000 Subject: [PATCH 03/14] review: mark exception hierarchy as @api; document extension-point and sealed-root roles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes: - §7 «Mark API and internal» applied to the exception hierarchy (six classes) - §12 «PrepareException dead code» — N/A (used downstream) - §5 «InitializationException asymmetric constructor» — WONT-FIX (intentional semantic shape, not asymmetry) See runtime/review/report.md for the full rationale on each. Context: the reviewer (which only sees core) flagged PrepareException as dead because nothing in core throws it. In fact it is a public extension point for driver implementations in downstream packages (dbschemix/pdo, dbschemix/pgsql, dbschemix/clickhouse) — they throw it from their ConnectionInterface / StatementInterface implementations when statement preparation fails, and Workflow::run() catches it through the generic Throwable handler and wraps it into ActionException. Changes: - All six exception classes (MigratorException, ActionException, ConnectionException, InitializationException, ConfigurationException, PrepareException) now carry @api markers, making the stability tier of the exception contract explicit — both for consumers that catch them and for driver implementors that throw them. - MigratorException gains a class-level docblock describing the sealed hierarchy: catch the base for generic handling or a concrete subtype for targeted handling. - PrepareException gains a class-level docblock describing its extension-point role for downstream driver packages and the workflow that wraps it into ActionException. - The constructor signature asymmetry the reviewer flagged is intentional: each constructor takes the domain data of its case plus the originating cause (filename for ActionException, driver for ConnectionException, description for InitializationException with no domain object). The two classes that inherit the parent signature (ConfigurationException, PrepareException) do so by design — to keep text flexible for user input validation and to give downstream driver implementors freedom in how they construct PrepareException. All checks green: Psalm (100% inferred), PHPStan, PHPUnit (86 tests), PHPCS. Co-Authored-By: Claude Opus 4.7 --- src/exception/ActionException.php | 3 +++ src/exception/ConfigurationException.php | 3 +++ src/exception/ConnectionException.php | 3 +++ src/exception/InitializationException.php | 3 +++ src/exception/MigratorException.php | 12 ++++++++++++ src/exception/PrepareException.php | 12 ++++++++++++ 6 files changed, 36 insertions(+) diff --git a/src/exception/ActionException.php b/src/exception/ActionException.php index 85bb390..0faa71b 100644 --- a/src/exception/ActionException.php +++ b/src/exception/ActionException.php @@ -6,6 +6,9 @@ use Throwable; +/** + * @api + */ final class ActionException extends MigratorException { public function __construct(string $filename, Throwable $previous) diff --git a/src/exception/ConfigurationException.php b/src/exception/ConfigurationException.php index f0af816..26a6332 100644 --- a/src/exception/ConfigurationException.php +++ b/src/exception/ConfigurationException.php @@ -4,6 +4,9 @@ namespace dbschemix\core\exception; +/** + * @api + */ final class ConfigurationException extends MigratorException { } diff --git a/src/exception/ConnectionException.php b/src/exception/ConnectionException.php index 80e75cb..8575724 100644 --- a/src/exception/ConnectionException.php +++ b/src/exception/ConnectionException.php @@ -7,6 +7,9 @@ use Throwable; use dbschemix\core\connection\DriverInterface; +/** + * @api + */ final class ConnectionException extends MigratorException { public function __construct(DriverInterface $driver, Throwable $previous) diff --git a/src/exception/InitializationException.php b/src/exception/InitializationException.php index 0ac3b40..bc905ec 100644 --- a/src/exception/InitializationException.php +++ b/src/exception/InitializationException.php @@ -6,6 +6,9 @@ use Throwable; +/** + * @api + */ final class InitializationException extends MigratorException { public function __construct(string $message, Throwable $previous) diff --git a/src/exception/MigratorException.php b/src/exception/MigratorException.php index 4f7b470..3284806 100644 --- a/src/exception/MigratorException.php +++ b/src/exception/MigratorException.php @@ -6,6 +6,18 @@ use RuntimeException; +/** + * Root of the dbschemix\core exception hierarchy. + * + * All exceptions thrown by the migrator descend from this class. Consumers + * may catch this base type for generic handling, or catch a concrete subtype + * (ActionException, ConfigurationException, ConnectionException, + * InitializationException, PrepareException) for targeted handling. Driver + * implementations in downstream packages may throw any subtype of this + * hierarchy. + * + * @api + */ abstract class MigratorException extends RuntimeException { } diff --git a/src/exception/PrepareException.php b/src/exception/PrepareException.php index 57a2fa8..7439dbe 100644 --- a/src/exception/PrepareException.php +++ b/src/exception/PrepareException.php @@ -4,6 +4,18 @@ namespace dbschemix\core\exception; +/** + * Thrown when a database driver cannot prepare a SQL statement for execution. + * + * Intended as an extension point for driver implementations in downstream + * packages (dbschemix/pdo, dbschemix/pgsql, dbschemix/clickhouse, etc.). The + * core workflow does not throw this directly; downstream drivers throw it + * from within ConnectionInterface / StatementInterface implementations and + * Workflow::run() catches it through the generic Throwable handler, wrapping + * it into ActionException with the original PrepareException as the cause. + * + * @api + */ final class PrepareException extends MigratorException { } From 5df564eed8d09df22745d8138b4c84c49c686666 Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 11:46:56 +0000 Subject: [PATCH 04/14] review: tighten callable to Closure in Workflow private methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes review item §4 (Methods and functions / Typing): src/internal/action/Workflow.php. Workflow::run() and Workflow::iteratorHandler() accepted `callable` as their native type with a PHPDoc-typed signature. The reviewer noted that the richest static typing for first-class callables — `Closure` with a typed signature — was not used, so Psalm could only verify the callable's shape through PHPDoc and not at runtime. All existing call sites already produce Closure instances: - `$fsHandler = static fn(filesystem\Action $fs): Iterator => $fs->up(...)` (arrow functions are Closures) - `$command->up(...)`, `$command->down(...)`, `$command->exec(...)` (first-class callable syntax produces Closures) Replaced `callable` with `Closure` in both the native type and the PHPDoc shape on Workflow::run and Workflow::iteratorHandler; added `use Closure;`. No call-site changes needed. Effect: PHP now rejects non-Closure callables (string function names, [$obj, 'method'] arrays, invokables) at the method boundary before they enter the body. Psalm and PHPStan now have a real native class type plus the typed Closure signature to reason about. All checks green: Psalm (100% inferred), PHPStan, PHPUnit (86), PHPCS (58). Co-Authored-By: Claude Opus 4.7 --- src/internal/action/Workflow.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/internal/action/Workflow.php b/src/internal/action/Workflow.php index eb73510..1449b2b 100644 --- a/src/internal/action/Workflow.php +++ b/src/internal/action/Workflow.php @@ -4,6 +4,7 @@ namespace dbschemix\core\internal\action; +use Closure; use Throwable; use DateTimeImmutable; use Iterator; @@ -253,10 +254,10 @@ private function getLastVersion(Migration $migration, CommandInterface $command) } /** - * @param callable(Context $context):bool $handler + * @param Closure(Context $context):bool $handler * @throws ActionException */ - private function run(callable $handler, Context $context, EventAction $action): void + private function run(Closure $handler, Context $context, EventAction $action): void { try { $handler($context); @@ -275,11 +276,11 @@ private function run(callable $handler, Context $context, EventAction $action): } /** - * @param callable(filesystem\Action):Iterator $handler + * @param Closure(filesystem\Action):Iterator $handler * @return iterable * @throws ConfigurationException */ - private function iteratorHandler(Migration $migration, callable $handler, bool $useException = true): iterable + private function iteratorHandler(Migration $migration, Closure $handler, bool $useException = true): iterable { try { $iterator = $handler(new filesystem\Action($migration->path)); From 7448c17dab448656dde76d99763370643d8a91e7 Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 11:57:35 +0000 Subject: [PATCH 05/14] review: document CommandInterface as driver-extension contract with explicit exception policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes review item §5 (@throws Throwable on CommandInterface::fetchApplied) as WONT-FIX with documentation; see runtime/review/report.md. The reviewer flagged `@throws Throwable` on the four CommandInterface methods as collapsing the "three tiers of failure" discipline. The framing confuses the contract's audience: CommandInterface is a driver-extension API, not a consumer API. - Consumers interact with Migrator, whose exception contract is fully typed (ActionException, ConfigurationException, ConnectionException, InitializationException). They never see Throwable. - Driver implementors (internal\command\Command, plus downstream packages dbschemix/pdo, dbschemix/pgsql, dbschemix/clickhouse) implement CommandInterface and may surface whatever their underlying DB client throws (\PDOException, mysqli_sql_exception, ClickHouse client exceptions, ...) plus library-typed exceptions such as PrepareException. - Workflow catches Throwable at its boundary (Workflow::run, Workflow::getAppliedMigrations) and wraps each into the appropriate typed MigratorException subtype before it propagates out. The three tiers of failure are restored at the boundary. Forcing driver implementors to pre-wrap into a typed exception would add boilerplate without typing benefit (Workflow's broad catch is unchanged either way) and would break existing downstream implementations. Change: CommandInterface now carries a class-level docblock that records audience (driver implementors), exception policy (Throwable on the port, typed exceptions at the workflow boundary), and the explicit invariant that driver implementors are not required to pre-wrap. All checks green: Psalm (100% inferred), PHPStan, PHPUnit (86), PHPCS (58). Co-Authored-By: Claude Opus 4.7 --- src/command/CommandInterface.php | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/command/CommandInterface.php b/src/command/CommandInterface.php index b1a9572..cdac53a 100644 --- a/src/command/CommandInterface.php +++ b/src/command/CommandInterface.php @@ -8,6 +8,27 @@ use dbschemix\core\Context; /** + * Command port: the contract a database driver implementation fulfills to + * run core migration workflow operations. + * + * Audience: driver implementors (the in-tree adapter + * `internal\command\Command` plus downstream packages such as + * `dbschemix/pdo`, `dbschemix/pgsql`, `dbschemix/clickhouse`). End consumers + * interact with `Migrator`, whose exception contract is fully typed. + * + * Exception policy: each method below is declared `@throws Throwable` + * intentionally. A driver implementation may surface whatever its + * underlying client throws (\PDOException, mysqli_sql_exception, ClickHouse + * client exceptions, ...) plus library-typed exceptions such as + * `PrepareException`. The core workflow catches these at its boundary + * (Workflow::run, Workflow::getAppliedMigrations) and wraps them into + * typed `MigratorException` subtypes — `ActionException` for migration + * failures, `InitializationException` for system-state reads — so the + * typed exception contract is restored before reaching the consumer of + * `Migrator`. Driver implementors are NOT required to pre-wrap their + * underlying exceptions; the workflow's `catch (Throwable)` is the + * wrapping point by design. + * * @api */ interface CommandInterface From c7b404c65dbc3a498d8e5ca6addc53498579445b Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 11:58:47 +0000 Subject: [PATCH 06/14] review: tighten FactoryInterface::makeBody return type to non-empty-string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes review item §3 (Typing — PHPDoc completes the contract): src/template/FactoryInterface.php:14. FactoryInterface::makeBody() previously had no @return annotation and a native string return type. The body is written verbatim to disk as a migration file (via Workflow::create -> filesystem\Action::create), so an empty string silently produces a no-op migration template. Added `@return non-empty-string` to make the contract explicit. The in-tree internal\template\Factory::makeBody implementation returns a heredoc with fixed -- @up / -- @down headers and is statically verified as non-empty. Downstream implementations that return empty now surface on static analysis instead of producing a useless file. All checks green: Psalm (100% inferred), PHPStan, PHPUnit (86), PHPCS (58). Co-Authored-By: Claude Opus 4.7 --- src/template/FactoryInterface.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/template/FactoryInterface.php b/src/template/FactoryInterface.php index 9fe8b43..e6d9aeb 100644 --- a/src/template/FactoryInterface.php +++ b/src/template/FactoryInterface.php @@ -15,5 +15,8 @@ interface FactoryInterface */ public function makeName(string $name): string; + /** + * @return non-empty-string + */ public function makeBody(): string; } From 080037fcfa530bfee2979809b38e46fa98cc1751 Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 12:12:47 +0000 Subject: [PATCH 07/14] review: document Event enum -> payload class mapping and clarify EventInterface role MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes review item §design-patterns "Anaemic EventInterface forces instanceof" (see runtime/review/report.md). The reviewer's framing was partially valid (subscribers that need structured data must instanceof onto the concrete event class) and partially wrong (the concrete classes already expose payload as public readonly properties — it's type narrowing, not "reach-back into internals"). All structural alternatives (fat EventInterface, generic Subscription, per-event callback signatures) trade away more than they gain. The actual hidden problem: the Event enum case -> concrete event class mapping was nowhere documented. A subscriber had to read Workflow.php and Migrator.php to know what to instanceof onto, turning an implicit @api contract into a guessing game. This commit makes that mapping explicit: - src/event/Event.php: each of the seven cases now carries a docblock with a one-line description of when it fires and an explicit "Dispatched payload: {@see ConcreteEvent} (properties)" line. The class-level docblock declares that the mapping is part of the @api contract. - src/event/EventInterface.php: class-level docblock explains the role split — thin surface (getName/getMessage) for generic logging, structured data via instanceof onto MigrateSuccessEvent / MigrateErrorEvent / ExceptionEvent, with the mapping in {@see Event}. The mapping itself: MigrateSuccess -> MigrateSuccessEvent (action, context) MigrateError -> MigrateErrorEvent (action, context, exception) InitializationError -> ExceptionEvent (dbName, exception) ConnectionError -> ExceptionEvent (dbName, exception) ConfigurationError -> ExceptionEvent (dbName, exception) FilesystemError -> ExceptionEvent (dbName, exception) FilesystemNotice -> ExceptionEvent (dbName, exception) No code changes; no type changes; no breaking changes. Subscribers may now instanceof with documented certainty instead of guessing. All checks green: Psalm (100% inferred), PHPStan, PHPUnit (86), PHPCS (58). Co-Authored-By: Claude Opus 4.7 --- src/event/Event.php | 47 ++++++++++++++++++++++++++++++++++++ src/event/EventInterface.php | 13 ++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/event/Event.php b/src/event/Event.php index d43609c..3f68350 100644 --- a/src/event/Event.php +++ b/src/event/Event.php @@ -5,21 +5,68 @@ namespace dbschemix\core\event; /** + * Library-wide event identifiers and the stable enum-to-payload mapping. + * + * Each case below documents the concrete {@see EventInterface} implementation + * a subscriber will receive when the event fires. Subscribers that need the + * structured payload (Context, Throwable, dbName, action) may safely + * `instanceof` onto the dispatched class — the mapping is part of the @api + * contract. + * * @api */ enum Event: string { + /** + * Reading the migration-tracking table failed during initialization. + * + * Dispatched payload: {@see ExceptionEvent} (dbName, exception). + */ case InitializationError = 'initialization-error-event'; + /** + * The driver could not open a connection or build a CommandInterface. + * + * Dispatched payload: {@see ExceptionEvent} (dbName, exception). + */ case ConnectionError = 'connection-error-event'; + /** + * Configuration is invalid (requested database is not in the list, + * table name is malformed, required arguments missing, ...). + * + * Dispatched payload: {@see ExceptionEvent} (dbName, exception). + */ case ConfigurationError = 'configuration-error-event'; + /** + * A filesystem operation failed (missing migration directory, unwritable + * path, unreadable file, ...). + * + * Dispatched payload: {@see ExceptionEvent} (dbName, exception). + */ case FilesystemError = 'filesystem-error-event'; + /** + * Filesystem soft event: directory exists but contains no migration files. + * Informational; not an error. The carried exception holds the notice + * message for logging convenience. + * + * Dispatched payload: {@see ExceptionEvent} (dbName, exception). + */ case FilesystemNotice = 'filesystem-notice-event'; + /** + * A single migration file has been applied successfully. + * + * Dispatched payload: {@see MigrateSuccessEvent} (action, context). + */ case MigrateSuccess = 'migrate-success-event'; + /** + * A single migration file failed to apply. + * + * Dispatched payload: {@see MigrateErrorEvent} (action, context, exception). + */ case MigrateError = 'migrate-error-event'; } diff --git a/src/event/EventInterface.php b/src/event/EventInterface.php index 32615b5..8d84dd8 100644 --- a/src/event/EventInterface.php +++ b/src/event/EventInterface.php @@ -5,6 +5,19 @@ namespace dbschemix\core\event; /** + * Common surface across all dispatched events. + * + * Provides generic-logging-friendly accessors (getName, getMessage) suitable + * for subscribers that only need a human-readable line. Structured payload + * data (Context, Throwable, dbName, action) lives on the concrete event + * class — {@see MigrateSuccessEvent}, {@see MigrateErrorEvent}, + * {@see ExceptionEvent} — as public readonly properties. + * + * Subscribers that need structured data should type-narrow via `instanceof` + * onto the concrete class. The Event enum case → concrete event class + * mapping is part of the @api contract and is documented per case in + * {@see Event}. + * * @api */ interface EventInterface From c8c62028a9741767ba8d453a6712aba5d0a1253d Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 12:28:55 +0000 Subject: [PATCH 08/14] review: rename joinBasename to joinSuffix; close PSR-12 nit as WONT-FIX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes review item coding-style §2 ("functions in namespace for a library") on src/internal/filesystem/functions.php as WONT-FIX; see runtime/review/report.md. Rationale: precedent for keeping function-in-namespace form is already set in src/package/functions.php (public package helpers) — see review item §3. Wrapping three internal one-line helpers in a final class for lazy autoload adds churn for negligible benefit. All three remain `@infection-ignore-all` deterministic transformers; the cost of eager autoload through composer.json "files" entry is negligible for a handful of pure functions. Same commit fixes a misnamed helper: - `joinBasename($path, $postfix)` → `joinSuffix($path, $suffix)`. The original name was misleading: the function does not join a basename, it appends a suffix to a directory path to produce a sibling directory (e.g. /path/to/dir + '-fixture' → /path/to/dir-fixture/). The new name matches the action and stays in the existing join* family alongside joinFilename. Parameter renamed for consistency. Two call sites in Action.php (fixture / repeatable directory construction) updated. All checks green: Psalm (100% inferred), PHPStan, PHPUnit (86), PHPCS (58). Co-Authored-By: Claude Opus 4.7 --- src/internal/filesystem/Action.php | 4 ++-- src/internal/filesystem/functions.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/internal/filesystem/Action.php b/src/internal/filesystem/Action.php index 23368ac..2f65d97 100644 --- a/src/internal/filesystem/Action.php +++ b/src/internal/filesystem/Action.php @@ -85,7 +85,7 @@ public function down(array $listApplied, Options $options = new Options()): Iter public function fixture(Options $options = new Options()): Iterator { $iternum = 0; - foreach ($this->makeIterator(joinBasename($this->path, '-fixture')) as $matchFilename) { + foreach ($this->makeIterator(joinSuffix($this->path, '-fixture')) as $matchFilename) { if ($options->limit > 0 && $iternum >= $options->limit) { return; } @@ -107,7 +107,7 @@ public function fixture(Options $options = new Options()): Iterator */ public function repeatable(): Iterator { - foreach ($this->makeIterator(joinBasename($this->path, '-repeatable')) as $matchFilename) { + foreach ($this->makeIterator(joinSuffix($this->path, '-repeatable')) as $matchFilename) { $filepath = $matchFilename[0]; $command = $this->prepareCommand($filepath, ActionType::UP); if ($command !== null) { diff --git a/src/internal/filesystem/functions.php b/src/internal/filesystem/functions.php index fe4ce1d..f4579c1 100644 --- a/src/internal/filesystem/functions.php +++ b/src/internal/filesystem/functions.php @@ -16,13 +16,13 @@ function normalizePath(string $path): string /** * @param non-empty-string $path - * @param non-empty-string $postfix + * @param non-empty-string $suffix * @return non-empty-string * @infection-ignore-all */ -function joinBasename(string $path, string $postfix): string +function joinSuffix(string $path, string $suffix): string { - return rtrim(trim($path), '/') . rtrim($postfix, '/') . '/'; + return rtrim(trim($path), '/') . rtrim($suffix, '/') . '/'; } /** From 99b7a2a8328ce3c347093f4e6c8d52f80da302ad Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 12:31:26 +0000 Subject: [PATCH 09/14] review: drop unused ActionType::SKIP enum case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes review item coding-style §8 ("ActionType has SKIP case never matched in prepareCommand"). Confirmed: - prepareCommand() callers pass only ActionType::UP and ActionType::DOWN. - ActionType carries @psalm-internal dbschemix\core\internal\filesystem (the narrowest scope in the codebase), so downstream packages (dbschemix/pdo, dbschemix/pgsql, dbschemix/clickhouse) have no legitimate path to it. - "skip" semantics in the project exist, but on the filename level (Action::makeIterator regex /.+(? --- src/internal/filesystem/ActionType.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/internal/filesystem/ActionType.php b/src/internal/filesystem/ActionType.php index dd3547e..e8f1708 100644 --- a/src/internal/filesystem/ActionType.php +++ b/src/internal/filesystem/ActionType.php @@ -11,5 +11,4 @@ enum ActionType: string { case UP = 'up'; case DOWN = 'down'; - case SKIP = 'skip'; } From b56492dc5da08ec7d57e8edca1448d280c7527b9 Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 12:39:05 +0000 Subject: [PATCH 10/14] review: close 4 remaining nits (Command query build, line separator, command\Options @api + invariants) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes review nits §7 (Command.php inline @var), §8 (\r\n vs no separator), §9.7 (command\Options namespace), and reopens the §6 VO-invariant pattern for command\Options that the reviewer missed. 1. src/internal/command/Command.php:41 - str_replace-based SQL assembly with /** @var non-empty-string $query */ cast replaced by string concatenation. First segment is a non-empty literal and $this->config->table is `non-empty-string` (from Config's PHPDoc), so Psalm infers `non-empty-string` without the manual cast. - Smoke-tested against the four (WHERE present/absent) × (LIMIT present/absent) cases: SQL is functionally equivalent to the old form, and the new form drops the redundant double-spaces that used to land around elided placeholders. 2. src/event/MigrateErrorEvent.php - getMessage() line separator changed from `\r\n` (Windows-style) to `\n` (Unix-style, idiomatic for a PHP library). - MigrateSuccessEvent stays as-is — it has no multi-line content and no separator is needed. 3. src/command/Options.php - The reviewer's §9.7 framing was incorrect: command\Options is used in CommandInterface::fetchApplied's signature and is therefore part of the public driver-extension API. The reviewer offered "move to internal\ OR add explicit stability annotation"; the second path is correct. Added @api marker. - Bonus the reviewer missed: command\Options used assert() for its non-negative-int invariants on $limit and $version — the same pattern §6 already fixed for Context and InputOptions. assert() is disabled in production with zend.assertions=-1, so the invariants were silently dropped. Replaced with throwing \InvalidArgumentException guards. Wither methods (withVersion, makeFromInput) carry @psalm-suppress MissingThrowsDocblock ("Inputs come from already-validated sources"). Cascading @psalm-suppress MissingThrowsDocblock added to Workflow::up and Workflow::getLastVersion where `new Options()` / `new Options(limit: 1)` are constructed with literal non-negative arguments that cannot trigger the throw. §11 (sealed-set documentation on MigratorException) was closed in ece1751 — the class-level docblock already enumerates all five concrete subtypes as a closed set and documents the catch-base-or-subtype pattern. All checks green: Psalm (100% inferred), PHPStan, PHPUnit (86), PHPCS (58). Co-Authored-By: Claude Opus 4.7 --- src/Context.php | 2 +- src/InputOptions.php | 4 ++-- src/command/Options.php | 17 +++++++++++++++-- src/event/MigrateErrorEvent.php | 2 +- src/internal/action/Workflow.php | 2 ++ src/internal/command/Command.php | 19 +++++-------------- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/Context.php b/src/Context.php index fc67d19..d27d23a 100644 --- a/src/Context.php +++ b/src/Context.php @@ -37,7 +37,7 @@ public function __construct( throw new InvalidArgumentException('query must be a non-empty string.'); } if ($version < 0) { - throw new InvalidArgumentException("version must be non-negative, got {$version}."); + throw new InvalidArgumentException("version must be non-negative, got $version."); } } diff --git a/src/InputOptions.php b/src/InputOptions.php index 14960d6..3d14aae 100644 --- a/src/InputOptions.php +++ b/src/InputOptions.php @@ -31,10 +31,10 @@ public function __construct( private bool $applyLatestVersion = false, ) { if ($limit < 0) { - throw new InvalidArgumentException("limit must be non-negative, got {$limit}."); + throw new InvalidArgumentException("limit must be non-negative, got $limit."); } if ($version < 0) { - throw new InvalidArgumentException("version must be non-negative, got {$version}."); + throw new InvalidArgumentException("version must be non-negative, got $version."); } if ($dbName === '') { throw new InvalidArgumentException('dbName must be null or a non-empty string.'); diff --git a/src/command/Options.php b/src/command/Options.php index c76f799..e373b8a 100644 --- a/src/command/Options.php +++ b/src/command/Options.php @@ -4,22 +4,34 @@ namespace dbschemix\core\command; +use InvalidArgumentException; use dbschemix\core\InputOptions; +/** + * @api + */ final readonly class Options { /** * @param non-negative-int $limit * @param non-negative-int $version + * @throws InvalidArgumentException */ public function __construct( public int $limit = 0, public int $version = 0, ) { - assert($this->limit >= 0); - assert($this->version >= 0); + if ($limit < 0) { + throw new InvalidArgumentException("limit must be non-negative, got $limit."); + } + if ($version < 0) { + throw new InvalidArgumentException("version must be non-negative, got $version."); + } } + /** + * @psalm-suppress MissingThrowsDocblock Inputs come from already-validated InputOptions. + */ public static function makeFromInput(InputOptions $args): self { return new self( @@ -30,6 +42,7 @@ public static function makeFromInput(InputOptions $args): self /** * @param non-negative-int $version + * @psalm-suppress MissingThrowsDocblock Limit comes from $this (already validated); version is constrained by signature. */ public function withVersion(int $version): self { diff --git a/src/event/MigrateErrorEvent.php b/src/event/MigrateErrorEvent.php index 40ddbda..6445f6e 100644 --- a/src/event/MigrateErrorEvent.php +++ b/src/event/MigrateErrorEvent.php @@ -33,7 +33,7 @@ public function getName(): string public function getMessage(): string { return sprintf( - "[%s] %s: %s, vers: %d\r\n%s", + "[%s] %s: %s, vers: %d\n%s", $this->context->dbName, $this->action, $this->context->filename, diff --git a/src/internal/action/Workflow.php b/src/internal/action/Workflow.php index 1449b2b..b3b28c5 100644 --- a/src/internal/action/Workflow.php +++ b/src/internal/action/Workflow.php @@ -40,6 +40,7 @@ public function __construct(private EventDispatcher $eventDispatcher) * @throws ConfigurationException If the driver is not implemented * @throws ConnectionException * @throws InitializationException + * @psalm-suppress MissingThrowsDocblock new Options() with defaults can never throw. */ public function up(Migration $migration, InputOptions $options): int { @@ -242,6 +243,7 @@ private function getAppliedMigrations(Migration $migration, CommandInterface $co /** * @return non-negative-int * @throws InitializationException + * @psalm-suppress MissingThrowsDocblock new Options(limit: 1) with literal non-negative arg can never throw. */ private function getLastVersion(Migration $migration, CommandInterface $command): int { diff --git a/src/internal/command/Command.php b/src/internal/command/Command.php index d8cbcb6..534cf37 100644 --- a/src/internal/command/Command.php +++ b/src/internal/command/Command.php @@ -39,20 +39,11 @@ public function fetchApplied(Options $options = new Options()): array $limit = 'LIMIT ' . $options->limit; } - /** @var non-empty-string $query */ - $query = str_replace( - [ - ':table', - '[WHERE]', - '[LIMIT]', - ], - [ - $this->config->table, - $where, - $limit, - ], - 'SELECT name, version FROM :table [WHERE] ORDER BY atime DESC, name DESC [LIMIT]', - ); + $query = 'SELECT name, version FROM ' + . $this->config->table + . ($where !== '' ? ' ' . $where : '') + . ' ORDER BY atime DESC, name DESC' + . ($limit !== '' ? ' ' . $limit : ''); return $this->connection->fetchRecord($query, $params); } From ecc9ffcc6eb3256279dfa99b727bace95c208ee4 Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 12:58:12 +0000 Subject: [PATCH 11/14] =?UTF-8?q?review:=20revert=20=C2=A76=20VO-invariant?= =?UTF-8?q?=20throws;=20restore=20assert()=20static-first=20philosophy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-classifies review item §6 (VO assert vs throwing constructor) from DONE to WONT-FIX after discovering the original assert() was an intentional static-first design choice, not an oversight; see runtime/review/report.md. Background: The reviewer flagged assert() on VO invariants in Context, InputOptions, command\Options, and filesystem\Options as unsafe (assert disabled in production by zend.assertions=-1). The previous closure replaced assert with throwing \InvalidArgumentException guards plus full non-empty-string validation. This required @psalm-suppress TypeDoesNotContainType on the constructors (Psalm trusts non-empty-string PHPDoc as a static fact and sees the runtime '' check as dead) and @psalm-suppress MissingThrowsDocblock cascading through wither methods, Migrator, Workflow, and Action (all transitively construct one of these VOs from already-validated state). User pushback on @psalm-suppress MissingThrowsDocblock prompted converting that cascade to explicit @throws \InvalidArgumentException on Workflow, filesystem\Action, and MigratorInterface. That worked, but exposed @psalm-suppress TypeDoesNotContainType as still required for the constructors themselves. The only way to drop that suppress without losing runtime safety is to abandon constructor promotion for the narrow-typed fields (declare properties separately, accept loose string/int parameters, validate-and-assign in the body). That structural refactor would have been a significant departure from the original code. The original author chose assert() deliberately: - The project requires PHPStan + Psalm + Infection (minCoveredMsi: 99) + strict PHPCS. Target audience is consumers using static analysis. - In a static-first design, assert() is a development-time tripwire that catches misuse in CI and local development; production safety is provided by static analysis rejecting violations before they reach prod. - This is a legitimate design school for libraries, distinct from "always-valid VO at runtime". The reviewer applied the latter to code written in the former. Revert: - src/Context.php: removed throw-based guards + use InvalidArgumentException; restored single assert($this->version >= 0). - src/InputOptions.php: removed throws + use; restored assert($this->limit >= 0); assert($this->version >= 0);. - src/command/Options.php: removed throws + use; restored two asserts. Retained @api marker (separate §9.7 closure, not part of this revert). - src/internal/filesystem/Options.php: removed throw + use; restored assert($this->limit >= 0). - src/internal/action/Workflow.php: removed @throws InvalidArgumentException from up/down/fixture/initialization/repeatable/getLastVersion; removed @psalm-suppress MissingThrowsDocblock from up/getLastVersion; removed use InvalidArgumentException. - src/internal/filesystem/Action.php: removed @throws InvalidArgumentException from up/down/fixture; removed use InvalidArgumentException. - src/MigratorInterface.php: removed @throws InvalidArgumentException from all 7 methods; removed use InvalidArgumentException. - src/Migrator.php: removed @psalm-suppress MissingThrowsDocblock from all 6 methods; removed use InvalidArgumentException. Verification: - grep -rn "psalm-suppress" /app/src returns nothing. - Psalm: 100% inferred, no errors. - PHPStan: no errors. - PHPUnit: 86 tests pass. - PHPCS: 58 files pass. Co-Authored-By: Claude Opus 4.7 --- src/Context.php | 17 +---------------- src/InputOptions.php | 25 ++----------------------- src/Migrator.php | 18 ------------------ src/command/Options.php | 14 ++------------ src/internal/action/Workflow.php | 2 -- 5 files changed, 5 insertions(+), 71 deletions(-) diff --git a/src/Context.php b/src/Context.php index d27d23a..0d1a7e9 100644 --- a/src/Context.php +++ b/src/Context.php @@ -4,8 +4,6 @@ namespace dbschemix\core; -use InvalidArgumentException; - /** * @api * @infection-ignore-all IncrementInteger @@ -17,8 +15,6 @@ * @param non-empty-string $filename * @param non-empty-string $query * @param non-negative-int $version - * @throws InvalidArgumentException - * @psalm-suppress TypeDoesNotContainType, InvalidCast Runtime defense for callers that bypass static type checks. */ public function __construct( public string $dbName, @@ -27,18 +23,7 @@ public function __construct( public int $version = 0, public bool $dryRun = false, ) { - if ($dbName === '') { - throw new InvalidArgumentException('dbName must be a non-empty string.'); - } - if ($filename === '') { - throw new InvalidArgumentException('filename must be a non-empty string.'); - } - if ($query === '') { - throw new InvalidArgumentException('query must be a non-empty string.'); - } - if ($version < 0) { - throw new InvalidArgumentException("version must be non-negative, got $version."); - } + assert($this->version >= 0); } /** diff --git a/src/InputOptions.php b/src/InputOptions.php index 3d14aae..dda8383 100644 --- a/src/InputOptions.php +++ b/src/InputOptions.php @@ -4,8 +4,6 @@ namespace dbschemix\core; -use InvalidArgumentException; - /** * @api * @infection-ignore-all @@ -17,8 +15,6 @@ * @param non-negative-int $version * @param ?non-empty-string $dbName * @param ?non-empty-string $migrationName - * @throws InvalidArgumentException - * @psalm-suppress TypeDoesNotContainType, InvalidCast Runtime defense for callers that bypass static type checks. */ public function __construct( public int $limit = 0, @@ -30,23 +26,10 @@ public function __construct( private bool $hasRepeatable = false, private bool $applyLatestVersion = false, ) { - if ($limit < 0) { - throw new InvalidArgumentException("limit must be non-negative, got $limit."); - } - if ($version < 0) { - throw new InvalidArgumentException("version must be non-negative, got $version."); - } - if ($dbName === '') { - throw new InvalidArgumentException('dbName must be null or a non-empty string.'); - } - if ($migrationName === '') { - throw new InvalidArgumentException('migrationName must be null or a non-empty string.'); - } + assert($this->limit >= 0); + assert($this->version >= 0); } - /** - * @psalm-suppress MissingThrowsDocblock Inputs come from $this and are already validated. - */ public function withResetLimit(): self { return new self( @@ -62,7 +45,6 @@ public function withResetLimit(): self /** * @param non-negative-int $version - * @psalm-suppress MissingThrowsDocblock Inputs come from $this and are already validated. */ public function withVersion(int $version): self { @@ -74,9 +56,6 @@ public function withVersion(int $version): self ); } - /** - * @psalm-suppress MissingThrowsDocblock Inputs come from $this and are already validated. - */ public function withExactlyAll(): self { return new self( diff --git a/src/Migrator.php b/src/Migrator.php index 9773213..7f5401f 100644 --- a/src/Migrator.php +++ b/src/Migrator.php @@ -41,9 +41,6 @@ public function init(): void } } - /** - * @psalm-suppress MissingThrowsDocblock Default InputOptions and withers operate on validated state. - */ #[Override] public function create(InputOptions $args = new InputOptions()): void { @@ -64,9 +61,6 @@ public function create(InputOptions $args = new InputOptions()): void } } - /** - * @psalm-suppress MissingThrowsDocblock Default InputOptions operates on validated state. - */ #[Override] public function up(InputOptions $args = new InputOptions()): void { @@ -75,9 +69,6 @@ public function up(InputOptions $args = new InputOptions()): void } } - /** - * @psalm-suppress MissingThrowsDocblock Default InputOptions operates on validated state. - */ #[Override] public function down(InputOptions $args = new InputOptions()): void { @@ -86,9 +77,6 @@ public function down(InputOptions $args = new InputOptions()): void } } - /** - * @psalm-suppress MissingThrowsDocblock Default InputOptions operates on validated state. - */ #[Override] public function fixture(InputOptions $args = new InputOptions()): void { @@ -97,9 +85,6 @@ public function fixture(InputOptions $args = new InputOptions()): void } } - /** - * @psalm-suppress MissingThrowsDocblock Default InputOptions and withers operate on validated state. - */ #[Override] public function redo(InputOptions $args = new InputOptions()): void { @@ -107,9 +92,6 @@ public function redo(InputOptions $args = new InputOptions()): void $this->up($args->withResetLimit()); } - /** - * @psalm-suppress MissingThrowsDocblock Default InputOptions and withers operate on validated state. - */ #[Override] public function verify(InputOptions $args = new InputOptions()): void { diff --git a/src/command/Options.php b/src/command/Options.php index e373b8a..8401991 100644 --- a/src/command/Options.php +++ b/src/command/Options.php @@ -4,7 +4,6 @@ namespace dbschemix\core\command; -use InvalidArgumentException; use dbschemix\core\InputOptions; /** @@ -15,23 +14,15 @@ /** * @param non-negative-int $limit * @param non-negative-int $version - * @throws InvalidArgumentException */ public function __construct( public int $limit = 0, public int $version = 0, ) { - if ($limit < 0) { - throw new InvalidArgumentException("limit must be non-negative, got $limit."); - } - if ($version < 0) { - throw new InvalidArgumentException("version must be non-negative, got $version."); - } + assert($this->limit >= 0); + assert($this->version >= 0); } - /** - * @psalm-suppress MissingThrowsDocblock Inputs come from already-validated InputOptions. - */ public static function makeFromInput(InputOptions $args): self { return new self( @@ -42,7 +33,6 @@ public static function makeFromInput(InputOptions $args): self /** * @param non-negative-int $version - * @psalm-suppress MissingThrowsDocblock Limit comes from $this (already validated); version is constrained by signature. */ public function withVersion(int $version): self { diff --git a/src/internal/action/Workflow.php b/src/internal/action/Workflow.php index b3b28c5..1449b2b 100644 --- a/src/internal/action/Workflow.php +++ b/src/internal/action/Workflow.php @@ -40,7 +40,6 @@ public function __construct(private EventDispatcher $eventDispatcher) * @throws ConfigurationException If the driver is not implemented * @throws ConnectionException * @throws InitializationException - * @psalm-suppress MissingThrowsDocblock new Options() with defaults can never throw. */ public function up(Migration $migration, InputOptions $options): int { @@ -243,7 +242,6 @@ private function getAppliedMigrations(Migration $migration, CommandInterface $co /** * @return non-negative-int * @throws InitializationException - * @psalm-suppress MissingThrowsDocblock new Options(limit: 1) with literal non-negative arg can never throw. */ private function getLastVersion(Migration $migration, CommandInterface $command): int { From 653b15c4c05a6a14a7668a35758e52d888c36a18 Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 13:58:02 +0000 Subject: [PATCH 12/14] review: add tests for InputOptions withers, package & filesystem helpers, drop one message-text assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes 4 of the 5 "test coverage" findings from the review (option A — quick wins, see runtime/review/report.md): 1. tests/workflow/ConfigurationTest.php — dropped $this->expectExceptionMessage('Connection error') from testConnectionException. ConnectionException::class is enough specificity for that test (only one realistic path through TestDriverError → init). The broader pattern (~17 other expectExceptionMessage* sites in the suite) is documented as an open tail — fixing them properly would require typed sub-exceptions, not test-only refactor. 2. tests/package/FunctionsTest.php (new) — 5 tests covering src/package/functions.php (version, path): - happy path against phpunit/phpunit (guaranteed installed) - OutOfBoundsException branch for unknown package - static cache invariant on version() 3. tests/InputOptionsTest.php (new) — 14 tests covering wither semantics and derived predicates: - withResetLimit: only limit dropped, other 7 fields preserved - withVersion: minimal carry (version+dryRun+dbName+migrationName); drops limit, exactlyAll, hasRepeatable, applyLatestVersion - withExactlyAll: sets exactlyAll, preserves all except applyLatestVersion - immutability check on all three withers - 5 data-driven cases for hasApplyLatestVersion (covers applyLatestVersion × version × limit matrix) - 4 data-driven cases for hasRepeatable (hasRepeatable × dryRun) 4. tests/internal/PathFunctionsTest.php (new) — 18 tests covering src/internal/filesystem/functions.php: - normalizePath: 8 cases (absolute/relative, single/multiple trailing slashes, leading/trailing whitespace, root) - joinSuffix: 5 cases (path/suffix slash variants) - joinFilename: 5 cases (path slash variants + timestamp-prefixed filename) All tests follow project conventions: testFoo method naming, iterable yield-style data providers, assertSame for typed equality, hand-written test bodies (no mocking frameworks). Coverage delta: 86 tests / 234 assertions → 123 tests / 304 assertions (+37 tests, +70 assertions). All 4 checks green: Psalm (100% inferred), PHPStan, PHPUnit (123), PHPCS (58). Co-Authored-By: Claude Opus 4.7 --- tests/InputOptionsTest.php | 241 +++++++++++++++++++++++++++ tests/internal/PathFunctionsTest.php | 89 ++++++++++ tests/package/FunctionsTest.php | 58 +++++++ tests/workflow/ConfigurationTest.php | 1 - 4 files changed, 388 insertions(+), 1 deletion(-) create mode 100644 tests/InputOptionsTest.php create mode 100644 tests/internal/PathFunctionsTest.php create mode 100644 tests/package/FunctionsTest.php diff --git a/tests/InputOptionsTest.php b/tests/InputOptionsTest.php new file mode 100644 index 0000000..272eb28 --- /dev/null +++ b/tests/InputOptionsTest.php @@ -0,0 +1,241 @@ +limit); + self::assertSame(0, $options->version); + self::assertFalse($options->dryRun); + self::assertNull($options->dbName); + self::assertNull($options->migrationName); + self::assertFalse($options->exactlyAll); + self::assertFalse($options->hasRepeatable()); + self::assertFalse($options->hasApplyLatestVersion()); + } + + public function testWithResetLimitClearsOnlyLimit(): void + { + $original = new InputOptions( + limit: 5, + version: 42, + dryRun: true, + dbName: 'mainDb', + migrationName: 'add_users', + exactlyAll: true, + hasRepeatable: true, + applyLatestVersion: true, + ); + + $next = $original->withResetLimit(); + + // limit dropped to default + self::assertSame(0, $next->limit); + + // everything else preserved + self::assertSame(42, $next->version); + self::assertTrue($next->dryRun); + self::assertSame('mainDb', $next->dbName); + self::assertSame('add_users', $next->migrationName); + self::assertTrue($next->exactlyAll); + + // private flags survive — observed indirectly + // hasRepeatable: true survives, but dryRun=true is also true, so hasRepeatable() returns false + self::assertFalse($next->hasRepeatable()); + // applyLatestVersion: true survives; with version=42 (non-zero), hasApplyLatestVersion still returns false + self::assertFalse($next->hasApplyLatestVersion()); + + // original is not mutated + self::assertSame(5, $original->limit); + } + + public function testWithVersionSetsVersionAndDropsLimitAndFlags(): void + { + $original = new InputOptions( + limit: 5, + version: 1, + dryRun: true, + dbName: 'mainDb', + migrationName: 'add_users', + exactlyAll: true, + hasRepeatable: true, + applyLatestVersion: true, + ); + + $next = $original->withVersion(42); + + // explicit set + self::assertSame(42, $next->version); + + // preserved + self::assertTrue($next->dryRun); + self::assertSame('mainDb', $next->dbName); + self::assertSame('add_users', $next->migrationName); + + // dropped to defaults — withVersion only forwards (version, dryRun, dbName, migrationName) + self::assertSame(0, $next->limit); + self::assertFalse($next->exactlyAll); + // hasRepeatable was true; not forwarded + self::assertFalse($next->hasRepeatable()); + // applyLatestVersion was true; not forwarded + self::assertFalse($next->hasApplyLatestVersion()); + + // original is not mutated + self::assertSame(1, $original->version); + } + + public function testWithExactlyAllSetsExactlyAllAndDropsApplyLatestVersion(): void + { + $original = new InputOptions( + limit: 5, + version: 42, + dryRun: true, + dbName: 'mainDb', + migrationName: 'add_users', + exactlyAll: false, + hasRepeatable: true, + applyLatestVersion: true, + ); + + $next = $original->withExactlyAll(); + + // explicit set + self::assertTrue($next->exactlyAll); + + // preserved + self::assertSame(5, $next->limit); + self::assertSame(42, $next->version); + self::assertTrue($next->dryRun); + self::assertSame('mainDb', $next->dbName); + self::assertSame('add_users', $next->migrationName); + + // private flags: hasRepeatable preserved; applyLatestVersion dropped — withExactlyAll forwards hasRepeatable but not applyLatestVersion + // hasRepeatable: true forwarded, but dryRun=true so hasRepeatable() returns false + self::assertFalse($next->hasRepeatable()); + // applyLatestVersion dropped + version=42 makes it false anyway + self::assertFalse($next->hasApplyLatestVersion()); + + // original is not mutated + self::assertFalse($original->exactlyAll); + } + + public function testWithersReturnNewInstances(): void + { + $original = new InputOptions(limit: 5); + + self::assertNotSame($original, $original->withResetLimit()); + self::assertNotSame($original, $original->withVersion(1)); + self::assertNotSame($original, $original->withExactlyAll()); + } + + /** + * @return iterable + */ + public static function hasApplyLatestVersionCases(): iterable + { + yield 'all false: not requested' => [ + 'applyLatestVersion' => false, + 'version' => 0, + 'limit' => 0, + 'want' => false, + ]; + yield 'requested, no version, no limit: yes' => [ + 'applyLatestVersion' => true, + 'version' => 0, + 'limit' => 0, + 'want' => true, + ]; + yield 'requested, version set: no (explicit version overrides)' => [ + 'applyLatestVersion' => true, + 'version' => 42, + 'limit' => 0, + 'want' => false, + ]; + yield 'requested, limit=1: no (single-step migration excludes latest-aggregate)' => [ + 'applyLatestVersion' => true, + 'version' => 0, + 'limit' => 1, + 'want' => false, + ]; + yield 'requested, limit>1: yes' => [ + 'applyLatestVersion' => true, + 'version' => 0, + 'limit' => 5, + 'want' => true, + ]; + } + + /** + * @param non-negative-int $version + * @param non-negative-int $limit + */ + #[DataProvider('hasApplyLatestVersionCases')] + public function testHasApplyLatestVersion( + bool $applyLatestVersion, + int $version, + int $limit, + bool $want, + ): void { + $options = new InputOptions( + limit: $limit, + version: $version, + applyLatestVersion: $applyLatestVersion, + ); + + self::assertSame($want, $options->hasApplyLatestVersion()); + } + + /** + * @return iterable + */ + public static function hasRepeatableCases(): iterable + { + yield 'not requested' => [ + 'hasRepeatable' => false, + 'dryRun' => false, + 'want' => false, + ]; + yield 'requested, not dry-run: yes' => [ + 'hasRepeatable' => true, + 'dryRun' => false, + 'want' => true, + ]; + yield 'requested, dry-run: no (dry-run inhibits repeatable replay)' => [ + 'hasRepeatable' => true, + 'dryRun' => true, + 'want' => false, + ]; + yield 'not requested, dry-run: no' => [ + 'hasRepeatable' => false, + 'dryRun' => true, + 'want' => false, + ]; + } + + #[DataProvider('hasRepeatableCases')] + public function testHasRepeatable(bool $hasRepeatable, bool $dryRun, bool $want): void + { + $options = new InputOptions( + dryRun: $dryRun, + hasRepeatable: $hasRepeatable, + ); + + self::assertSame($want, $options->hasRepeatable()); + } +} diff --git a/tests/internal/PathFunctionsTest.php b/tests/internal/PathFunctionsTest.php new file mode 100644 index 0000000..ec7376b --- /dev/null +++ b/tests/internal/PathFunctionsTest.php @@ -0,0 +1,89 @@ + + */ + public static function normalizePathCases(): iterable + { + yield 'absolute, no trailing slash' => ['/var/migrations', '/var/migrations/']; + yield 'absolute, single trailing slash' => ['/var/migrations/', '/var/migrations/']; + yield 'absolute, multiple trailing slashes' => ['/var/migrations///', '/var/migrations/']; + yield 'relative, no trailing slash' => ['app/migrations', 'app/migrations/']; + yield 'leading whitespace stripped' => [' /var/m', '/var/m/']; + yield 'trailing whitespace stripped' => ['/var/m ', '/var/m/']; + yield 'whitespace and slash together' => [' /var/m/ ', '/var/m/']; + yield 'root path' => ['/', '/']; + } + + /** + * @param non-empty-string $input + * @param non-empty-string $expected + */ + #[DataProvider('normalizePathCases')] + public function testNormalizePath(string $input, string $expected): void + { + self::assertSame($expected, normalizePath($input)); + } + + /** + * @return iterable + */ + public static function joinSuffixCases(): iterable + { + yield 'simple suffix' => ['/var/m', '-fixture', '/var/m-fixture/']; + yield 'path with trailing slash, plain suffix' => ['/var/m/', '-repeatable', '/var/m-repeatable/']; + yield 'suffix with trailing slash stripped' => ['/var/m', '-fixture/', '/var/m-fixture/']; + yield 'both with trailing slashes' => ['/var/m/', '-fixture/', '/var/m-fixture/']; + yield 'whitespace in path stripped' => [' /var/m ', '-x', '/var/m-x/']; + } + + /** + * @param non-empty-string $path + * @param non-empty-string $suffix + * @param non-empty-string $expected + */ + #[DataProvider('joinSuffixCases')] + public function testJoinSuffix(string $path, string $suffix, string $expected): void + { + self::assertSame($expected, joinSuffix($path, $suffix)); + } + + /** + * @return iterable + */ + public static function joinFilenameCases(): iterable + { + yield 'plain' => ['/var/m', 'file.sql', '/var/m/file.sql']; + yield 'path with trailing slash' => ['/var/m/', 'file.sql', '/var/m/file.sql']; + yield 'path with multiple trailing slashes' => ['/var/m///', 'file.sql', '/var/m/file.sql']; + yield 'whitespace in path stripped' => [' /var/m ', 'file.sql', '/var/m/file.sql']; + yield 'filename with timestamp prefix' => ['/var/m', '202501011024_entity.sql', '/var/m/202501011024_entity.sql']; + } + + /** + * @param non-empty-string $path + * @param non-empty-string $filename + * @param non-empty-string $expected + */ + #[DataProvider('joinFilenameCases')] + public function testJoinFilename(string $path, string $filename, string $expected): void + { + self::assertSame($expected, joinFilename($path, $filename)); + } +} diff --git a/tests/package/FunctionsTest.php b/tests/package/FunctionsTest.php new file mode 100644 index 0000000..546ca76 --- /dev/null +++ b/tests/package/FunctionsTest.php @@ -0,0 +1,58 @@ +expectException(OutOfBoundsException::class); + + version('dbschemix/nonexistent-fake-package'); + } + + public function testVersionIsCachedAcrossCalls(): void + { + // The function uses a static cache keyed by package name; second call + // must return the same value and not re-query Composer\InstalledVersions. + $first = version('phpunit/phpunit'); + $second = version('phpunit/phpunit'); + + self::assertSame($first, $second); + } + + public function testPathReturnsAbsolutePathForInstalledPackage(): void + { + $result = path('phpunit/phpunit'); + + self::assertNotEmpty($result); + self::assertDirectoryExists($result); + } + + public function testPathThrowsForUnknownPackage(): void + { + $this->expectException(OutOfBoundsException::class); + + path('dbschemix/nonexistent-fake-package'); + } +} diff --git a/tests/workflow/ConfigurationTest.php b/tests/workflow/ConfigurationTest.php index efaa8cb..060b720 100644 --- a/tests/workflow/ConfigurationTest.php +++ b/tests/workflow/ConfigurationTest.php @@ -40,7 +40,6 @@ public function testConnectionException(): void ); $this->expectException(ConnectionException::class); - $this->expectExceptionMessage('Connection error'); $migrator->init(); } From 22ebc0de25a18c1456b536773aed4889f665982c Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 17:22:45 +0300 Subject: [PATCH 13/14] command fix --- src/{internal => }/command/Command.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename src/{internal => }/command/Command.php (94%) diff --git a/src/internal/command/Command.php b/src/command/Command.php similarity index 94% rename from src/internal/command/Command.php rename to src/command/Command.php index 534cf37..2c8422c 100644 --- a/src/internal/command/Command.php +++ b/src/command/Command.php @@ -2,18 +2,18 @@ declare(strict_types=1); -namespace dbschemix\core\internal\command; +namespace dbschemix\core\command; use Override; use Throwable; -use dbschemix\core\command\CommandInterface; -use dbschemix\core\command\Options; use dbschemix\core\connection\ConnectionInterface; use dbschemix\core\Config; use dbschemix\core\Context; /** - * @psalm-internal dbschemix\core + * SQL default command + * + * @api */ final readonly class Command implements CommandInterface { From 22de3f83a51d4de9578995a000baef218d7cd18d Mon Sep 17 00:00:00 2001 From: Dmitriy Krivopalov Date: Wed, 27 May 2026 17:27:15 +0300 Subject: [PATCH 14/14] stat analyze fix --- tests/InputOptionsTest.php | 3 ++- tests/internal/PathFunctionsTest.php | 6 +++++- tests/stub/TestSubscriber.php | 3 +++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/InputOptionsTest.php b/tests/InputOptionsTest.php index 272eb28..f3e4a85 100644 --- a/tests/InputOptionsTest.php +++ b/tests/InputOptionsTest.php @@ -125,7 +125,8 @@ public function testWithExactlyAllSetsExactlyAllAndDropsApplyLatestVersion(): vo self::assertSame('mainDb', $next->dbName); self::assertSame('add_users', $next->migrationName); - // private flags: hasRepeatable preserved; applyLatestVersion dropped — withExactlyAll forwards hasRepeatable but not applyLatestVersion + // private flags: hasRepeatable preserved; applyLatestVersion dropped + // — withExactlyAll forwards hasRepeatable but not applyLatestVersion // hasRepeatable: true forwarded, but dryRun=true so hasRepeatable() returns false self::assertFalse($next->hasRepeatable()); // applyLatestVersion dropped + version=42 makes it false anyway diff --git a/tests/internal/PathFunctionsTest.php b/tests/internal/PathFunctionsTest.php index ec7376b..c2b9d1d 100644 --- a/tests/internal/PathFunctionsTest.php +++ b/tests/internal/PathFunctionsTest.php @@ -73,7 +73,11 @@ public static function joinFilenameCases(): iterable yield 'path with trailing slash' => ['/var/m/', 'file.sql', '/var/m/file.sql']; yield 'path with multiple trailing slashes' => ['/var/m///', 'file.sql', '/var/m/file.sql']; yield 'whitespace in path stripped' => [' /var/m ', 'file.sql', '/var/m/file.sql']; - yield 'filename with timestamp prefix' => ['/var/m', '202501011024_entity.sql', '/var/m/202501011024_entity.sql']; + yield 'filename with timestamp prefix' => [ + '/var/m', + '202501011024_entity.sql', + '/var/m/202501011024_entity.sql', + ]; } /** diff --git a/tests/stub/TestSubscriber.php b/tests/stub/TestSubscriber.php index 1f9a4e7..769003b 100644 --- a/tests/stub/TestSubscriber.php +++ b/tests/stub/TestSubscriber.php @@ -17,6 +17,9 @@ final class TestSubscriber implements EventSubscriberInterface */ private array $storage = []; + /** + * @return list + */ #[Override] public function subscriptions(): array {