[RFC]: Named exception constructors for the MySQL adapter #82

Proposed Version

0.5.0

Basic Information

core#189 established a convention: no exception is constructed with a message assembled at the throw site. Every throw goes through a static named constructor on a component-owned exception class, with the message template beside it as a final public const string. That work is merged on core 0.6.x across six pull requests, leaving one throw new in the whole package. This proposes the same for this adapter.

This package has 27 throw sites and no Exception namespace. Every one reaches into core's PhpDb\Adapter\Exception\* classes and supplies its message inline:

Thrown types: Exception\RuntimeException 18, Exception\InvalidQueryException 2, InvalidConnectionParametersException 2, Exception\InvalidConnectionParametersException 2, InvalidArgumentException 2, Exception\InvalidArgumentException 1. The unqualified and Exception\-prefixed spellings of one class are used interchangeably.

The convention, as merged in core#190 to core#195 and documented in docs/book/exceptions.md:

• Named constructors are static, return self and start with for.
• Each has a single final public const string template beside it, named for the method: forMissingDsn() uses MISSING_DSN.
• new self(self::CONST) when the template has no placeholder, new self(sprintf(self::CONST, ...)) when it does.
• Quote a %s holding a caller-supplied identifier; leave a class name, method name or count unquoted.
• There is one marker interface, PhpDb\Exception\ExceptionInterface, and it extends Throwable. Components do not declare their own. A class inherits the marker from the core class it extends, so no implements clause is needed or wanted.
• A condition earns its own exception class only when it carries something a named constructor cannot: extra state with accessors, an external interface contract, or a condition a consumer can act on. Everything else is a named constructor on the component's SPL-shaped class.

Background

Duplicated inside this package. Connection configuration must be an array of parameters passed via $options["connection"] appears twice, in Container/ConnectionInterfaceFactory.php:30 and Container/PdoConnectionInterfaceFactory.php:30.

Driver error text passed through verbatim at three sites: Connection.php:261 and Statement.php:79 throw $this->resource->error as the whole message, and Result.php:346 throws $statement->error. Core added a DRIVER_ERROR template of '%s' for this shape.

Messages composed at the throw site at Pdo/Connection.php:137 ("Connect Error: {$e->getMessage()}"), Statement.php:150 ("Statement couldn't be produced with sql: {$sql}") and Pdo/Connection.php:198 (sprintf('The "%s" connection parameter contains invalid characters', $name)).

The two Sql/Ddl/ColumnOptionTrait.php messages are the longest in the package and the only ones naming a pattern constant in their text, so they are the clearest candidates for templates.

The same conditions are worded differently in every package

These are not adapter-specific messages. They are one condition reported four slightly different ways, which is what core#189 found inside core and what no adapter can see on its own.

Nine conditions, roughly twenty-five throw sites, four packages.

Where a shared template should live

core#189 made every template a final public const string, which is what lets an adapter stop hard-coding literals. Three cases follow from that:

• Core already names the condition — reuse core's constant rather than redeclaring the string. The rollback, extension, prepared, query-result and driver-error templates all exist on PhpDb\Adapter\Exception\RuntimeException.
• Shared between adapters, absent from core — the connection-configuration and connect-failure messages. These want one home. The proposal is to add them to core as adapter-facing templates; the alternative is three copies that drift again.
• Genuinely adapter-specific — stays in the adapter. An adapter-specific message does not belong in a core constant, which is the line core drew when it left InvalidConnectionParametersException with a plain constructor.

Where wording has drifted, core's is canonical, because every adapter already depends on it.

Considerations

• This depends on core 0.6.0. The convention, the guards and the public constants are merged on 0.6.x but unreleased; the current tag is 0.5.0. Nothing here can be built until it ships.
• No adapter-owned ExceptionInterface. Core removed the five component interfaces in core#190. Mysql\Exception\RuntimeException extending PhpDb\Adapter\Exception\RuntimeException implements the single marker by inheritance, so the class body carries no implements.
• too-many-methods has a threshold of 10, verified against mago 1.50.0, so a message catalogue trips it by construction. Core excludes the paths in its own mago.toml rather than baselining:
  too-many-methods = {{ exclude = ["test/", "src/Mysql/Exception/"] }}
• #[CoversMethod] restricts coverage attribution. Once a test class declares any, PHPUnit records coverage only for the listed methods, so a PR touching an undeclared method fails codecov/patch with passing tests. This failed core#194 on a method four existing tests already exercised.
• Call factories in the test body, not the data provider. A provider runs outside the coverage-collected region, so factories called there record no coverage at all while every assertion still passes.
• Verify each template against the original literal rather than by eye. Core did this with a throwaway script before touching any call site, and it caught wording drift that reading would not.
• No VunerablePlatformQuoteException fallout here. Core#194 removed that class, and this package has no references to it, unlike the two sibling adapters.

Proposal(s)

• PR 1: add PhpDb\Mysql\Exception. RuntimeException and InvalidArgumentException extending core's, plus the mago.toml exclude. No throw sites change.
• PR 2: Result.php and Statement.php, 12 sites. Includes the three driver-error passthroughs reusing core's DRIVER_ERROR, and converging This statement has already been prepared on core's wording.
• PR 3: Connection.php and Pdo/Connection.php, 10 sites. Includes converging the two rollback messages on core's, dropping the full stop this package added, and the Connect Error composition.
• PR 4: Driver.php and Container/*, 3 sites. Collapse the two duplicated connection-configuration messages onto one template, per whatever phpdb-sqlite#41 settles.
• PR 5: Sql/Ddl/ColumnOptionTrait.php, 2 sites.

Test plan, per PR: every named constructor gets a test asserting the returned type and the rendered message, driven by a #[DataProvider] per exception class; existing message assertions move from literals onto the templates; each PR runs mago format --check, mago lint and mago analyze, and regenerates the baselines it moves.

Appendix/Additional Info

• core#189 is the originating RFC. core#190 carries the convention doc, the single marker interface and the structural guards. core#194 covers the Adapter component this package builds on.
• Sibling RFCs: [RFC]: Named exception constructors for the PostgreSQL adapter phpdb-pgsql#15 and [RFC]: Named exception constructors, and where shared message templates should live phpdb-sqlite#41. All three follow the same structure and defer the shared-template question to phpdb-sqlite.
• Counts and line references are from 0.5.x at the time of filing.