[RFC]: Named exception constructors for the PostgreSQL adapter #15

Proposed Version

0.1.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 16 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 9, Exception\InvalidQueryException 2, Exception\InvalidArgumentException 2, InvalidConnectionParametersException 2, Exception\InvalidConnectionParametersException 1.

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

Two spellings of one condition inside this package. Container/ConnectionInterfaceFactory.php:32 says Connection configuration must be an array of parameters passed via $options["connection"]; Container/PdoConnectionInterfaceFactory.php:44 says Connection configuration must be an array of parameters — the same guard, one message truncated.

Driver error text passed through verbatim at Connection.php:170 (pg_last_error($this->resource)) and Statement.php:88 (pg_last_error()). Core added a DRIVER_ERROR template of '%s' for this shape.

Two ways of naming the same method in adjacent guards. Connection.php:107 prefixes self::class . '::connect: '; :120 uses sprintf('%s: …', __METHOD__). :128 also interpolates $this->connectionParameters['charset'].

One message concatenated inside a match arm at Connection.php:293: 'Connection parameter "' . $name . '" is not valid for Pgsql adapter'.

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. Pgsql\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/Pgsql/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.
• core#194 removed VunerablePlatformQuoteException. test/integration/AdapterPlatformTest.php names it at lines 7, 24 and 35. Those move to PhpDb\Adapter\Exception\RuntimeException, which the class already extended, so a catch of that type is unaffected. This must follow core 0.6.0 shipping, and is worth its own small PR ahead of the rest.

Proposal(s)

• PR 1: update the VunerablePlatformQuoteException references in test/integration/AdapterPlatformTest.php once core 0.6.0 ships. Unblocks CI; independent of everything below.
• PR 2: add PhpDb\Pgsql\Exception. RuntimeException and InvalidArgumentException extending core's, plus the mago.toml exclude. No throw sites change.
• PR 3: Connection.php, 9 sites. Includes the two pg_last_error() passthroughs reusing core's DRIVER_ERROR, settling one way of naming the method in the connect guards, and the match-arm concatenation.
• PR 4: Pdo/Connection.php, Driver.php, Result.php, Statement.php, 5 sites. Reuse core's templates for the query-result and extension messages, converging required for this Driver on core's adapter wording.
• PR 5: Container/*, 2 sites. Collapse the two connection-configuration spellings onto one template, per whatever phpdb-sqlite#41 settles.

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 MySQL adapter phpdb-mysql#82 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.1.x at the time of filing.