[RFC]: Named exception constructors, and where shared message templates should live #41

Proposed Version

0.3.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 is the smallest case by far — 3 throw sites and no Exception namespace — and it is mostly a reuse exercise rather than a refactor, because all three of its messages are already said elsewhere:

So the question this RFC mainly asks is not how to restructure this package, but where the shared templates should live so three adapters stop each keeping their own copy. The sibling RFCs both defer that decision here.

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

All three sites are covered by the table above; there is no package-local duplication to report beyond it. Thrown types: InvalidConnectionParametersException 1, Exception\InvalidConnectionParametersException 1, Exception\RuntimeException 1.

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. Sqlite\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/Sqlite/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/unit/AdapterPlatformTest.php names it at lines 9, 155 and 164. 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.
• Three sites may not justify a namespace. With every message shared, it is worth deciding whether this package needs PhpDb\Sqlite\Exception at all or should simply consume shared templates. Adding a namespace holding nothing adapter-specific would be structure for its own sake. Decide here, then apply consistently to the siblings.
• too-many-methods would not be reached here, so unlike the siblings no mago.toml change is expected.

Proposal(s)

• PR 1: update the VunerablePlatformQuoteException references in test/unit/AdapterPlatformTest.php once core 0.6.0 ships. Unblocks CI; independent of everything below.
• PR 2: settle where the three shared templates live, in discussion here rather than in code. The options are core as adapter-facing templates, one template per adapter, or a shared namespace. Both sibling RFCs defer to this.
• PR 3: convert the 3 throw sites to whatever PR 2 decides, converging A dsn was not provided and the Connect Error concatenation on the agreed wording.

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 for the PostgreSQL adapter phpdb-pgsql#15. All three follow the same structure and defer the shared-template question to phpdb-sqlite.
• Counts and line references are from 0.3.x at the time of filing.