Skip to content

Conversation

@apfelbox
Copy link
Member

@apfelbox apfelbox commented Jul 3, 2025

Please ignore the CI, as I will bump deps + update CI in the next PR.

@apfelbox apfelbox added the enhancement New feature or request label Jul 3, 2025
@apfelbox apfelbox requested review from 21christiansc and Copilot July 3, 2025 14:52
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ValidJsonVerifier and InvalidJsonElement classes to detect and report invalid JSON types.
  • Updates to SimpleNormalizer to invoke the verifier when in debug mode.
  • Tests in SimpleNormalizerTest covering 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 $isDebug but the validJsonVerifier argument 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 @return annotation is inaccurate: this method returns InvalidJsonElement|null, not a generic mixed. Please update it to @return InvalidJsonElement|null for clarity.
	 * @return mixed Returns null if everything is valid, otherwise the invalid value.

tests/Normalizer/SimpleNormalizerTest.php:64

  • [nitpick] This test only covers the normalize method. To ensure full coverage, consider data-driving this case for normalizeArray and normalizeMap, similar to the enabled tests.
	public function testJsonVerifierDisabled () : void

@21christiansc 21christiansc merged commit b4ccdc5 into 1.x Jul 3, 2025
1 check failed
@21christiansc 21christiansc deleted the validate-json branch July 3, 2025 15:17
21christiansc added a commit that referenced this pull request Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants