From 549c744c64391b705b9b149d97f0629b0fdb0369 Mon Sep 17 00:00:00 2001 From: Henrique Moody Date: Thu, 26 Dec 2024 20:05:11 +0100 Subject: [PATCH] Customize overwriting file and line in ValidationException I've already changed the `ValidationException` so as not to let the file and line from the Validator.php [1]. However, one could go even further when creating more customizations on top of this library, and allowing to customize the line could be very useful. What motivated me making this change because it will be handy when I get back to work on [Assertion][]. [1]: 75a9b8e94f9e74a6cf377e02b0b472eff0b1f2bd [Assertion]: https://github.com/Respect/Assertion --- library/Exceptions/ValidationException.php | 38 ++++++++++++++++--- library/Validator.php | 19 +++++++--- library/ValidatorDefaults.php | 14 +++++++ .../ValidationExceptionStackTraceTest.php | 36 +++++++++++++++++- tests/library/Stubs/MyValidator.php | 30 +++++++++++++++ 5 files changed, 125 insertions(+), 12 deletions(-) create mode 100644 tests/library/Stubs/MyValidator.php diff --git a/library/Exceptions/ValidationException.php b/library/Exceptions/ValidationException.php index 7343cc10a..226c1709c 100644 --- a/library/Exceptions/ValidationException.php +++ b/library/Exceptions/ValidationException.php @@ -11,20 +11,23 @@ use InvalidArgumentException; +use function array_shift; +use function in_array; use function realpath; final class ValidationException extends InvalidArgumentException implements Exception { - /** @param array $messages */ + /** + * @param array $messages + * @param array $ignoredBacktracePaths + */ public function __construct( string $message, private readonly string $fullMessage, private readonly array $messages, + array $ignoredBacktracePaths = [], ) { - if (realpath($this->file) === realpath(__DIR__ . '/../Validator.php')) { - $this->file = $this->getTrace()[0]['file'] ?? $this->file; - $this->line = $this->getTrace()[0]['line'] ?? $this->line; - } + $this->overwriteFileAndLine($ignoredBacktracePaths); parent::__construct($message); } @@ -38,4 +41,29 @@ public function getMessages(): array { return $this->messages; } + + /** @param array $ignoredBacktracePaths */ + private function overwriteFileAndLine(array $ignoredBacktracePaths = []): void + { + if ($ignoredBacktracePaths === []) { + return; + } + + $currentTrace = $this->getTrace(); + $currentFile = $this->file; + $currentLine = $this->line; + while (in_array(realpath($currentFile), $ignoredBacktracePaths, true)) { + $top = array_shift($currentTrace); + if ($top === false || !isset($top['file']) || !isset($top['line'])) { + $currentFile = $currentLine = null; + break; + } + + $currentFile = $top['file']; + $currentLine = $top['line']; + } + + $this->file = $currentFile ?? $this->file; + $this->line = $currentLine ?? $this->line; + } } diff --git a/library/Validator.php b/library/Validator.php index 71f540fdc..dfd380495 100644 --- a/library/Validator.php +++ b/library/Validator.php @@ -36,10 +36,12 @@ final class Validator implements Rule private ?string $template = null; + /** @param array $ignoredBacktracePaths */ public function __construct( private readonly Factory $factory, private readonly Formatter $formatter, private readonly Translator $translator, + private readonly array $ignoredBacktracePaths, ) { } @@ -48,7 +50,8 @@ public static function create(Rule ...$rules): self $validator = new self( ValidatorDefaults::getFactory(), ValidatorDefaults::getFormatter(), - ValidatorDefaults::getTranslator() + ValidatorDefaults::getTranslator(), + ValidatorDefaults::getIgnoredBacktracePaths() ); $validator->rules = $rules; @@ -89,7 +92,8 @@ public function assert(mixed $input, array|string|Throwable|callable|null $templ $exception = new ValidationException( $this->formatter->main($result, $templates, $this->translator), $this->formatter->full($result, $templates, $this->translator), - $this->formatter->array($result, $templates, $this->translator) + $this->formatter->array($result, $templates, $this->translator), + $this->ignoredBacktracePaths ); if (!is_callable($template)) { @@ -107,6 +111,13 @@ public function setTemplates(array $templates): self return $this; } + public function addRule(Rule $rule): self + { + $this->rules[] = $rule; + + return $this; + } + /** @return array */ public function getRules(): array { @@ -166,8 +177,6 @@ public static function __callStatic(string $ruleName, array $arguments): self */ public function __call(string $ruleName, array $arguments): self { - $this->rules[] = $this->factory->rule($ruleName, $arguments); - - return $this; + return $this->addRule($this->factory->rule($ruleName, $arguments)); } } diff --git a/library/ValidatorDefaults.php b/library/ValidatorDefaults.php index 57391e456..ec9aad515 100644 --- a/library/ValidatorDefaults.php +++ b/library/ValidatorDefaults.php @@ -22,6 +22,9 @@ final class ValidatorDefaults private static ?Translator $translator = null; + /** @var array */ + private static array $ignoredBacktracePaths = [__DIR__ . '/Validator.php']; + public static function setFactory(Factory $factory): void { self::$factory = $factory; @@ -63,4 +66,15 @@ public static function getTranslator(): Translator return self::$translator; } + + /** @return array*/ + public static function getIgnoredBacktracePaths(): array + { + return self::$ignoredBacktracePaths; + } + + public static function setIgnoredBacktracePaths(string ...$ignoredBacktracePaths): void + { + self::$ignoredBacktracePaths = $ignoredBacktracePaths; + } } diff --git a/tests/feature/ValidationExceptionStackTraceTest.php b/tests/feature/ValidationExceptionStackTraceTest.php index 6238111fe..2422825f5 100644 --- a/tests/feature/ValidationExceptionStackTraceTest.php +++ b/tests/feature/ValidationExceptionStackTraceTest.php @@ -8,8 +8,9 @@ declare(strict_types=1); use Respect\Validation\Exceptions\ValidationException; +use Respect\Validation\Test\Stubs\MyValidator; -test('Should overwrite stack trace when in Validator', function (): void { +test('Should overwrite file and line in the Validator class', function (): void { try { v::intType()->assert('string'); } catch (ValidationException $e) { @@ -18,7 +19,16 @@ } }); -test('Should not overwrite stack trace when created manually', function (): void { +test('Should overwrite file and line via the ValidatorDefaults class', function (): void { + try { + MyValidator::assertIntType('string'); + } catch (ValidationException $e) { + expect($e->getFile())->toBe(__FILE__); + expect($e->getLine())->toBe(__LINE__ - 3); + } +}); + +test('Should not overwrite file and line when created manually', function (): void { try { throw new ValidationException('message', 'fullMessage', ['id' => 'message']); } catch (ValidationException $e) { @@ -26,3 +36,25 @@ expect($e->getLine())->toBe(__LINE__ - 3); } }); + + +test('Should not overwrite file and line when file cannot be ever traced', function (): void { + try { + throw new ValidationException('message', 'fullMessage', ['id' => 'message'], ['/tmp/unknown']); + } catch (ValidationException $e) { + expect($e->getFile())->toBe(__FILE__); + expect($e->getLine())->toBe(__LINE__ - 3); + } +}); + + +test('Should go not overwrite file and line', function (): void { + try { + $trace = array_unique(array_filter(array_map(fn($trace) => $trace['file'] ?? null, debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS)))); + $trace[] = __FILE__; + throw new ValidationException('message', 'fullMessage', ['id' => 'message'], $trace); + } catch (ValidationException $e) { + expect($e->getFile())->toBe(__FILE__); + expect($e->getLine())->toBe(__LINE__ - 3); + } +}); diff --git a/tests/library/Stubs/MyValidator.php b/tests/library/Stubs/MyValidator.php new file mode 100644 index 000000000..3a328ad04 --- /dev/null +++ b/tests/library/Stubs/MyValidator.php @@ -0,0 +1,30 @@ + + * SPDX-License-Identifier: MIT + */ + +declare(strict_types=1); + +namespace Respect\Validation\Test\Stubs; + +use Respect\Validation\Exceptions\ValidationException; +use Respect\Validation\Validator; +use Respect\Validation\ValidatorDefaults; + +final class MyValidator +{ + public static function assertIntType(mixed $input): void + { + $originalIgnoredBacktracePaths = ValidatorDefaults::getIgnoredBacktracePaths(); + ValidatorDefaults::setIgnoredBacktracePaths(__FILE__, ...$originalIgnoredBacktracePaths); + try { + Validator::intType()->assert($input); + } catch (ValidationException $exception) { + // This is a workaround to avoid changing exceptions that are thrown in other places. + ValidatorDefaults::setIgnoredBacktracePaths(...$originalIgnoredBacktracePaths); + throw $exception; + } + } +}