[RFC]: Require a PlatformInterface in the DDL process* methods #212

Proposed Version

Next minor

Basic Information

The process* methods on CreateTable, AlterTable and DropTable take ?PlatformInterface $adapterPlatform = null, but use it unconditionally: they call quoteIdentifier(), quoteTrustedValue(), processExpression() and resolveTable(), none of which accept null. Their only caller, AbstractSql::buildSqlString(), always passes a non-null platform.

This accounts for 21 analyzer findings in src/Sql/Ddl:

• possibly-null-argument (8) and possible-method-access-on-null (6).
• less-specific-nested-return-statement (4) and invalid-return-statement (1): calling a method on a possibly-null platform yields mixed, so the arrays these methods return lose their element types.
• unused-parameter (2): CreateTable::processCombinedby() and processStatementEnd() receive the platform but don't use it.

Background

AbstractSql::getSqlString() supplies a default platform before calling buildSqlString(), so no path reaches these methods with null.

Considerations

• These are protected methods. Making the parameter required narrows it for callers, but subclasses that override with ?PlatformInterface $adapterPlatform = null remain compatible, since a child may widen a parameter.
• A subclass that calls parent::processX() without arguments, or with null, would get an ArgumentCountError or TypeError. None of the driver packages do this.

Proposal(s)

Change the signature of each DDL process* method to PlatformInterface $adapterPlatform. Keep the parameter on processCombinedby() and processStatementEnd(), since buildSqlString() passes it positionally to every process* method.

Appendix/Additional Info

Finding counts are from mago analyze --ignore-baseline src/Sql once the docblock typing in #204 is applied.