Repository: CuyZ/Valinor (php)

Issue:
`Normalizer`: `float` precision
Hey there,

as we already had [issues](https://github.com/CuyZ/Valinor/issues/360) in valinor regarding `float` and `int` strict comparison, I wanted to bring this topic up.

I am not sure how different languages handle this, but I would assume that `int` can be a `float` as well, however, to prevent that, we sometimes used `JSON_PRESERVE_ZERO_FRACTION`.

Valinors `JsonNormalizer` is just casting floats to string which drops the zero fraction and converts floats to `int` in case they do not have decimals.

Should we either add a way to enable this behavior or is this ignored on purpose?
Current workaround is to use the array normalizer and `json_encode` on our own in combination with `JSON_PRESERVE_ZERO_FRACTION`.


Pull request:
`JsonNormalizer` accepting `json_encode` flags
As discussed in #476, this PR introduces a new `JsonNormalizer#withOptions` method accepting the following `JSON_*` constant values as an integer bitmask:

- `JSON_HEX_QUOT`
- `JSON_HEX_TAG`
- `JSON_HEX_AMP`
- `JSON_HEX_APOS`
- `JSON_INVALID_UTF8_IGNORE`
- `JSON_INVALID_UTF8_SUBSTITUTE`
- `JSON_NUMERIC_CHECK`
- `JSON_PRESERVE_ZERO_FRACTION`
- `JSON_UNESCAPED_LINE_TERMINATORS`
- `JSON_UNESCAPED_SLASHES`
- `JSON_UNESCAPED_UNICODE`

Since we should always enable `JSON_THROW_ON_ERROR`, I haven't added that one to the possible constants as it is enabled by-default.

Closes #476

Patch:
diff --git a/docs/pages/serialization/normalizing-json.md b/docs/pages/serialization/normalizing-json.md
index b341b4d..a935d92 100644
--- a/docs/pages/serialization/normalizing-json.md
+++ b/docs/pages/serialization/normalizing-json.md
@@ -72,8 +72,54 @@ $normalizer = (new \CuyZ\Valinor\MapperBuilder())
 $normalizer->normalize($users);
 ```
 
+## Passing `json_encode` flags
+
+By default, the JSON normalizer will only use `JSON_THROW_ON_ERROR` to encode
+non-boolean scalar values. There might be use-cases where projects will need
+flags like `JSON_JSON_PRESERVE_ZERO_FRACTION`.
+
+This can be achieved by passing these flags to the
+`JsonNormalizer::withOptions()` method:
+
+```php
+namespace My\App;
+
+$normalizer = (new \CuyZ\Valinor\MapperBuilder())
+    ->normalizer(\CuyZ\Valinor\Normalizer\Format::json())
+    ->withOptions(\JSON_PRESERVE_ZERO_FRACTION);
+
+$lowerManhattanAsJson = $normalizer->normalize(
+    new \My\App\Coordinates(
+        longitude: 40.7128,
+        latitude: -74.0000
+    )
+);
+
+// `$lowerManhattanAsJson` is a valid JSON string representing the data:
+// {"longitude":40.7128,"latitude":-74.0000}
+```
+
+The method accepts an int-mask of the following `JSON_*` constant
+representations ([see official doc for more information]):
+
+- `JSON_HEX_QUOT`
+- `JSON_HEX_TAG`
+- `JSON_HEX_AMP`
+- `JSON_HEX_APOS`
+- `JSON_INVALID_UTF8_IGNORE`
+- `JSON_INVALID_UTF8_SUBSTITUTE`
+- `JSON_NUMERIC_CHECK`
+- `JSON_PRESERVE_ZERO_FRACTION`
+- `JSON_UNESCAPED_LINE_TERMINATORS`
+- `JSON_UNESCAPED_SLASHES`
+- `JSON_UNESCAPED_UNICODE`
+
+`JSON_THROW_ON_ERROR` is always enforced and thus is not accepted.
+
 [default transformations]: normalizer.md#supported-transformations
 
 [registered transformers]: extending-normalizer.md
 
 [can be streamed to a PHP resource]: #streaming-to-a-php-resource
+
+[see official doc for more information]: https://www.php.net/manual/en/json.constants.php
diff --git a/src/Normalizer/Formatter/JsonFormatter.php b/src/Normalizer/Formatter/JsonFormatter.php
index 930daf9..feec835 100644
--- a/src/Normalizer/Formatter/JsonFormatter.php
+++ b/src/Normalizer/Formatter/JsonFormatter.php
@@ -16,6 +16,8 @@ use function is_null;
 use function is_scalar;
 use function json_encode;
 
+use const JSON_THROW_ON_ERROR;
+
 /** @internal */
 final class JsonFormatter implements StreamFormatter
 {
@@ -24,6 +26,7 @@ final class JsonFormatter implements StreamFormatter
      */
     public function __construct(
         private mixed $resource,
+        private int $jsonEncodingOptions,
     ) {}
 
     public function format(mixed $value): void
@@ -33,7 +36,11 @@ final class JsonFormatter implements StreamFormatter
         } elseif (is_bool($value)) {
             $this->write($value ? 'true' : 'false');
         } elseif (is_scalar($value)) {
-            $this->write(json_encode($value, JSON_THROW_ON_ERROR));
+            /**
+             * @phpstan-ignore-next-line / Due to the new json encoding options feature, it is not possible to let SA
+             *                             tools understand that JSON_THROW_ON_ERROR is always set.
+             */
+            $this->write(json_encode($value, $this->jsonEncodingOptions));
         } elseif (is_iterable($value)) {
             // Note: when a generator is formatted, it is considered as a list
             // if its first key is 0. This is done early because the first JSON
@@ -59,7 +66,9 @@ final class JsonFormatter implements StreamFormatter
                 $isFirst = false;
 
                 if (! $isList) {
-                    $this->write('"' . $key . '":');
+                    $key = json_encode((string)$key, $this->jsonEncodingOptions);
+
+                    $this->write($key . ':');
                 }
 
                 $this->format($val);
diff --git a/src/Normalizer/JsonNormalizer.php b/src/Normalizer/JsonNormalizer.php
index b8716f2..872955e 100644
--- a/src/Normalizer/JsonNormalizer.php
+++ b/src/Normalizer/JsonNormalizer.php
@@ -6,7 +6,6 @@ namespace CuyZ\Valinor\Normalizer;
 
 use CuyZ\Valinor\Normalizer\Formatter\JsonFormatter;
 use CuyZ\Valinor\Normalizer\Transformer\RecursiveTransformer;
-
 use RuntimeException;
 
 use function fclose;
@@ -15,6 +14,19 @@ use function get_debug_type;
 use function is_resource;
 use function stream_get_contents;
 
+use const JSON_HEX_AMP;
+use const JSON_HEX_APOS;
+use const JSON_HEX_QUOT;
+use const JSON_HEX_TAG;
+use const JSON_INVALID_UTF8_IGNORE;
+use const JSON_INVALID_UTF8_SUBSTITUTE;
+use const JSON_NUMERIC_CHECK;
+use const JSON_PRESERVE_ZERO_FRACTION;
+use const JSON_THROW_ON_ERROR;
+use const JSON_UNESCAPED_LINE_TERMINATORS;
+use const JSON_UNESCAPED_SLASHES;
+use const JSON_UNESCAPED_UNICODE;
+
 /**
  * @api
  *
@@ -22,9 +34,71 @@ use function stream_get_contents;
  */
 final class JsonNormalizer implements Normalizer
 {
+    private const ACCEPTABLE_JSON_OPTIONS = JSON_HEX_QUOT
+    | JSON_HEX_TAG
+    | JSON_HEX_AMP
+    | JSON_HEX_APOS
+    | JSON_INVALID_UTF8_IGNORE
+    | JSON_INVALID_UTF8_SUBSTITUTE
+    | JSON_NUMERIC_CHECK
+    | JSON_PRESERVE_ZERO_FRACTION
+    | JSON_UNESCAPED_LINE_TERMINATORS
+    | JSON_UNESCAPED_SLASHES
+    | JSON_UNESCAPED_UNICODE
+    | JSON_THROW_ON_ERROR;
+
+    private RecursiveTransformer $transformer;
+
+    private int $jsonEncodingOptions;
+
+    /**
+     * Internal note
+     * -------------
+     *
+     * We could use the `int-mask-of<JsonNormalizer::JSON_*>` annotation
+     * to let PHPStan infer the type of the accepted options, but some caveats
+     * were found:
+     * - SA tools are not able to infer that we end up having only accepted
+     *   options. Might be fixed with https://github.com/phpstan/phpstan/issues/9384
+     *   for PHPStan but Psalm does have some (not all) issues as well.
+     * - Using this annotation provokes *severe* performance issues when
+     *   running PHPStan analysis, therefore it is preferable to avoid it.
+     */
     public function __construct(
-        private RecursiveTransformer $transformer,
-    ) {}
+        RecursiveTransformer $transformer,
+        int $jsonEncodingOptions = JSON_THROW_ON_ERROR,
+    ) {
+        $this->transformer = $transformer;
+        $this->jsonEncodingOptions = (self::ACCEPTABLE_JSON_OPTIONS & $jsonEncodingOptions) | JSON_THROW_ON_ERROR;
+    }
+
+    /**
+     * By default, the JSON normalizer will only use `JSON_THROW_ON_ERROR` to
+     * encode non-boolean scalar values. There might be use-cases where projects
+     * will need flags like `JSON_JSON_PRESERVE_ZERO_FRACTION`.
+     *
+     * This can be achieved by passing these flags to this method:
+     *
+     * ```php
+     * $normalizer = (new \CuyZ\Valinor\MapperBuilder())
+     *     ->normalizer(\CuyZ\Valinor\Normalizer\Format::json())
+     *     ->withOptions(\JSON_PRESERVE_ZERO_FRACTION);
+     *
+     * $lowerManhattanAsJson = $normalizer->normalize(
+     *     new \My\App\Coordinates(
+     *         longitude: 40.7128,
+     *         latitude: -74.0000
+     *     )
+     * );
+     *
+     * // `$lowerManhattanAsJson` is a valid JSON string representing the data:
+     * // {"longitude":40.7128,"latitude":-74.0000}
+     * ```
+     */
+    public function withOptions(int $options): self
+    {
+        return new self($this->transformer, $options);
+    }
 
     public function normalize(mixed $value): string
     {
@@ -33,7 +107,7 @@ final class JsonNormalizer implements Normalizer
         /** @var resource $resource */
         $resource = fopen('php://memory', 'w');
 
-        (new JsonFormatter($resource))->format($value);
+        (new JsonFormatter($resource, $this->jsonEncodingOptions))->format($value);
 
         rewind($resource);
 
@@ -80,6 +154,6 @@ final class JsonNormalizer implements Normalizer
             throw new RuntimeException('Expected a valid resource, got ' . get_debug_type($resource));
         }
 
-        return new StreamNormalizer($this->transformer, new JsonFormatter($resource));
+        return new StreamNormalizer($this->transformer, new JsonFormatter($resource, $this->jsonEncodingOptions));
     }
 }


Test patch:
diff --git a/tests/Integration/Normalizer/NormalizerTest.php b/tests/Integration/Normalizer/NormalizerTest.php
index d120799..64a4885 100644
--- a/tests/Integration/Normalizer/NormalizerTest.php
+++ b/tests/Integration/Normalizer/NormalizerTest.php
@@ -29,6 +29,9 @@ use Traversable;
 
 use function array_merge;
 
+use const JSON_HEX_TAG;
+use const JSON_THROW_ON_ERROR;
+
 final class NormalizerTest extends IntegrationTestCase
 {
     /**
@@ -42,6 +45,7 @@ final class NormalizerTest extends IntegrationTestCase
         string $expectedJson,
         array $transformers = [],
         array $transformerAttributes = [],
+        int $jsonEncodingOptions = JSON_THROW_ON_ERROR,
     ): void {
         $builder = $this->mapperBuilder();
 
@@ -56,7 +60,9 @@ final class NormalizerTest extends IntegrationTestCase
         }
 
         $arrayResult = $builder->normalizer(Format::array())->normalize($input);
-        $jsonResult = $builder->normalizer(Format::json())->normalize($input);
+        $jsonNormalizer = $builder->normalizer(Format::json())->withOptions($jsonEncodingOptions);
+
+        $jsonResult = $jsonNormalizer->normalize($input);
 
         self::assertSame($expectedArray, $arrayResult);
         self::assertSame($expectedJson, $jsonResult);
@@ -141,18 +147,20 @@ final class NormalizerTest extends IntegrationTestCase
 
         yield 'array of scalar' => [
             'input' => [
+                0 => 'first value',
                 'string' => 'foo',
                 'integer' => 42,
                 'float' => 1337.404,
                 'boolean' => true,
             ],
             'expected array' => [
+                0 => 'first value',
                 'string' => 'foo',
                 'integer' => 42,
                 'float' => 1337.404,
                 'boolean' => true,
             ],
-            'expected json' => '{"string":"foo","integer":42,"float":1337.404,"boolean":true}',
+            'expected json' => '{"0":"first value","string":"foo","integer":42,"float":1337.404,"boolean":true}',
         ];
 
         yield 'array with transformer' => [
@@ -303,13 +311,13 @@ final class NormalizerTest extends IntegrationTestCase
         ];
 
         yield 'date with default transformer' => [
-            'input' => new DateTimeImmutable('1971-11-08'),
+            'input' => new DateTimeImmutable('1971-11-08', new DateTimeZone('UTC')),
             'expected array' => '1971-11-08T00:00:00.000000+00:00',
             'expected json' => '"1971-11-08T00:00:00.000000+00:00"',
         ];
 
         yield 'date with transformer' => [
-            'input' => new DateTimeImmutable('1971-11-08'),
+            'input' => new DateTimeImmutable('1971-11-08', new DateTimeZone('UTC')),
             'expected array' => '1971-11-08',
             'expected json' => '"1971-11-08"',
             'transformers' => [
@@ -641,6 +649,28 @@ final class NormalizerTest extends IntegrationTestCase
                 SomeKeyTransformerInterface::class,
             ],
         ];
+
+        yield 'object with float property containing zero fraction' => [
+            'input' => new class () {
+                public function __construct(
+                    public float $value = 1.0,
+                ) {}
+            },
+            'expected array' => ['value' => 1.0],
+            'expected_json' => '{"value":1.0}',
+            'transformers' => [],
+            'transformerAttributes' => [],
+            'jsonEncodingOptions' => JSON_PRESERVE_ZERO_FRACTION,
+        ];
+
+        yield 'array with key and value containing ampersand' => [
+            'input' => ['foo&bar' => 'bar&baz'],
+            'expected array' => ['foo&bar' => 'bar&baz'],
+            'expected_json' => '{"foo\u0026bar":"bar\u0026baz"}',
+            'transformers' => [],
+            'transformerAttributes' => [],
+            'jsonEncodingOptions' => JSON_HEX_AMP,
+        ];
     }
 
     public function test_generator_of_scalar_yields_expected_array(): void
@@ -1036,6 +1066,33 @@ final class NormalizerTest extends IntegrationTestCase
         // @phpstan-ignore-next-line
         $this->mapperBuilder()->normalizer(Format::json())->streamTo('foo');
     }
+
+    public function test_json_transformer_will_always_throw_on_error(): void
+    {
+        $normalizer = $this->mapperBuilder()->normalizer(Format::json());
+        self::assertSame(JSON_THROW_ON_ERROR, (fn () => $this->jsonEncodingOptions)->call($normalizer));
+
+        $normalizer = $normalizer->withOptions(JSON_HEX_TAG);
+        self::assertSame(JSON_THROW_ON_ERROR | JSON_HEX_TAG, (fn () => $this->jsonEncodingOptions)->call($normalizer));
+
+        $normalizer = $normalizer->withOptions(JSON_HEX_TAG & ~JSON_THROW_ON_ERROR);
+        self::assertSame(JSON_THROW_ON_ERROR | JSON_HEX_TAG, (fn () => $this->jsonEncodingOptions)->call($normalizer));
+    }
+
+    public function test_json_transformer_only_accepts_acceptable_json_options(): void
+    {
+        $normalizer = $this->mapperBuilder()->normalizer(Format::json())->withOptions(JSON_FORCE_OBJECT);
+        self::assertSame(JSON_THROW_ON_ERROR, (fn () => $this->jsonEncodingOptions)->call($normalizer));
+
+        $normalizer = $this->mapperBuilder()->normalizer(Format::json())->withOptions(JSON_PARTIAL_OUTPUT_ON_ERROR);
+        self::assertSame(JSON_THROW_ON_ERROR, (fn () => $this->jsonEncodingOptions)->call($normalizer));
+
+        $normalizer = $this->mapperBuilder()->normalizer(Format::json())->withOptions(JSON_PRETTY_PRINT);
+        self::assertSame(JSON_THROW_ON_ERROR, (fn () => $this->jsonEncodingOptions)->call($normalizer));
+
+        $normalizer = $this->mapperBuilder()->normalizer(Format::json())->withOptions(JSON_FORCE_OBJECT | JSON_PARTIAL_OUTPUT_ON_ERROR | JSON_PRETTY_PRINT);
+        self::assertSame(JSON_THROW_ON_ERROR, (fn () => $this->jsonEncodingOptions)->call($normalizer));
+    }
 }
 
 final class BasicObject
diff --git a/tests/Unit/Normalizer/Formatter/JsonFormatterTest.php b/tests/Unit/Normalizer/Formatter/JsonFormatterTest.php
index 47c530d..2af117c 100644
--- a/tests/Unit/Normalizer/Formatter/JsonFormatterTest.php
+++ b/tests/Unit/Normalizer/Formatter/JsonFormatterTest.php
@@ -10,6 +10,8 @@ use PHPUnit\Framework\TestCase;
 
 use function fopen;
 
+use const JSON_THROW_ON_ERROR;
+
 final class JsonFormatterTest extends TestCase
 {
     public function test_invalid_closure_type_given_to_formatter_throws_exception(): void
@@ -21,6 +23,6 @@ final class JsonFormatterTest extends TestCase
         /** @var resource $resource */
         $resource = fopen('php://memory', 'r+');
 
-        (new JsonFormatter($resource))->format(fn () => 42);
+        (new JsonFormatter($resource, JSON_THROW_ON_ERROR))->format(fn () => 42);
     }
 }