feat: implement output format handling with CLI and JSON renderers#14
feat: implement output format handling with CLI and JSON renderers#14konradmichalik merged 7 commits intomainfrom
Conversation
|
Warning Rate limit exceeded@jackd248 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe changes introduce a new, centralized output rendering system for translation validation results. Output format handling is refactored to use a strongly typed enum and dedicated renderer classes for CLI and JSON formats. The command logic is simplified by delegating result summarization to a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Command as ValidateTranslationCommand
participant Output
participant Renderer as CliRenderer/JsonRenderer
User->>Command: Execute validation command
Command->>Output: Create Output (with format, result, issues, etc.)
Command->>Output: Call summarize()
Output->>Renderer: Instantiate renderer (CLI or JSON) based on format
Output->>Renderer: Call renderResult()
Renderer->>Output: Render output, return exit code
Output->>Command: Return exit code
Command->>User: Return exit code and formatted output
Possibly related PRs
Poem
✨ 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: 5
🧹 Nitpick comments (5)
tests/src/Validator/AbstractValidatorTest.php (1)
45-48: Consider removing unnecessary method override.The
validatemethod override simply calls the parent implementation without modification. Unless this is needed for specific testing scenarios, consider removing it to reduce code noise.- public function validate(array $files, ?string $parserClass): array - { - return parent::validate($files, $parserClass); - }tests/src/Result/OutputTest.php (1)
67-80: Clarify the invalid renderer test logic.The test expects a
TypeErrorwhenFormatType::tryFrom('invalid')returns null, but this seems to test PHP's type system rather than the Output class's error handling logic. The static analysis confirms that null will be passed to the constructor.Consider testing the actual error handling within the
summarize()method instead:public function testSummarizeInvalidRendererClass(): void { - $this->expectException(\TypeError::class); - $this->expectExceptionMessage('must be of type MoveElevator\\ComposerTranslationValidator\\Result\\FormatType, null given'); + $this->loggerMock->expects($this->once()) + ->method('error') + ->with('Renderer class {class} does not exist.', $this->anything()); - new Output( + $output = new Output( $this->loggerMock, $this->output, $this->inputMock, - FormatType::tryFrom('invalid'), // This will be null, causing class_exists to fail + FormatType::CLI, // Use valid format but mock missing renderer class ResultType::SUCCESS, [] ); + + // Mock a scenario where the renderer class doesn't exist + $exitCode = $output->summarize(); + $this->assertSame(Command::FAILURE, $exitCode); }This would test the actual error handling logic rather than PHP's type system.
src/Result/CliRenderer.php (2)
46-48: Improve conditional formatting logic.The ternary operator for choosing between verbose and non-verbose output could be more readable and maintainable.
- $this->output->isVerbose() - ? $this->io->success($message) - : $this->output->writeln('<fg=green>'.$message.'</>'); + if ($this->output->isVerbose()) { + $this->io->success($message); + } else { + $this->output->writeln('<fg=green>'.$message.'</>'); + }
65-67: Verbose output placement could be improved.The verbose explanation is shown inside the path loop, which means it will be repeated for each path. Consider moving it outside the path loop to show it once per validator.
$this->io->section(sprintf('Validator: <fg=cyan>%s</>', $validator)); + if ($this->output->isVerbose()) { + $this->io->writeln(sprintf('Explanation: %s', $validatorInstance->explain())); + } foreach ($paths as $path => $sets) { - if ($this->output->isVerbose()) { - $this->io->writeln(sprintf('Explanation: %s', $validatorInstance->explain())); - }tests/src/Result/JsonRendererTest.php (1)
51-62: Extract duplicated test data to improve maintainability.The test issues array is duplicated across multiple test methods with only validator class names changing. Consider extracting this to a helper method or data provider.
+ private function createSampleIssues(string $validatorClass = 'MoveElevator\ComposerTranslationValidator\Validator\DuplicateKeysValidator'): array + { + return [ + $validatorClass => [ + 'file1.xlf' => [ + 'set1' => [ + 'file' => 'file1.xlf', + 'issues' => ['keyA' => 2], + 'parser' => 'MoveElevator\ComposerTranslationValidator\Parser\XliffParser', + 'type' => 'DuplicateKeysValidator', + ], + ], + ], + ]; + } public function testRenderResultError(): void { - $issues = [ - 'MoveElevator\ComposerTranslationValidator\Validator\DuplicateKeysValidator' => [ - 'file1.xlf' => [ - 'set1' => [ - 'file' => 'file1.xlf', - 'issues' => ['keyA' => 2], - 'parser' => 'MoveElevator\ComposerTranslationValidator\Parser\XliffParser', - 'type' => 'DuplicateKeysValidator', - ], - ], - ], - ]; + $issues = $this->createSampleIssues();Also applies to: 83-94, 116-127, 150-161
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/Command/ValidateTranslationCommand.php(3 hunks)src/Result/CliRenderer.php(1 hunks)src/Result/FormatType.php(1 hunks)src/Result/JsonRenderer.php(1 hunks)src/Result/Output.php(1 hunks)src/Result/RendererInterface.php(1 hunks)tests/src/Command/ValidateTranslationCommandTest.php(2 hunks)tests/src/Result/CliRendererTest.php(1 hunks)tests/src/Result/FormatTypeTest.php(1 hunks)tests/src/Result/JsonRendererTest.php(1 hunks)tests/src/Result/OutputTest.php(1 hunks)tests/src/Validator/AbstractValidatorTest.php(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/Result/RendererInterface.php (2)
src/Result/JsonRenderer.php (1)
renderResult(36-56)src/Result/CliRenderer.php (1)
renderResult(34-52)
tests/src/Validator/AbstractValidatorTest.php (1)
src/Validator/AbstractValidator.php (1)
validate(28-70)
tests/src/Command/ValidateTranslationCommandTest.php (1)
src/Command/ValidateTranslationCommand.php (2)
ValidateTranslationCommand(25-197)execute(88-179)
🪛 GitHub Actions: CGL
tests/src/Validator/AbstractValidatorTest.php
[error] 1-1: EditorConfig: This file has no final new line given.
tests/src/Command/ValidateTranslationCommandTest.php
[error] 1-1: EditorConfig: This file has no final new line given.
🪛 PHPStan (2.1.15)
tests/src/Result/FormatTypeTest.php
14-14: Call to method PHPUnit\Framework\Assert::assertSame() with 'cli' and 'cli' will always evaluate to true.
Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.
(method.alreadyNarrowedType)
15-15: Call to method PHPUnit\Framework\Assert::assertSame() with 'json' and 'json' will always evaluate to true.
Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.
(method.alreadyNarrowedType)
20-20: Call to method PHPUnit\Framework\Assert::assertSame() with MoveElevator\ComposerTranslationValidator\Result\FormatType::CLI and MoveElevator\ComposerTranslationValidator\Result\FormatType::CLI will always evaluate to true.
Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.
(method.alreadyNarrowedType)
21-21: Call to method PHPUnit\Framework\Assert::assertSame() with MoveElevator\ComposerTranslationValidator\Result\FormatType::JSON and MoveElevator\ComposerTranslationValidator\Result\FormatType::JSON will always evaluate to true.
Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.
(method.alreadyNarrowedType)
22-22: Call to method PHPUnit\Framework\Assert::assertNull() with null will always evaluate to true.
Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.
(method.alreadyNarrowedType)
tests/src/Result/OutputTest.php
76-76: Parameter #4 $format of class MoveElevator\ComposerTranslationValidator\Result\Output constructor expects MoveElevator\ComposerTranslationValidator\Result\FormatType, null given.
(argument.type)
🔇 Additional comments (26)
src/Result/FormatType.php (1)
7-11: Well-designed enum implementation.The string-backed enum provides excellent type safety for output format handling while maintaining string compatibility. This is a clean approach that prevents magic strings throughout the codebase.
src/Result/RendererInterface.php (1)
7-10: Clean interface design following SRP.The interface correctly abstracts the rendering responsibility with a clear contract. The return type of
intappropriately represents the command exit code.tests/src/Result/FormatTypeTest.php (1)
12-23: Comprehensive enum testing with expected static analysis warnings.The test methods properly verify enum value consistency and
tryFrom()behavior. The PHPStan warnings about "always evaluating to true" are false positives - these assertions are necessary to catch regressions if enum values change.tests/src/Validator/AbstractValidatorTest.php (2)
58-64: Reflection-based testing approach is acceptable.Using reflection to access and modify the private
issuesproperty is a pragmatic approach for testing thepostProcess()functionality. This allows proper validation of the post-processing behavior.
177-187: Well-structured test for post-processing behavior.The test properly verifies that issues added during post-processing are included in the final validation results. The test setup and assertions are clear and comprehensive.
src/Result/JsonRenderer.php (2)
13-27: Well-designed constructor with appropriate dependencies.The constructor properly injects all necessary dependencies for JSON rendering. The type annotations are comprehensive and the parameter documentation is helpful.
36-56: Excellent JSON rendering implementation.The method demonstrates several best practices:
- Proper JSON encoding flags (
JSON_THROW_ON_ERROR,JSON_PRETTY_PRINT,JSON_UNESCAPED_UNICODE)- Clear conditional logic for success/failure messaging
- Proper handling of dry-run mode messaging
- Consistent return of status code
The structured output format with status, message, and issues provides a clean API for consumers.
src/Result/Output.php (2)
15-33: Well-structured constructor with proper dependency injection.The constructor properly accepts all required dependencies and initializes the SymfonyStyle instance. The PHPDoc annotation for the
$issuesparameter clearly defines the expected array structure.
53-64: Renderer constructor signatures are consistent
src/Result/CliRenderer.phpandsrc/Result/JsonRenderer.phpboth define
public function __construct( LoggerInterface $logger, OutputInterface $output, InputInterface $input, ResultType $resultType, array $issues, bool $dryRun = false, bool $strict = false )
Since PHP interfaces cannot enforce constructors and both renderers match this signature, dynamic instantiation will work as expected. No changes required.tests/src/Result/CliRendererTest.php (4)
17-29: Excellent test setup with proper mocking and output buffering.The test setup properly creates mocks for dependencies and uses BufferedOutput to capture console output for assertions. This is a solid foundation for testing CLI output.
31-45: Clear test case for success scenario.The test properly validates both the exit code and output content for successful validation results.
47-78: Comprehensive error scenario test with realistic data structure.The test uses a realistic issues array structure that matches the expected format from the validators. The assertions properly verify both exit code and output content.
157-197: Good use of reflection for testing protected methods.Testing the protected
renderIssuesmethod via reflection ensures comprehensive coverage of the rendering logic. The verbosity setting and output assertion are appropriate.tests/src/Result/OutputTest.php (2)
31-46: Solid test for CLI format rendering.The test properly validates that the Output class delegates to the CLI renderer and returns the expected exit code and output content.
48-65: Comprehensive JSON format test with proper output validation.The test correctly verifies JSON output structure, ensuring the status, message, and other fields are properly formatted.
src/Command/ValidateTranslationCommand.php (4)
10-11: Good addition of necessary imports for the new output system.The imports for
FormatTypeandOutputare properly added to support the refactored output handling.
63-63: Excellent use of enum value as default option.Using
FormatType::CLI->valueinstead of a hardcoded string improves type safety and ensures consistency with the enum definition.
161-167: Robust format validation with clear error handling.The validation properly uses
FormatType::tryFrom()to parse the format option and provides a clear error message for invalid formats. This prevents runtime errors and gives users actionable feedback.
169-178: Clean delegation to centralized Output class.The refactoring successfully removes the complexity of output handling from the command and delegates it to a specialized class. All required dependencies are properly passed to the Output constructor.
tests/src/Command/ValidateTranslationCommandTest.php (4)
76-76: Good fix for the validator class name escaping.The escaped backslashes in the validator class name ensure proper handling of the namespace separator in the test.
173-194: Comprehensive test for JSON format with error scenarios.The test properly validates JSON output structure, including status, message, and issues fields. The assertions verify both the JSON content and the command exit code.
196-211: Excellent test for invalid format handling.This test verifies the new format validation logic, ensuring that invalid format options are properly rejected with appropriate error messages.
213-245: Good coverage for null option handling.These tests ensure that the command gracefully handles null values for optional parameters, which is important for robustness.
src/Result/CliRenderer.php (1)
19-21: Complex type annotation is well-documented.The multi-dimensional array type annotation clearly describes the expected structure of the issues parameter, which helps with maintainability and IDE support.
tests/src/Result/JsonRendererTest.php (2)
30-47: Comprehensive test coverage for success scenario.The test properly verifies all aspects of successful validation: exit code, JSON status, message, and empty issues array.
114-146: Well-designed test for strict mode behavior.The test correctly verifies that warnings become failures in strict mode, which is important for CI/CD pipelines that need strict validation.
| $validatorInstance = new $validator($this->logger); | ||
| /* @var ValidatorInterface $validatorInstance */ | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for dynamic validator instantiation.
The dynamic instantiation of validator classes without error handling could cause runtime exceptions if a validator class doesn't exist or has incompatible constructor requirements.
- $validatorInstance = new $validator($this->logger);
- /* @var ValidatorInterface $validatorInstance */
+ try {
+ $validatorInstance = new $validator($this->logger);
+ /** @var ValidatorInterface $validatorInstance */
+ } catch (\Exception $e) {
+ $this->logger->error(sprintf('Failed to instantiate validator %s: %s', $validator, $e->getMessage()));
+ continue;
+ }Also, the PHPDoc annotation should use /** instead of /* for proper documentation.
🤖 Prompt for AI Agents
In src/Result/CliRenderer.php around lines 60 to 62, add try-catch error
handling around the dynamic instantiation of the validator class to catch
exceptions if the class does not exist or the constructor fails. Also, change
the PHPDoc annotation from /* to /** to ensure proper documentation syntax.
| $exitCode = $renderer->renderResult(); | ||
|
|
||
| $this->assertSame(0, $exitCode); | ||
| $output = json_decode($this->output->fetch(), true); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for JSON decoding.
The JSON decoding operations don't check for potential parsing failures, which could cause false test passes if the output is malformed.
- $output = json_decode($this->output->fetch(), true);
+ $jsonOutput = $this->output->fetch();
+ $output = json_decode($jsonOutput, true);
+ $this->assertNotNull($output, 'Failed to decode JSON output: ' . $jsonOutput);Also applies to: 75-75, 108-108, 142-142, 176-176
🤖 Prompt for AI Agents
In tests/src/Result/JsonRendererTest.php at lines 43, 75, 108, 142, and 176, the
json_decode calls lack error handling for parsing failures. Update each
json_decode call to check if the result is null or if json_last_error() returns
an error, and add assertions or exceptions to fail the test if JSON decoding
fails, ensuring malformed output does not cause false test passes.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/src/Result/OutputTest.php (1)
39-39: Consider testing with actual validation data.Both tests pass an empty array as the last parameter (presumably validation results). Testing with actual validation data would provide more realistic test coverage and ensure the renderers handle different data scenarios correctly.
Consider adding test data like:
$validationErrors = [ 'Missing translation key: app.title', 'Invalid translation format in: app.description' ];Also applies to: 56-56
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/src/Command/ValidateTranslationCommandTest.php(2 hunks)tests/src/Result/JsonRendererTest.php(1 hunks)tests/src/Result/OutputTest.php(1 hunks)tests/src/Validator/AbstractValidatorTest.php(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/src/Validator/AbstractValidatorTest.php
- tests/src/Result/JsonRendererTest.php
- tests/src/Command/ValidateTranslationCommandTest.php
🔇 Additional comments (2)
tests/src/Result/OutputTest.php (2)
1-16: LGTM! Clean imports and proper namespace structure.The file header, strict types declaration, and imports are well-organized and follow PHP best practices.
17-29: LGTM! Proper test setup with appropriate mocking.The class structure and setUp method follow standard PHPUnit patterns. The use of
BufferedOutputfor capturing console output is the right approach for testing output formatting.
| public function testSummarizeJsonFormat(): void | ||
| { | ||
| $output = new Output( | ||
| $this->loggerMock, | ||
| $this->output, | ||
| $this->inputMock, | ||
| FormatType::JSON, | ||
| ResultType::SUCCESS, | ||
| [] | ||
| ); | ||
|
|
||
| $exitCode = $output->summarize(); | ||
|
|
||
| $this->assertSame(Command::SUCCESS, $exitCode); | ||
| $jsonOutput = json_decode($this->output->fetch(), true); | ||
| $this->assertSame(Command::SUCCESS, $jsonOutput['status']); | ||
| $this->assertSame('Language validation succeeded.', $jsonOutput['message']); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add JSON error handling and expand test coverage.
The JSON format test has two issues:
- Missing error handling for
json_decode() - Only tests success scenarios
Apply this diff to improve JSON error handling:
public function testSummarizeJsonFormat(): void
{
$output = new Output(
$this->loggerMock,
$this->output,
$this->inputMock,
FormatType::JSON,
ResultType::SUCCESS,
[]
);
$exitCode = $output->summarize();
$this->assertSame(Command::SUCCESS, $exitCode);
- $jsonOutput = json_decode($this->output->fetch(), true);
+ $jsonOutput = json_decode($this->output->fetch(), true);
+ $this->assertNotNull($jsonOutput, 'JSON output should be valid');
+ $this->assertJson($this->output->fetch(), 'Output should be valid JSON');
$this->assertSame(Command::SUCCESS, $jsonOutput['status']);
$this->assertSame('Language validation succeeded.', $jsonOutput['message']);
}Also consider adding a test for failure scenarios with validation errors to ensure the JSON renderer properly formats error messages.
📝 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.
| public function testSummarizeJsonFormat(): void | |
| { | |
| $output = new Output( | |
| $this->loggerMock, | |
| $this->output, | |
| $this->inputMock, | |
| FormatType::JSON, | |
| ResultType::SUCCESS, | |
| [] | |
| ); | |
| $exitCode = $output->summarize(); | |
| $this->assertSame(Command::SUCCESS, $exitCode); | |
| $jsonOutput = json_decode($this->output->fetch(), true); | |
| $this->assertSame(Command::SUCCESS, $jsonOutput['status']); | |
| $this->assertSame('Language validation succeeded.', $jsonOutput['message']); | |
| } | |
| public function testSummarizeJsonFormat(): void | |
| { | |
| $output = new Output( | |
| $this->loggerMock, | |
| $this->output, | |
| $this->inputMock, | |
| FormatType::JSON, | |
| ResultType::SUCCESS, | |
| [] | |
| ); | |
| $exitCode = $output->summarize(); | |
| $this->assertSame(Command::SUCCESS, $exitCode); | |
| $jsonOutput = json_decode($this->output->fetch(), true); | |
| $this->assertNotNull($jsonOutput, 'JSON output should be valid'); | |
| $this->assertJson($this->output->fetch(), 'Output should be valid JSON'); | |
| $this->assertSame(Command::SUCCESS, $jsonOutput['status']); | |
| $this->assertSame('Language validation succeeded.', $jsonOutput['message']); | |
| } |
🤖 Prompt for AI Agents
In tests/src/Result/OutputTest.php around lines 48 to 65, the
testSummarizeJsonFormat method lacks error handling for json_decode and only
tests success cases. Update the test to check for json_decode errors by
verifying the result is not null and json_last_error() returns JSON_ERROR_NONE.
Additionally, add a new test method that simulates failure scenarios with
validation errors, asserting that the JSON output correctly includes error
messages and the appropriate failure status.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/Result/CliRenderer.php (1)
62-62: Fix PHPDoc comment syntax.The comment should use proper PHPDoc syntax with
/**instead of/*for documentation.- /* @var ValidatorInterface $validatorInstance */ + /** @var ValidatorInterface $validatorInstance */
🧹 Nitpick comments (2)
src/Result/RendererFactory.php (1)
18-18: Consider removing null from return type annotation.The return type annotation indicates that
nullcan be returned, but the match expression only handles the two enum cases without a default case that returnsnull. SinceFormatTypeis an enum with defined values, this method should never returnnull.- * @return CliRenderer|JsonRenderer|null + * @return CliRenderer|JsonRenderersrc/Result/Output.php (1)
57-57: Use English for error messages.The error message is in German, which breaks consistency with the rest of the codebase that uses English.
- 'Renderer für das Format {format} existiert nicht.', + 'No renderer available for format {format}.',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Result/CliRenderer.php(1 hunks)src/Result/Output.php(1 hunks)src/Result/RendererFactory.php(1 hunks)tests/src/Result/JsonRendererTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/src/Result/JsonRendererTest.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Result/CliRenderer.php (5)
src/Utility/PathUtility.php (2)
PathUtility(7-33)normalizeFolderPath(9-32)src/Result/Output.php (2)
Output(15-66)__construct(22-33)tests/src/Validator/AbstractValidatorTest.php (3)
__construct(71-73)explain(40-43)renderIssueSets(50-53)src/Result/RendererInterface.php (1)
renderResult(9-9)src/Validator/ResultType.php (2)
notFullySuccessful(20-23)resolveErrorToCommandExitCode(25-36)
🪛 PHPStan (2.1.15)
src/Result/CliRenderer.php
63-63: Dead catch - Exception is never thrown in the try block.
(catch.neverThrown)
🪛 GitHub Check: cgl
src/Result/CliRenderer.php
[failure] 63-63:
Dead catch - Exception is never thrown in the try block.
🔇 Additional comments (5)
src/Result/RendererFactory.php (1)
20-34: Excellent factory implementation.Clean factory pattern using modern PHP match expression. The comprehensive parameter list and strong typing provide good maintainability and type safety.
src/Result/Output.php (1)
44-53: Good implementation of factory pattern addressing previous concerns.The use of
RendererFactory::create()successfully addresses the previous review feedback about avoiding dynamic class construction with string concatenation. This approach provides better type safety and maintainability.src/Result/CliRenderer.php (3)
60-66: The try-catch block is necessary despite static analysis warnings.The static analysis tool incorrectly flags this as a "dead catch" block. Dynamic class instantiation (
new $validator()) can throw exceptions in several scenarios:
- Class doesn't exist
- Constructor throws exceptions
- Class cannot be instantiated
The error handling is appropriate and necessary for robust operation.
34-52: Well-structured rendering logic with appropriate exit code handling.The method properly:
- Renders issues first
- Provides contextual success/failure messages based on dry-run mode
- Uses appropriate styling (warning vs error)
- Returns correct exit codes via
ResultType::resolveErrorToCommandExitCode()
57-85: Comprehensive issue rendering with good error handling.The method effectively:
- Handles validator instantiation errors gracefully
- Provides verbose explanations when requested
- Uses proper path normalization
- Delegates issue set rendering to validator instances
- Maintains good visual formatting with spacing
…output validation in OutputTest
Summary by CodeRabbit