From 28c25a9b7832f9f69a9f891d943e34eb22e446f7 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 28 Sep 2026 14:50:45 +1000 Subject: [PATCH 1/4] Introduce the named exception constructor convention - Document the convention, the constant naming rule and the three tests for when a condition earns its own class in docs/book/exceptions.md - Point TableGateway\Exception\RuntimeException at Exception\RuntimeException rather than Exception\InvalidArgumentException, so it is a \RuntimeException - Declare the @throws the retype exposed on AbstractTableGateway, TableGateway::__construct and GlobalAdapterFeature::preInitialize, which the wrong parent had been covering - Extend ContainerException from Exception\RuntimeException so it carries the package marker interface, and move its message to a SERVICE template - Guard the marker interface, the SPL parents and the constant naming rule in ExceptionHierarchyTest, scanning src so new classes are covered on sight - Assert ContainerException's message against its template and drop the file's stale lint baseline entries Refs #189 Signed-off-by: Simon Mundy --- docs/book/exceptions.md | 73 ++++++++ lint-baseline.toml | 18 -- mkdocs.yml | 1 + src/Exception/ContainerException.php | 7 +- src/TableGateway/AbstractTableGateway.php | 11 ++ .../Exception/RuntimeException.php | 2 +- .../Feature/GlobalAdapterFeature.php | 1 + src/TableGateway/TableGateway.php | 1 + .../unit/Exception/ContainerExceptionTest.php | 27 ++- .../unit/Exception/ExceptionHierarchyTest.php | 161 ++++++++++++++++++ .../TestAsset/ExceptionClassLocator.php | 119 +++++++++++++ 11 files changed, 393 insertions(+), 28 deletions(-) create mode 100644 docs/book/exceptions.md create mode 100644 test/unit/Exception/ExceptionHierarchyTest.php create mode 100644 test/unit/Exception/TestAsset/ExceptionClassLocator.php diff --git a/docs/book/exceptions.md b/docs/book/exceptions.md new file mode 100644 index 00000000..b9423bda --- /dev/null +++ b/docs/book/exceptions.md @@ -0,0 +1,73 @@ +# Exceptions + +Every exception this package throws is built by a `static` named constructor on a +component-owned exception class. The message template sits beside it as a +`final public const string`. No exception is constructed with a message assembled at +the throw site. + +## The convention + +```php +namespace PhpDb\TableGateway\Exception; + +use PhpDb\Exception; + +use function sprintf; + +class RuntimeException extends Exception\RuntimeException implements ExceptionInterface +{ + final public const string NO_PRIMARY_KEY = 'No information was provided to the RowGatewayFeature' + . ' and/or no MetadataFeature could be consulted to find the primary key' + . ' necessary for RowGateway object creation.'; + + final public const string INVALID_MAGIC_CALL = 'Invalid method (%s) called, caught by %s::__call()'; + + public static function forNoPrimaryKey(): self + { + return new self(self::NO_PRIMARY_KEY); + } + + public static function forInvalidMagicCall(string $method, string $class): self + { + return new self(sprintf(self::INVALID_MAGIC_CALL, $method, $class)); + } +} +``` + +- Named constructors are `static`, return `self` and start with `for`. +- Each one has a single `final public const string` template beside it. +- The constant name mirrors the method name: `forNoPrimaryKey()` uses `NO_PRIMARY_KEY`, + `forInvalidMagicCall()` uses `INVALID_MAGIC_CALL`. Strip the `for` prefix and + upper-snake the rest. +- Use `new self(self::CONST)` when the template has no placeholder and + `new self(sprintf(self::CONST, ...))` when it does. `sprintf()` on a template with no + `%` placeholder returns its own argument, so it is not written. +- Quote a `%s` that holds a caller-supplied identifier, such as a column or parameter + name. Leave a class name, method name or count unquoted. + +The constants are `final` because the component base classes are not. Adapter packages +extend `PhpDb\Adapter\Exception\RuntimeException`, so without `final` a subclass could +redefine a published template. + +## When a condition earns its own exception class + +Naming a condition in a class instead of a method is a departure from the convention, +not an application of it. A condition earns its own class only when it carries something +a named constructor cannot: + +1. **Extra state, with accessors.** `PhpDb\Adapter\Exception\InvalidConnectionParametersException` + holds the connection parameters it rejected. +2. **An external interface contract.** `PhpDb\Exception\ContainerException` implements + PSR-11's `ContainerExceptionInterface`. +3. **A condition a consumer can act on.** `PhpDb\Adapter\Exception\InvalidQueryException` + is thrown by every adapter and caught to log the failing SQL or retry. + +Everything else is a named constructor on the component's SPL-shaped class. + +## Component namespaces + +Each component owns an `Exception` namespace holding an `ExceptionInterface` that extends +`PhpDb\Exception\ExceptionInterface`, plus the SPL-shaped classes it needs. A class named +after an SPL exception extends that SPL exception, so `catch (\RuntimeException)` and +`catch (PhpDb\Exception\ExceptionInterface)` both behave as a reader expects. +`PhpDbTest\Exception\ExceptionHierarchyTest` enforces both rules across `src/`. diff --git a/lint-baseline.toml b/lint-baseline.toml index b2f822ad..f4f30de4 100644 --- a/lint-baseline.toml +++ b/lint-baseline.toml @@ -2868,24 +2868,6 @@ code = "prefer-test-attribute" message = "Use `#[Test]` attribute instead of `test` prefix on method `testInvokeUsesDefaultSeparatorWhenSeparatorKeyIsAbsent`." count = 1 -[[issues]] -file = "test/unit/Exception/ContainerExceptionTest.php" -code = "assertion-style" -message = "Inconsistent assertions style." -count = 4 - -[[issues]] -file = "test/unit/Exception/ContainerExceptionTest.php" -code = "prefer-test-attribute" -message = "Use `#[Test]` attribute instead of `test` prefix on method `testForServiceCreatesFormattedExceptionMessage`." -count = 1 - -[[issues]] -file = "test/unit/Exception/ContainerExceptionTest.php" -code = "prefer-test-attribute" -message = "Use `#[Test]` attribute instead of `test` prefix on method `testImplementsContainerExceptionInterface`." -count = 1 - [[issues]] file = "test/unit/ResultSet/AbstractResultSetIntegrationTest.php" code = "assertion-style" diff --git a/mkdocs.yml b/mkdocs.yml index 76af623d..186ddbfe 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -31,6 +31,7 @@ nav: - Metadata Objects: metadata/objects.md - Examples: metadata/examples.md - Profiler: profiler.md + - Exceptions: exceptions.md - "Application Integration": - "Integrating in a Laminas MVC application": application-integration/usage-in-a-laminas-mvc-application.md - "Integrating in a Mezzio application": application-integration/usage-in-a-mezzio-application.md diff --git a/src/Exception/ContainerException.php b/src/Exception/ContainerException.php index ec21ff90..bac0c6dc 100644 --- a/src/Exception/ContainerException.php +++ b/src/Exception/ContainerException.php @@ -5,12 +5,13 @@ namespace PhpDb\Exception; use Psr\Container\ContainerExceptionInterface; -use RuntimeException as SplRuntimeException; use function sprintf; -final class ContainerException extends SplRuntimeException implements ContainerExceptionInterface +final class ContainerException extends RuntimeException implements ContainerExceptionInterface { + final public const string SERVICE = 'Failed to create service "%s" in factory %s Reason: %s'; + public static function forService( string $serviceName, string $factoryClass, @@ -18,7 +19,7 @@ public static function forService( ): self { return new self( sprintf( - 'Failed to create service "%s" in factory %s Reason: %s', + self::SERVICE, $serviceName, $factoryClass, $reason, diff --git a/src/TableGateway/AbstractTableGateway.php b/src/TableGateway/AbstractTableGateway.php index 0d0c2f51..feddc57c 100644 --- a/src/TableGateway/AbstractTableGateway.php +++ b/src/TableGateway/AbstractTableGateway.php @@ -51,6 +51,7 @@ abstract class AbstractTableGateway implements TableGatewayInterface protected string|int|false|null $lastInsertValue = null; + /** @throws Exception\RuntimeException */ #[Override] public function delete(Where|Closure|array|string $where): int { @@ -67,6 +68,7 @@ public function delete(Where|Closure|array|string $where): int return $this->executeDelete($delete); } + /** @throws Exception\RuntimeException */ public function deleteWith(Delete $delete): int { $this->initialize(); @@ -84,6 +86,7 @@ public function getColumns(): array return $this->columns; } + /** @throws Exception\RuntimeException */ public function getFeatureSet(): Feature\FeatureSet { if (! $this->isInitialized) { @@ -98,6 +101,7 @@ public function getLastInsertValue(): string|int|false|null return $this->lastInsertValue; } + /** @throws Exception\RuntimeException */ public function getResultSetPrototype(): ResultSetInterface { if (! $this->isInitialized) { @@ -107,6 +111,7 @@ public function getResultSetPrototype(): ResultSetInterface return $this->resultSetPrototype; } + /** @throws Exception\RuntimeException */ public function getSql(): Sql { if (! $this->isInitialized) { @@ -161,6 +166,7 @@ public function initialize(): void $this->isInitialized = true; } + /** @throws Exception\RuntimeException */ #[Override] public function insert(array $set): int { @@ -173,6 +179,7 @@ public function insert(array $set): int return $this->executeInsert($insert); } + /** @throws Exception\RuntimeException */ public function insertWith(Insert $insert): int { if (! $this->isInitialized) { @@ -187,6 +194,7 @@ public function isInitialized(): bool return $this->isInitialized; } + /** @throws Exception\RuntimeException */ #[Override] public function select(Where|Closure|string|array|null $where = null): ResultSetInterface { @@ -205,6 +213,7 @@ public function select(Where|Closure|string|array|null $where = null): ResultSet return $this->selectWith($select); } + /** @throws Exception\RuntimeException */ public function selectWith(Select $select): ResultSetInterface { if (! $this->isInitialized) { @@ -214,6 +223,7 @@ public function selectWith(Select $select): ResultSetInterface return $this->executeSelect($select); } + /** @throws Exception\RuntimeException */ #[Override] public function update( array $set, @@ -240,6 +250,7 @@ public function update( return $this->executeUpdate($update); } + /** @throws Exception\RuntimeException */ public function updateWith(Update $update): int { if (! $this->isInitialized) { diff --git a/src/TableGateway/Exception/RuntimeException.php b/src/TableGateway/Exception/RuntimeException.php index 7d2902e4..7f5ae93c 100644 --- a/src/TableGateway/Exception/RuntimeException.php +++ b/src/TableGateway/Exception/RuntimeException.php @@ -6,4 +6,4 @@ use PhpDb\Exception; -class RuntimeException extends Exception\InvalidArgumentException implements ExceptionInterface {} +class RuntimeException extends Exception\RuntimeException implements ExceptionInterface {} diff --git a/src/TableGateway/Feature/GlobalAdapterFeature.php b/src/TableGateway/Feature/GlobalAdapterFeature.php index 2e2f8eb0..45b8d0ca 100644 --- a/src/TableGateway/Feature/GlobalAdapterFeature.php +++ b/src/TableGateway/Feature/GlobalAdapterFeature.php @@ -48,6 +48,7 @@ public static function setStaticAdapter(AdapterInterface $adapter): void /** * after initialization, retrieve the original adapter as "master" + * @throws Exception\RuntimeException */ public function preInitialize(): void { diff --git a/src/TableGateway/TableGateway.php b/src/TableGateway/TableGateway.php index d2024437..6fff28aa 100644 --- a/src/TableGateway/TableGateway.php +++ b/src/TableGateway/TableGateway.php @@ -18,6 +18,7 @@ class TableGateway extends AbstractTableGateway * @param Feature\FeatureSet|Feature\FeatureInterface|Feature\FeatureInterface[]|null $features * * @throws Exception\InvalidArgumentException + * @throws Exception\RuntimeException */ public function __construct( TableIdentifier|array|string $table, diff --git a/test/unit/Exception/ContainerExceptionTest.php b/test/unit/Exception/ContainerExceptionTest.php index 5356edcc..d14a42ff 100644 --- a/test/unit/Exception/ContainerExceptionTest.php +++ b/test/unit/Exception/ContainerExceptionTest.php @@ -5,28 +5,43 @@ namespace PhpDbTest\Exception; use PhpDb\Exception\ContainerException; +use PhpDb\Exception\ExceptionInterface; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerExceptionInterface; +use function sprintf; + #[Group('unit')] #[CoversMethod(ContainerException::class, 'forService')] final class ContainerExceptionTest extends TestCase { - public function testForServiceCreatesFormattedExceptionMessage(): void + #[Test] + public function forServiceIsAPsrContainerException(): void { $exception = ContainerException::forService('Svc', 'Factory', 'reason'); - self::assertStringContainsString('Svc', $exception->getMessage()); - self::assertStringContainsString('Factory', $exception->getMessage()); - self::assertStringContainsString('reason', $exception->getMessage()); + self::assertInstanceOf(ContainerExceptionInterface::class, $exception); } - public function testImplementsContainerExceptionInterface(): void + #[Test] + public function forServiceIsReachableThroughThePackageMarkerInterface(): void { $exception = ContainerException::forService('Svc', 'Factory', 'reason'); - self::assertInstanceOf(ContainerExceptionInterface::class, $exception); + self::assertInstanceOf(ExceptionInterface::class, $exception); + } + + #[Test] + public function forServiceRendersItsTemplate(): void + { + $exception = ContainerException::forService('Svc', 'Factory', 'reason'); + + self::assertSame( + sprintf(ContainerException::SERVICE, 'Svc', 'Factory', 'reason'), + $exception->getMessage(), + ); } } diff --git a/test/unit/Exception/ExceptionHierarchyTest.php b/test/unit/Exception/ExceptionHierarchyTest.php new file mode 100644 index 00000000..a6e1db08 --- /dev/null +++ b/test/unit/Exception/ExceptionHierarchyTest.php @@ -0,0 +1,161 @@ +> + */ + private const array SPL_PARENTS = [ + 'DomainException' => DomainException::class, + 'InvalidArgumentException' => InvalidArgumentException::class, + 'LengthException' => LengthException::class, + 'LogicException' => LogicException::class, + 'OutOfBoundsException' => OutOfBoundsException::class, + 'OutOfRangeException' => OutOfRangeException::class, + 'OverflowException' => OverflowException::class, + 'RangeException' => RangeException::class, + 'RuntimeException' => RuntimeException::class, + 'UnderflowException' => UnderflowException::class, + 'UnexpectedValueException' => UnexpectedValueException::class, + ]; + + /** + * PhpDb\Adapter\Exception\VunerablePlatformQuoteException is exempt from the naming + * guard: its factory is named for its arguments rather than the fault, and holds its + * message inline. PR 5 of the exception RFC removes the class, resolving both. + */ + private const string UNCONVERTED_CLASS = 'VunerablePlatformQuoteException'; + + /** @return array */ + public static function exceptionClassProvider(): array + { + $cases = []; + foreach (ExceptionClassLocator::concreteClasses() as $class) { + $cases[$class] = [$class]; + } + + ksort($cases); + + return $cases; + } + + /** @return array */ + public static function namedConstructorProvider(): array + { + $cases = []; + foreach (ExceptionClassLocator::concreteClasses() as $shortName => $class) { + if (self::UNCONVERTED_CLASS === $shortName) { + continue; + } + + foreach (ExceptionClassLocator::namedConstructors($class) as $method) { + $cases["{$class}::{$method}()"] = [ + $class, + $method, + ExceptionClassLocator::templateConstantFor($method), + ]; + } + } + + ksort($cases); + + return $cases; + } + + /** @return array */ + public static function splShapedExceptionProvider(): array + { + $cases = []; + foreach (ExceptionClassLocator::concreteClasses() as $shortName => $class) { + $splParent = self::SPL_PARENTS[$shortName] ?? null; + if (null === $splParent) { + continue; + } + + $cases[$class] = [$class, $splParent]; + } + + ksort($cases); + + return $cases; + } + + #[Test] + #[DataProvider('exceptionClassProvider')] + public function exceptionIsReachableThroughThePackageMarkerInterface(string $class): void + { + self::assertTrue( + is_subclass_of($class, ExceptionInterface::class), + "{$class} does not implement " . ExceptionInterface::class, + ); + } + + #[Test] + #[DataProvider('splShapedExceptionProvider')] + public function exceptionNamedAfterAnSplClassExtendsThatSplClass(string $class, string $splParent): void + { + self::assertTrue( + is_subclass_of($class, $splParent), + "{$class} is named after {$splParent} but does not extend it", + ); + } + + #[Test] + #[DataProvider('namedConstructorProvider')] + public function namedConstructorHasATemplateConstantMirroringItsName( + string $class, + string $method, + string $expectedConstant, + ): void { + self::assertArrayHasKey( + $expectedConstant, + (new ReflectionClass($class))->getConstants(), + "{$class}::{$method}() has no {$expectedConstant} template constant", + ); + } +} diff --git a/test/unit/Exception/TestAsset/ExceptionClassLocator.php b/test/unit/Exception/TestAsset/ExceptionClassLocator.php new file mode 100644 index 00000000..997ba802 --- /dev/null +++ b/test/unit/Exception/TestAsset/ExceptionClassLocator.php @@ -0,0 +1,119 @@ + + */ + public static function concreteClasses(): iterable + { + $srcDir = + dirname( + path: __DIR__, + levels: 4, + ) . '/src'; + $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($srcDir)); + + /** @var SplFileInfo $file */ + foreach ($iterator as $file) { + if (! $file->isFile() || 'php' !== $file->getExtension()) { + continue; + } + + if ('Exception' !== $file->getPathInfo()->getFilename()) { + continue; + } + + $class = self::classFor($file, $srcDir); + if (interface_exists($class) || ! class_exists($class)) { + continue; + } + + yield $file->getBasename('.php') => $class; + } + } + + /** + * The named constructors a class declares itself. + * + * @param class-string $class + * @return list + */ + public static function namedConstructors(string $class): array + { + $methods = []; + foreach ((new ReflectionClass($class))->getMethods(ReflectionMethod::IS_STATIC) as $method) { + $name = $method->getName(); + if ($method->getDeclaringClass()->getName() !== $class || ! str_starts_with($name, 'for')) { + continue; + } + + $methods[] = $name; + } + + return $methods; + } + + /** + * The template constant a named constructor's name implies. + */ + public static function templateConstantFor(string $method): string + { + $condition = substr( + string: $method, + offset: 3, + ); + + return strtoupper((string) preg_replace( + pattern: '/(?getPathname(), strlen($srcDir) + 1); + $className = substr( + string: $relative, + offset: 0, + length: -4, + ); + + return 'PhpDb\\' + . str_replace( + search: '/', + replace: '\\', + subject: $className, + ); + } +} From fc72ca4d121f9f744ab7b008327a38858f9f1538 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 28 Sep 2026 20:27:52 +1000 Subject: [PATCH 2/4] Replace exception class scanning with an explicit class to parent map - List every concrete exception in ExceptionHierarchyTest::EXCEPTION_CLASSES, mapped to the SPL class it must extend, in place of scanning src - Assert the SPL parent for every class rather than only those named after an SPL exception, and drop the name-based SPL_PARENTS map - Move the named constructor helpers into the test and delete the ExceptionClassLocator test asset Refs #189 Signed-off-by: Simon Mundy --- .../unit/Exception/ExceptionHierarchyTest.php | 150 +++++++++++------- .../TestAsset/ExceptionClassLocator.php | 119 -------------- 2 files changed, 91 insertions(+), 178 deletions(-) delete mode 100644 test/unit/Exception/TestAsset/ExceptionClassLocator.php diff --git a/test/unit/Exception/ExceptionHierarchyTest.php b/test/unit/Exception/ExceptionHierarchyTest.php index a6e1db08..25fbaced 100644 --- a/test/unit/Exception/ExceptionHierarchyTest.php +++ b/test/unit/Exception/ExceptionHierarchyTest.php @@ -4,79 +4,91 @@ namespace PhpDbTest\Exception; -use DomainException; +use Exception; use InvalidArgumentException; -use LengthException; -use LogicException; -use OutOfBoundsException; -use OutOfRangeException; -use OverflowException; -use PhpDb\Exception\ExceptionInterface; -use PhpDbTest\Exception\TestAsset\ExceptionClassLocator; +use PhpDb\Adapter\Exception as AdapterException; +use PhpDb\Exception as DbException; +use PhpDb\ResultSet\Exception as ResultSetException; +use PhpDb\RowGateway\Exception as RowGatewayException; +use PhpDb\Sql\Exception as SqlException; +use PhpDb\TableGateway\Exception as TableGatewayException; use PHPUnit\Framework\Attributes\CoversNothing; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -use RangeException; use ReflectionClass; +use ReflectionMethod; use RuntimeException; use Throwable; -use UnderflowException; use UnexpectedValueException; +use function array_keys; use function is_subclass_of; use function ksort; +use function preg_replace; +use function str_starts_with; +use function strtoupper; +use function substr; /** * Structural guards over the exception namespaces. * - * A class named after an SPL exception extends that SPL exception, every concrete - * exception is reachable through the package marker interface, and every named - * constructor has the template constant its name implies. + * Every concrete exception extends its expected SPL parent and is reachable through the + * package marker interface, and every named constructor has the template constant its + * name implies. */ #[Group('unit')] #[CoversNothing] final class ExceptionHierarchyTest extends TestCase { /** - * Short class name to the SPL class it claims by name. + * Every concrete exception class in the package, mapped to the SPL class it must extend. * - * ErrorException is absent deliberately. Both PhpDb\Exception\ErrorException and - * PhpDb\Adapter\Exception\ErrorException extend \Exception rather than - * \ErrorException, and \ErrorException's third constructor argument is $severity - * where \Exception's is $previous, so correcting the parent would change a released - * constructor contract. + * A new exception class comes under the guards only once it is listed here. * - * @var array> + * Both ErrorException classes map to \Exception rather than \ErrorException. + * \ErrorException's third constructor argument is $severity where \Exception's is + * $previous, so correcting the parent would change a released constructor contract. + * + * @var array, class-string> */ - private const array SPL_PARENTS = [ - 'DomainException' => DomainException::class, - 'InvalidArgumentException' => InvalidArgumentException::class, - 'LengthException' => LengthException::class, - 'LogicException' => LogicException::class, - 'OutOfBoundsException' => OutOfBoundsException::class, - 'OutOfRangeException' => OutOfRangeException::class, - 'OverflowException' => OverflowException::class, - 'RangeException' => RangeException::class, - 'RuntimeException' => RuntimeException::class, - 'UnderflowException' => UnderflowException::class, - 'UnexpectedValueException' => UnexpectedValueException::class, + private const array EXCEPTION_CLASSES = [ + AdapterException\ErrorException::class => Exception::class, + AdapterException\InvalidArgumentException::class => InvalidArgumentException::class, + AdapterException\InvalidConnectionParametersException::class => RuntimeException::class, + AdapterException\InvalidQueryException::class => UnexpectedValueException::class, + AdapterException\RuntimeException::class => RuntimeException::class, + AdapterException\UnexpectedValueException::class => UnexpectedValueException::class, + AdapterException\VunerablePlatformQuoteException::class => RuntimeException::class, + DbException\ContainerException::class => RuntimeException::class, + DbException\ErrorException::class => Exception::class, + DbException\InvalidArgumentException::class => InvalidArgumentException::class, + DbException\RuntimeException::class => RuntimeException::class, + DbException\UnexpectedValueException::class => UnexpectedValueException::class, + ResultSetException\InvalidArgumentException::class => InvalidArgumentException::class, + ResultSetException\RuntimeException::class => RuntimeException::class, + RowGatewayException\InvalidArgumentException::class => InvalidArgumentException::class, + RowGatewayException\RuntimeException::class => RuntimeException::class, + SqlException\InvalidArgumentException::class => InvalidArgumentException::class, + SqlException\RuntimeException::class => RuntimeException::class, + TableGatewayException\InvalidArgumentException::class => InvalidArgumentException::class, + TableGatewayException\RuntimeException::class => RuntimeException::class, ]; /** - * PhpDb\Adapter\Exception\VunerablePlatformQuoteException is exempt from the naming - * guard: its factory is named for its arguments rather than the fault, and holds its - * message inline. PR 5 of the exception RFC removes the class, resolving both. + * Exempt from the naming guard: its factory is named for its arguments rather than + * the fault, and holds its message inline. PR 5 of the exception RFC removes the + * class, resolving both. */ - private const string UNCONVERTED_CLASS = 'VunerablePlatformQuoteException'; + private const string UNCONVERTED_CLASS = AdapterException\VunerablePlatformQuoteException::class; - /** @return array */ + /** @return array */ public static function exceptionClassProvider(): array { $cases = []; - foreach (ExceptionClassLocator::concreteClasses() as $class) { - $cases[$class] = [$class]; + foreach (self::EXCEPTION_CLASSES as $class => $splParent) { + $cases[$class] = [$class, $splParent]; } ksort($cases); @@ -88,16 +100,16 @@ public static function exceptionClassProvider(): array public static function namedConstructorProvider(): array { $cases = []; - foreach (ExceptionClassLocator::concreteClasses() as $shortName => $class) { - if (self::UNCONVERTED_CLASS === $shortName) { + foreach (array_keys(self::EXCEPTION_CLASSES) as $class) { + if (self::UNCONVERTED_CLASS === $class) { continue; } - foreach (ExceptionClassLocator::namedConstructors($class) as $method) { + foreach (self::namedConstructors($class) as $method) { $cases["{$class}::{$method}()"] = [ $class, $method, - ExceptionClassLocator::templateConstantFor($method), + self::templateConstantFor($method), ]; } } @@ -107,41 +119,61 @@ public static function namedConstructorProvider(): array return $cases; } - /** @return array */ - public static function splShapedExceptionProvider(): array + /** + * The named constructors a class declares itself. + * + * @param class-string $class + * @return list + */ + private static function namedConstructors(string $class): array { - $cases = []; - foreach (ExceptionClassLocator::concreteClasses() as $shortName => $class) { - $splParent = self::SPL_PARENTS[$shortName] ?? null; - if (null === $splParent) { + $methods = []; + foreach ((new ReflectionClass($class))->getMethods(ReflectionMethod::IS_STATIC) as $method) { + $name = $method->getName(); + if ($method->getDeclaringClass()->getName() !== $class || ! str_starts_with($name, 'for')) { continue; } - $cases[$class] = [$class, $splParent]; + $methods[] = $name; } - ksort($cases); + return $methods; + } - return $cases; + /** + * The template constant a named constructor's name implies. + */ + private static function templateConstantFor(string $method): string + { + $condition = substr( + string: $method, + offset: 3, + ); + + return strtoupper((string) preg_replace( + pattern: '/(? - */ - public static function concreteClasses(): iterable - { - $srcDir = - dirname( - path: __DIR__, - levels: 4, - ) . '/src'; - $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($srcDir)); - - /** @var SplFileInfo $file */ - foreach ($iterator as $file) { - if (! $file->isFile() || 'php' !== $file->getExtension()) { - continue; - } - - if ('Exception' !== $file->getPathInfo()->getFilename()) { - continue; - } - - $class = self::classFor($file, $srcDir); - if (interface_exists($class) || ! class_exists($class)) { - continue; - } - - yield $file->getBasename('.php') => $class; - } - } - - /** - * The named constructors a class declares itself. - * - * @param class-string $class - * @return list - */ - public static function namedConstructors(string $class): array - { - $methods = []; - foreach ((new ReflectionClass($class))->getMethods(ReflectionMethod::IS_STATIC) as $method) { - $name = $method->getName(); - if ($method->getDeclaringClass()->getName() !== $class || ! str_starts_with($name, 'for')) { - continue; - } - - $methods[] = $name; - } - - return $methods; - } - - /** - * The template constant a named constructor's name implies. - */ - public static function templateConstantFor(string $method): string - { - $condition = substr( - string: $method, - offset: 3, - ); - - return strtoupper((string) preg_replace( - pattern: '/(?getPathname(), strlen($srcDir) + 1); - $className = substr( - string: $relative, - offset: 0, - length: -4, - ); - - return 'PhpDb\\' - . str_replace( - search: '/', - replace: '\\', - subject: $className, - ); - } -} From 01a9406b7b9ebea3b11a48354dd3d07f0773823c Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Tue, 29 Sep 2026 13:22:45 +1000 Subject: [PATCH 3/4] Make PhpDb\Exception\ExceptionInterface the only marker, extending Throwable Two changes to the marker interface, both surfaced while reviewing #189's convention against the code. The root interface did not extend Throwable, which was unintentional. It left the interface usable only in a catch clause: a parameter typed against it was not known to be throwable and had no getMessage(), so `mago analyze` reported non-existent-method and invalid-throw on any code that tried. It now extends Throwable, and ExceptionHierarchyTest pins that so it cannot regress. The interface carries a one-line docblock saying what it marks, and nothing about the change itself. Each component also declared its own ExceptionInterface extending the root one. Those are removed: Adapter, ResultSet, RowGateway, Sql and TableGateway. Every concrete exception class extends its counterpart in PhpDb\Exception, which is where the marker is declared, so all 20 still implement it by inheritance and the 15 redundant `implements` clauses are gone. docs/book/exceptions.md records the single-interface rule and shows how to catch a whole component by naming its classes. BC: the five component interfaces ship in 0.5.0, so `catch (PhpDb\Sql\Exception\ExceptionInterface)` and the equivalents stop resolving. Nothing in this repository or the adapter packages catches one; phpdb-mysql names them in @throws docblocks only, which its own RFC covers. Signed-off-by: Simon Mundy --- docs/book/exceptions.md | 22 +++++++++++++++---- src/Adapter/Exception/ErrorException.php | 2 +- src/Adapter/Exception/ExceptionInterface.php | 9 -------- .../Exception/InvalidArgumentException.php | 2 +- .../InvalidConnectionParametersException.php | 2 +- .../Exception/InvalidQueryException.php | 2 +- src/Adapter/Exception/RuntimeException.php | 2 +- .../Exception/UnexpectedValueException.php | 2 +- .../VunerablePlatformQuoteException.php | 2 +- src/Exception/ExceptionInterface.php | 5 ++++- .../Exception/ExceptionInterface.php | 9 -------- .../Exception/InvalidArgumentException.php | 2 +- src/ResultSet/Exception/RuntimeException.php | 2 +- .../Exception/ExceptionInterface.php | 9 -------- .../Exception/InvalidArgumentException.php | 2 +- src/RowGateway/Exception/RuntimeException.php | 2 +- src/Sql/Exception/ExceptionInterface.php | 9 -------- .../Exception/InvalidArgumentException.php | 2 +- src/Sql/Exception/RuntimeException.php | 2 +- .../Exception/ExceptionInterface.php | 9 -------- .../Exception/InvalidArgumentException.php | 2 +- .../Exception/RuntimeException.php | 2 +- .../unit/Exception/ExceptionHierarchyTest.php | 9 ++++++++ 23 files changed, 46 insertions(+), 65 deletions(-) delete mode 100644 src/Adapter/Exception/ExceptionInterface.php delete mode 100644 src/ResultSet/Exception/ExceptionInterface.php delete mode 100644 src/RowGateway/Exception/ExceptionInterface.php delete mode 100644 src/Sql/Exception/ExceptionInterface.php delete mode 100644 src/TableGateway/Exception/ExceptionInterface.php diff --git a/docs/book/exceptions.md b/docs/book/exceptions.md index b9423bda..7f6bae3b 100644 --- a/docs/book/exceptions.md +++ b/docs/book/exceptions.md @@ -66,8 +66,22 @@ Everything else is a named constructor on the component's SPL-shaped class. ## Component namespaces -Each component owns an `Exception` namespace holding an `ExceptionInterface` that extends -`PhpDb\Exception\ExceptionInterface`, plus the SPL-shaped classes it needs. A class named -after an SPL exception extends that SPL exception, so `catch (\RuntimeException)` and +There is exactly one marker interface, `PhpDb\Exception\ExceptionInterface`, and every +exception this package throws implements it. It extends `Throwable`, so it can be used as a +type and not only in a `catch` clause. + +Components do **not** declare their own `ExceptionInterface`. Each owns an `Exception` +namespace holding the SPL-shaped classes it needs, and those extend their counterparts in +`PhpDb\Exception`, which is where the marker is declared — so a component class inherits it +rather than restating it. A sub-namespace such as `PhpDb\Sql\Predicate\Exception` does the +same through its parent component. + +To catch every failure from one component, name its classes: + +```php +catch (PhpDb\Sql\Exception\InvalidArgumentException | PhpDb\Sql\Exception\RuntimeException $e) +``` + +A class named after an SPL exception extends that SPL exception, so `catch (\RuntimeException)` and `catch (PhpDb\Exception\ExceptionInterface)` both behave as a reader expects. -`PhpDbTest\Exception\ExceptionHierarchyTest` enforces both rules across `src/`. +`PhpDbTest\Exception\ExceptionHierarchyTest` enforces both rules across `src/`, for every class listed in its `EXCEPTION_CLASSES` map. diff --git a/src/Adapter/Exception/ErrorException.php b/src/Adapter/Exception/ErrorException.php index d2455bcf..0eef9de1 100644 --- a/src/Adapter/Exception/ErrorException.php +++ b/src/Adapter/Exception/ErrorException.php @@ -6,4 +6,4 @@ use PhpDb\Exception; -class ErrorException extends Exception\ErrorException implements ExceptionInterface {} +class ErrorException extends Exception\ErrorException {} diff --git a/src/Adapter/Exception/ExceptionInterface.php b/src/Adapter/Exception/ExceptionInterface.php deleted file mode 100644 index 43ba2552..00000000 --- a/src/Adapter/Exception/ExceptionInterface.php +++ /dev/null @@ -1,9 +0,0 @@ -isSubclassOf(Throwable::class), + DbException\ExceptionInterface::class . ' must extend Throwable, or it cannot be used as a type', + ); + } } From 2886bc78e5560dc771fbdeeceeac9910954b07c8 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Tue, 29 Sep 2026 13:31:51 +1000 Subject: [PATCH 4/4] Drop a redundant constructor argument and a PR reference from source - ContainerException::forService() passed 0 explicitly as $code, which is the RuntimeException default. Infection reported two escaped mutants on it, IncrementInteger and DecrementInteger, because nothing asserts the code. Removing the argument removes the literal rather than adding an assertion for a value PHP already supplies; getCode() is unchanged and Covered Code MSI for the file goes from 33% to 100% - ExceptionHierarchyTest's naming-guard exemption named the pull request that would remove the class. The reason for the exemption stays; the reference to work that will be merged and forgotten does not Signed-off-by: Simon Mundy --- src/Exception/ContainerException.php | 1 - test/unit/Exception/ExceptionHierarchyTest.php | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Exception/ContainerException.php b/src/Exception/ContainerException.php index bac0c6dc..20d159ff 100644 --- a/src/Exception/ContainerException.php +++ b/src/Exception/ContainerException.php @@ -24,7 +24,6 @@ public static function forService( $factoryClass, $reason, ), - 0, ); } } diff --git a/test/unit/Exception/ExceptionHierarchyTest.php b/test/unit/Exception/ExceptionHierarchyTest.php index 1eede54b..6593f630 100644 --- a/test/unit/Exception/ExceptionHierarchyTest.php +++ b/test/unit/Exception/ExceptionHierarchyTest.php @@ -77,9 +77,8 @@ final class ExceptionHierarchyTest extends TestCase ]; /** - * Exempt from the naming guard: its factory is named for its arguments rather than - * the fault, and holds its message inline. PR 5 of the exception RFC removes the - * class, resolving both. + * Exempt from the naming guard: its factory is named for its arguments rather than the + * fault, and holds its message inline. */ private const string UNCONVERTED_CLASS = AdapterException\VunerablePlatformQuoteException::class;