Add ValidJsonVerifier, that checks, that every normalizer correctly generated JSON-compatible values#13
Merged
21christiansc merged 1 commit into1.xfrom Jul 3, 2025
Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a debug-only mechanism to verify that normalized values are JSON-compatible, ensuring that only scalars, arrays, or empty objects are output. Key changes include:
- Addition of
ValidJsonVerifierandInvalidJsonElementclasses to detect and report invalid JSON types. - Updates to
SimpleNormalizerto invoke the verifier when in debug mode. - Tests in
SimpleNormalizerTestcovering enabled/disabled verifier and invalid-value exceptions. - Configuration update to pass the debug flag to
SimpleNormalizer.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Normalizer/SimpleNormalizerTest.php | New tests for JSON verifier enabled/disabled and error cases |
| src/Normalizer/Validator/ValidJsonVerifier.php | Implementation of JSON-type checker |
| src/Normalizer/Validator/InvalidJsonElement.php | Data holder for reporting invalid JSON elements |
| src/Normalizer/SimpleNormalizer.php | Hook into verifier in public API methods |
| src/Exception/IncompleteNormalizationException.php | New exception for incomplete normalization |
| config/services.yaml | Pass %kernel.debug% to SimpleNormalizer |
| CHANGELOG.md | Release notes for 1.4.0 feature |
Comments suppressed due to low confidence (3)
config/services.yaml:14
- The SimpleNormalizer service is configured with
$isDebugbut thevalidJsonVerifierargument is not wired. As written, debug mode won’t actually inject the verifier. Consider adding:
$validJsonVerifier: '@Torr\SimpleNormalizer\Normalizer\Validator\ValidJsonVerifier'
$isDebug: '%kernel.debug%'
src/Normalizer/Validator/ValidJsonVerifier.php:38
- The
@returnannotation is inaccurate: this method returnsInvalidJsonElement|null, not a genericmixed. Please update it to@return InvalidJsonElement|nullfor clarity.
* @return mixed Returns null if everything is valid, otherwise the invalid value.
tests/Normalizer/SimpleNormalizerTest.php:64
- [nitpick] This test only covers the
normalizemethod. To ensure full coverage, consider data-driving this case fornormalizeArrayandnormalizeMap, similar to the enabled tests.
public function testJsonVerifierDisabled () : void
… generated JSON-compatible values
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ignore the CI, as I will bump deps + update CI in the next PR.