feat: add HtmlTagValidator for HTML tag consistency across translations#45
feat: add HtmlTagValidator for HTML tag consistency across translations#45konradmichalik merged 3 commits intomainfrom
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ValidateTranslationCommand
participant ValidatorRegistry
participant HtmlTagValidator
participant Parser
User->>ValidateTranslationCommand: Run validation
ValidateTranslationCommand->>ValidatorRegistry: getAvailableValidators()
ValidatorRegistry-->>ValidateTranslationCommand: List of validators (incl. HtmlTagValidator)
loop For each translation file
ValidateTranslationCommand->>Parser: Parse file
Parser-->>ValidateTranslationCommand: Parsed data
ValidateTranslationCommand->>HtmlTagValidator: processFile(parsed file)
end
ValidateTranslationCommand->>HtmlTagValidator: postProcess()
HtmlTagValidator-->>ValidateTranslationCommand: Issues (if any)
ValidateTranslationCommand->>User: Output results (including HTML tag issues)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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: 2
🔭 Outside diff range comments (1)
src/Validator/ValidatorRegistry.php (1)
24-39: Missing import statement forHtmlTagValidatorThe
HtmlTagValidatorclass is referenced but not imported. Add the import statement to avoid potential issues.namespace MoveElevator\ComposerTranslationValidator\Validator; +use MoveElevator\ComposerTranslationValidator\Validator\HtmlTagValidator; + class ValidatorRegistry
🧹 Nitpick comments (1)
src/Validator/HtmlTagValidator.php (1)
156-167: Consider expanding attribute name patternThe current regex pattern
\w+for attribute names won't match common attributes likedata-*oraria-*. Consider using a more inclusive pattern.- if (preg_match_all('/(\w+)=(["\'])([^"\']*)\2/i', $attributeString, $matches, PREG_SET_ORDER)) { + if (preg_match_all('/([\w\-:]+)=(["\'])([^"\']*)\2/i', $attributeString, $matches, PREG_SET_ORDER)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(1 hunks)src/Command/ValidateTranslationCommand.php(1 hunks)src/Validator/HtmlTagValidator.php(1 hunks)src/Validator/ValidatorRegistry.php(1 hunks)tests/src/Fixtures/config/unsupported.txt(1 hunks)tests/src/Fixtures/translations/xliff/fail/de.locallang.xlf(1 hunks)tests/src/Fixtures/translations/xliff/fail/locallang.xlf(1 hunks)tests/src/Validator/HtmlTagValidatorTest.php(1 hunks)tests/src/Validator/ValidatorRegistryTest.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Validator/ValidatorRegistry.php (1)
src/Validator/HtmlTagValidator.php (1)
HtmlTagValidator(37-376)
🪛 GitHub Actions: CGL
tests/src/Fixtures/config/unsupported.txt
[error] 1-1: EditorConfigCLI: This file has no final new line.
🪛 PHPMD (2.15.0)
src/Validator/HtmlTagValidator.php
265-265: Avoid unused local variables such as '$fileInfo'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (10)
tests/src/Fixtures/translations/xliff/fail/locallang.xlf (1)
21-23: LGTM! Test fixture correctly demonstrates HTML tag mismatch.The intentional mismatch between
<strong>opening and</b>closing tags serves as an appropriate negative test case for the HtmlTagValidator to detect HTML tag inconsistencies.tests/src/Fixtures/translations/xliff/fail/de.locallang.xlf (1)
22-25: LGTM! Proper HTML tags create effective test scenario.The correctly matched
<strong></strong>tags in the German translation provide a good contrast to the mismatched tags in the English source, enabling the HtmlTagValidator to detect cross-language HTML inconsistencies.src/Command/ValidateTranslationCommand.php (1)
137-138: LGTM! Help text accurately describes the new validator.The addition of HtmlTagValidator to the available validators list with a clear, concise description aligns perfectly with the PR objectives for HTML tag consistency validation.
tests/src/Validator/ValidatorRegistryTest.php (2)
29-29: LGTM! Import added for the new validator.The HtmlTagValidator import is correctly placed in alphabetical order with other validator imports.
46-46: LGTM! Test properly validates HtmlTagValidator registration.The assertion correctly checks for HtmlTagValidator presence in the registry, and the count update from 7 to 8 accurately reflects the addition of the new validator.
Also applies to: 49-49
README.md (1)
84-91: Documentation updates look good!The HtmlTagValidator documentation is well-written and follows the established pattern. The change from hex colors to CSS color names improves readability.
src/Validator/HtmlTagValidator.php (3)
42-68: Well-structured file processing logicThe
processFilemethod properly handles edge cases and builds the data structure for post-processing.
97-149: HTML parsing implementation covers common cases wellThe tag matching and structure validation logic is sound for typical HTML content in translations. Note that edge cases like namespaced tags, HTML comments, or CDATA sections are not handled, but these are rare in translation strings.
313-319: Potential issue with file order assumptionThe method assumes all issues share the same file structure by using only the first issue to determine file order. This could lead to missing columns if different issues involve different files.
Consider iterating through all issues to collect all unique filenames first:
$allFileNames = []; foreach ($issues as $issue) { $details = $issue->getDetails(); $files = $details['files'] ?? []; $allFileNames = array_merge($allFileNames, array_keys($files)); } $fileOrder = array_unique($allFileNames);tests/src/Validator/HtmlTagValidatorTest.php (1)
40-299: Excellent test coverage!The test suite comprehensively covers all aspects of the HtmlTagValidator including edge cases, error conditions, and output formatting. The use of mocks and reflection is appropriate for testing private methods.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/Validator/HtmlTagValidator.php (1)
260-260: Remove unused variable$_The loop variable is assigned but never used.
- foreach ($files as $fileName => $_) { + foreach (array_keys($files) as $fileName) {
🧹 Nitpick comments (1)
src/Validator/HtmlTagValidator.php (1)
92-149: LGTM! Comprehensive HTML structure analysis.The method effectively analyzes HTML structure with proper tag matching and error detection. The stack-based approach correctly identifies unclosed and unmatched tags.
Consider documenting that this parser doesn't handle CDATA sections or HTML comments, though for translation validation this is likely acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Validator/HtmlTagValidator.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Validator/HtmlTagValidator.php (8)
src/FileDetector/FileSet.php (2)
FileSet(26-60)getPath(43-46)src/Parser/JsonParser.php (1)
JsonParser(29-123)src/Parser/PhpParser.php (1)
PhpParser(29-144)src/Parser/XliffParser.php (1)
XliffParser(29-126)src/Parser/YamlParser.php (1)
YamlParser(30-114)src/Result/Issue.php (3)
Issue(26-73)getDetails(46-49)getValidatorType(56-59)src/Validator/AbstractValidator.php (1)
AbstractValidator(34-223)src/Validator/ResultType.php (2)
toString(57-64)toColorString(66-73)
🪛 PHPMD (2.15.0)
src/Validator/HtmlTagValidator.php
260-260: Avoid unused local variables such as '$_'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (9)
src/Validator/HtmlTagValidator.php (9)
37-41: LGTM! Well-structured class declaration.The property type annotation clearly documents the expected data structure, which will help with maintainability.
42-68: LGTM! Clean implementation of file processing.The method properly extracts keys, analyzes HTML structure, and stores data for cross-file comparison in
postProcess().
70-90: LGTM! Proper cross-file validation logic.The method correctly identifies HTML inconsistencies across files and creates issues appropriately. The empty file/parser parameters are handled by
distributeIssuesForDisplay().
151-167: LGTM! Solid attribute extraction for standard cases.The method handles the common attribute formats well. Edge cases like unquoted or boolean attributes are rare in translation contexts.
169-234: LGTM! Thorough HTML consistency validation.The method effectively identifies key inconsistencies: missing/extra tags, structure errors, and class attribute differences. Using the first file as reference is a pragmatic approach.
236-250: LGTM! Clear and informative issue formatting.The method generates well-structured messages that clearly identify the problematic translation key and all associated inconsistencies.
281-337: LGTM! Excellent visual presentation of translation comparisons.The table format with HTML tag highlighting provides clear visibility of inconsistencies across files. The consistent column ordering ensures readable output.
339-344: LGTM! Safe regex handling with good color choices.The null coalescing operators properly handle potential regex failures, and the color distinction between opening and closing tags enhances readability.
346-369: LGTM! Well-implemented validator configuration methods.The validator appropriately:
- Supports all major translation file formats
- Properly resets state between validation runs
- Uses WARNING level for HTML inconsistencies (correct severity)
- Always shows detailed output for better debugging
Examples:
Summary by CodeRabbit