Uh oh!
There was an error while loading. Please reload this page.
Refactor ColumnDefinitionParser - #466
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## master #466 +/- ##
============================================
+ Coverage 96.64% 96.71% +0.06% + Complexity 409 407 -2
============================================
Files 66 66 Lines 1192 1216 +24 ============================================
+ Hits 1152 1176 +24
Misses 40 40 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the ColumnDefinitionParser class to extend from AbstractColumnDefinitionParser instead of directly implementing the parsing logic. The refactoring delegates core parsing logic to the parent class while maintaining PostgreSQL-specific type handling through the new parseTypeParams() method.
- Simplified
parseDefinition()method to return raw parsed components instead of fully processed info - Introduced
parseTypeParams()using match expressions to handle PostgreSQL-specific type parameter parsing - Removed unnecessary imports and psalm-suppress annotation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Column/ColumnDefinitionParser.php | Refactored to extend AbstractColumnDefinitionParser, simplified parsing methods, and added PostgreSQL-specific type parameter handling via match expression |
| tests/ArrayParserTest.php | Removed unnecessary @psalm-suppress PropertyNotSetInConstructor annotation |
| CHANGELOG.md | Added entry documenting the breaking change refactor |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $typeDetails = $matches[4] ?? $matches[2] ?? ''; | ||
| if ($typeDetails !== '') { | ||
| if ($type === 'enum') { |
There was a problem hiding this comment.
'enum' here and in other drivers should be an abstract type and must be parsable.
There was a problem hiding this comment.
Enum couldn't use in column definition directly.
| 'varbit', | ||
| 'varchar' => $this->parseSizeInfo($params), | ||
| default => [], | ||
| }; |
There was a problem hiding this comment.
Abstract, pseudo, or user-defined types must also be parsable.
There was a problem hiding this comment.
Why need to parse abstract and pseudo type? What case when this is needed?
Isn't custom types are support parameters?
Related to yiisoft/db#1108