Skip to content

Commit deb6df7

Browse files
author
Enno Woortmann
committed
Strengthen ComposedRequiredPromotion test coverage
- Assert specific composition exception types (AllOfException, AnyOfException, OneOfException, ConditionalException) in CanBeOmitted tests instead of the generic Exception base class - Assert assertInstanceOf on the collected error in all three implicit-null error-collection tests; add allOf and oneOf variants of that test (previously only anyOf was covered) - assertNullableStringProperty/assertNullableIntProperty now unconditionally assert null is present: composition-transferred non-promoted properties are always nullable regardless of implicitNull, and this is now verified in both modes rather than only under implicitNull=true - Add testMultiplePropertiesOnlySomePromoted: two transferred properties where only one meets the promotion condition - Cover three defensive guard paths in the post processor with new schemas and tests: AnyOfEmptyComposition (empty branches array), AnyOfRequiredOnlyBranches (required name absent from schema properties), and AnyOfUntypedPropertyInBranches (getType() returns null, no promotion)
1 parent 586a84f commit deb6df7

5 files changed

Lines changed: 216 additions & 16 deletions

File tree

tests/ComposedValue/ComposedRequiredPromotionTest.php

Lines changed: 153 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,16 @@
44

55
namespace PHPModelGenerator\Tests\ComposedValue;
66

7+
use PHPModelGenerator\Exception\ComposedValue\AllOfException;
8+
use PHPModelGenerator\Exception\ComposedValue\AnyOfException;
9+
use PHPModelGenerator\Exception\ComposedValue\ConditionalException;
10+
use PHPModelGenerator\Exception\ComposedValue\OneOfException;
711
use PHPModelGenerator\Exception\ErrorRegistryException;
812
use PHPModelGenerator\Model\GeneratorConfiguration;
913
use PHPModelGenerator\Tests\AbstractPHPModelGeneratorTestCase;
1014
use PHPUnit\Framework\Attributes\DataProvider;
15+
use ReflectionClass;
16+
use ReflectionMethod;
1117

1218
/**
1319
* Tests for CompositionRequiredPromotionPostProcessor.
@@ -67,7 +73,7 @@ public function testAllOfNotRequiredInAnyBranchIsNotPromoted(bool $implicitNull)
6773
$implicitNull,
6874
);
6975

70-
$this->assertNullableStringProperty($className, 'name', $implicitNull);
76+
$this->assertNullableStringProperty($className, 'name');
7177
}
7278

7379
/**
@@ -106,10 +112,37 @@ public function testAllOfPromotedPropertyCanBeOmittedWithoutRequiredValueExcepti
106112
false,
107113
);
108114

109-
$this->expectException(\Exception::class);
115+
$this->expectException(AllOfException::class);
110116
new $className([]);
111117
}
112118

119+
/**
120+
* allOf + implicitNull=true + error-collection: absent promoted property collects exactly
121+
* one allOf composition error and no spurious type error.
122+
*/
123+
public function testAllOfPromotedPropertyAbsentUnderImplicitNullCollectsOnlyCompositionError(): void
124+
{
125+
$className = $this->generateClassFromFile(
126+
'AllOfRequiredInAnyBranch.json',
127+
(new GeneratorConfiguration())->setCollectErrors(true),
128+
false,
129+
true,
130+
);
131+
132+
try {
133+
new $className([]);
134+
$this->fail('Expected ErrorRegistryException');
135+
} catch (ErrorRegistryException $e) {
136+
$errors = $e->getErrors();
137+
$this->assertCount(1, $errors, 'Only the composition error should be collected');
138+
$this->assertInstanceOf(
139+
AllOfException::class,
140+
$errors[0],
141+
'The collected error must be an AllOfException',
142+
);
143+
}
144+
}
145+
113146
// -------------------------------------------------------------------------
114147
// anyOf
115148
// -------------------------------------------------------------------------
@@ -143,7 +176,7 @@ public function testAnyOfRequiredInSomeBranchesIsNotPromoted(bool $implicitNull)
143176
$implicitNull,
144177
);
145178

146-
$this->assertNullableStringProperty($className, 'name', $implicitNull);
179+
$this->assertNullableStringProperty($className, 'name');
147180
}
148181

149182
/**
@@ -159,7 +192,7 @@ public function testAnyOfNotRequiredInAnyBranchIsNotPromoted(bool $implicitNull)
159192
$implicitNull,
160193
);
161194

162-
$this->assertNullableStringProperty($className, 'name', $implicitNull);
195+
$this->assertNullableStringProperty($className, 'name');
163196
}
164197

165198
/**
@@ -195,10 +228,51 @@ public function testAnyOfPromotedPropertyCanBeOmittedWithoutRequiredValueExcepti
195228
false,
196229
);
197230

198-
$this->expectException(\Exception::class);
231+
$this->expectException(AnyOfException::class);
199232
new $className([]);
200233
}
201234

235+
/**
236+
* anyOf: empty composition array → no promotable names, no crash.
237+
*/
238+
public function testAnyOfEmptyCompositionDoesNotCrash(): void
239+
{
240+
$className = $this->generateClassFromFile('AnyOfEmptyComposition.json', null, false, false);
241+
242+
// An object with empty anyOf generates successfully; no branch properties to promote.
243+
$rc = new ReflectionClass($className);
244+
$this->assertEmpty($rc->getProperties(ReflectionMethod::IS_PUBLIC));
245+
}
246+
247+
/**
248+
* anyOf: branches carry only required constraints, no properties — the promoted property name
249+
* does not appear in the schema's property list, so promoteProperty returns early.
250+
*/
251+
public function testAnyOfRequiredOnlyBranchesDoesNotCrash(): void
252+
{
253+
$className = $this->generateClassFromFile('AnyOfRequiredOnlyBranches.json', null, false, false);
254+
255+
// Branches have no 'properties', so no branch properties are transferred to the schema.
256+
$rc = new ReflectionClass($className);
257+
$this->assertEmpty($rc->getProperties(ReflectionMethod::IS_PUBLIC));
258+
}
259+
260+
/**
261+
* anyOf: branches define an untyped property (no 'type' keyword) in required —
262+
* promoteProperty returns early when getType() is null. No crash, and the
263+
* property must not be incorrectly promoted to non-nullable.
264+
*/
265+
public function testAnyOfUntypedPropertyInBranchesDoesNotCrash(): void
266+
{
267+
$className = $this->generateClassFromFile('AnyOfUntypedPropertyInBranches.json', null, false, false);
268+
269+
// The property exists but has no scalar type — the generator emits 'mixed'.
270+
// The important thing is that promotion did not apply (no PropertyType to strip nullable from).
271+
$returnType = $this->getReturnType($className, 'getValue');
272+
$this->assertNotNull($returnType, 'Getter must exist for transferred untyped property');
273+
$this->assertTrue($returnType->allowsNull(), 'Untyped property must not be promoted to non-nullable');
274+
}
275+
202276
// -------------------------------------------------------------------------
203277
// oneOf
204278
// -------------------------------------------------------------------------
@@ -232,7 +306,7 @@ public function testOneOfRequiredInSomeBranchesIsNotPromoted(bool $implicitNull)
232306
$implicitNull,
233307
);
234308

235-
$this->assertNullableStringProperty($className, 'name', $implicitNull);
309+
$this->assertNullableStringProperty($className, 'name');
236310
}
237311

238312
/**
@@ -248,7 +322,7 @@ public function testOneOfNotRequiredInAnyBranchIsNotPromoted(bool $implicitNull)
248322
$implicitNull,
249323
);
250324

251-
$this->assertNullableStringProperty($className, 'name', $implicitNull);
325+
$this->assertNullableStringProperty($className, 'name');
252326
}
253327

254328
/**
@@ -284,10 +358,37 @@ public function testOneOfPromotedPropertyCanBeOmittedWithoutRequiredValueExcepti
284358
false,
285359
);
286360

287-
$this->expectException(\Exception::class);
361+
$this->expectException(OneOfException::class);
288362
new $className([]);
289363
}
290364

365+
/**
366+
* oneOf + implicitNull=true + error-collection: absent promoted property collects exactly
367+
* one oneOf composition error and no spurious type error.
368+
*/
369+
public function testOneOfPromotedPropertyAbsentUnderImplicitNullCollectsOnlyCompositionError(): void
370+
{
371+
$className = $this->generateClassFromFile(
372+
'OneOfRequiredInAllBranches.json',
373+
(new GeneratorConfiguration())->setCollectErrors(true),
374+
false,
375+
true,
376+
);
377+
378+
try {
379+
new $className([]);
380+
$this->fail('Expected ErrorRegistryException');
381+
} catch (ErrorRegistryException $e) {
382+
$errors = $e->getErrors();
383+
$this->assertCount(1, $errors, 'Only the composition error should be collected');
384+
$this->assertInstanceOf(
385+
OneOfException::class,
386+
$errors[0],
387+
'The collected error must be a OneOfException',
388+
);
389+
}
390+
}
391+
291392
// -------------------------------------------------------------------------
292393
// if/then/else
293394
// -------------------------------------------------------------------------
@@ -323,7 +424,7 @@ public function testIfThenOnlyNotPromoted(bool $implicitNull): void
323424
$implicitNull,
324425
);
325426

326-
$this->assertNullableStringProperty($className, 'name', $implicitNull);
427+
$this->assertNullableStringProperty($className, 'name');
327428
}
328429

329430
/**
@@ -339,7 +440,7 @@ public function testIfThenElseOneBranchRequiredIsNotPromoted(bool $implicitNull)
339440
$implicitNull,
340441
);
341442

342-
$this->assertNullableStringProperty($className, 'name', $implicitNull);
443+
$this->assertNullableStringProperty($className, 'name');
343444
}
344445

345446
/**
@@ -378,7 +479,7 @@ public function testIfThenElsePromotedPropertyCanBeOmittedWithoutRequiredValueEx
378479
false,
379480
);
380481

381-
$this->expectException(\Exception::class);
482+
$this->expectException(ConditionalException::class);
382483
new $className([]);
383484
}
384485

@@ -396,7 +497,7 @@ public function testPromotionSuppressesNullUnderImplicitNull(): void
396497
}
397498

398499
/**
399-
* implicitNull=true + error collection: when a promoted property is absent, only the
500+
* anyOf + implicitNull=true + error-collection: when a promoted property is absent, only the
400501
* composition error is collected. No spurious InvalidTypeException must be added.
401502
*/
402503
public function testPromotedPropertyAbsentUnderImplicitNullCollectsOnlyCompositionError(): void
@@ -414,9 +515,36 @@ public function testPromotedPropertyAbsentUnderImplicitNullCollectsOnlyCompositi
414515
} catch (ErrorRegistryException $e) {
415516
$errors = $e->getErrors();
416517
$this->assertCount(1, $errors, 'Only the composition error should be collected');
518+
$this->assertInstanceOf(
519+
AnyOfException::class,
520+
$errors[0],
521+
'The collected error must be an AnyOfException',
522+
);
417523
}
418524
}
419525

526+
// -------------------------------------------------------------------------
527+
// Multiple properties: only some promoted
528+
// -------------------------------------------------------------------------
529+
530+
/**
531+
* anyOf with two transferred properties: 'name' is required in every branch (→ promoted),
532+
* 'age' is not required in any branch (→ stays nullable).
533+
*/
534+
#[DataProvider('implicitNullDataProvider')]
535+
public function testMultiplePropertiesOnlySomePromoted(bool $implicitNull): void
536+
{
537+
$className = $this->generateClassFromFile(
538+
'AnyOfMultiplePropertiesSomePromoted.json',
539+
null,
540+
false,
541+
$implicitNull,
542+
);
543+
544+
$this->assertNonNullableStringProperty($className, 'name');
545+
$this->assertNullableIntProperty($className, 'age');
546+
}
547+
420548
// -------------------------------------------------------------------------
421549
// Helpers
422550
// -------------------------------------------------------------------------
@@ -430,15 +558,24 @@ private function assertNonNullableStringProperty(string $className, string $prop
430558
$this->assertFalse($returnType->allowsNull(), 'Return type must not allow null for promoted property');
431559
}
432560

433-
private function assertNullableStringProperty(string $className, string $property, bool $implicitNull): void
561+
private function assertNullableStringProperty(string $className, string $property): void
434562
{
435563
$getter = 'get' . ucfirst($property);
436564

437565
$returnTypeNames = $this->getReturnTypeNames($className, $getter);
438566
$this->assertContains('string', $returnTypeNames, "Return type should contain 'string'");
567+
// Composition-transferred non-promoted properties are always nullable: the property is
568+
// optional, so a valid object may not provide it. This holds regardless of implicitNull.
569+
$this->assertContains('null', $returnTypeNames, 'Non-promoted property must remain nullable');
570+
}
439571

440-
if ($implicitNull) {
441-
$this->assertContains('null', $returnTypeNames, 'Non-promoted property should be nullable');
442-
}
572+
private function assertNullableIntProperty(string $className, string $property): void
573+
{
574+
$getter = 'get' . ucfirst($property);
575+
576+
$returnTypeNames = $this->getReturnTypeNames($className, $getter);
577+
$this->assertContains('int', $returnTypeNames, "Return type should contain 'int'");
578+
// Same reasoning as assertNullableStringProperty.
579+
$this->assertContains('null', $returnTypeNames, 'Non-promoted property must remain nullable');
443580
}
444581
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "object",
3+
"anyOf": []
4+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
{
2+
"type": "object",
3+
"anyOf": [
4+
{
5+
"type": "object",
6+
"properties": {
7+
"name": {
8+
"type": "string"
9+
},
10+
"age": {
11+
"type": "integer"
12+
}
13+
},
14+
"required": ["name"]
15+
},
16+
{
17+
"type": "object",
18+
"properties": {
19+
"name": {
20+
"type": "string"
21+
},
22+
"age": {
23+
"type": "integer"
24+
}
25+
},
26+
"required": ["name"]
27+
}
28+
]
29+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"type": "object",
3+
"anyOf": [
4+
{
5+
"required": ["name"]
6+
},
7+
{
8+
"required": ["name"]
9+
}
10+
]
11+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"type": "object",
3+
"anyOf": [
4+
{
5+
"type": "object",
6+
"properties": {
7+
"value": {}
8+
},
9+
"required": ["value"]
10+
},
11+
{
12+
"type": "object",
13+
"properties": {
14+
"value": {}
15+
},
16+
"required": ["value"]
17+
}
18+
]
19+
}

0 commit comments

Comments
 (0)