feat: add SchemaValidator and integrate it into XliffParser#4
feat: add SchemaValidator and integrate it into XliffParser#4konradmichalik merged 2 commits intomainfrom
Conversation
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConsoleApp
participant SchemaValidator
participant XliffParser
participant FileDetector
User->>ConsoleApp: Run validation command
ConsoleApp->>SchemaValidator: validate(fileDetector, parserClass, files)
loop For each file
SchemaValidator->>XliffParser: Instantiate parser (dynamic)
SchemaValidator->>FileDetector: Detect and load file
SchemaValidator->>SchemaValidator: Validate XML schema
alt Errors found
SchemaValidator->>ConsoleApp: Output warning and error table
else No errors
SchemaValidator->>ConsoleApp: Output success checkmark
end
end
SchemaValidator-->>ConsoleApp: Return validation result
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/Parser/XliffParser.php (1)
66-71: Fix the return type annotation to match actual return types.The method now returns validators of different types, but the return type annotation is too restrictive.
Apply this diff to fix the type annotation:
- /** - * @return array<int, class-string<MismatchValidator>> - */ + /** + * @return array<int, class-string<ValidatorInterface>> + */You'll also need to add the import for
ValidatorInterface:+use MoveElevator\ComposerTranslationValidator\Validator\ValidatorInterface;
🧹 Nitpick comments (2)
src/Validator/SchemaValidator.php (2)
29-34: Consider clarifying the @throws annotation.The
@throws \ReflectionExceptionannotation is present, but it's not immediately clear where this exception could be thrown from this method directly.If the exception comes from
ParserUtility::resolveParserClass()or the dynamic class instantiation on line 45, consider making this more explicit in the documentation.
77-81: Consider making the error message cleanup more robust.The regular expression for cleaning XML element prefixes from error messages could be fragile with different XML namespaces or error message formats.
Consider adding fallback handling or making the regex more flexible:
$message = preg_replace( "/^Element ('(?:\{[^}]+\})?[^']+'):?\s*/", '', $error['message'] - ); + ) ?: $error['message']; // Fallback to original message if regex fails
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(2 hunks)composer.json(1 hunks)src/Parser/XliffParser.php(2 hunks)src/Validator/SchemaValidator.php(1 hunks)tests/Build/app/composer.json(1 hunks)
🧰 Additional context used
🪛 PHPStan (2.1.15)
src/Parser/XliffParser.php
70-70: Method MoveElevator\ComposerTranslationValidator\Parser\XliffParser::getValidators() should return array<int, class-string<MoveElevator\ComposerTranslationValidator\Validator\MismatchValidator>> but returns array<int, string>.
(return.type)
🔇 Additional comments (9)
tests/Build/app/composer.json (1)
5-7: LGTM! Development dependency addition is well-justified.The addition of
composer/composeras a development dependency aligns with the new testing instructions in the README and uses an appropriate version constraint.composer.json (2)
14-22: Dependency updates properly support the new SchemaValidator functionality.The changes are well-justified:
- PHP 8.1 requirement is appropriate for modern development
ext-libxmlis needed for XML schema validation- New Symfony packages (
config,translation) support the SchemaValidator implementation- Version constraints are consistent across Symfony dependencies
30-30: Good cleanup of duplicate dependency.Removing
symfony/translationfrom require-dev makes sense since it's now in the main requirements.README.md (2)
12-14: Documentation improvements enhance clarity and completeness.Good formatting fixes by removing
.phpextensions and properly documenting the newSchemaValidator.
31-38: Testing section provides valuable user guidance.The new testing instructions complement the development dependency changes and give users a clear way to verify functionality.
src/Parser/XliffParser.php (1)
9-9: Import addition is correct.The new import for
SchemaValidatoris properly placed and follows the existing import pattern.src/Validator/SchemaValidator.php (3)
18-27: Well-structured validator class with proper dependency injection.The constructor properly initializes dependencies and the SymfonyStyle instance for consistent console output.
69-90: Excellent error reporting implementation.The table-based error display with proper styling and column formatting provides clear, actionable feedback to users.
96-111: Good use of debug output with appropriate verbosity controls.The debug output implementation properly respects verbosity levels and provides useful feedback during validation.
Summary by CodeRabbit
New Features
Documentation
Chores