Refactor ColumnDefinitionParser

[vjik]

Q	A
Is bugfix?	❌
New feature?	❌
Breaks BC?	✔️

Related PRs:

• Add ColumnDefinitionParser db-sqlite#405
• Refactor ColumnDefinitionParser db-mysql#447
• Add ColumnDefinitionParser class db-mssql#416
• Refactor ColumnDefinitionParser db-pgsql#466
• Refactor ColumnDefinitionParser db-oracle#381

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.62%. Comparing base (853feda) to head (3d8f279).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1108   +/-   ##
=========================================
  Coverage     98.62%   98.62%           
+ Complexity     1648     1647    -1     
=========================================
  Files           119      119           
  Lines          4280     4280           
=========================================
  Hits           4221     4221           
  Misses           59       59           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR refactors the ColumnDefinitionParser class into an abstract base class (AbstractColumnDefinitionParser) with a new interface (ColumnDefinitionParserInterface). The refactoring extracts database-specific type parameter parsing (such as enum value extraction and size/scale parsing) into an abstract method parseTypeParams(), which must be implemented by database-specific parsers in related packages (MySQL, PostgreSQL, MSSQL, Oracle, SQLite). This improves separation of concerns and allows each database driver to handle type-specific parsing according to its own syntax requirements.

Key changes:

• Introduced ColumnDefinitionParserInterface defining the contract for column definition parsers
• Renamed ColumnDefinitionParser to AbstractColumnDefinitionParser and made it abstract with parseTypeParams() method
• Removed enum-specific parsing from the base class (moved to database-specific implementations)
• Updated test infrastructure with stub implementations and adjusted test cases

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Syntax/ColumnDefinitionParserInterface.php	New interface defining the column definition parser contract
src/Syntax/AbstractColumnDefinitionParser.php	Refactored from concrete class to abstract class, extracted type parameter parsing to abstract method
src/Schema/Column/AbstractColumnFactory.php	Changed columnDefinitionParser() from concrete to abstract method returning interface
tests/Support/Stub/StubColumnDefinitionParser.php	New stub implementation for testing purposes
tests/Support/Stub/StubColumnFactory.php	Added implementation of abstract columnDefinitionParser() method
tests/Common/CommonColumnDefinitionParserTest.php	Updated to use interface and made createColumnDefinitionParser() abstract
tests/Provider/ColumnDefinitionParserProvider.php	Removed enum test cases (moved to database-specific tests)
tests/Db/Schema/Column/ColumnFactoryTest.php	Updated data provider usage and removed size assertion for stub tests
tests/Common/CommonColumnTest.php	Code formatting improvement (multi-line array_map)
UPGRADE.md	Documented breaking changes and API modifications
CHANGELOG.md	Added entry for the refactoring
.github/workflows/db-mariadb.yml	Added YII_MYSQL_TYPE environment variable for MariaDB distinction

Comments suppressed due to low confidence (1)

src/Syntax/AbstractColumnDefinitionParser.php:92

• The return type annotation at line 82 declares the fourth element as string|null, but substr() at line 92 always returns a string (never null). This creates an inconsistency in the type declaration.

Consider updating the return type to:

@psalm-return list{string, string|null, string|null, string}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The StubColumnDefinitionParser returns an empty array from parseTypeParams(), which means it doesn't parse type parameters like size and scale. However, test data in ColumnFactoryTest.php includes definitions with parameters (e.g., 'char(1)', 'decimal(10,2)' at lines 90-91) that won't be parsed.

Consider implementing basic parameter parsing in the stub:

protected function parseTypeParams(string $type, string $params): array
{
    return $this->parseSizeInfo($params);
}

This would make the stub more realistic and allow proper testing of size/scale parsing.

[Tigrov]

Why not to use return $this->parseSizeInfo($params); as AI suggests?

[vjik]

It depends by type. Different types requires different parsing. This is DBMS-specific thing.