From 88812cc4079d93826877a46ae8011e203dee22fa Mon Sep 17 00:00:00 2001 From: Toon Verwerft Date: Wed, 14 Oct 2026 18:54:19 +0200 Subject: [PATCH 1/5] Require the reflecta branch with the optional lenses veewee/reflecta#39 adds optional_index() and optional_property(), stacked on #37. Until 1.1.0 is tagged the branch comes in through a VCS repository aliased as 1.1.0, so this cannot be merged before the constraint is back on a release. --- composer.json | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 137a44f..9dffc2f 100644 --- a/composer.json +++ b/composer.json @@ -20,6 +20,12 @@ "email": "toonverwerft@gmail.com" } ], + "repositories": [ + { + "type": "vcs", + "url": "https://github.com/veewee/reflecta" + } + ], "require": { "php": "~8.4.0 || ~8.5.0", "php-standard-library/collection": "^6.1", @@ -34,7 +40,7 @@ "php-standard-library/str": "^6.1", "php-standard-library/type": "^6.1", "php-standard-library/vec": "^6.1", - "veewee/reflecta": "^1.0.0", + "veewee/reflecta": "dev-feature/optional-lenses as 1.1.0", "veewee/xml": "^4.10", "php-soap/engine": "^2.20", "php-soap/wsdl": "^1.19", From d6baa194be0080216570ae9f7e2c1bb57ca748a9 Mon Sep 17 00:00:00 2001 From: Toon Verwerft Date: Wed, 14 Oct 2026 18:57:26 +0200 Subject: [PATCH 2/5] Read absent elements and properties without throwing The built-in lenses were index() and property(), which throw for every absent element, attribute or property, and the exception was caught again by optional() or runLens. reflecta's optional_index() and optional_property() return null in those cases without throwing, which is what the caught exception turned into before, so they replace the built-in lenses unwrapped whether or not the property is optional. Custom lenses from ProvidesObjectEncoderLens and ProvidesObjectDecoderLens keep the optional() wrapping and the runLens catch. --- src/Encoder/ObjectAccess.php | 40 ++++---- tests/Fixture/Encoder/ThrowingLensEncoder.php | 73 +++++++++++++++ .../IntermediateThrowScenariosTest.php | 9 ++ .../Integration/NoIntermediateThrowsTest.php | 50 ++++++++++ .../Encoder/ObjectEncoderCustomLensTest.php | 93 +++++++++++++++++++ 5 files changed, 249 insertions(+), 16 deletions(-) create mode 100644 tests/Fixture/Encoder/ThrowingLensEncoder.php create mode 100644 tests/Integration/NoIntermediateThrowsTest.php create mode 100644 tests/Unit/Encoder/ObjectEncoderCustomLensTest.php diff --git a/src/Encoder/ObjectAccess.php b/src/Encoder/ObjectAccess.php index 64ebd84..4fdecf3 100644 --- a/src/Encoder/ObjectAccess.php +++ b/src/Encoder/ObjectAccess.php @@ -15,9 +15,9 @@ use VeeWee\Reflecta\Iso\IsoInterface; use VeeWee\Reflecta\Lens\LensInterface; use function Psl\Vec\sort_by; -use function VeeWee\Reflecta\Lens\index; use function VeeWee\Reflecta\Lens\optional; -use function VeeWee\Reflecta\Lens\property; +use function VeeWee\Reflecta\Lens\optional_index; +use function VeeWee\Reflecta\Lens\optional_property; final class ObjectAccess { @@ -89,8 +89,10 @@ private static function build(Context $context): self $shouldLensBeOptional = self::shouldLensBeOptional($propertyTypeMeta); $normalizedProperties[$normalizedName] = $property; - $encoderLenses[$normalizedName] = self::createEncoderLensForType($shouldLensBeOptional, $normalizedName, $encoder, $type, $property); - $decoderLenses[$normalizedName] = self::createDecoderLensForType($shouldLensBeOptional, $name, $encoder, $type, $property); + $encoderLenses[$normalizedName] = self::createCustomEncoderLens($shouldLensBeOptional, $encoder, $type, $property) + ?? optional_property($normalizedName); + $decoderLenses[$normalizedName] = self::createCustomDecoderLens($shouldLensBeOptional, $encoder, $type, $property) + ?? optional_index($name); $isos[$normalizedName] = $encoder->iso($propertyContext); $isAnyPropertyQualified = $isAnyPropertyQualified || $propertyTypeMeta->isQualified()->unwrapOr(false); @@ -106,40 +108,46 @@ private static function build(Context $context): self } /** - * @return LensInterface + * @return LensInterface|null */ - private static function createEncoderLensForType( + private static function createCustomEncoderLens( bool $shouldLensBeOptional, - string $normalizedName, XmlEncoder $encoder, Type $type, Property $property, - ): LensInterface { + ): ?LensInterface { $lens = match (true) { - $encoder instanceof Feature\DecoratingEncoder => self::createEncoderLensForType($shouldLensBeOptional, $normalizedName, $encoder->decoratedEncoder(), $type, $property), + $encoder instanceof Feature\DecoratingEncoder => self::createCustomEncoderLens($shouldLensBeOptional, $encoder->decoratedEncoder(), $type, $property), $encoder instanceof Feature\ProvidesObjectEncoderLens => $encoder::createObjectEncoderLens($type, $property), - default => property($normalizedName) + default => null, }; + if ($lens === null) { + return null; + } + return $shouldLensBeOptional ? optional($lens) : $lens; } /** - * @return LensInterface|null, array|null, mixed, mixed> + * @return LensInterface|null, array|null, mixed, mixed>|null */ - private static function createDecoderLensForType( + private static function createCustomDecoderLens( bool $shouldLensBeOptional, - string $name, XmlEncoder $encoder, Type $type, Property $property, - ): LensInterface { + ): ?LensInterface { $lens = match(true) { - $encoder instanceof Feature\DecoratingEncoder => self::createDecoderLensForType($shouldLensBeOptional, $name, $encoder->decoratedEncoder(), $type, $property), + $encoder instanceof Feature\DecoratingEncoder => self::createCustomDecoderLens($shouldLensBeOptional, $encoder->decoratedEncoder(), $type, $property), $encoder instanceof Feature\ProvidesObjectDecoderLens => $encoder::createObjectDecoderLens($type, $property), - default => index($name), + default => null, }; + if ($lens === null) { + return null; + } + return $shouldLensBeOptional ? optional($lens) : $lens; } diff --git a/tests/Fixture/Encoder/ThrowingLensEncoder.php b/tests/Fixture/Encoder/ThrowingLensEncoder.php new file mode 100644 index 0000000..4978b35 --- /dev/null +++ b/tests/Fixture/Encoder/ThrowingLensEncoder.php @@ -0,0 +1,73 @@ + + * @template-implements Feature\ProvidesObjectEncoderLens + * @template-implements Feature\ProvidesObjectDecoderLens + */ +final class ThrowingLensEncoder implements + Feature\DisregardXsiInformation, + Feature\ProvidesObjectDecoderLens, + Feature\ProvidesObjectEncoderLens, + XmlEncoder +{ + public static int $encoderLensReads = 0; + public static int $decoderLensReads = 0; + + /** + * @return Lens + */ + public static function createObjectEncoderLens(Type $parentType, Property $currentProperty): Lens + { + $lens = property($currentProperty->getName()); + + /** @var Lens */ + return Lens::readonly(static function (object $data) use ($lens): mixed { + self::$encoderLensReads++; + + return $lens->get($data); + }); + } + + /** + * @return Lens + */ + public static function createObjectDecoderLens(Type $parentType, Property $currentProperty): Lens + { + $lens = index($currentProperty->getName()); + + /** @var Lens */ + return Lens::readonly(static function (array $data) use ($lens): mixed { + self::$decoderLensReads++; + + return $lens->get($data); + }); + } + + /** + * @return Iso + */ + public function iso(Context $context): Iso + { + return new Iso( + static fn (?string $value): string => $value === null ? '' : '' . $value . '', + static fn (Element|string $xml): string => ($xml instanceof Element ? $xml : Element::fromString($xml))->element()->textContent, + ); + } +} diff --git a/tests/Integration/IntermediateThrowScenariosTest.php b/tests/Integration/IntermediateThrowScenariosTest.php index 6f1b0d2..325ee1f 100644 --- a/tests/Integration/IntermediateThrowScenariosTest.php +++ b/tests/Integration/IntermediateThrowScenariosTest.php @@ -141,6 +141,15 @@ public static function provideScenarios(): iterable ), ]; + yield 'encode object of a class that does not declare the properties' => [ + 'operation' => static fn (): string => $literal->encode('test', [new class { + public int $id = 1; + }])->getRequest(), + 'expected' => self::literalRequest( + '1
' + ), + ]; + yield 'encode uninitialised typed properties' => [ 'operation' => static function () use ($typed): string { $item = new Item(); diff --git a/tests/Integration/NoIntermediateThrowsTest.php b/tests/Integration/NoIntermediateThrowsTest.php new file mode 100644 index 0000000..3c862bd --- /dev/null +++ b/tests/Integration/NoIntermediateThrowsTest.php @@ -0,0 +1,50 @@ + + */ + public static function provideScenarios(): iterable + { + foreach (IntermediateThrowScenariosTest::provideScenarios() as $name => $scenario) { + if (!in_array($name, self::STILL_THROWING, true)) { + yield $name => $scenario; + } + } + } +} diff --git a/tests/Unit/Encoder/ObjectEncoderCustomLensTest.php b/tests/Unit/Encoder/ObjectEncoderCustomLensTest.php new file mode 100644 index 0000000..0bd3c7c --- /dev/null +++ b/tests/Unit/Encoder/ObjectEncoderCustomLensTest.php @@ -0,0 +1,93 @@ + + + + + + + + + + + XSD; + + protected function setUp(): void + { + ThrowingLensEncoder::$encoderLensReads = 0; + ThrowingLensEncoder::$decoderLensReads = 0; + } + + public function test_it_decodes_through_custom_lenses_that_throw_on_a_missing_element(): void + { + $actual = $this->iso()->from('1x'); + + static::assertEquals((object) ['id' => 1, 'custom' => null, 'any' => ['x']], $actual); + static::assertSame(1, ThrowingLensEncoder::$decoderLensReads); + } + + public function test_it_decodes_through_custom_lenses_when_the_element_is_present(): void + { + $actual = $this->iso()->from('1c'); + + static::assertEquals((object) ['id' => 1, 'custom' => 'c', 'any' => []], $actual); + static::assertSame(1, ThrowingLensEncoder::$decoderLensReads); + } + + public function test_it_encodes_through_custom_lenses_that_throw_on_a_missing_property(): void + { + $actual = $this->iso()->to((object) ['id' => 1, 'any' => ['x']]); + + static::assertSame('1x', $actual); + static::assertSame(1, ThrowingLensEncoder::$encoderLensReads); + } + + public function test_it_encodes_through_custom_lenses_when_the_property_is_present(): void + { + $actual = $this->iso()->to((object) ['id' => 1, 'custom' => 'c']); + + static::assertSame('1c', $actual); + static::assertSame(1, ThrowingLensEncoder::$encoderLensReads); + } + + /** + * @return \VeeWee\Reflecta\Iso\IsoInterface + */ + private function iso(): \VeeWee\Reflecta\Iso\IsoInterface + { + $metadata = self::createMetadataFromWsdl(self::SCHEMA, 'type="tns:user"', use: BindingUse::LITERAL); + $registry = EncoderRegistry::default() + ->addComplexTypeConverter('https://test', 'customType', new ThrowingLensEncoder()); + $context = new Context( + $metadata->getTypes()->fetchByNameAndXmlNamespace('user', 'https://test')->getXsdType(), + $metadata, + $registry, + self::buildNamespaces(), + ); + + return (new ObjectEncoder(stdClass::class))->iso($context); + } +} From aeab842aa0b5a8ff65315c3fa40a7444592de108 Mon Sep 17 00:00:00 2001 From: Toon Verwerft Date: Wed, 14 Oct 2026 18:59:13 +0200 Subject: [PATCH 3/5] Decode untyped scalars without throwing An untyped value went through Psl's int, float and bool coercion in turn and every miss threw a CoercionException, so a plain string threw three or four times. The decoder now checks the same rules up front: the int and float checks repeat Psl's string coercion, including its trimming of leading zeros that decodes '0-5' as -5. The test compares the result with Psl itself over every short string, so a Psl upgrade that changes those rules shows up as a failure. --- src/Encoder/SimpleType/ScalarTypeEncoder.php | 63 ++++--- .../Integration/NoIntermediateThrowsTest.php | 60 +++++- .../ScalarTypeEncoderUntypedDecodeTest.php | 175 ++++++++++++++++++ 3 files changed, 267 insertions(+), 31 deletions(-) create mode 100644 tests/Unit/Encoder/SimpleType/ScalarTypeEncoderUntypedDecodeTest.php diff --git a/src/Encoder/SimpleType/ScalarTypeEncoder.php b/src/Encoder/SimpleType/ScalarTypeEncoder.php index 0e5612d..55e398b 100644 --- a/src/Encoder/SimpleType/ScalarTypeEncoder.php +++ b/src/Encoder/SimpleType/ScalarTypeEncoder.php @@ -3,7 +3,6 @@ namespace Soap\Encoding\Encoder\SimpleType; -use Psl\Type; use Soap\Encoding\Encoder\Context; use Soap\Encoding\Encoder\XmlEncoder; use Soap\Encoding\Exception\RestrictionException; @@ -12,6 +11,8 @@ use function is_float; use function is_int; use function is_string; +use function ltrim; +use function preg_match; /** * @implements XmlEncoder @@ -41,36 +42,40 @@ public function iso(Context $context): Iso is_bool($value) => (new BoolTypeEncoder())->iso($context)->to($value), default => throw RestrictionException::unsupportedValueType($context->type, $value) }, - static function (string $value) use ($context): mixed { - try { - return Type\int()->coerce($value); - } catch (Type\Exception\CoercionException) { - } + self::decodeUntyped(...), + )); + } + + /** + * Decodes like coercing to int, float, the 'true' or 'false' literal and string in that order, without throwing on each miss. + * The int and float checks repeat Psl's IntType and FloatType coercion of a string. + */ + private static function decodeUntyped(string $value): int|float|bool|string + { + $int = (int) $value; + if ($value === (string) $int) { + return $int; + } - try { - return Type\float()->coerce($value); - } catch (Type\Exception\CoercionException) { - } + // Psl trims leading zeros before comparing, which also turns '0-5' into -5. + $trimmed = ltrim($value, '0'); + $int = (int) $trimmed; + if ($trimmed === (string) $int) { + return $int; + } - try { - return Type\converted( - Type\string(), - Type\bool(), - static fn (string $value): bool => match ($value) { - 'true' => true, - 'false' => false, - default => throw RestrictionException::unexpectedEnumType( - $context->type, - ['true', 'false'], - $value - ) - } - )->coerce($value); - } catch (Type\Exception\CoercionException) { - } + if ($trimmed === '' && $value !== '') { + return 0; + } - return Type\string()->coerce($value); - } - )); + if (preg_match('/^[+-]?(\d+([.]\d*)?([eE][+-]?\d+)?|[.]\d+([eE][+-]?\d+)?)$/', $value) === 1) { + return (float) $value; + } + + return match ($value) { + 'true' => true, + 'false' => false, + default => $value, + }; } } diff --git a/tests/Integration/NoIntermediateThrowsTest.php b/tests/Integration/NoIntermediateThrowsTest.php index 3c862bd..b0a3842 100644 --- a/tests/Integration/NoIntermediateThrowsTest.php +++ b/tests/Integration/NoIntermediateThrowsTest.php @@ -8,20 +8,28 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; +use Soap\Encoding\Encoder\SimpleType\ScalarTypeEncoder; +use Soap\Encoding\Encoder\SimpleType\SimpleListEncoder; +use Soap\Encoding\Encoder\SoapEnc\ApacheMapEncoder; +use Soap\Encoding\Encoder\SoapEnc\SoapObjectEncoder; use Soap\Encoding\Test\Support\Throws\AssertsNoThrows; +use Soap\Encoding\Test\Unit\ContextCreatorTrait; +use Soap\Engine\Metadata\Model\TypeMeta; +use Soap\Engine\Metadata\Model\XsdType; use function in_array; +use function var_export; #[CoversNothing] #[Group('no-throws')] final class NoIntermediateThrowsTest extends TestCase { use AssertsNoThrows; + use ContextCreatorTrait; /** - * The untyped scalar decode and the engine type lookup for soap-enc arrays of a builtin item type still throw. + * The engine type lookup for soap-enc arrays of a builtin item type still throws. */ private const STILL_THROWING = [ - 'decode untyped values', 'decode soap-enc array of a builtin type', ]; @@ -36,6 +44,54 @@ public function test_it_does_not_throw_internally(Closure $operation, mixed $exp static::assertNoThrows($operation); } + /** + * @param Closure(): mixed $decode + */ + #[DataProvider('provideUntypedDecodes')] + public function test_it_decodes_untyped_values_without_throwing(Closure $decode, mixed $expected): void + { + static::assertEquals($expected, $decode()); + + static::assertNoThrows($decode); + } + + /** + * @return iterable + */ + public static function provideUntypedDecodes(): iterable + { + $scalar = ScalarTypeEncoder::default()->iso(self::createContext(XsdType::any())); + foreach (['1.5' => 1.5, '1e3' => 1000.0, 'abc' => 'abc', '' => '', 'true' => true, 'false' => false, '12' => 12] as $value => $expected) { + $value = (string) $value; + yield 'scalar ' . var_export($value, true) => [ + 'decode' => static fn (): mixed => $scalar->from($value), + 'expected' => $expected, + ]; + } + + $list = (new SimpleListEncoder(ScalarTypeEncoder::default()))->iso(self::createContext( + XsdType::any()->withMeta(static fn (TypeMeta $meta): TypeMeta => $meta->withIsList(true)->withIsSimple(true)) + )); + yield 'simple list' => [ + 'decode' => static fn (): mixed => $list->from('1 1.5 abc true'), + 'expected' => [1, 1.5, 'abc', true], + ]; + + $map = (new ApacheMapEncoder())->iso(self::createContext(XsdType::guess('Map'))); + yield 'apache map values' => [ + 'decode' => static fn (): mixed => $map->from( + 'aabcb1.5' + ), + 'expected' => ['a' => 'abc', 'b' => 1.5], + ]; + + $object = (new SoapObjectEncoder())->iso(self::createContext(XsdType::guess('Struct'))); + yield 'soap-enc struct values' => [ + 'decode' => static fn (): mixed => $object->from('abc1.5false'), + 'expected' => (object) ['a' => 'abc', 'b' => 1.5, 'c' => false], + ]; + } + /** * @return iterable */ diff --git a/tests/Unit/Encoder/SimpleType/ScalarTypeEncoderUntypedDecodeTest.php b/tests/Unit/Encoder/SimpleType/ScalarTypeEncoderUntypedDecodeTest.php new file mode 100644 index 0000000..dd10fe0 --- /dev/null +++ b/tests/Unit/Encoder/SimpleType/ScalarTypeEncoderUntypedDecodeTest.php @@ -0,0 +1,175 @@ + + */ + public static function provideNamedCases(): iterable + { + yield 'zero' => ['0', 0]; + yield 'one is an int, not a bool' => ['1', 1]; + yield 'int' => ['7', 7]; + yield 'negative int' => ['-12', -12]; + yield 'int max' => ['9223372036854775807', PHP_INT_MAX]; + yield 'leading zeros' => ['007', 7]; + yield 'more leading zeros' => ['0012', 12]; + yield 'only zeros' => ['000', 0]; + yield 'two zeros' => ['00', 0]; + yield 'psl quirk: zeros before a minus are trimmed into a negative int' => ['0-5', -5]; + yield 'psl quirk with two zeros' => ['00-5', -5]; + yield 'negative with leading zero is a float' => ['-012', -12.0]; + yield 'plus sign is a float' => ['+5', 5.0]; + yield 'negative zero is a float' => ['-0', -0.0]; + yield 'above int max is a float' => ['9223372036854775808', 9223372036854775808.0]; + yield 'far above int max is a float' => ['99999999999999999999', 99999999999999999999.0]; + yield 'decimal' => ['1.5', 1.5]; + yield 'decimal without leading digit' => ['.5', 0.5]; + yield 'decimal without trailing digit' => ['5.', 5.0]; + yield 'exponent' => ['1e3', 1000.0]; + yield 'negative upper case exponent' => ['1E-3', 0.001]; + yield 'signed exponent' => ['+1.5e+2', 150.0]; + yield 'overflowing exponent' => ['1e999', INF]; + yield 'decimal with trailing newline' => ["1.5\n", 1.5]; + yield 'int with trailing newline is a float' => ["12\n", 12.0]; + yield 'leading space' => [' 12', ' 12']; + yield 'trailing space' => ['12 ', '12 ']; + yield 'leading tab' => ["\t1", "\t1"]; + yield 'underscore separator' => ['1_000', '1_000']; + yield 'hexadecimal' => ['0x1A', '0x1A']; + yield 'decimal comma' => ['1,5', '1,5']; + yield 'exponent without digits' => ['1e', '1e']; + yield 'exponent without mantissa' => ['e1', 'e1']; + yield 'double minus' => ['--1', '--1']; + yield 'plus' => ['+', '+']; + yield 'minus' => ['-', '-']; + yield 'dot' => ['.', '.']; + yield 'empty' => ['', '']; + yield 'true' => ['true', true]; + yield 'false' => ['false', false]; + yield 'upper case true stays a string' => ['TRUE', 'TRUE']; + yield 'capitalised false stays a string' => ['False', 'False']; + yield 'yes' => ['yes', 'yes']; + yield 'on' => ['on', 'on']; + yield 'null' => ['null', 'null']; + yield 'nan' => ['NaN', 'NaN']; + yield 'inf' => ['INF', 'INF']; + yield 'word' => ['abc', 'abc']; + } + + private static function decode(string $value): mixed + { + set_error_handler(static function (int $severity, string $message, string $file, int $line): never { + throw new ErrorException($message, 0, $severity, $file, $line); + }); + + try { + return ScalarTypeEncoder::default()->iso(self::createContext(XsdType::any()))->from($value); + } finally { + restore_error_handler(); + } + } + + private static function oracle(string $value): int|float|bool|string + { + try { + return Type\int()->coerce($value); + } catch (Type\Exception\CoercionException) { + } + + try { + return Type\float()->coerce($value); + } catch (Type\Exception\CoercionException) { + } + + return match ($value) { + 'true' => true, + 'false' => false, + default => $value, + }; + } + + /** + * @return list + */ + private static function sweep(): array + { + $values = ['']; + $layer = ['']; + for ($length = 1; $length <= self::SWEEP_MAX_LENGTH; $length++) { + $next = []; + foreach ($layer as $prefix) { + foreach (self::SWEEP_ALPHABET as $char) { + $next[] = $prefix . $char; + } + } + $values = [...$values, ...$next]; + $layer = $next; + } + + return $values; + } + + /** + * @return list + */ + private static function boundaries(): array + { + return [ + '9223372036854775807', + '9223372036854775808', + '-9223372036854775808', + '-9223372036854775809', + '09223372036854775807', + '+9223372036854775807', + '9223372036854775807.0', + '18446744073709551616', + ]; + } +} From 3b1f01fde29e9d99385625c368c3692f336550bc Mon Sep 17 00:00:00 2001 From: Toon Verwerft Date: Thu, 15 Oct 2026 08:47:10 +0200 Subject: [PATCH 4/5] Require the released veewee/reflecta 1.1 --- composer.json | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/composer.json b/composer.json index 9dffc2f..bd34deb 100644 --- a/composer.json +++ b/composer.json @@ -20,12 +20,6 @@ "email": "toonverwerft@gmail.com" } ], - "repositories": [ - { - "type": "vcs", - "url": "https://github.com/veewee/reflecta" - } - ], "require": { "php": "~8.4.0 || ~8.5.0", "php-standard-library/collection": "^6.1", @@ -40,7 +34,7 @@ "php-standard-library/str": "^6.1", "php-standard-library/type": "^6.1", "php-standard-library/vec": "^6.1", - "veewee/reflecta": "dev-feature/optional-lenses as 1.1.0", + "veewee/reflecta": "^1.1", "veewee/xml": "^4.10", "php-soap/engine": "^2.20", "php-soap/wsdl": "^1.19", From 6566d77dd23db3e892513abf8eba3daf23b3bee0 Mon Sep 17 00:00:00 2001 From: Toon Verwerft Date: Thu, 15 Oct 2026 09:14:00 +0200 Subject: [PATCH 5/5] Move the non-throwing scalar coercion into Coerce\Scalar The untyped decode lived as a private method on ScalarTypeEncoder. As its own class the int, float and bool steps can be reused by other encoders and tested one by one against their Psl counterparts. The class is internal, so it adds no BC surface. --- src/Coerce/Scalar.php | 66 ++++++ src/Encoder/SimpleType/ScalarTypeEncoder.php | 38 +--- tests/Unit/Coerce/ScalarTest.php | 200 ++++++++++++++++++ .../ScalarTypeEncoderUntypedDecodeTest.php | 157 +------------- 4 files changed, 279 insertions(+), 182 deletions(-) create mode 100644 src/Coerce/Scalar.php create mode 100644 tests/Unit/Coerce/ScalarTest.php diff --git a/src/Coerce/Scalar.php b/src/Coerce/Scalar.php new file mode 100644 index 0000000..42ea5b3 --- /dev/null +++ b/src/Coerce/Scalar.php @@ -0,0 +1,66 @@ +coerce() accepts. + */ + public static function int(string $value): ?int + { + $int = (int) $value; + if ($value === (string) $int) { + return $int; + } + + // Psl trims leading zeros before comparing, which also turns '0-5' into -5. + $trimmed = ltrim($value, '0'); + $int = (int) $trimmed; + if ($trimmed === (string) $int) { + return $int; + } + + if ($trimmed === '' && $value !== '') { + return 0; + } + + return null; + } + + /** + * Accepts exactly the strings Psl\Type\float()->coerce() accepts. + */ + public static function float(string $value): ?float + { + if (preg_match('/^[+-]?(\d+([.]\d*)?([eE][+-]?\d+)?|[.]\d+([eE][+-]?\d+)?)$/', $value) === 1) { + return (float) $value; + } + + return null; + } + + public static function bool(string $value): ?bool + { + return match ($value) { + 'true' => true, + 'false' => false, + default => null, + }; + } + + /** + * Returns the first of int, float and bool that accepts the value, else the value itself. + */ + public static function guess(string $value): int|float|bool|string + { + return self::int($value) ?? self::float($value) ?? self::bool($value) ?? $value; + } +} diff --git a/src/Encoder/SimpleType/ScalarTypeEncoder.php b/src/Encoder/SimpleType/ScalarTypeEncoder.php index 55e398b..22c6e91 100644 --- a/src/Encoder/SimpleType/ScalarTypeEncoder.php +++ b/src/Encoder/SimpleType/ScalarTypeEncoder.php @@ -3,6 +3,7 @@ namespace Soap\Encoding\Encoder\SimpleType; +use Soap\Encoding\Coerce\Scalar; use Soap\Encoding\Encoder\Context; use Soap\Encoding\Encoder\XmlEncoder; use Soap\Encoding\Exception\RestrictionException; @@ -11,8 +12,6 @@ use function is_float; use function is_int; use function is_string; -use function ltrim; -use function preg_match; /** * @implements XmlEncoder @@ -42,40 +41,7 @@ public function iso(Context $context): Iso is_bool($value) => (new BoolTypeEncoder())->iso($context)->to($value), default => throw RestrictionException::unsupportedValueType($context->type, $value) }, - self::decodeUntyped(...), + Scalar::guess(...), )); } - - /** - * Decodes like coercing to int, float, the 'true' or 'false' literal and string in that order, without throwing on each miss. - * The int and float checks repeat Psl's IntType and FloatType coercion of a string. - */ - private static function decodeUntyped(string $value): int|float|bool|string - { - $int = (int) $value; - if ($value === (string) $int) { - return $int; - } - - // Psl trims leading zeros before comparing, which also turns '0-5' into -5. - $trimmed = ltrim($value, '0'); - $int = (int) $trimmed; - if ($trimmed === (string) $int) { - return $int; - } - - if ($trimmed === '' && $value !== '') { - return 0; - } - - if (preg_match('/^[+-]?(\d+([.]\d*)?([eE][+-]?\d+)?|[.]\d+([eE][+-]?\d+)?)$/', $value) === 1) { - return (float) $value; - } - - return match ($value) { - 'true' => true, - 'false' => false, - default => $value, - }; - } } diff --git a/tests/Unit/Coerce/ScalarTest.php b/tests/Unit/Coerce/ScalarTest.php new file mode 100644 index 0000000..efcbe8a --- /dev/null +++ b/tests/Unit/Coerce/ScalarTest.php @@ -0,0 +1,200 @@ + Scalar::guess($value)), true)); + } + + /** + * @return iterable + */ + public static function provideMethods(): iterable + { + yield 'int' => [Scalar::int(...), self::intOracle(...)]; + yield 'float' => [Scalar::float(...), self::floatOracle(...)]; + yield 'bool' => [Scalar::bool(...), self::boolOracle(...)]; + yield 'guess' => [Scalar::guess(...), self::guessOracle(...)]; + } + + /** + * @param Closure(string): mixed $method + * @param Closure(string): mixed $oracle + */ + #[DataProvider('provideMethods')] + public function test_it_matches_its_oracle_for_every_short_string(Closure $method, Closure $oracle): void + { + $mismatches = []; + foreach ([...self::sweep(), ...self::boundaries()] as $value) { + $expected = var_export($oracle($value), true); + $actual = var_export(self::strict(static fn () => $method($value)), true); + if ($expected !== $actual) { + $mismatches[] = var_export($value, true) . ': expected ' . $expected . ', got ' . $actual; + } + } + + static::assertSame([], $mismatches); + } + + /** + * @return iterable + */ + public static function provideNamedCases(): iterable + { + yield 'zero' => ['0', 0]; + yield 'one is an int, not a bool' => ['1', 1]; + yield 'int' => ['7', 7]; + yield 'negative int' => ['-12', -12]; + yield 'int max' => ['9223372036854775807', PHP_INT_MAX]; + yield 'leading zeros' => ['007', 7]; + yield 'more leading zeros' => ['0012', 12]; + yield 'only zeros' => ['000', 0]; + yield 'two zeros' => ['00', 0]; + yield 'psl quirk: zeros before a minus are trimmed into a negative int' => ['0-5', -5]; + yield 'psl quirk with two zeros' => ['00-5', -5]; + yield 'negative with leading zero is a float' => ['-012', -12.0]; + yield 'plus sign is a float' => ['+5', 5.0]; + yield 'negative zero is a float' => ['-0', -0.0]; + yield 'above int max is a float' => ['9223372036854775808', 9223372036854775808.0]; + yield 'far above int max is a float' => ['99999999999999999999', 99999999999999999999.0]; + yield 'decimal' => ['1.5', 1.5]; + yield 'decimal without leading digit' => ['.5', 0.5]; + yield 'decimal without trailing digit' => ['5.', 5.0]; + yield 'exponent' => ['1e3', 1000.0]; + yield 'negative upper case exponent' => ['1E-3', 0.001]; + yield 'signed exponent' => ['+1.5e+2', 150.0]; + yield 'overflowing exponent' => ['1e999', INF]; + yield 'decimal with trailing newline' => ["1.5\n", 1.5]; + yield 'int with trailing newline is a float' => ["12\n", 12.0]; + yield 'leading space' => [' 12', ' 12']; + yield 'trailing space' => ['12 ', '12 ']; + yield 'leading tab' => ["\t1", "\t1"]; + yield 'underscore separator' => ['1_000', '1_000']; + yield 'hexadecimal' => ['0x1A', '0x1A']; + yield 'decimal comma' => ['1,5', '1,5']; + yield 'exponent without digits' => ['1e', '1e']; + yield 'exponent without mantissa' => ['e1', 'e1']; + yield 'double minus' => ['--1', '--1']; + yield 'plus' => ['+', '+']; + yield 'minus' => ['-', '-']; + yield 'dot' => ['.', '.']; + yield 'empty' => ['', '']; + yield 'true' => ['true', true]; + yield 'false' => ['false', false]; + yield 'upper case true stays a string' => ['TRUE', 'TRUE']; + yield 'capitalised false stays a string' => ['False', 'False']; + yield 'yes' => ['yes', 'yes']; + yield 'on' => ['on', 'on']; + yield 'null' => ['null', 'null']; + yield 'nan' => ['NaN', 'NaN']; + yield 'inf' => ['INF', 'INF']; + yield 'word' => ['abc', 'abc']; + } + + private static function strict(Closure $call): mixed + { + set_error_handler(static function (int $severity, string $message, string $file, int $line): never { + throw new ErrorException($message, 0, $severity, $file, $line); + }); + + try { + return $call(); + } finally { + restore_error_handler(); + } + } + + private static function intOracle(string $value): ?int + { + try { + return Type\int()->coerce($value); + } catch (Type\Exception\CoercionException) { + return null; + } + } + + private static function floatOracle(string $value): ?float + { + try { + return Type\float()->coerce($value); + } catch (Type\Exception\CoercionException) { + return null; + } + } + + private static function boolOracle(string $value): ?bool + { + return match ($value) { + 'true' => true, + 'false' => false, + default => null, + }; + } + + private static function guessOracle(string $value): int|float|bool|string + { + return self::intOracle($value) ?? self::floatOracle($value) ?? self::boolOracle($value) ?? $value; + } + + /** + * @return list + */ + private static function sweep(): array + { + $values = ['']; + $layer = ['']; + for ($length = 1; $length <= self::SWEEP_MAX_LENGTH; $length++) { + $next = []; + foreach ($layer as $prefix) { + foreach (self::SWEEP_ALPHABET as $char) { + $next[] = $prefix . $char; + } + } + $values = [...$values, ...$next]; + $layer = $next; + } + + return $values; + } + + /** + * @return list + */ + private static function boundaries(): array + { + return [ + '9223372036854775807', + '9223372036854775808', + '-9223372036854775808', + '-9223372036854775809', + '09223372036854775807', + '+9223372036854775807', + '9223372036854775807.0', + '18446744073709551616', + ]; + } +} diff --git a/tests/Unit/Encoder/SimpleType/ScalarTypeEncoderUntypedDecodeTest.php b/tests/Unit/Encoder/SimpleType/ScalarTypeEncoderUntypedDecodeTest.php index dd10fe0..6375778 100644 --- a/tests/Unit/Encoder/SimpleType/ScalarTypeEncoderUntypedDecodeTest.php +++ b/tests/Unit/Encoder/SimpleType/ScalarTypeEncoderUntypedDecodeTest.php @@ -3,173 +3,38 @@ namespace Soap\Encoding\Test\Unit\Encoder\SimpleType; -use ErrorException; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; -use Psl\Type; +use Soap\Encoding\Coerce\Scalar; use Soap\Encoding\Encoder\SimpleType\ScalarTypeEncoder; use Soap\Encoding\Test\Unit\ContextCreatorTrait; use Soap\Engine\Metadata\Model\XsdType; -use function restore_error_handler; -use function set_error_handler; use function var_export; -/** - * Untyped values are decoded as the first of int, float, bool and string that accepts them. - * The int and float rules are Psl's coercion rules, so Psl is the oracle here: a Psl upgrade that changes them turns this test red. - */ #[CoversClass(ScalarTypeEncoder::class)] final class ScalarTypeEncoderUntypedDecodeTest extends TestCase { use ContextCreatorTrait; - private const SWEEP_ALPHABET = ['0', '1', '9', '+', '-', '.', 'e', 'E', ' ', "\n", 'x', 't']; - private const SWEEP_MAX_LENGTH = 4; - - #[DataProvider('provideNamedCases')] - public function test_it_decodes_untyped_values(string $value, int|float|bool|string $expected): void - { - static::assertSame(var_export($expected, true), var_export(self::oracle($value), true), 'The oracle disagrees with the expected value'); - static::assertSame(var_export($expected, true), var_export(self::decode($value), true)); - } - - public function test_it_decodes_like_psl_for_every_short_string(): void + #[DataProvider('provideValues')] + public function test_it_decodes_untyped_values_as_scalar_guess(string $value, int|float|bool|string $expected): void { - $mismatches = []; - foreach ([...self::sweep(), ...self::boundaries()] as $value) { - $expected = var_export(self::oracle($value), true); - $actual = var_export(self::decode($value), true); - if ($expected !== $actual) { - $mismatches[] = var_export($value, true) . ': expected ' . $expected . ', got ' . $actual; - } - } + $decoded = ScalarTypeEncoder::default()->iso(self::createContext(XsdType::any()))->from($value); - static::assertSame([], $mismatches); + static::assertSame(var_export($expected, true), var_export($decoded, true)); + static::assertSame(var_export(Scalar::guess($value), true), var_export($decoded, true)); } /** * @return iterable */ - public static function provideNamedCases(): iterable + public static function provideValues(): iterable { - yield 'zero' => ['0', 0]; - yield 'one is an int, not a bool' => ['1', 1]; yield 'int' => ['7', 7]; - yield 'negative int' => ['-12', -12]; - yield 'int max' => ['9223372036854775807', PHP_INT_MAX]; - yield 'leading zeros' => ['007', 7]; - yield 'more leading zeros' => ['0012', 12]; - yield 'only zeros' => ['000', 0]; - yield 'two zeros' => ['00', 0]; - yield 'psl quirk: zeros before a minus are trimmed into a negative int' => ['0-5', -5]; - yield 'psl quirk with two zeros' => ['00-5', -5]; - yield 'negative with leading zero is a float' => ['-012', -12.0]; - yield 'plus sign is a float' => ['+5', 5.0]; - yield 'negative zero is a float' => ['-0', -0.0]; - yield 'above int max is a float' => ['9223372036854775808', 9223372036854775808.0]; - yield 'far above int max is a float' => ['99999999999999999999', 99999999999999999999.0]; - yield 'decimal' => ['1.5', 1.5]; - yield 'decimal without leading digit' => ['.5', 0.5]; - yield 'decimal without trailing digit' => ['5.', 5.0]; - yield 'exponent' => ['1e3', 1000.0]; - yield 'negative upper case exponent' => ['1E-3', 0.001]; - yield 'signed exponent' => ['+1.5e+2', 150.0]; - yield 'overflowing exponent' => ['1e999', INF]; - yield 'decimal with trailing newline' => ["1.5\n", 1.5]; - yield 'int with trailing newline is a float' => ["12\n", 12.0]; - yield 'leading space' => [' 12', ' 12']; - yield 'trailing space' => ['12 ', '12 ']; - yield 'leading tab' => ["\t1", "\t1"]; - yield 'underscore separator' => ['1_000', '1_000']; - yield 'hexadecimal' => ['0x1A', '0x1A']; - yield 'decimal comma' => ['1,5', '1,5']; - yield 'exponent without digits' => ['1e', '1e']; - yield 'exponent without mantissa' => ['e1', 'e1']; - yield 'double minus' => ['--1', '--1']; - yield 'plus' => ['+', '+']; - yield 'minus' => ['-', '-']; - yield 'dot' => ['.', '.']; - yield 'empty' => ['', '']; - yield 'true' => ['true', true]; - yield 'false' => ['false', false]; - yield 'upper case true stays a string' => ['TRUE', 'TRUE']; - yield 'capitalised false stays a string' => ['False', 'False']; - yield 'yes' => ['yes', 'yes']; - yield 'on' => ['on', 'on']; - yield 'null' => ['null', 'null']; - yield 'nan' => ['NaN', 'NaN']; - yield 'inf' => ['INF', 'INF']; - yield 'word' => ['abc', 'abc']; - } - - private static function decode(string $value): mixed - { - set_error_handler(static function (int $severity, string $message, string $file, int $line): never { - throw new ErrorException($message, 0, $severity, $file, $line); - }); - - try { - return ScalarTypeEncoder::default()->iso(self::createContext(XsdType::any()))->from($value); - } finally { - restore_error_handler(); - } - } - - private static function oracle(string $value): int|float|bool|string - { - try { - return Type\int()->coerce($value); - } catch (Type\Exception\CoercionException) { - } - - try { - return Type\float()->coerce($value); - } catch (Type\Exception\CoercionException) { - } - - return match ($value) { - 'true' => true, - 'false' => false, - default => $value, - }; - } - - /** - * @return list - */ - private static function sweep(): array - { - $values = ['']; - $layer = ['']; - for ($length = 1; $length <= self::SWEEP_MAX_LENGTH; $length++) { - $next = []; - foreach ($layer as $prefix) { - foreach (self::SWEEP_ALPHABET as $char) { - $next[] = $prefix . $char; - } - } - $values = [...$values, ...$next]; - $layer = $next; - } - - return $values; - } - - /** - * @return list - */ - private static function boundaries(): array - { - return [ - '9223372036854775807', - '9223372036854775808', - '-9223372036854775808', - '-9223372036854775809', - '09223372036854775807', - '+9223372036854775807', - '9223372036854775807.0', - '18446744073709551616', - ]; + yield 'psl quirk' => ['0-5', -5]; + yield 'float' => ['1.5', 1.5]; + yield 'bool' => ['true', true]; + yield 'string' => ['abc', 'abc']; } }