feat: add PlaceholderConsistencyValidator for validating placeholder consistency across translation files#30
Conversation
…consistency across translation files
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ValidatorRegistry
participant PlaceholderConsistencyValidator
participant FileParser
User->>CLI: Run translation validation command
CLI->>ValidatorRegistry: Get available validators
ValidatorRegistry-->>CLI: Return validators (incl. PlaceholderConsistencyValidator)
CLI->>PlaceholderConsistencyValidator: Validate translation files
PlaceholderConsistencyValidator->>FileParser: Parse translation files
FileParser-->>PlaceholderConsistencyValidator: Return parsed keys/values
PlaceholderConsistencyValidator->>PlaceholderConsistencyValidator: Extract placeholders
PlaceholderConsistencyValidator->>PlaceholderConsistencyValidator: Compare placeholders across files
PlaceholderConsistencyValidator-->>CLI: Report issues (warnings) if inconsistencies found
CLI-->>User: Output validation results
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ 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
🧹 Nitpick comments (2)
src/Validator/PlaceholderConsistencyValidator.php (2)
129-165: Consider improving the consistency checking approach.The current implementation uses the first file as reference, which might not be optimal if the first file contains errors. Consider using a more sophisticated approach like finding the most common placeholder set or allowing explicit reference file specification.
private function findPlaceholderInconsistencies(array $fileData): array { if (count($fileData) < 2) { return []; } $inconsistencies = []; $allPlaceholders = []; - // Collect all placeholders from all files for this key + // Collect all placeholders from all files for this key foreach ($fileData as $fileName => $data) { $allPlaceholders[$fileName] = $data['placeholders']; } - // Compare placeholders between files + // Find the most common placeholder set as reference $fileNames = array_keys($allPlaceholders); - $referenceFile = $fileNames[0]; + $placeholderCounts = []; + foreach ($allPlaceholders as $placeholders) { + $key = serialize(sort($placeholders)); + $placeholderCounts[$key] = ($placeholderCounts[$key] ?? 0) + 1; + } + $mostCommonKey = array_key_first($placeholderCounts); + $referenceFile = $fileNames[0]; // Fallback to first file + foreach ($allPlaceholders as $fileName => $placeholders) { + if (serialize(sort($placeholders)) === $mostCommonKey) { + $referenceFile = $fileName; + break; + } + } $referencePlaceholders = $allPlaceholders[$referenceFile];
270-279: Consider potential issues with overlapping placeholder replacements.The current string replacement approach might cause issues if one placeholder is a substring of another. Consider using a more robust replacement strategy.
private function highlightPlaceholders(string $value): string { $placeholders = $this->extractPlaceholders($value); + + // Sort by length descending to avoid substring replacement issues + usort($placeholders, fn($a, $b) => strlen($b) - strlen($a)); foreach ($placeholders as $placeholder) { $value = str_replace($placeholder, "<fg=yellow>{$placeholder}</>", $value); } return $value; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.md(1 hunks)src/Command/ValidateTranslationCommand.php(1 hunks)src/Result/ValidationResultCliRenderer.php(2 hunks)src/Validator/PlaceholderConsistencyValidator.php(1 hunks)src/Validator/ValidatorRegistry.php(1 hunks)tests/src/Fixtures/translations/xliff/fail/locallang.xlf(1 hunks)tests/src/Validator/PlaceholderConsistencyValidatorTest.php(1 hunks)tests/src/Validator/ValidatorRegistryTest.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Validator/ValidatorRegistry.php (1)
src/Validator/PlaceholderConsistencyValidator.php (1)
PlaceholderConsistencyValidator(16-304)
🪛 PHPMD (2.15.0)
src/Validator/PlaceholderConsistencyValidator.php
191-191: Avoid unused local variables such as '$fileInfo'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (14)
tests/src/Fixtures/translations/xliff/fail/locallang.xlf (1)
13-13: LGTM! Appropriate test fixture for placeholder validation.The addition of the
%splaceholder to the test fixture creates a realistic test case for the PlaceholderConsistencyValidator to detect placeholder inconsistencies across translation files.src/Validator/ValidatorRegistry.php (1)
18-18: LGTM! Validator correctly registered.The
PlaceholderConsistencyValidatoris properly added to the available validators registry, maintaining consistency with existing validator registration patterns.README.md (1)
84-84: LGTM! Documentation accurately describes the new validator.The documentation entry correctly describes the PlaceholderConsistencyValidator with appropriate function description, supported formats (XLIFF, YAML), and warning level classification.
src/Command/ValidateTranslationCommand.php (1)
107-108: LGTM! Help text accurately describes the new validator.The help text entry for PlaceholderConsistencyValidator is clear, concise, and follows the established format. The description accurately reflects the validator's functionality and maintains consistency with the ValidatorRegistry ordering.
tests/src/Validator/ValidatorRegistryTest.php (3)
9-9: LGTM! Import statement correctly added.The import for PlaceholderConsistencyValidator is properly added to support the test assertions.
22-22: LGTM! Test assertion correctly added.The assertion for PlaceholderConsistencyValidator follows the established pattern and ensures the new validator is properly registered.
24-24: LGTM! Expected count correctly updated.The expected count is properly updated from 4 to 5 to reflect the addition of the PlaceholderConsistencyValidator.
src/Result/ValidationResultCliRenderer.php (2)
45-55: LGTM! Good UX enhancement.The header method provides clear context about the tool's purpose and capabilities. The formatting is consistent with Symfony console standards and the usage hint is helpful for users.
120-120: Good integration of the header display.The placement of the header call at the beginning of verbose output is logical and ensures users get context before seeing detailed validation results.
tests/src/Validator/PlaceholderConsistencyValidatorTest.php (1)
1-441: Excellent comprehensive test coverage.The test suite thoroughly covers the validator's functionality including:
- All supported placeholder syntaxes (Symfony, ICU, Twig, printf, Laravel)
- Edge cases with mixed placeholder styles
- Consistency validation across multiple files
- Output formatting and rendering
- Internal state management
- Complex scenarios with multiple keys and files
The use of reflection for testing private methods is appropriate and the test structure is well-organized.
src/Validator/PlaceholderConsistencyValidator.php (4)
21-47: LGTM! Clean file processing implementation.The file processing logic correctly extracts keys and placeholders, with appropriate error handling for invalid files.
82-122: Comprehensive placeholder extraction with good regex patterns.The method supports multiple placeholder syntaxes with well-crafted regex patterns. The array_unique() call prevents duplicates, which is good practice.
284-287: LGTM! Correct parser support declaration.The validator correctly declares support for both XLIFF and YAML parsers as intended.
295-303: LGTM! Appropriate configuration for warning-level validation.The validator correctly returns WARNING result type and enables detailed output, which is appropriate for placeholder consistency checks.
| $details = $issue->getDetails(); | ||
| $files = $details['files'] ?? []; | ||
|
|
||
| foreach ($files as $fileName => $fileInfo) { |
There was a problem hiding this comment.
Fix unused variable.
The variable $fileInfo is declared but never used in the loop body.
- foreach ($files as $fileName => $fileInfo) {
+ foreach ($files as $fileName => $data) {
if (!empty($fileName)) {Or if you don't need the array values:
- foreach ($files as $fileName => $fileInfo) {
+ foreach (array_keys($files) as $fileName) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| foreach ($files as $fileName => $fileInfo) { | |
| foreach ($files as $fileName => $data) { | |
| if (!empty($fileName)) { | |
| // …rest of loop body… |
| foreach ($files as $fileName => $fileInfo) { | |
| foreach (array_keys($files) as $fileName) { | |
| if (!empty($fileName)) { | |
| // …rest of loop body… |
🧰 Tools
🪛 PHPMD (2.15.0)
191-191: Avoid unused local variables such as '$fileInfo'. (Unused Code Rules)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In src/Validator/PlaceholderConsistencyValidator.php at line 191, the foreach
loop declares the variable $fileInfo but it is never used in the loop body. To
fix this, remove the unused variable by changing the loop to only iterate over
the keys, for example, use foreach ($files as $fileName) instead of foreach
($files as $fileName => $fileInfo).
# Conflicts: # README.md # src/Command/ValidateTranslationCommand.php # src/Validator/ValidatorRegistry.php # tests/src/Validator/ValidatorRegistryTest.php
Summary by CodeRabbit
New Features
Documentation
Tests