From 43c0500af8a3783a5cb5b0152bc3c16a65e9a505 Mon Sep 17 00:00:00 2001 From: John Charman Date: Thu, 23 Oct 2025 16:00:49 +0100 Subject: [PATCH 1/5] Extract getProcessorFromProperty method --- src/Attribute/Builder.php | 41 ++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/Attribute/Builder.php b/src/Attribute/Builder.php index c7337a1..2ccc815 100644 --- a/src/Attribute/Builder.php +++ b/src/Attribute/Builder.php @@ -53,30 +53,35 @@ private function fromClass(string $class, string $processes = ''): Processor continue; } - $type = $property->getType(); + $processors[] = $this->getProcessorFromProperty($property); + } - if ($type === null) { - throw CannotProcessProperty::noTypeHint($property->getName()); - } + return new FieldSet($processes, ...$processors); + } - if (!($type instanceof ReflectionNamedType)) { - throw CannotProcessProperty::compoundPropertyType($property->getName()); - } + private function getProcessorFromProperty(ReflectionProperty $property): Processor + { + $type = $property->getType(); - $processorType = $this->getProcessorTypeFromPropertyType($type->getName()); - $processorTypeAttribute = current($property->getAttributes(OverrideProcessorType::class)); - if ($processorTypeAttribute !== false) { - $processorType = $processorTypeAttribute->newInstance()->type; - } + if ($type === null) { + throw CannotProcessProperty::noTypeHint($property->getName()); + } - $processors[] = match ($processorType) { - ProcessorType::Field => $this->makeField($property), - ProcessorType::Fieldset => $this->fromClass($type->getName(), $property->getName()), - ProcessorType::Collection => $this->makeCollection($property), - }; + if (!($type instanceof ReflectionNamedType)) { + throw CannotProcessProperty::compoundPropertyType($property->getName()); } - return new FieldSet($processes, ...$processors); + $processorType = $this->getProcessorTypeFromPropertyType($type->getName()); + $processorTypeAttribute = current($property->getAttributes(OverrideProcessorType::class)); + if ($processorTypeAttribute !== false) { + $processorType = $processorTypeAttribute->newInstance()->type; + } + + return match ($processorType) { + ProcessorType::Field => $this->makeField($property), + ProcessorType::Fieldset => $this->fromClass($type->getName(), $property->getName()), + ProcessorType::Collection => $this->makeCollection($property), + }; } private function getProcessorTypeFromPropertyType(string $type): ProcessorType From 82a0350a8b00013f9c2578bd16eb968fa9bfab0a Mon Sep 17 00:00:00 2001 From: John Charman Date: Fri, 24 Oct 2025 10:52:14 +0100 Subject: [PATCH 2/5] Add intersection typehint exception --- src/Attribute/Builder.php | 10 ++++++++-- src/Exception/CannotProcessProperty.php | 6 ++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Attribute/Builder.php b/src/Attribute/Builder.php index 2ccc815..c16ab24 100644 --- a/src/Attribute/Builder.php +++ b/src/Attribute/Builder.php @@ -67,10 +67,16 @@ private function getProcessorFromProperty(ReflectionProperty $property): Process throw CannotProcessProperty::noTypeHint($property->getName()); } - if (!($type instanceof ReflectionNamedType)) { - throw CannotProcessProperty::compoundPropertyType($property->getName()); + if ($type instanceof \ReflectionIntersectionType) { + throw CannotProcessProperty::intersectionTypeHint($property->getName()); } + if ($type instanceof \ReflectionUnionType) { + throw CannotProcessProperty::compoundPropertyType($property->getName()); //TODO support scalar unions + } + + assert($type instanceof \ReflectionNamedType); + $processorType = $this->getProcessorTypeFromPropertyType($type->getName()); $processorTypeAttribute = current($property->getAttributes(OverrideProcessorType::class)); if ($processorTypeAttribute !== false) { diff --git a/src/Exception/CannotProcessProperty.php b/src/Exception/CannotProcessProperty.php index e0d873f..92ed2fb 100644 --- a/src/Exception/CannotProcessProperty.php +++ b/src/Exception/CannotProcessProperty.php @@ -21,6 +21,12 @@ public static function noTypeHint(string $propertyName): self return new self($message); } + public static function intersectionTypeHint(string $propertyName): self + { + $message = sprintf('Property %s uses an intersection type, these are not currently supported', $propertyName); + return new self($message); + } + public static function compoundPropertyType(string $propertyName): self { $message = sprintf('Property %s uses a compound type hint, these are not currently supported', $propertyName); From 9bfe16b630dc447be7347be025bec7314623a4e8 Mon Sep 17 00:00:00 2001 From: John Charman Date: Fri, 24 Oct 2025 10:59:05 +0100 Subject: [PATCH 3/5] Remove empty class fixture Can be replaced with anonymous class --- tests/Attribute/BuilderTest.php | 13 ++++++------- tests/MembraneTest.php | 3 +-- tests/fixtures/Attribute/EmptyClass.php | 9 --------- 3 files changed, 7 insertions(+), 18 deletions(-) delete mode 100644 tests/fixtures/Attribute/EmptyClass.php diff --git a/tests/Attribute/BuilderTest.php b/tests/Attribute/BuilderTest.php index eff3bf2..301dd8b 100644 --- a/tests/Attribute/BuilderTest.php +++ b/tests/Attribute/BuilderTest.php @@ -48,7 +48,6 @@ use Membrane\Tests\Fixtures\Attribute\Docs\BlogPostRegexAndMaxLength; use Membrane\Tests\Fixtures\Attribute\Docs\BlogPostRequiredFields; use Membrane\Tests\Fixtures\Attribute\Docs\BlogPostWithAllOf; -use Membrane\Tests\Fixtures\Attribute\EmptyClass; use Membrane\Tests\Fixtures\Attribute\EmptyClassWithIgnoredProperty; use Membrane\Tests\Fixtures\Attribute\EnumProperties; use Membrane\Tests\Fixtures\Attribute\EnumPropertiesWithAttributes; @@ -177,8 +176,8 @@ public function nestedCollectionThrowsException(): void public static function dataSetOfClassesToBuild(): array { return [ - EmptyClass::class => [ - new ClassWithAttributes(EmptyClass::class), + 'empty class' => [ + new ClassWithAttributes((new class() {})::class), new FieldSet(''), ], EmptyClassWithIgnoredProperty::class => [ @@ -201,7 +200,7 @@ public static function dataSetOfClassesToBuild(): array new ClassWithAttributes(ClassWithIntPropertyIsIntValidator::class), new FieldSet('', new Field('integerProperty', new IsInt())), ], - EnumPropeties::class => [ + EnumProperties::class => [ new ClassWithAttributes(EnumProperties::class), new FieldSet( '', @@ -300,10 +299,10 @@ public function BuildingProcessorsTest(Specification $specification, FieldSet $e public static function dataSetOfInputsAndOutputs(): array { return [ - EmptyClass::class => [ - new ClassWithAttributes(EmptyClass::class), + 'empty class' => [ + new ClassWithAttributes((new class() {})::class), [], - Result::noResult([]), + Result::noResult([]) ], EmptyClassWithIgnoredProperty::class => [ new ClassWithAttributes(EmptyClassWithIgnoredProperty::class), diff --git a/tests/MembraneTest.php b/tests/MembraneTest.php index f878514..8a4031f 100644 --- a/tests/MembraneTest.php +++ b/tests/MembraneTest.php @@ -21,7 +21,6 @@ use Membrane\Result\Message; use Membrane\Result\MessageSet; use Membrane\Result\Result; -use Membrane\Tests\Fixtures\Attribute\EmptyClass; use Membrane\Validator\FieldSet\RequiredFields; use Membrane\Validator\Type\IsList; use PHPUnit\Framework\Attributes\CoversClass; @@ -102,7 +101,7 @@ public static function provideSpecifications(): Generator yield 'Attributes' => [ new class { }, - new ClassWithAttributes(EmptyClass::class), + new ClassWithAttributes((new class () {})::class), ]; yield 'Request' => [ new \GuzzleHttp\Psr7\Request('get', ''), diff --git a/tests/fixtures/Attribute/EmptyClass.php b/tests/fixtures/Attribute/EmptyClass.php deleted file mode 100644 index e34742a..0000000 --- a/tests/fixtures/Attribute/EmptyClass.php +++ /dev/null @@ -1,9 +0,0 @@ - Date: Fri, 24 Oct 2025 11:26:29 +0100 Subject: [PATCH 4/5] Add failing test for building union type --- tests/Attribute/BuilderTest.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/Attribute/BuilderTest.php b/tests/Attribute/BuilderTest.php index 301dd8b..04d9291 100644 --- a/tests/Attribute/BuilderTest.php +++ b/tests/Attribute/BuilderTest.php @@ -16,6 +16,7 @@ use Membrane\Filter\Type\ToBackedEnum; use Membrane\Filter\Type\ToString; use Membrane\Processor\AfterSet; +use Membrane\Processor\AnyOf; use Membrane\Processor\BeforeSet; use Membrane\Processor\Collection; use Membrane\Processor\Field; @@ -57,6 +58,7 @@ use Membrane\Validator\FieldSet\RequiredFields; use Membrane\Validator\String\Length; use Membrane\Validator\String\Regex; +use Membrane\Validator\Type\IsFloat; use Membrane\Validator\Type\IsInt; use Membrane\Validator\Type\IsList; use Membrane\Validator\Type\IsString; @@ -92,6 +94,7 @@ #[UsesClass(Regex::class)] #[UsesClass(AllOf::class)] #[UsesClass(Count::class)] +#[UsesClass(AnyOf::class)] #[UsesClass(WithNamedArguments::class)] class BuilderTest extends TestCase { @@ -281,7 +284,17 @@ public static function dataSetOfClassesToBuild(): array ) ), ], - + 'class with union type' => [ + new ClassWithAttributes((new class () {private float|int $number;})::class), + new FieldSet( + '', + new AnyOf( + 'number', + new Field('number', new IsFloat()), + new Field('number', new IsInt()), + ), + ), + ] ]; } From ef1cc1d101a4f24f7c1066887f821f2affecd86c Mon Sep 17 00:00:00 2001 From: John Charman Date: Fri, 24 Oct 2025 15:14:39 +0100 Subject: [PATCH 5/5] Update builder to allow conditional When attributes --- src/Attribute/Builder.php | 79 +++++++++++---- src/Attribute/When.php | 15 +++ src/Exception/CannotProcessProperty.php | 8 +- .../Exception/CannotProcessOpenAPI.php | 1 - tests/Attribute/BuilderTest.php | 96 +++++++++++++++---- 5 files changed, 154 insertions(+), 45 deletions(-) create mode 100644 src/Attribute/When.php diff --git a/src/Attribute/Builder.php b/src/Attribute/Builder.php index c16ab24..fe2aa78 100644 --- a/src/Attribute/Builder.php +++ b/src/Attribute/Builder.php @@ -10,6 +10,7 @@ use Membrane\Builder\Builder as BuilderInterface; use Membrane\Builder\Specification; use Membrane\Exception\CannotProcessProperty; +use Membrane\Filter; use Membrane\Processor; use Membrane\Processor\AfterSet; use Membrane\Processor\BeforeSet; @@ -17,9 +18,9 @@ use Membrane\Processor\Field; use Membrane\Processor\FieldSet; use Membrane\Processor\ProcessorType; +use Membrane\Validator; use ReflectionAttribute; use ReflectionClass; -use ReflectionNamedType; use ReflectionProperty; use function array_map; @@ -72,10 +73,26 @@ private function getProcessorFromProperty(ReflectionProperty $property): Process } if ($type instanceof \ReflectionUnionType) { - throw CannotProcessProperty::compoundPropertyType($property->getName()); //TODO support scalar unions + $processors = []; + + foreach ($type->getTypes() as $subType) { + if ($subType instanceof \ReflectionIntersectionType) { + throw CannotProcessProperty::intersectionTypeHint($property->getName()); + } + + if (!in_array($subType->getName(), ['bool', 'float', 'int', 'string', 'null', 'true', 'false'])) { + throw CannotProcessProperty::compoundPropertyType($property->getName()); + } + + $processors [] = $this->makeField($property->getName(), ...$this + ->getFiltersOrValidators($property, $subType->getName())); + } + + // AnyOf is faster than OneOf since it performs less checks + return new Processor\AnyOf($property->getName(), ...$processors); } - assert($type instanceof \ReflectionNamedType); + assert($type instanceof \ReflectionNamedType); // proof by exhaustion (all alternatives have been checked above) $processorType = $this->getProcessorTypeFromPropertyType($type->getName()); $processorTypeAttribute = current($property->getAttributes(OverrideProcessorType::class)); @@ -84,12 +101,39 @@ private function getProcessorFromProperty(ReflectionProperty $property): Process } return match ($processorType) { - ProcessorType::Field => $this->makeField($property), + ProcessorType::Field => $this->makeField($property->getName(), ...$this + ->getFiltersOrValidators($property, $type->getName())), ProcessorType::Fieldset => $this->fromClass($type->getName(), $property->getName()), ProcessorType::Collection => $this->makeCollection($property), }; } + /** @return array */ + private function getFiltersOrValidators( + ReflectionProperty $property, + string $type + ): array { + $reflectionAttributes = $property->getAttributes(); + + $result = []; + foreach ($reflectionAttributes as $reflectionAttribute) { + $attribute = $reflectionAttribute->newInstance(); + + switch (true) { + case $attribute instanceof When: + if ($attribute->typeIs === $type) { + $result [] = $attribute->filterOrValidator->class; + } + break; + case $attribute instanceof FilterOrValidator: + $result [] = $attribute->class; + break; + } + } + + return $result; + } + private function getProcessorTypeFromPropertyType(string $type): ProcessorType { if ( @@ -107,28 +151,20 @@ enum_exists($type) }; } - private function makeField(ReflectionProperty $property): Field - { - $attributes = $property->getAttributes( - FilterOrValidator::class, - ReflectionAttribute::IS_INSTANCEOF - ); - - return new Field( - $property->getName(), - ...array_map(fn($reflectionAttribute) => $reflectionAttribute->newInstance()->class, $attributes) - ); + private function makeField( + string $propertyName, + Filter|Validator ...$filtersOrValidators + ): Field { + return new Field($propertyName, ...$filtersOrValidators); } private function makeCollection(ReflectionProperty $property): Processor { - $subtype = (current($property->getAttributes(Subtype::class)) ?: null) - ?->newInstance() - ?->type; - - if ($subtype === null) { + $subtype = current($property->getAttributes(Subtype::class)); + if ($subtype === false) { throw CannotProcessProperty::noSubtypeHint($property->getName()); } + $subtype = $subtype->newInstance()->type; $subProcessorType = $this->getProcessorTypeFromPropertyType($subtype); @@ -141,7 +177,8 @@ private function makeCollection(ReflectionProperty $property): Processor $processors[] = match ($subProcessorType) { ProcessorType::Fieldset => $this->fromClass($subtype, $property->getName()), - ProcessorType::Field => $this->makeField($property), + ProcessorType::Field => $this->makeField($property->getName(), ...$this + ->getFiltersOrValidators($property, $subtype)), ProcessorType::Collection => throw CannotProcessProperty::nestedCollection($property->getName()) }; diff --git a/src/Attribute/When.php b/src/Attribute/When.php new file mode 100644 index 0000000..feac0a5 --- /dev/null +++ b/src/Attribute/When.php @@ -0,0 +1,15 @@ +build($specification); } - #[Test] - public function compoundPropertyThrowsException(): void - { - $specification = new ClassWithAttributes(ClassWithCompoundPropertyType::class); - $builder = new Builder(); - - self::expectException(CannotProcessProperty::class); - self::expectExceptionMessage( - 'Property compoundProperty uses a compound type hint, these are not currently supported' - ); - - $builder->build($specification); - } - #[Test] public function nestedCollectionThrowsException(): void { @@ -284,23 +274,87 @@ public static function dataSetOfClassesToBuild(): array ) ), ], - 'class with union type' => [ + 'union type' => [ new ClassWithAttributes((new class () {private float|int $number;})::class), new FieldSet( '', new AnyOf( 'number', + new Field('number'), + new Field('number'), + ), + ), + ], + 'union type, When float, isfloat?' => [ + new ClassWithAttributes((new class () { + #[When('float', new FilterOrValidator(new IsFloat()))] + private float|int $number; + })::class), + new FieldSet( + '', + new AnyOf( + 'number', + new Field('number'), new Field('number', new IsFloat()), + ), + ), + ], + 'union type. When float, isfloat? When int, isInt?' => [ + new ClassWithAttributes((new class () { + #[When('float', new FilterOrValidator(new IsFloat()))] + #[When('int', new FilterOrValidator(new IsInt()))] + private float|int $number; + })::class), + new FieldSet( + '', + new AnyOf( + 'number', new Field('number', new IsInt()), + new Field('number', new IsFloat()), + ), + ), + ], + 'union type. When string NumericString?' => [ + new ClassWithAttributes((new class () { + #[When('string', new FilterOrValidator(new NumericString()))] + #[When('string', new FilterOrValidator(new ToNumber()))] + #[FilterOrValidator(new Maximum(3.14))] + private float|int|string $number; + })::class), + new FieldSet( + '', + new AnyOf( + 'number', + new Field('number', new NumericString(), new ToNumber(), new Maximum(3.14)), + new Field('number', new Maximum(3.14)), + new Field('number', new Maximum(3.14)), ), ), - ] + ], + 'union type. Whens and FilterOrValidators mixed together' => [ + new ClassWithAttributes((new class () { + #[When('string', new FilterOrValidator(new NumericString()))] + #[When('string', new FilterOrValidator(new ToNumber()))] + #[FilterOrValidator(new Maximum(3.14))] + #[When('string', new FilterOrValidator(new ToString()))] + private float|int|string $number; + })::class), + new FieldSet( + '', + new AnyOf( + 'number', + new Field('number', new NumericString(), new ToNumber(), new Maximum(3.14), new ToString()), + new Field('number', new Maximum(3.14)), + new Field('number', new Maximum(3.14)), + ), + ), + ], ]; } - #[DataProvider('dataSetOfClassesToBuild')] #[Test] - public function BuildingProcessorsTest(Specification $specification, FieldSet $expected): void + #[DataProvider('dataSetOfClassesToBuild')] + public function buildingProcessorsTest(Specification $specification, FieldSet $expected): void { $builder = new Builder(); @@ -381,8 +435,8 @@ public static function dataSetOfInputsAndOutputs(): array ]; } - #[DataProvider('dataSetOfInputsAndOutputs')] #[Test] + #[DataProvider('dataSetOfInputsAndOutputs')] public function InputsAndOutputsTest(Specification $specification, mixed $input, mixed $expected): void { $builder = new Builder(); @@ -563,8 +617,8 @@ public static function dataSetsWithDocExamples(): array ]; } - #[DataProvider('dataSetsWithDocExamples')] #[Test] + #[DataProvider('dataSetsWithDocExamples')] public function docExamplesTest(Specification $specification, array $input, Result $expected): void { $builder = new Builder();