Conversation
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ValidateTranslationCommand
participant Validators
participant Output
User->>CLI: Run validate-translations [--format=cli|json]
CLI->>ValidateTranslationCommand: Parse options
ValidateTranslationCommand->>Validators: Validate files
Validators-->>ValidateTranslationCommand: Return structured issues
ValidateTranslationCommand->>ValidateTranslationCommand: summarize(issues, format)
alt format == "json"
ValidateTranslationCommand->>Output: Render JSON result
else format == "cli"
ValidateTranslationCommand->>Output: Render CLI result
end
Output-->>User: Display result
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 phpcs (3.7.2)src/Command/ValidateTranslationCommand.phpWarning: PHP Startup: Invalid date.timezone value 'UTC', using 'UTC' instead in Unknown on line 0 Fatal error: Uncaught Error: Class "Phar" not found in /usr/local/bin/phpcs:3 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)src/Command/ValidateTranslationCommand.php (5)
🪛 PHPMD (2.15.0)src/Command/ValidateTranslationCommand.php219-219: Avoid using static access to class '\MoveElevator\ComposerTranslationValidator\Utility\PathUtility' in method 'renderIssues'. (Clean Code Rules) (StaticAccess) 🔇 Additional comments (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: 7
🔭 Outside diff range comments (2)
src/Validator/MismatchValidator.php (1)
17-18: Update property type annotation to match new structure.The type annotation for
$keyArraydoesn't match the actual structure being stored.- /** @var array<string, array<string>> */ + /** @var array<string, array<string, string|null>> */ protected array $keyArray = [];src/Command/ValidateTranslationCommand.php (1)
169-190: Fix documentation but logic is soundThe CLI rendering preserves existing behavior correctly. Address documentation:
+ /** + * Renders validation results as CLI output. + * + * @param array<class-string<ValidatorInterface>, array<string, array<string, array<mixed>>>> $issues + * + * @return int Command exit code + */ - private function renderCliResult(array $issues): int + private function _renderCliResult(array $issues): intThe backward compatibility is well maintained.
🧹 Nitpick comments (2)
src/Validator/AbstractValidator.php (1)
58-63: Good enhancement for structured issue metadata.The change from simple key-value storage to a richer array structure with file, issues, parser, and type metadata is a solid improvement that enables better output formatting capabilities.
Consider updating the docblock for the
$issuesproperty at line 14 to reflect the new structure:- /** @var array<string, array<mixed>> */ + /** @var array<int, array{file: string, issues: array<mixed>, parser: class-string<ParserInterface>, type: string}> */src/Validator/SchemaValidator.php (1)
77-83: Fix multi-line function call formatting.Apply this diff to fix the formatting issue flagged by static analysis:
- $table->addRow([ + $table->addRow( + [ '<fg=red>'.$errors['file'].'</>', LIBXML_ERR_WARNING === (int) $error['level'] ? 'WARNING' : 'ERROR', $error['code'], trim((string) $message), $error['line'], - ]); + ] + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.md(1 hunks)src/Command/ValidateTranslationCommand.php(3 hunks)src/Parser/XliffParser.php(1 hunks)src/Validator/AbstractValidator.php(1 hunks)src/Validator/DuplicatesValidator.php(1 hunks)src/Validator/MismatchValidator.php(2 hunks)src/Validator/SchemaValidator.php(2 hunks)tests/composer.json(1 hunks)tests/src/Validator/DuplicatesValidatorTest.php(1 hunks)tests/src/Validator/MismatchValidatorTest.php(4 hunks)tests/src/Validator/SchemaValidatorTest.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/Validator/AbstractValidator.php (2)
src/Parser/AbstractParser.php (1)
getFileName(28-31)src/Parser/ParserInterface.php (1)
getFileName(23-23)
src/Parser/XliffParser.php (2)
src/Parser/YamlParser.php (1)
getContentByKey(50-64)src/Parser/ParserInterface.php (1)
getContentByKey(21-21)
src/Command/ValidateTranslationCommand.php (5)
src/Validator/SchemaValidator.php (2)
explain(91-95)renderIssueSets(52-89)src/Validator/MismatchValidator.php (2)
explain(113-117)renderIssueSets(80-111)src/Validator/DuplicatesValidator.php (2)
explain(74-78)renderIssueSets(46-72)src/Validator/ValidatorInterface.php (2)
explain(23-23)renderIssueSets(21-21)src/Utility/PathUtility.php (2)
PathUtility(7-33)normalizeFolderPath(9-32)
src/Validator/MismatchValidator.php (5)
src/Parser/XliffParser.php (1)
getContentByKey(41-56)src/Parser/YamlParser.php (1)
getContentByKey(50-64)src/Parser/ParserInterface.php (2)
getContentByKey(21-21)getFileName(23-23)src/Parser/AbstractParser.php (1)
getFileName(28-31)src/Validator/ValidatorInterface.php (1)
renderIssueSets(21-21)
🪛 phpcs (3.7.2)
src/Command/ValidateTranslationCommand.php
[ERROR] 123-123: Missing short description in doc comment
(Generic.Commenting.DocComment.MissingShort)
[ERROR] 123-123: Doc comment for parameter "$format" missing
(PEAR.Commenting.FunctionComment.MissingParamTag)
[ERROR] 124-124: Missing parameter comment
(PEAR.Commenting.FunctionComment.MissingParamComment)
[ERROR] 127-127: Missing @return tag in function comment
(PEAR.Commenting.FunctionComment.MissingReturn)
[ERROR] 128-128: Private method name "ValidateTranslationCommand::summarize" must be prefixed with an underscore
(PEAR.NamingConventions.ValidFunctionName.PrivateNoUnderscore)
[ERROR] 141-141: Missing short description in doc comment
(Generic.Commenting.DocComment.MissingShort)
[ERROR] 142-142: Missing parameter comment
(PEAR.Commenting.FunctionComment.MissingParamComment)
[ERROR] 143-143: Missing @return tag in function comment
(PEAR.Commenting.FunctionComment.MissingReturn)
[ERROR] 144-144: Private method name "ValidateTranslationCommand::renderJsonResult" must be prefixed with an underscore
(PEAR.NamingConventions.ValidFunctionName.PrivateNoUnderscore)
[ERROR] 166-166: Missing short description in doc comment
(Generic.Commenting.DocComment.MissingShort)
[ERROR] 167-167: Missing parameter comment
(PEAR.Commenting.FunctionComment.MissingParamComment)
[ERROR] 168-168: Missing @return tag in function comment
(PEAR.Commenting.FunctionComment.MissingReturn)
[ERROR] 169-169: Private method name "ValidateTranslationCommand::renderCliResult" must be prefixed with an underscore
(PEAR.NamingConventions.ValidFunctionName.PrivateNoUnderscore)
[ERROR] 192-192: Missing short description in doc comment
(Generic.Commenting.DocComment.MissingShort)
[ERROR] 193-193: Missing parameter comment
(PEAR.Commenting.FunctionComment.MissingParamComment)
[ERROR] 194-194: Missing @return tag in function comment
(PEAR.Commenting.FunctionComment.MissingReturn)
[ERROR] 195-195: Private method name "ValidateTranslationCommand::renderIssues" must be prefixed with an underscore
(PEAR.NamingConventions.ValidFunctionName.PrivateNoUnderscore)
src/Validator/DuplicatesValidator.php
[ERROR] 39-39: Missing parameter name
(PEAR.Commenting.FunctionComment.MissingParamName)
[ERROR] 45-45: Missing @return tag in function comment
(PEAR.Commenting.FunctionComment.MissingReturn)
src/Validator/SchemaValidator.php
[ERROR] 38-38: Missing parameter name
(PEAR.Commenting.FunctionComment.MissingParamName)
[ERROR] 77-77: Opening parenthesis of a multi-line function call must be the last content on the line
(PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket)
[ERROR] 83-83: Closing parenthesis of a multi-line function call must be on a line by itself
(PEAR.Functions.FunctionCallSignature.CloseBracketLine)
src/Validator/MismatchValidator.php
[ERROR] 71-71: Missing short description in doc comment
(Generic.Commenting.DocComment.MissingShort)
[ERROR] 71-71: Doc comment for parameter "$input" missing
(PEAR.Commenting.FunctionComment.MissingParamTag)
[ERROR] 71-71: Doc comment for parameter "$output" missing
(PEAR.Commenting.FunctionComment.MissingParamTag)
[ERROR] 71-71: Doc comment for parameter "$issueSets" missing
(PEAR.Commenting.FunctionComment.MissingParamTag)
[ERROR] 72-72: Missing parameter name
(PEAR.Commenting.FunctionComment.MissingParamName)
[ERROR] 79-79: Missing @return tag in function comment
(PEAR.Commenting.FunctionComment.MissingReturn)
🪛 PHPMD (2.15.0)
src/Command/ValidateTranslationCommand.php
206-206: Avoid using static access to class '\MoveElevator\ComposerTranslationValidator\Utility\PathUtility' in method 'renderIssues'. (Clean Code Rules)
(StaticAccess)
🔇 Additional comments (10)
README.md (1)
42-42: LGTM! Clear documentation of the new format option.The usage documentation correctly reflects the new
--formatoption with appropriate syntax showing the available values (cli|json).tests/composer.json (1)
31-33: Good test coverage for the new JSON format feature.The addition of the
translation:xliff:fail:jsonscript properly tests the new--format=jsonoption. The trailing comma on line 31 follows good JSON formatting practices.tests/src/Validator/SchemaValidatorTest.php (1)
126-160: SchemaValidator test data structure matches implementationThe
SchemaValidator::renderIssueSetsmethod expects each set item to have a'file'key and an'issues'array where each issue provides'level','code','message', and'line'. The test’s nested'file','column','parser', and'type'keys are simply ignored by the CLI renderer and do not break the existing assertions.All required fields line up:
- Outer array items supply
'file'and'issues'.- Each issue includes
'level','code','message', and'line'.No discrepancies found—tests will continue to pass against the current implementation.
src/Parser/XliffParser.php (1)
45-51: Approved: XliffParser source→target fallback is safe.Verified that no existing calls to getContentByKey in the codebase expect a
nullwhen the source is empty. The new fallback from ‘source’ to ‘target’:
- Does not break any current consumer (no null checks on empty source found)
- Aligns with existing XliffParser tests in tests/src/Parser/XliffParserTest.php
This change can be merged as-is.
tests/src/Validator/DuplicatesValidatorTest.php (1)
68-89: Consistent data structure enhancement across validator tests.The restructured test data follows the same pattern as SchemaValidatorTest, providing consistent metadata structure (file, parser, type) across all validators. This consistency supports the JSON output feature effectively.
tests/src/Validator/MismatchValidatorTest.php (1)
39-40: Test data correctly updated for new issue structure.All test data has been properly updated to match the new structured format used by MismatchValidator. The change from dash to
<missing>in the output improves clarity.Also applies to: 87-114, 152-167, 178-179
src/Validator/MismatchValidator.php (2)
29-32: Good enhancement to store values alongside keys.Storing the actual translation values enables richer comparison and better debugging information in the output.
80-111: Well-structured refactoring of the render method.The new implementation dynamically builds table headers based on all files and properly handles missing values with clear visual indicators.
src/Command/ValidateTranslationCommand.php (2)
41-42: LGTM: Format option properly configuredThe new format option is well-designed with appropriate default value and short option for usability.
118-120: LGTM: Clean format delegationProper separation of concerns by delegating format handling to the dedicated summarize method.
| * @param array<string, array<int, array{ | ||
| * file: string, | ||
| * issues: array<string, int>, | ||
| * parser: string, | ||
| * type: string | ||
| * }>> $issueSets | ||
| */ | ||
| public function renderIssueSets(InputInterface $input, OutputInterface $output, array $issueSets): void |
There was a problem hiding this comment.
Update matches the new issue structure correctly.
The parameter type annotation and implementation properly handle the new structured format from AbstractValidator.
Fix the docblock to address static analysis warnings:
/**
- * @param array<string, array<int, array{
+ * @param InputInterface $input
+ * @param OutputInterface $output
+ * @param array<string, array<int, array{
* file: string,
* issues: array<string, int>,
* parser: string,
* type: string
* }>> $issueSets
+ * @return void
*/📝 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.
| * @param array<string, array<int, array{ | |
| * file: string, | |
| * issues: array<string, int>, | |
| * parser: string, | |
| * type: string | |
| * }>> $issueSets | |
| */ | |
| public function renderIssueSets(InputInterface $input, OutputInterface $output, array $issueSets): void | |
| * @param InputInterface $input | |
| * @param OutputInterface $output | |
| * @param array<string, array<int, array{ | |
| * file: string, | |
| * issues: array<string, int>, | |
| * parser: string, | |
| * type: string | |
| * }>> $issueSets | |
| * @return void | |
| */ | |
| public function renderIssueSets(InputInterface $input, OutputInterface $output, array $issueSets): void |
🧰 Tools
🪛 phpcs (3.7.2)
[ERROR] 39-39: Missing parameter name
(PEAR.Commenting.FunctionComment.MissingParamName)
[ERROR] 45-45: Missing @return tag in function comment
(PEAR.Commenting.FunctionComment.MissingReturn)
🤖 Prompt for AI Agents
In src/Validator/DuplicatesValidator.php around lines 39 to 46, update the
docblock for the $issueSets parameter to precisely match the new issue structure
used by AbstractValidator, ensuring all nested array keys and types are
correctly annotated. This will resolve static analysis warnings by providing
accurate type information for the complex array structure expected by the
renderIssueSets method.
| * @param array<string, array<int, array{ | ||
| * file: string, | ||
| * issues: array<int, array{ | ||
| * level: string, | ||
| * code: int, | ||
| * message: string, | ||
| * file: string, | ||
| * line: int, | ||
| * column: int | ||
| * }>, | ||
| * parser: string, | ||
| * type: string | ||
| * }>> $issueSets | ||
| */ |
There was a problem hiding this comment.
Parameter type annotation correctly updated.
The type annotation properly reflects the new structured format.
Fix the docblock to address static analysis warnings:
/**
- * @param array<string, array<int, array{
+ * @param InputInterface $input
+ * @param OutputInterface $output
+ * @param array<string, array<int, array{
* file: string,
* issues: array<int, array{
* level: string,
* code: int,
* message: string,
* file: string,
* line: int,
* column: int
* }>,
* parser: string,
* type: string
* }>> $issueSets
+ * @return void
*/Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 phpcs (3.7.2)
[ERROR] 38-38: Missing parameter name
(PEAR.Commenting.FunctionComment.MissingParamName)
[ERROR] 51-51: Missing @return tag in function comment
(PEAR.Commenting.FunctionComment.MissingReturn)
🤖 Prompt for AI Agents
In src/Validator/SchemaValidator.php around lines 38 to 51, update the docblock
parameter type annotation for $issueSets to match the new structured format
shown, ensuring it accurately describes the nested array structure with keys
file, issues, parser, and type, and the issues array containing level, code,
message, file, line, and column. This will fix static analysis warnings by
providing precise type information.
src/Validator/MismatchValidator.php
Outdated
|
|
||
| $rows = []; | ||
| foreach ($issues as $key => $files) { | ||
| // Sammle alle Dateinamen (Spalten) und baue die Zeilen |
There was a problem hiding this comment.
Replace German comment with English.
- // Sammle alle Dateinamen (Spalten) und baue die Zeilen
+ // Collect all filenames (columns) and build the rows📝 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.
| // Sammle alle Dateinamen (Spalten) und baue die Zeilen | |
| // Collect all filenames (columns) and build the rows |
🤖 Prompt for AI Agents
In src/Validator/MismatchValidator.php at line 86, replace the German comment
"Sammle alle Dateinamen (Spalten) und baue die Zeilen" with its English
equivalent, such as "Collect all filenames (columns) and build the rows" to
maintain consistent language usage in the codebase.
| * @param array<string, array<int, array{ | ||
| * key: string, | ||
| * files: array<int, array{ | ||
| * file: string, | ||
| * value: string|null | ||
| * }> | ||
| * }>> $issueSets | ||
| */ |
There was a problem hiding this comment.
Fix docblock to address static analysis warnings.
Add the missing parameter documentation:
/**
+ * @param InputInterface $input
+ * @param OutputInterface $output
* @param array<string, array<int, array{
* key: string,
* files: array<int, array{
* file: string,
* value: string|null
* }>
* }>> $issueSets
+ * @return void
*/📝 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.
| * @param array<string, array<int, array{ | |
| * key: string, | |
| * files: array<int, array{ | |
| * file: string, | |
| * value: string|null | |
| * }> | |
| * }>> $issueSets | |
| */ | |
| /** | |
| * @param InputInterface $input | |
| * @param OutputInterface $output | |
| * @param array<string, array<int, array{ | |
| * key: string, | |
| * files: array<int, array{ | |
| * file: string, | |
| * value: string|null | |
| * }> | |
| * }>> $issueSets | |
| * @return void | |
| */ |
🧰 Tools
🪛 phpcs (3.7.2)
[ERROR] 72-72: Missing parameter name
(PEAR.Commenting.FunctionComment.MissingParamName)
[ERROR] 79-79: Missing @return tag in function comment
(PEAR.Commenting.FunctionComment.MissingReturn)
🤖 Prompt for AI Agents
In src/Validator/MismatchValidator.php around lines 72 to 79, the docblock for
the method is missing documentation for one or more parameters, causing static
analysis warnings. Review the method signature and add complete @param
annotations for all parameters, ensuring the types and structure match the
actual arguments, including any missing parameters not currently documented.
| private function renderJsonResult(array $issues): int | ||
| { | ||
| $result = [ | ||
| 'status' => Command::SUCCESS, | ||
| 'message' => 'Language validation succeeded.', | ||
| 'issues' => $issues, | ||
| ]; | ||
|
|
||
| if (!empty($issues)) { | ||
| $result['message'] = 'Language validation failed.'; | ||
| $result['status'] = Command::FAILURE; | ||
| if ($this->dryRun) { | ||
| $result['message'] = 'Language validation failed and completed in dry-run mode.'; | ||
| $result['status'] = Command::SUCCESS; | ||
| } | ||
| } | ||
|
|
||
| $this->output->writeln(json_encode($result, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE)); | ||
|
|
||
| return $result['status']; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve documentation and consider JSON structure
The JSON rendering logic is solid, but consider these improvements:
- Add proper PHPDoc documentation
- Consider following naming convention with underscore prefix
+ /**
+ * Renders validation results as JSON output.
+ *
+ * @param array<class-string<ValidatorInterface>, array<string, array<string, array<mixed>>>> $issues
+ *
+ * @return int Command exit code
+ */
- private function renderJsonResult(array $issues): int
+ private function _renderJsonResult(array $issues): intThe JSON structure includes raw issue data which provides comprehensive information for programmatic consumption. This is appropriate for the intended use case.
🧰 Tools
🪛 phpcs (3.7.2)
[ERROR] 144-144: Private method name "ValidateTranslationCommand::renderJsonResult" must be prefixed with an underscore
(PEAR.NamingConventions.ValidFunctionName.PrivateNoUnderscore)
🤖 Prompt for AI Agents
In src/Command/ValidateTranslationCommand.php around lines 144 to 164, add
PHPDoc comments above the renderJsonResult method to describe its purpose,
parameters, and return type for better code documentation. Also, consider
renaming the $issues parameter or any internal variables with an underscore
prefix if it aligns with the project's naming conventions to improve code
consistency. The JSON structure itself is fine and does not require changes.
| private function renderIssues(array $issues): void | ||
| { | ||
| foreach ($issues as $validator => $paths) { | ||
| $validatorInstance = new $validator($this->logger); | ||
| /* @var ValidatorInterface $validatorInstance */ | ||
|
|
||
| $this->io->section(sprintf('Validator: <fg=cyan>%s</>', $validator)); | ||
| foreach ($paths as $path => $sets) { | ||
| if ($this->output->isVerbose()) { | ||
| $this->io->writeln(sprintf('Explanation: %s', $validatorInstance->explain())); | ||
| } | ||
| $this->io->writeln(sprintf('<fg=gray>Folder Path: %s</>', PathUtility::normalizeFolderPath($path))); | ||
| $this->io->newLine(); | ||
| $validatorInstance->renderIssueSets( | ||
| $this->input, | ||
| $this->output, | ||
| $sets | ||
| ); | ||
|
|
||
| $this->io->newLine(); | ||
| $this->io->newLine(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix documentation and method naming
The issue rendering logic is correct. Address the documentation and naming:
+ /**
+ * Renders validation issues using validator-specific formatters.
+ *
+ * @param array<class-string<ValidatorInterface>, array<string, array<string, array<mixed>>>> $issues
+ */
- private function renderIssues(array $issues): void
+ private function _renderIssues(array $issues): voidUpdate the call on line 171:
- $this->renderIssues($issues);
+ $this->_renderIssues($issues);The static access to PathUtility::normalizeFolderPath() is appropriate for a utility function.
📝 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.
| private function renderIssues(array $issues): void | |
| { | |
| foreach ($issues as $validator => $paths) { | |
| $validatorInstance = new $validator($this->logger); | |
| /* @var ValidatorInterface $validatorInstance */ | |
| $this->io->section(sprintf('Validator: <fg=cyan>%s</>', $validator)); | |
| foreach ($paths as $path => $sets) { | |
| if ($this->output->isVerbose()) { | |
| $this->io->writeln(sprintf('Explanation: %s', $validatorInstance->explain())); | |
| } | |
| $this->io->writeln(sprintf('<fg=gray>Folder Path: %s</>', PathUtility::normalizeFolderPath($path))); | |
| $this->io->newLine(); | |
| $validatorInstance->renderIssueSets( | |
| $this->input, | |
| $this->output, | |
| $sets | |
| ); | |
| $this->io->newLine(); | |
| $this->io->newLine(); | |
| } | |
| } | |
| } | |
| /** | |
| * Renders validation issues using validator-specific formatters. | |
| * | |
| * @param array<class-string<ValidatorInterface>, array<string, array<string, array<mixed>>>> $issues | |
| */ | |
| private function _renderIssues(array $issues): void | |
| { | |
| foreach ($issues as $validator => $paths) { | |
| $validatorInstance = new $validator($this->logger); | |
| /* @var ValidatorInterface $validatorInstance */ | |
| $this->io->section(sprintf('Validator: <fg=cyan>%s</>', $validator)); | |
| foreach ($paths as $path => $sets) { | |
| if ($this->output->isVerbose()) { | |
| $this->io->writeln(sprintf('Explanation: %s', $validatorInstance->explain())); | |
| } | |
| $this->io->writeln(sprintf('<fg=gray>Folder Path: %s</>', PathUtility::normalizeFolderPath($path))); | |
| $this->io->newLine(); | |
| $validatorInstance->renderIssueSets( | |
| $this->input, | |
| $this->output, | |
| $sets | |
| ); | |
| $this->io->newLine(); | |
| $this->io->newLine(); | |
| } | |
| } | |
| } |
🧰 Tools
🪛 phpcs (3.7.2)
[ERROR] 195-195: Private method name "ValidateTranslationCommand::renderIssues" must be prefixed with an underscore
(PEAR.NamingConventions.ValidFunctionName.PrivateNoUnderscore)
🪛 PHPMD (2.15.0)
206-206: Avoid using static access to class '\MoveElevator\ComposerTranslationValidator\Utility\PathUtility' in method 'renderIssues'. (Clean Code Rules)
(StaticAccess)
🤖 Prompt for AI Agents
In src/Command/ValidateTranslationCommand.php between lines 195 and 218, the
method renderIssues is correctly implemented but requires improved documentation
and possibly a clearer method name to better reflect its purpose. Review and
update the method's PHPDoc comment to accurately describe its functionality and
consider renaming the method if needed for clarity. The call to
PathUtility::normalizeFolderPath() is correctly used as a static method and does
not require changes.
| private function summarize(array $issues, string $format = 'cli'): int | ||
| { | ||
| foreach ($issues as $validator => $paths) { | ||
| $validatorInstance = new $validator($this->logger); | ||
| /* @var ValidatorInterface $validatorInstance */ | ||
| if ('json' === $format) { | ||
| return $this->renderJsonResult($issues); | ||
| } | ||
| if ('cli' === $format) { | ||
| return $this->renderCliResult($issues); | ||
| } | ||
| $this->io->error('Invalid output format specified. Use "cli" or "json".'); | ||
|
|
||
| $this->io->section(sprintf('Validator: <fg=cyan>%s</>', $validator)); | ||
| foreach ($paths as $path => $sets) { | ||
| if ($this->output->isVerbose()) { | ||
| $this->io->writeln(sprintf('Explanation: %s', $validatorInstance->explain())); | ||
| } | ||
| $this->io->writeln(sprintf('<fg=gray>Folder Path: %s</>', PathUtility::normalizeFolderPath($path))); | ||
| $this->io->newLine(); | ||
| $validatorInstance->renderIssueSets( | ||
| $this->input, | ||
| $this->output, | ||
| $sets | ||
| ); | ||
| return Command::FAILURE; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix documentation and consider naming convention
The method logic is correct, but address these issues:
- Add proper PHPDoc with method description, parameter documentation, and return type
- Consider following the private method naming convention (prefix with underscore)
+ /**
+ * Summarizes validation results in the specified format.
+ *
+ * @param array<class-string<ValidatorInterface>, array<string, array<string, array<mixed>>>> $issues
+ * @param string $format Output format ('cli' or 'json')
+ *
+ * @return int Command exit code
+ *
+ * @throws \JsonException
+ */
- private function summarize(array $issues, string $format = 'cli'): int
+ private function _summarize(array $issues, string $format = 'cli'): intAnd update the call on line 120:
- return $this->summarize($issues, $format);
+ return $this->_summarize($issues, $format);📝 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.
| private function summarize(array $issues, string $format = 'cli'): int | |
| { | |
| foreach ($issues as $validator => $paths) { | |
| $validatorInstance = new $validator($this->logger); | |
| /* @var ValidatorInterface $validatorInstance */ | |
| if ('json' === $format) { | |
| return $this->renderJsonResult($issues); | |
| } | |
| if ('cli' === $format) { | |
| return $this->renderCliResult($issues); | |
| } | |
| $this->io->error('Invalid output format specified. Use "cli" or "json".'); | |
| $this->io->section(sprintf('Validator: <fg=cyan>%s</>', $validator)); | |
| foreach ($paths as $path => $sets) { | |
| if ($this->output->isVerbose()) { | |
| $this->io->writeln(sprintf('Explanation: %s', $validatorInstance->explain())); | |
| } | |
| $this->io->writeln(sprintf('<fg=gray>Folder Path: %s</>', PathUtility::normalizeFolderPath($path))); | |
| $this->io->newLine(); | |
| $validatorInstance->renderIssueSets( | |
| $this->input, | |
| $this->output, | |
| $sets | |
| ); | |
| return Command::FAILURE; | |
| } | |
| /** | |
| * Summarizes validation results in the specified format. | |
| * | |
| * @param array<class-string<ValidatorInterface>, array<string, array<string, array<mixed>>>> $issues | |
| * @param string $format Output format ('cli' or 'json') | |
| * | |
| * @return int Command exit code | |
| * | |
| * @throws \JsonException | |
| */ | |
| private function _summarize(array $issues, string $format = 'cli'): int | |
| { | |
| if ('json' === $format) { | |
| return $this->renderJsonResult($issues); | |
| } | |
| if ('cli' === $format) { | |
| return $this->renderCliResult($issues); | |
| } | |
| $this->io->error('Invalid output format specified. Use "cli" or "json".'); | |
| return Command::FAILURE; | |
| } |
🧰 Tools
🪛 phpcs (3.7.2)
[ERROR] 128-128: Private method name "ValidateTranslationCommand::summarize" must be prefixed with an underscore
(PEAR.NamingConventions.ValidFunctionName.PrivateNoUnderscore)
🤖 Prompt for AI Agents
In src/Command/ValidateTranslationCommand.php around lines 128 to 139, add a
PHPDoc block above the summarize method describing its purpose, documenting the
array $issues parameter, the string $format parameter, and the int return type.
Rename the method to _summarize to follow the private method naming convention.
Also, update any calls to this method (such as on line 120) to use the new name
_summarize.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/Validator/MismatchValidator.php (2)
69-77: Fix docblock to address static analysis warnings.
84-84: Replace German comment with English.
🧹 Nitpick comments (11)
tests/src/Parser/ParserRegistryTest.php (1)
23-28: Add PHPDoc comment for better documentation.The test method is well-structured and covers appropriate test scenarios for parser resolution. However, it's missing a PHPDoc comment as flagged by the static analysis tool.
+ /** + * Tests that ParserRegistry::resolveParserClass returns the correct parser class + * for known file extensions and null for unknown extensions. + */ public function testResolveParserClass(): voidThe static access warnings from PHPMD can be ignored as they are false positives - testing static methods necessarily requires static access.
tests/src/Validator/SchemaValidatorTest.php (1)
187-193: Add PHPDoc comment for consistency.The test method functionality is correct, but it's missing a PHPDoc comment as flagged by static analysis.
+ /** + * Test that the validator supports only XliffParser. + */ public function testSupportsParser(): voidsrc/Validator/AbstractValidator.php (1)
82-82: Add PHPDoc comment for the public method.The visibility change from
protectedtopublicis appropriate for broader access, but the method needs a proper PHPDoc comment as flagged by static analysis.+ /** + * Perform additional processing after validation. + * This method can be overridden by subclasses to perform post-processing tasks. + */ public function postProcess(): voidtests/src/Validator/DuplicatesValidatorTest.php (1)
117-123: Add PHPDoc comment for consistency.The test method is correctly implemented but needs a PHPDoc comment as flagged by static analysis to maintain consistency with coding standards.
+ /** + * Test that the validator supports only XliffParser. + */ public function testSupportsParser(): voidtests/src/Command/ValidateTranslationCommandTest.php (3)
116-137: Fix code style violations in the new test method.The test logic correctly validates JSON output format functionality. However, there are code style violations that should be addressed.
Apply this diff to fix the formatting and documentation issues:
+ /** + * Test command execution with JSON format output. + */ public function testExecuteWithJsonFormat(): void { $application = new Application(); $application->add(new ValidateTranslationCommand()); $command = $application->find('validate-translations'); $commandTester = new CommandTester($command); - $commandTester->execute([ + $commandTester->execute( + [ 'path' => [__DIR__.'/../Fixtures/translations/xliff/success'], '--format' => 'json', - ]); + ] + );
139-154: Fix code style violations in the file detector test.The test logic is sound and properly validates the file detector functionality.
Apply this diff to fix the formatting and documentation issues:
+ /** + * Test command execution with valid file detector. + */ public function testExecuteWithValidFileDetector(): void { $application = new Application(); $application->add(new ValidateTranslationCommand()); $command = $application->find('validate-translations'); $commandTester = new CommandTester($command); - $commandTester->execute([ + $commandTester->execute( + [ 'path' => [__DIR__.'/../Fixtures/translations/xliff/success'], '--file-detector' => \MoveElevator\ComposerTranslationValidator\FileDetector\PrefixFileDetector::class, - ]); + ] + );
156-171: Fix code style violations in the verbose output test.The test correctly validates verbose output functionality with error scenarios.
Apply this diff to fix the formatting and documentation issues:
+ /** + * Test command execution with errors and verbose output. + */ public function testExecuteWithErrorsAndVerboseOutput(): void { $application = new Application(); $application->add(new ValidateTranslationCommand()); $command = $application->find('validate-translations'); $commandTester = new CommandTester($command); - $commandTester->execute([ + $commandTester->execute( + [ 'path' => [__DIR__.'/../Fixtures/translations/xliff/fail'], - ], ['verbosity' => \Symfony\Component\Console\Output\OutputInterface::VERBOSITY_VERBOSE]); + ], + ['verbosity' => \Symfony\Component\Console\Output\OutputInterface::VERBOSITY_VERBOSE] + );tests/src/Validator/MismatchValidatorTest.php (1)
196-205: Fix code style violations in the new test method.The test logic correctly validates the supported parsers functionality.
Apply this diff to fix the formatting and documentation issues:
+ /** + * Test that the validator supports the expected parsers. + */ public function testSupportsParser(): void { $logger = $this->createMock(LoggerInterface::class); $validator = new MismatchValidator($logger); - $this->assertSame([ + $this->assertSame( + [ \MoveElevator\ComposerTranslationValidator\Parser\XliffParser::class, \MoveElevator\ComposerTranslationValidator\Parser\YamlParser::class, - ], $validator->supportsParser()); + ], + $validator->supportsParser() + );tests/src/Validator/AbstractValidatorTest.php (3)
121-131: LGTM! Good test coverage for validation without issues.The test correctly validates the scenario where no issues are found and the validator returns an empty result.
Consider adding a doc comment:
+ /** + * Test validation with no issues found. + */ public function testValidateWithNoIssues(): void
133-154: LGTM! Comprehensive test for validation with issues.The test correctly validates the structured return format when issues are found, including file, issues, parser, and type metadata.
Consider adding a doc comment:
+ /** + * Test validation with issues found. + */ public function testValidateWithIssues(): void
156-166: LGTM! Appropriate test for debug logging.The test ensures that debug logging is triggered during validation, providing good coverage of the logging functionality.
Consider adding a doc comment:
+ /** + * Test that debug logging is called during validation. + */ public function testValidateWithDebugLogging(): void
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/Validator/AbstractValidator.php(2 hunks)src/Validator/MismatchValidator.php(2 hunks)tests/src/Command/ValidateTranslationCommandTest.php(1 hunks)tests/src/FileDetector/CollectorTest.php(1 hunks)tests/src/Parser/ParserRegistryTest.php(1 hunks)tests/src/Utility/PathUtilityTest.php(1 hunks)tests/src/Validator/AbstractValidatorTest.php(3 hunks)tests/src/Validator/DuplicatesValidatorTest.php(2 hunks)tests/src/Validator/MismatchValidatorTest.php(5 hunks)tests/src/Validator/SchemaValidatorTest.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
tests/src/Utility/PathUtilityTest.php (1)
src/Utility/PathUtility.php (2)
PathUtility(7-33)normalizeFolderPath(9-32)
tests/src/FileDetector/CollectorTest.php (1)
src/FileDetector/Collector.php (2)
Collector(11-65)collectFiles(25-64)
src/Validator/AbstractValidator.php (3)
src/Parser/AbstractParser.php (1)
getFileName(28-31)src/Parser/ParserInterface.php (1)
getFileName(23-23)src/Validator/MismatchValidator.php (1)
postProcess(37-67)
🪛 phpcs (3.7.2)
tests/src/Parser/ParserRegistryTest.php
[ERROR] 23-23: Missing doc comment for function testResolveParserClass()
(PEAR.Commenting.FunctionComment.Missing)
tests/src/Utility/PathUtilityTest.php
[ERROR] 87-87: Missing doc comment for function testNormalizeFolderPathWithUnrelatedPathWithoutTrailingSlash()
(PEAR.Commenting.FunctionComment.Missing)
tests/src/FileDetector/CollectorTest.php
[ERROR] 108-108: Missing doc comment for function testCollectFilesWithNoMatchingFilesAndVerboseOutput()
(PEAR.Commenting.FunctionComment.Missing)
tests/src/Command/ValidateTranslationCommandTest.php
[ERROR] 116-116: Missing doc comment for function testExecuteWithJsonFormat()
(PEAR.Commenting.FunctionComment.Missing)
[ERROR] 124-124: Opening parenthesis of a multi-line function call must be the last content on the line
(PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket)
[ERROR] 127-127: Closing parenthesis of a multi-line function call must be on a line by itself
(PEAR.Functions.FunctionCallSignature.CloseBracketLine)
[ERROR] 139-139: Missing doc comment for function testExecuteWithValidFileDetector()
(PEAR.Commenting.FunctionComment.Missing)
[ERROR] 147-147: Opening parenthesis of a multi-line function call must be the last content on the line
(PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket)
[ERROR] 150-150: Closing parenthesis of a multi-line function call must be on a line by itself
(PEAR.Functions.FunctionCallSignature.CloseBracketLine)
[ERROR] 156-156: Missing doc comment for function testExecuteWithErrorsAndVerboseOutput()
(PEAR.Commenting.FunctionComment.Missing)
[ERROR] 164-164: Opening parenthesis of a multi-line function call must be the last content on the line
(PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket)
[ERROR] 166-166: Closing parenthesis of a multi-line function call must be on a line by itself
(PEAR.Functions.FunctionCallSignature.CloseBracketLine)
tests/src/Validator/AbstractValidatorTest.php
[ERROR] 45-45: Missing doc comment for function renderIssueSets()
(PEAR.Commenting.FunctionComment.Missing)
[ERROR] 50-50: Missing doc comment for function postProcess()
(PEAR.Commenting.FunctionComment.Missing)
[ERROR] 121-121: Missing doc comment for function testValidateWithNoIssues()
(PEAR.Commenting.FunctionComment.Missing)
[ERROR] 133-133: Missing doc comment for function testValidateWithIssues()
(PEAR.Commenting.FunctionComment.Missing)
[ERROR] 156-156: Missing doc comment for function testValidateWithDebugLogging()
(PEAR.Commenting.FunctionComment.Missing)
src/Validator/AbstractValidator.php
[ERROR] 82-82: Missing doc comment for function postProcess()
(PEAR.Commenting.FunctionComment.Missing)
src/Validator/MismatchValidator.php
[ERROR] 69-69: Missing short description in doc comment
(Generic.Commenting.DocComment.MissingShort)
[ERROR] 69-69: Doc comment for parameter "$input" missing
(PEAR.Commenting.FunctionComment.MissingParamTag)
[ERROR] 69-69: Doc comment for parameter "$output" missing
(PEAR.Commenting.FunctionComment.MissingParamTag)
[ERROR] 69-69: Doc comment for parameter "$issueSets" missing
(PEAR.Commenting.FunctionComment.MissingParamTag)
[ERROR] 70-70: Missing parameter name
(PEAR.Commenting.FunctionComment.MissingParamName)
[ERROR] 77-77: Missing @return tag in function comment
(PEAR.Commenting.FunctionComment.MissingReturn)
tests/src/Validator/DuplicatesValidatorTest.php
[ERROR] 117-117: Missing doc comment for function testSupportsParser()
(PEAR.Commenting.FunctionComment.Missing)
tests/src/Validator/MismatchValidatorTest.php
[ERROR] 196-196: Missing doc comment for function testSupportsParser()
(PEAR.Commenting.FunctionComment.Missing)
[ERROR] 201-201: Opening parenthesis of a multi-line function call must be the last content on the line
(PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket)
[ERROR] 204-204: Closing parenthesis of a multi-line function call must be on a line by itself
(PEAR.Functions.FunctionCallSignature.CloseBracketLine)
tests/src/Validator/SchemaValidatorTest.php
[ERROR] 187-187: Missing doc comment for function testSupportsParser()
(PEAR.Commenting.FunctionComment.Missing)
🪛 PHPMD (2.15.0)
tests/src/Parser/ParserRegistryTest.php
25-25: Avoid using static access to class '\MoveElevator\ComposerTranslationValidator\Parser\ParserRegistry' in method 'testResolveParserClass'. (Clean Code Rules)
(StaticAccess)
26-26: Avoid using static access to class '\MoveElevator\ComposerTranslationValidator\Parser\ParserRegistry' in method 'testResolveParserClass'. (Clean Code Rules)
(StaticAccess)
27-27: Avoid using static access to class '\MoveElevator\ComposerTranslationValidator\Parser\ParserRegistry' in method 'testResolveParserClass'. (Clean Code Rules)
(StaticAccess)
tests/src/Utility/PathUtilityTest.php
91-91: Avoid using static access to class '\MoveElevator\ComposerTranslationValidator\Utility\PathUtility' in method 'testNormalizeFolderPathWithUnrelatedPathWithoutTrailingSlash'. (Clean Code Rules)
(StaticAccess)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Code Coverage Report
🔇 Additional comments (9)
tests/src/Validator/SchemaValidatorTest.php (1)
126-160: Well-structured test data for enhanced output format.The updated test data structure with
'file','issues','parser', and'type'keys provides richer metadata that supports the new JSON output format functionality. This change aligns well with the PR objectives.src/Validator/AbstractValidator.php (1)
58-64: Enhanced issue data structure supports JSON output format.The new structured format with
'file','issues','parser', and'type'keys provides comprehensive metadata that enables flexible output formatting, including the new JSON format. This change aligns perfectly with the PR objectives.tests/src/Validator/DuplicatesValidatorTest.php (1)
68-89: Consistent test data structure update.The updated test data structure matches the changes made across other validator tests and supports the new structured issue format for JSON output functionality.
tests/src/Validator/MismatchValidatorTest.php (3)
39-43: LGTM! Test expectations updated correctly.The test correctly reflects the new internal data structure where keys are stored as associative arrays with null values instead of simple arrays.
87-116: LGTM! Test expectations align with new issue structure.The updated test expectations correctly reflect the new structured format for issues, where each issue contains a 'key' and 'files' array with detailed file information.
180-181: LGTM! Output format update is consistent.The change from dash to
<missing>for missing values improves readability and aligns with the updated rendering logic.src/Validator/MismatchValidator.php (3)
29-32: LGTM! Enhanced data structure supports richer metadata.The refactored processFile method now stores key-value pairs instead of just keys, enabling better issue reporting with actual content values.
54-64: LGTM! Structured issue format supports JSON output.The new issue structure with 'key' and 'files' entries provides the necessary metadata for both CLI and JSON output formats, aligning with the PR objectives.
80-109: LGTM! Refactored rendering method handles new data structure correctly.The completely rewritten renderIssueSets method properly handles the new structured issue format and builds the table output correctly. The use of
<missing>for null values improves readability.
Summary by CodeRabbit
New Features
--formatoption.Bug Fixes
Refactor
Tests
Documentation