feat: add support for PHP translation files and update validators#32
feat: add support for PHP translation files and update validators#32konradmichalik merged 3 commits intomainfrom
Conversation
WalkthroughThis update introduces comprehensive support for PHP translation files in addition to existing XLIFF, YAML, and JSON formats. It adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant FileDetectorRegistry
participant DirectoryFileDetector
participant SuffixFileDetector
participant ParserRegistry
participant PhpParser
participant Validators
User->>CLI: Run validation command
CLI->>FileDetectorRegistry: Get available detectors
CLI->>DirectoryFileDetector: Group files by language directories
DirectoryFileDetector-->>CLI: Return grouped files
CLI->>ParserRegistry: Get available parsers
CLI->>PhpParser: Parse PHP translation files
PhpParser-->>CLI: Extract translation keys/values
CLI->>Validators: Run validators on parsed data
Validators-->>CLI: Report validation results
CLI-->>User: Display results (incl. PHP support)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (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
🧹 Nitpick comments (2)
src/Validator/MismatchValidator.php (2)
10-12: Import added for PhpParser – consider keeping import list sorted
The newuse …\PhpParser;line is necessary. Optional: run your CS-fixer to keep theuseblock alphabetised, as other validators may expect that ordering.
217-220: PhpParser support added – duplication across validators
Great to see PHP now covered. Every validator now maintains its own hard-coded array of supported parsers; this is starting to repeat. Consider extracting a singleSUPPORTED_PARSERSconstant (or asking each parser whether it is supported) to avoid drift in future additions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
README.md(3 hunks)src/Command/ValidateTranslationCommand.php(1 hunks)src/FileDetector/FileDetectorRegistry.php(1 hunks)src/FileDetector/LaravelFileDetector.php(1 hunks)src/FileDetector/SuffixFileDetector.php(1 hunks)src/Parser/ParserRegistry.php(1 hunks)src/Parser/PhpParser.php(1 hunks)src/Result/ValidationResultCliRenderer.php(1 hunks)src/Validator/DuplicateKeysValidator.php(2 hunks)src/Validator/DuplicateValuesValidator.php(2 hunks)src/Validator/EmptyValuesValidator.php(2 hunks)src/Validator/MismatchValidator.php(2 hunks)src/Validator/PlaceholderConsistencyValidator.php(2 hunks)tests/src/FileDetector/CollectorTest.php(2 hunks)tests/src/FileDetector/FileDetectorRegistryTest.php(2 hunks)tests/src/FileDetector/LaravelFileDetectorTest.php(1 hunks)tests/src/Fixtures/config/unsupported.txt(1 hunks)tests/src/Fixtures/translations/php/fail/invalid.php(1 hunks)tests/src/Fixtures/translations/php/fail/messages.de.php(1 hunks)tests/src/Fixtures/translations/php/fail/messages.en.php(1 hunks)tests/src/Fixtures/translations/php/laravel/de/messages.php(1 hunks)tests/src/Fixtures/translations/php/laravel/en/messages.php(1 hunks)tests/src/Fixtures/translations/php/success/messages.de.php(1 hunks)tests/src/Fixtures/translations/php/success/messages.en.php(1 hunks)tests/src/Parser/ParserRegistryTest.php(3 hunks)tests/src/Parser/PhpParserTest.php(1 hunks)tests/src/Validator/DuplicateKeysValidatorTest.php(1 hunks)tests/src/Validator/EmptyValuesValidatorTest.php(2 hunks)tests/src/Validator/MismatchValidatorTest.php(1 hunks)tests/src/Validator/PlaceholderConsistencyValidatorTest.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (12)
src/FileDetector/FileDetectorRegistry.php (1)
src/FileDetector/LaravelFileDetector.php (1)
LaravelFileDetector(7-41)
src/Parser/ParserRegistry.php (1)
src/Parser/PhpParser.php (1)
PhpParser(7-122)
tests/src/FileDetector/FileDetectorRegistryTest.php (1)
src/FileDetector/LaravelFileDetector.php (1)
LaravelFileDetector(7-41)
tests/src/Validator/PlaceholderConsistencyValidatorTest.php (1)
src/Parser/PhpParser.php (1)
PhpParser(7-122)
tests/src/Validator/EmptyValuesValidatorTest.php (1)
src/Parser/PhpParser.php (1)
PhpParser(7-122)
tests/src/Validator/MismatchValidatorTest.php (1)
src/Parser/PhpParser.php (1)
PhpParser(7-122)
tests/src/Parser/ParserRegistryTest.php (2)
src/Parser/PhpParser.php (1)
PhpParser(7-122)src/Parser/ParserRegistry.php (2)
ParserRegistry(9-51)resolveParserClass(27-50)
src/Validator/DuplicateKeysValidator.php (4)
src/Parser/JsonParser.php (1)
JsonParser(7-101)src/Parser/PhpParser.php (1)
PhpParser(7-122)src/Parser/XliffParser.php (1)
XliffParser(7-81)src/Parser/YamlParser.php (1)
YamlParser(9-93)
src/Validator/DuplicateValuesValidator.php (1)
src/Parser/PhpParser.php (1)
PhpParser(7-122)
src/Validator/MismatchValidator.php (1)
src/Parser/PhpParser.php (1)
PhpParser(7-122)
src/FileDetector/LaravelFileDetector.php (1)
src/FileDetector/SuffixFileDetector.php (1)
mapTranslationSet(14-27)
src/Parser/PhpParser.php (1)
src/Parser/AbstractParser.php (1)
AbstractParser(7-51)
🪛 GitHub Actions: CGL
tests/src/Fixtures/config/unsupported.txt
[error] 1-1: EditorConfigCLI: This file has no final new line given.
🪛 LanguageTool
README.md
[style] ~83-~83: This phrase is redundant (‘L’ stands for ‘language’). Use simply “yaml”.
Context: ...n.wikipedia.org/wiki/YAML) | Supports yaml language files. | [Symfony F...
(ACRONYM_TAUTOLOGY)
🪛 PHPMD (2.15.0)
src/Parser/PhpParser.php
57-57: Avoid unused parameters such as '$attribute'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (42)
src/Result/ValidationResultCliRenderer.php (1)
49-49: LGTM! Documentation accurately reflects PHP support.The updated description correctly includes PHP in the list of supported translation file formats, aligning with the PR's objective to add PHP translation file support.
src/Command/ValidateTranslationCommand.php (1)
89-89: LGTM! Help text consistently updated for PHP support.The command help text now accurately includes PHP among the supported translation file formats, maintaining consistency with other documentation updates in this PR.
tests/src/FileDetector/CollectorTest.php (2)
97-97: LGTM! Test expectation correctly updated for new file detector.The increment from 3 to 4 expected debug calls is correct, accounting for the addition of the LaravelFileDetector which would generate an additional "No files found" debug log.
115-115: LGTM! Consistent test expectation update.The test expectation is correctly updated to match the behavioral change from adding the LaravelFileDetector, maintaining consistency with the previous test method.
src/FileDetector/FileDetectorRegistry.php (1)
17-17: LGTM! LaravelFileDetector properly added to registry.The addition of LaravelFileDetector::class to the registry correctly integrates the new detector for Laravel-style PHP translation files, maintaining the established pattern for file detector registration.
tests/src/Validator/PlaceholderConsistencyValidatorTest.php (2)
10-10: LGTM: Import added for PHP parser support.The import statement correctly adds the PhpParser class to support the new PHP translation file functionality.
233-233: LGTM: Test updated to reflect expanded parser support.The test correctly verifies that PlaceholderConsistencyValidator now supports four parsers including the newly added PhpParser. This aligns with the broader PHP translation file support implementation.
src/Parser/ParserRegistry.php (1)
20-20: LGTM: PhpParser correctly integrated into parser registry.Adding PhpParser to the available parsers array enables PHP translation file support throughout the system. This change follows the established pattern and is essential for the new functionality.
tests/src/FileDetector/FileDetectorRegistryTest.php (2)
9-9: LGTM: Import added for Laravel file detector.The import statement correctly adds the LaravelFileDetector class needed for the test assertions.
22-23: LGTM: Test assertions updated for new file detector.The test correctly verifies that LaravelFileDetector is included in the available detectors and updates the expected count from 2 to 3. This properly reflects the addition of Laravel-style PHP translation file detection support.
tests/src/Fixtures/translations/php/fail/invalid.php (1)
1-6: LGTM: Well-designed test fixture for error handling.This fixture correctly implements an invalid PHP translation file scenario by:
- Including output (echo statement) that could interfere with parsing
- Returning a string instead of the required array structure
This will properly test the PhpParser's error handling capabilities and validate that it correctly rejects malformed PHP translation files.
tests/src/Validator/DuplicateKeysValidatorTest.php (1)
66-72: LGTM: Test updated to verify complete parser support.The test correctly verifies that DuplicateKeysValidator supports all four parser classes including the newly added PhpParser. The formatting is clear and maintains consistency with other validator tests throughout the codebase.
tests/src/Validator/EmptyValuesValidatorTest.php (1)
9-9: LGTM - Consistent test updates for PHP parser support.The import and test assertion correctly reflect the addition of
PhpParsersupport to theEmptyValuesValidator. The changes are minimal and properly aligned with the broader PHP translation file support.Also applies to: 127-127
tests/src/Fixtures/translations/php/laravel/de/messages.php (1)
1-14: Well-structured Laravel PHP translation fixture.The file correctly follows Laravel translation conventions:
- Returns an array as expected by the
PhpParser- Uses proper nested structure for grouped translations (
auth,validation)- Contains realistic German translations with Laravel-style placeholders (
:attribute)- Follows Laravel directory structure pattern (
de/messages.php)This fixture will effectively support testing of PHP translation file parsing and validation.
tests/src/Validator/MismatchValidatorTest.php (1)
193-193: LGTM - Consistent test update for PHP parser support.The addition of
PhpParser::classto the expected supported parsers correctly reflects theMismatchValidator's extended support for PHP translation files.tests/src/Parser/ParserRegistryTest.php (1)
10-10: LGTM - Comprehensive test updates for PHP parser registry integration.The changes correctly validate the
PhpParserintegration:
- Import statement properly added
- Assertion correctly checks for
PhpParser::classinclusion in available parsers- Count accurately updated from 3 to 4 parsers
- Extension resolution test properly verifies
.phpfiles resolve toPhpParser::classThese changes ensure the
ParserRegistrycorrectly recognizes and resolves PHP translation files.Also applies to: 24-25, 48-49
src/Validator/DuplicateValuesValidator.php (1)
9-9: LGTM - Consistent validator update for PHP parser support.The changes correctly extend
DuplicateValuesValidatorto support PHP translation files:
- Import statement properly added in alphabetical order
PhpParser::classcorrectly added to thesupportsParser()method return array- Changes align with the broader PHP translation file support integration
The existing validator logic works seamlessly with the
PhpParserthrough theParserInterface.Also applies to: 88-88
src/Validator/DuplicateKeysValidator.php (2)
7-11: LGTM! Consistent import pattern for PHP parser support.The addition of PhpParser import and reordering of existing imports follows the established pattern for supporting new parser types.
66-66: LGTM! PHP parser correctly added to supported parsers.The PhpParser::class is properly included in the supportsParser() array, maintaining consistency with other validators in the codebase.
src/Validator/EmptyValuesValidator.php (2)
9-9: LGTM! PHP parser import added consistently.The PhpParser import follows the same pattern used in other validators to support PHP translation files.
69-69: LGTM! PHP parser support properly implemented.The addition of PhpParser::class to the supportsParser() array is consistent with the other validators being updated in this PR.
tests/src/Fixtures/translations/php/success/messages.de.php (1)
1-21: LGTM! Well-structured PHP translation fixture.This test fixture effectively demonstrates proper PHP translation file structure with:
- Nested array organization
- Multiple placeholder formats (%parameter% and {parameter})
- Appropriate German translations for testing
The structure aligns well with typical Laravel/Symfony PHP translation file conventions.
src/FileDetector/SuffixFileDetector.php (1)
20-20: LGTM! Regex correctly updated to detect PHP translation files.The addition of
\.phpto the regex pattern properly extends file detection to include PHP translation files while maintaining the existing detection logic for other formats.src/Validator/PlaceholderConsistencyValidator.php (2)
10-10: LGTM! PHP parser import added for placeholder validation.The PhpParser import is essential for this validator as it needs to validate placeholder consistency in PHP translation files, which commonly use various placeholder formats.
288-288: LGTM! PHP parser support crucial for placeholder validation.Adding PhpParser::class support is particularly important for this validator since PHP translation files frequently use different placeholder syntaxes (Laravel :parameter, Symfony %parameter%, etc.) that this validator is designed to handle.
tests/src/Fixtures/translations/php/fail/messages.de.php (1)
1-21: Fixture looks correct for negative-case testing
The structure, placeholder variety (%placeholder%,{placeholder}) and the deliberately unmatched key (missing_from_en) are all valuable for exercising the new validators. No issues spotted.tests/src/Fixtures/translations/php/laravel/en/messages.php (1)
1-14: Valid Laravel-style fixture
File follows Laravel directory convention and omits the closing?>, which is best practice. Keys and nested structure look fine.tests/src/Fixtures/translations/php/fail/messages.en.php (1)
1-21: Negative-case English fixture looks good
Mirrors the German “fail” file and introducesmissing_from_deto trigger mismatch validation. Content and placeholder formats are intentional.tests/src/Fixtures/translations/php/success/messages.en.php (1)
1-20: Positive-case fixture looks fine
Provides the fully-matched key set for “success” scenarios; placeholders and nesting are consistent.README.md (5)
14-14: LGTM! Clear and accurate format support statement.The addition of PHP to the supported translation file formats is clear and consistent with the implementation.
40-51: Excellent usage examples demonstrating PHP translation file validation.The examples clearly show how to validate both Laravel-style (directory-based) and Symfony-style (filename-based) PHP translation files, which will help users understand the new functionality.
83-83: Good correction of terminology.Fixing "Yaml" to "YAML" improves consistency with standard acronym capitalization.
85-85: Comprehensive PHP format documentation in the table.The table entry clearly describes PHP translation file support with appropriate framework references and examples.
103-131: Thorough documentation of PHP translation file structures.The section provides clear examples of both Laravel and Symfony styles, showing the different organizational patterns and syntax conventions. The explanation of automatic language detection and dot notation for nested keys is particularly helpful.
src/FileDetector/LaravelFileDetector.php (1)
7-41: Well-implemented Laravel file detector with robust pattern matching.The implementation correctly:
- Groups files by base filename for Laravel-style directory structures
- Uses appropriate regex patterns for language codes (
[a-z]{2}(?:[-_][A-Z]{2})?)- Handles both Unix and Windows path separators via
str_replace('\\', '/', $file)- Validates PHP file extensions with case-insensitive matching
The logic is clean and follows the expected Laravel translation file organization pattern.
tests/src/Parser/PhpParserTest.php (1)
1-268: Comprehensive and well-structured test suite.The test coverage is excellent, covering:
- Constructor validation with various file states
- Key extraction from nested arrays with dot notation
- Content retrieval with proper type checking
- Language detection for both Laravel and Symfony patterns
- Edge cases like empty arrays, mixed types, and invalid files
- Proper cleanup of temporary test files
The tests follow good practices with descriptive method names, clear assertions, and proper resource management.
src/Parser/PhpParser.php (5)
16-21: Proper error handling in constructor with meaningful exception messages.The try-catch block appropriately wraps the translation loading and provides clear error messages that include both the file path and underlying error details.
32-55: Well-implemented recursive key extraction with dot notation.The recursive closure correctly flattens nested arrays into dot-notated keys, handling both array and scalar values appropriately. The logic properly builds the full key path and accumulates results.
57-73: Correct implementation of content retrieval by dot-notated key.The method properly:
- Splits the dot-notated key into parts
- Navigates the nested array structure
- Returns only string values (null for non-strings)
- Includes helpful comment explaining the unused
$attributeparameterNote: The static analysis warning about unused
$attributeparameter is a false positive since this parameter is required by theParserInterface.
83-99: Robust language detection supporting both Laravel and Symfony patterns.The method correctly implements:
- Laravel pattern detection from directory names (
en/messages.php)- Symfony pattern detection from filenames (
messages.en.php)- Proper regex patterns for language codes with optional country codes
- Graceful fallback to empty string when no pattern matches
The regex patterns are well-formed and match the expected language code formats.
101-122: Secure and defensive PHP file inclusion with proper error handling.The implementation correctly:
- Disables error reporting temporarily to prevent issues
- Uses output buffering to catch any unwanted output
- Validates that the included file returns an array
- Restores error reporting in a finally block
- Provides clear error messages for validation failures
This approach safely handles potentially malicious or malformed PHP files while maintaining system stability.
tests/src/FileDetector/LaravelFileDetectorTest.php (1)
1-219: Comprehensive test coverage for Laravel file detector with excellent edge case handling.The test suite thoroughly validates:
- Basic Laravel-style file grouping by base filename
- Complex locale support (en_US, de_DE, fr-FR patterns)
- Proper filtering to include only PHP files
- Validation of language code patterns (rejecting invalid codes)
- Cross-platform path support (Windows and Unix)
- Edge cases like empty inputs and mixed file structures
- Multiple project structures with same filenames
The tests are well-organized with descriptive method names and clear assertions that validate the expected grouping behavior.
…ion file grouping
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/FileDetector/DirectoryFileDetector.php (1)
32-33: Consider making the language code regex more flexible.The current regex
/^[a-z]{2}(?:[-_][A-Z]{2})?$/is quite restrictive and may not support all valid language codes used in real-world projects. Consider supporting:
- 3-letter language codes (e.g.,
filfor Filipino)- Script codes (e.g.,
zh-Hans,zh-Hant)- Mixed case variations
- && preg_match('/^[a-z]{2}(?:[-_][A-Z]{2})?$/', $languageDir) + && preg_match('/^[a-z]{2,3}(?:[-_][A-Za-z]{2,4})?(?:[-_][A-Z]{2})?$/i', $languageDir)docs/file-detector.md (1)
33-38: Add language specifications to fenced code blocks.The markdown linter indicates that fenced code blocks should have language specifications for better syntax highlighting and accessibility.
-``` +```text translations/ ├── locallang.xlf ├── de.locallang.xlf └── fr.locallang.xlf-
+text
translations/
├── messages.en.yaml
├── messages.de.yaml
├── validators.en.yaml
└── validators.de.yaml-``` +```text resources/lang/ ├── en/ │ ├── messages.php │ └── auth.php ├── de/ │ ├── messages.php │ └── auth.phpAlso applies to: 41-47, 50-58
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/file-detector.md(1 hunks)src/FileDetector/DirectoryFileDetector.php(1 hunks)src/FileDetector/FileDetectorRegistry.php(1 hunks)tests/src/FileDetector/DirectoryFileDetectorTest.php(1 hunks)tests/src/FileDetector/FileDetectorRegistryTest.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/FileDetector/FileDetectorRegistry.php
- tests/src/FileDetector/FileDetectorRegistryTest.php
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/file-detector.md
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
50-50: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (7)
src/FileDetector/DirectoryFileDetector.php (1)
20-41: LGTM! Well-structured implementation.The method correctly implements the directory-based detection pattern with proper path normalization and grouping logic. The regex patterns effectively filter valid translation files and language directories.
docs/file-detector.md (1)
3-88: Excellent documentation structure and content.The documentation provides comprehensive coverage of all three detection strategies with clear examples, configuration guidance, and troubleshooting tips. This will be very helpful for users implementing different translation file organization patterns.
tests/src/FileDetector/DirectoryFileDetectorTest.php (5)
12-39: Excellent test coverage for basic functionality.The test correctly verifies the core directory-based grouping functionality with multiple languages and file types. The assertions are clear and comprehensive.
131-154: Good validation of language code filtering.This test effectively verifies that the detector correctly ignores invalid language codes like full language names, numeric codes, and codes that are too short. This is crucial for ensuring the detector only processes valid translation files.
156-175: Excellent cross-platform compatibility testing.Testing Windows-style paths ensures the detector works correctly across different operating systems. The path normalization logic is properly validated here.
201-231: Good test for mixed directory structures.This test correctly verifies that the detector only processes files following the directory-based pattern while ignoring other patterns like suffix-based files. This ensures the detector stays focused on its specific responsibility.
10-258: Comprehensive and well-structured test suite.The test suite provides excellent coverage of the DirectoryFileDetector functionality, including edge cases, cross-platform compatibility, and various file formats. The tests are well-organized and clearly demonstrate the expected behavior.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests