26158351968

Committed 20 May 2026 10:28AM UTC coverage: 98.466% (-0.09%) from 98.554%

Build # 26158351968

Build Type

Pull #128

github

Committed by

wol-soft

Commit Message

Fix if/then/else branch conflict detection for null-typed and per-branch checks

IfValidatorFactory previously required both then and else branches to conflict
with the parent before throwing, and stripped null from parent type names — so a
null-typed branch against a non-null parent was silently accepted. Now each branch
is checked independently and null from isNullable() is included in the parent set.
getBranchTypeNames() distinguishes null-typed branches ({type: null}) from truly
untyped branches via getTypeHint(), so null-typed branches correctly intersect (or
conflict) with the parent type. CLAUDE.md extended with review-learning policy and
line-number-in-docblock rule. New test coverage for all affected edge cases.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Pull Request Pull Request #128: Filter composition ordering

Coverage Stats

772 of 792 new or added lines in 22 files covered. (97.47%)

5 existing lines in 3 files now uncovered.

5455 of 5540 relevant lines covered (98.47%)

592.57 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

99.22

/src/Model/Validator/Factory/Composition/IfValidatorFactory.php

<?php

declare(strict_types=1);

namespace PHPModelGenerator\Model\Validator\Factory\Composition;

use PHPModelGenerator\Exception\SchemaException;
use PHPModelGenerator\Model\Property\BaseProperty;
use PHPModelGenerator\Model\Property\CompositionPropertyDecorator;
use PHPModelGenerator\Model\Property\PropertyInterface;
use PHPModelGenerator\Model\Property\PropertyType;
use PHPModelGenerator\Model\Schema;
use PHPModelGenerator\Model\SchemaDefinition\JsonSchema;
use PHPModelGenerator\Model\Validator;
use PHPModelGenerator\Model\Validator\ComposedPropertyValidator;
use PHPModelGenerator\Model\Validator\ConditionalPropertyValidator;
use PHPModelGenerator\Model\Validator\RequiredPropertyValidator;
use PHPModelGenerator\PropertyProcessor\Filter\CompositionCompatibilityChecker;
use PHPModelGenerator\PropertyProcessor\PropertyFactory;
use PHPModelGenerator\SchemaProcessor\SchemaProcessor;
use PHPModelGenerator\Utils\RenderHelper;
use PHPModelGenerator\Utils\TypeIntersection;

class IfValidatorFactory
    extends AbstractCompositionValidatorFactory
    implements ComposedPropertiesValidatorFactoryInterface
{
    /**
     * @throws SchemaException
     */
    public function modify(
        SchemaProcessor $schemaProcessor,
        Schema $schema,
        PropertyInterface $property,
        JsonSchema $propertySchema,
    ): void {
        if (!isset($propertySchema->getJson()[$this->key]) || $this->shouldSkip($property, $propertySchema)) {
            return;
        }

        if (!isset($propertySchema->getJson()['then']) && !isset($propertySchema->getJson()['else'])) {
            throw new SchemaException(
                sprintf(
                    'Incomplete conditional composition for property %s in file %s',
                    $property->getName(),
                    $property->getJsonSchema()->getFile(),
                ),
            );
        }

        // Inherit the parent type into if/then/else sub-schemas before the filter check so
        // that sub-schemas that inherit 'object' are correctly recognised as object-typed.
        // Object-typed sub-schemas create nested schemas whose properties are processed
        // independently and are not subject to ComposedItem $value reset.
        $propertySchema = $this->inheritPropertyType($propertySchema);
        $json = $propertySchema->getJson();

        // Check for filter keywords in if/then/else sub-schemas after type inheritance.
        // TODO: filters inside if/then/else sub-schemas cannot be correctly applied
        // (ComposedItem.phptpl resets $value to $originalModelData after each branch).
        // Proper per-branch filter chaining is deferred to a follow-up topic.
        foreach (['if', 'then', 'else'] as $keyword) {
            if (
                isset($json[$keyword])
                && is_array($json[$keyword])
                && CompositionCompatibilityChecker::branchContainsFilter($json[$keyword])
            ) {
                throw new SchemaException(sprintf(
                    'A filter keyword inside an if/then/else composition branch is not supported'
                        . ' for property %s in file %s (%s sub-schema).',
                    $property->getName(),
                    $property->getJsonSchema()->getFile(),
                    $keyword,
                ));
            }
        }

        $propertyFactory = new PropertyFactory();

        $onlyForDefinedValues = !($property instanceof BaseProperty)
            && (!$property->isRequired()
                && $schemaProcessor->getGeneratorConfiguration()->isImplicitNullAllowed());

        /** @var array<string, CompositionPropertyDecorator|null> $properties */
        $properties = [];

        foreach (['if', 'then', 'else'] as $keyword) {
            if (!isset($json[$keyword])) {
                $properties[$keyword] = null;
                continue;
            }

            $compositionSchema = $propertySchema->navigate($keyword);

            $compositionProperty = new CompositionPropertyDecorator(
                $property->getName(),
                $compositionSchema,
                $propertyFactory->create(
                    $schemaProcessor,
                    $schema,
                    $property->getName(),
                    $compositionSchema,
                    $property->isRequired(),
                ),
            );

            $compositionProperty->onResolve(static function () use ($compositionProperty): void {
                $compositionProperty->filterValidators(
                    static fn(Validator $validator): bool =>
                        !is_a($validator->getValidator(), RequiredPropertyValidator::class) &&
                        !is_a($validator->getValidator(), ComposedPropertyValidator::class),
                );
            });

            $properties[$keyword] = $compositionProperty;
        }

        $this->applyIfThenElseTypeSemantics($property, $properties);

        $property->addValidator(
            new ConditionalPropertyValidator(
                $schemaProcessor->getGeneratorConfiguration(),
                $property,
                array_values(array_filter($properties)),
                array_values(array_filter([$properties['then'], $properties['else']])),
                [
                    'ifProperty' => $properties['if'],
                    'thenProperty' => $properties['then'],
                    'elseProperty' => $properties['else'],
                    'schema' => $schema,
                    'generatorConfiguration' => $schemaProcessor->getGeneratorConfiguration(),
                    'viewHelper' => new RenderHelper($schemaProcessor->getGeneratorConfiguration()),
                    'onlyForDefinedValues' => $onlyForDefinedValues,
                ],
            ),
            100,
        );
    }

    /**
     * Apply type widening and conflict detection for property-level if/then/else.
     *
     * When both then and else branches are present, a coordinated callback fires after both
     * resolve to:
     * - Detect unsatisfiable schemas: if the parent property has a declared type that is
     *   incompatible (empty intersection) with BOTH then and else types, throw SchemaException.
     * - Widen the property type: when the parent property has no declared type, compute the
     *   union of then and else types (anyOf-like semantics) and apply it to the parent property.
     *
     * If else is absent, the absent branch accepts any value when `if` evaluates to false, so
     * the composition cannot constrain the type further; widening is skipped (property stays
     * mixed). Conflict detection also requires both branches to be present — a single conflicting
     * branch does not make the schema unsatisfiable.
     *
     * The parent type is captured at call time (before branch resolution) to avoid reading a
     * value that may have been mutated by concurrent onResolve side-effects.
     *
     * @param array<string, CompositionPropertyDecorator|null> $properties
     *
     * @throws SchemaException
     */
    private function applyIfThenElseTypeSemantics(
        PropertyInterface $property,
        array $properties,
    ): void {
        $thenProperty = $properties['then'];
        $elseProperty = $properties['else'];

        if ($thenProperty === null || $elseProperty === null) {
            // Absent branch = untyped (accepts any value on that path) — no type widening or
            // conflict detection possible without both branches.
            return;
        }

        // Capture before any branch-resolution callback can mutate the parent type.
        $originalParentType = $property->getType(true);

        $resolvedCount = 0;
        $onBothResolved = function () use (
            &$resolvedCount,
            $property,
            $thenProperty,
            $elseProperty,
            $originalParentType,
        ): void {
            $resolvedCount++;
            if ($resolvedCount < 2) {
                return;
            }

            // Object-level if/then/else creates nested schemas in the branches; type merging for
            // that case is owned by PropertyMerger. Skip here to avoid false conflict detection
            // (the parent 'object' type and branch generated-class types appear disjoint to
            // TypeIntersection::compute even though they are semantically compatible).
            if ($thenProperty->getNestedSchema() !== null || $elseProperty->getNestedSchema() !== null) {
                return;
            }

            if ($originalParentType !== null) {
                $this->detectIfThenElseConflict(
                    $property,
                    $originalParentType,
                    $thenProperty,
                    $elseProperty,
                );
                // Parent type constrains the property independently; widening is not applied.
                return;
            }

            // No parent type: widen to the union of then and else types (anyOf-like semantics).
            $this->transferPropertyType($property, [$thenProperty, $elseProperty], false);
        };

        $thenProperty->onResolve($onBothResolved);
        $elseProperty->onResolve($onBothResolved);
    }

    /**
     * Throw a SchemaException when the parent property's declared type is incompatible with
     * BOTH the then and the else branch types (empty intersection on both sides).
     *
     * A schema is unsatisfiable under this condition: no value can simultaneously satisfy the
     * parent type constraint and whichever composition branch fires. The schema is NOT
     * unsatisfiable when only one branch conflicts (values that satisfy the compatible branch
     * are still valid).
     *
     * @throws SchemaException
     */
    private function detectIfThenElseConflict(
        PropertyInterface $property,
        PropertyType $originalParentType,
        CompositionPropertyDecorator $thenProperty,
        CompositionPropertyDecorator $elseProperty,
    ): void {
        // isNullable() encodes the null part of a multi-type (e.g. ["string","null"]) separately
        // from getNames() — it is not present as a name string. Append 'null' so that a
        // null-typed branch is not incorrectly flagged as conflicting with a nullable parent.
        $parentNames = $originalParentType->isNullable()
            ? [...$originalParentType->getNames(), 'null']
            : $originalParentType->getNames();

        $thenTypes = $this->getBranchTypeNames($thenProperty);
        $elseTypes = $this->getBranchTypeNames($elseProperty);

        // A null return means the branch is truly untyped (no type keyword) — it accepts
        // any value and cannot conflict with the parent type.
        $thenConflicts = $thenTypes !== null
            && empty(TypeIntersection::compute($parentNames, $thenTypes));
        $elseConflicts = $elseTypes !== null
            && empty(TypeIntersection::compute($parentNames, $elseTypes));

        if ($thenConflicts || $elseConflicts) {
            throw new SchemaException(sprintf(
                "Property '%s' has an if/then/else composition branch with a type incompatible"
                    . " with the property's declared type (file %s)."
                    . ' No value can satisfy both constraints.',
                $property->getName(),
                $property->getJsonSchema()->getFile(),
            ));
        }
    }

    /**
     * Returns the full set of type names for a composition branch, or null when the branch
     * is truly untyped (no type keyword — accepts any value).
     *
     * NullModifier sets getType() to PHP null but adds a TypeHintDecorator containing 'null',
     * so we distinguish null-typed branches (effective types: ['null']) from truly untyped
     * branches (getType() also null, but no 'null' in the type hint).
     *
     * @return string[]|null null when the branch has no type constraint
     */
    private function getBranchTypeNames(CompositionPropertyDecorator $branch): ?array
    {
        $type = $branch->getType();

        if ($type !== null) {
            return $type->getNames();
        }

        // NullModifier-processed branch: getType() is PHP null but typeHint contains 'null'.
        if (str_contains($branch->getTypeHint(), 'null')) {
            return ['null'];
        }

        return null;
    }
}

1	<?php
2
3	declare(strict_types=1);
4
5	namespace PHPModelGenerator\Model\Validator\Factory\Composition;
6
7	use PHPModelGenerator\Exception\SchemaException;
8	use PHPModelGenerator\Model\Property\BaseProperty;
9	use PHPModelGenerator\Model\Property\CompositionPropertyDecorator;
10	use PHPModelGenerator\Model\Property\PropertyInterface;
11	use PHPModelGenerator\Model\Property\PropertyType;
12	use PHPModelGenerator\Model\Schema;
13	use PHPModelGenerator\Model\SchemaDefinition\JsonSchema;
14	use PHPModelGenerator\Model\Validator;
15	use PHPModelGenerator\Model\Validator\ComposedPropertyValidator;
16	use PHPModelGenerator\Model\Validator\ConditionalPropertyValidator;
17	use PHPModelGenerator\Model\Validator\RequiredPropertyValidator;
18	use PHPModelGenerator\PropertyProcessor\Filter\CompositionCompatibilityChecker;
19	use PHPModelGenerator\PropertyProcessor\PropertyFactory;
20	use PHPModelGenerator\SchemaProcessor\SchemaProcessor;
21	use PHPModelGenerator\Utils\RenderHelper;
22	use PHPModelGenerator\Utils\TypeIntersection;
23
24	class IfValidatorFactory
25	extends AbstractCompositionValidatorFactory
26	implements ComposedPropertiesValidatorFactoryInterface
27	{
28	/**
29	* @throws SchemaException
30	*/
31	public function modify(	2,358✔
32	SchemaProcessor $schemaProcessor,
33	Schema $schema,
34	PropertyInterface $property,
35	JsonSchema $propertySchema,
36	): void {
37	if (!isset($propertySchema->getJson()[$this->key]) \|\| $this->shouldSkip($property, $propertySchema)) {	2,358✔
38	return;	2,357✔
39	}
40
41	if (!isset($propertySchema->getJson()['then']) && !isset($propertySchema->getJson()['else'])) {	79✔
42	throw new SchemaException(	1✔
43	sprintf(	1✔
44	'Incomplete conditional composition for property %s in file %s',	1✔
45	$property->getName(),	1✔
46	$property->getJsonSchema()->getFile(),	1✔
47	),	1✔
48	);	1✔
49	}
50
51	// Inherit the parent type into if/then/else sub-schemas before the filter check so
52	// that sub-schemas that inherit 'object' are correctly recognised as object-typed.
53	// Object-typed sub-schemas create nested schemas whose properties are processed
54	// independently and are not subject to ComposedItem $value reset.
55	$propertySchema = $this->inheritPropertyType($propertySchema);	78✔
56	$json = $propertySchema->getJson();	78✔
57
58	// Check for filter keywords in if/then/else sub-schemas after type inheritance.
59	// TODO: filters inside if/then/else sub-schemas cannot be correctly applied
60	// (ComposedItem.phptpl resets $value to $originalModelData after each branch).
61	// Proper per-branch filter chaining is deferred to a follow-up topic.
62	foreach (['if', 'then', 'else'] as $keyword) {	78✔
63	if (
64	isset($json[$keyword])	78✔
65	&& is_array($json[$keyword])	78✔
66	&& CompositionCompatibilityChecker::branchContainsFilter($json[$keyword])	78✔
67	) {
68	throw new SchemaException(sprintf(	1✔
69	'A filter keyword inside an if/then/else composition branch is not supported'	1✔
70	. ' for property %s in file %s (%s sub-schema).',	1✔
71	$property->getName(),	1✔
72	$property->getJsonSchema()->getFile(),	1✔
73	$keyword,	1✔
74	));	1✔
75	}
76	}
77
78	$propertyFactory = new PropertyFactory();	77✔
79
80	$onlyForDefinedValues = !($property instanceof BaseProperty)	77✔
81	&& (!$property->isRequired()	77✔
82	&& $schemaProcessor->getGeneratorConfiguration()->isImplicitNullAllowed());	77✔
83
84	/** @var array<string, CompositionPropertyDecorator\|null> $properties */
85	$properties = [];	77✔
86
87	foreach (['if', 'then', 'else'] as $keyword) {	77✔
88	if (!isset($json[$keyword])) {	77✔
89	$properties[$keyword] = null;	19✔
90	continue;	19✔
91	}
92
93	$compositionSchema = $propertySchema->navigate($keyword);	77✔
94
95	$compositionProperty = new CompositionPropertyDecorator(	77✔
96	$property->getName(),	77✔
97	$compositionSchema,	77✔
98	$propertyFactory->create(	77✔
99	$schemaProcessor,	77✔
100	$schema,	77✔
101	$property->getName(),	77✔
102	$compositionSchema,	77✔
103	$property->isRequired(),	77✔
104	),	77✔
105	);	77✔
106
107	$compositionProperty->onResolve(static function () use ($compositionProperty): void {	77✔
108	$compositionProperty->filterValidators(	77✔
109	static fn(Validator $validator): bool =>	77✔
110	!is_a($validator->getValidator(), RequiredPropertyValidator::class) &&	77✔
111	!is_a($validator->getValidator(), ComposedPropertyValidator::class),	77✔
112	);	77✔
113	});	77✔
114
115	$properties[$keyword] = $compositionProperty;	77✔
116	}
117
118	$this->applyIfThenElseTypeSemantics($property, $properties);	77✔
119
120	$property->addValidator(	74✔
121	new ConditionalPropertyValidator(	74✔
122	$schemaProcessor->getGeneratorConfiguration(),	74✔
123	$property,	74✔
124	array_values(array_filter($properties)),	74✔
125	array_values(array_filter([$properties['then'], $properties['else']])),	74✔
126	[	74✔
127	'ifProperty' => $properties['if'],	74✔
128	'thenProperty' => $properties['then'],	74✔
129	'elseProperty' => $properties['else'],	74✔
130	'schema' => $schema,	74✔
131	'generatorConfiguration' => $schemaProcessor->getGeneratorConfiguration(),	74✔
132	'viewHelper' => new RenderHelper($schemaProcessor->getGeneratorConfiguration()),	74✔
133	'onlyForDefinedValues' => $onlyForDefinedValues,	74✔
134	],	74✔
135	),	74✔
136	100,	74✔
137	);	74✔
138	}
139
140	/**
141	* Apply type widening and conflict detection for property-level if/then/else.
142	*
143	* When both then and else branches are present, a coordinated callback fires after both
144	* resolve to:
145	* - Detect unsatisfiable schemas: if the parent property has a declared type that is
146	* incompatible (empty intersection) with BOTH then and else types, throw SchemaException.
147	* - Widen the property type: when the parent property has no declared type, compute the
148	* union of then and else types (anyOf-like semantics) and apply it to the parent property.
149	*
150	* If else is absent, the absent branch accepts any value when `if` evaluates to false, so
151	* the composition cannot constrain the type further; widening is skipped (property stays
152	* mixed). Conflict detection also requires both branches to be present — a single conflicting
153	* branch does not make the schema unsatisfiable.
154	*
155	* The parent type is captured at call time (before branch resolution) to avoid reading a
156	* value that may have been mutated by concurrent onResolve side-effects.
157	*
158	* @param array<string, CompositionPropertyDecorator\|null> $properties
159	*
160	* @throws SchemaException
161	*/
162	private function applyIfThenElseTypeSemantics(	77✔
163	PropertyInterface $property,
164	array $properties,
165	): void {
166	$thenProperty = $properties['then'];	77✔
167	$elseProperty = $properties['else'];	77✔
168
169	if ($thenProperty === null \|\| $elseProperty === null) {	77✔
170	// Absent branch = untyped (accepts any value on that path) — no type widening or
171	// conflict detection possible without both branches.
172	return;	19✔
173	}
174
175	// Capture before any branch-resolution callback can mutate the parent type.
176	$originalParentType = $property->getType(true);	58✔
177
178	$resolvedCount = 0;	58✔
179	$onBothResolved = function () use (	58✔
180	&$resolvedCount,	58✔
181	$property,	58✔
182	$thenProperty,	58✔
183	$elseProperty,	58✔
184	$originalParentType,	58✔
185	): void {	58✔
186	$resolvedCount++;	58✔
187	if ($resolvedCount < 2) {	58✔
188	return;	58✔
189	}
190
191	// Object-level if/then/else creates nested schemas in the branches; type merging for
192	// that case is owned by PropertyMerger. Skip here to avoid false conflict detection
193	// (the parent 'object' type and branch generated-class types appear disjoint to
194	// TypeIntersection::compute even though they are semantically compatible).
195	if ($thenProperty->getNestedSchema() !== null \|\| $elseProperty->getNestedSchema() !== null) {	58✔
196	return;	40✔
197	}
198
199	if ($originalParentType !== null) {	18✔
200	$this->detectIfThenElseConflict(	16✔
201	$property,	16✔
202	$originalParentType,	16✔
203	$thenProperty,	16✔
204	$elseProperty,	16✔
205	);	16✔
206	// Parent type constrains the property independently; widening is not applied.
207	return;	13✔
208	}
209
210	// No parent type: widen to the union of then and else types (anyOf-like semantics).
211	$this->transferPropertyType($property, [$thenProperty, $elseProperty], false);	2✔
212	};	58✔
213
214	$thenProperty->onResolve($onBothResolved);	58✔
215	$elseProperty->onResolve($onBothResolved);	58✔
216	}
217
218	/**
219	* Throw a SchemaException when the parent property's declared type is incompatible with
220	* BOTH the then and the else branch types (empty intersection on both sides).
221	*
222	* A schema is unsatisfiable under this condition: no value can simultaneously satisfy the
223	* parent type constraint and whichever composition branch fires. The schema is NOT
224	* unsatisfiable when only one branch conflicts (values that satisfy the compatible branch
225	* are still valid).
226	*
227	* @throws SchemaException
228	*/
229	private function detectIfThenElseConflict(	16✔
230	PropertyInterface $property,
231	PropertyType $originalParentType,
232	CompositionPropertyDecorator $thenProperty,
233	CompositionPropertyDecorator $elseProperty,
234	): void {
235	// isNullable() encodes the null part of a multi-type (e.g. ["string","null"]) separately
236	// from getNames() — it is not present as a name string. Append 'null' so that a
237	// null-typed branch is not incorrectly flagged as conflicting with a nullable parent.
238	$parentNames = $originalParentType->isNullable()	16✔
239	? [...$originalParentType->getNames(), 'null']	1✔
240	: $originalParentType->getNames();	15✔
241
242	$thenTypes = $this->getBranchTypeNames($thenProperty);	16✔
243	$elseTypes = $this->getBranchTypeNames($elseProperty);	16✔
244
245	// A null return means the branch is truly untyped (no type keyword) — it accepts
246	// any value and cannot conflict with the parent type.
247	$thenConflicts = $thenTypes !== null	16✔
248	&& empty(TypeIntersection::compute($parentNames, $thenTypes));	16✔
249	$elseConflicts = $elseTypes !== null	16✔
250	&& empty(TypeIntersection::compute($parentNames, $elseTypes));	16✔
251
252	if ($thenConflicts \|\| $elseConflicts) {	16✔
253	throw new SchemaException(sprintf(	3✔
254	"Property '%s' has an if/then/else composition branch with a type incompatible"	3✔
255	. " with the property's declared type (file %s)."	3✔
256	. ' No value can satisfy both constraints.',	3✔
257	$property->getName(),	3✔
258	$property->getJsonSchema()->getFile(),	3✔
259	));	3✔
260	}
261	}
262
263	/**
264	* Returns the full set of type names for a composition branch, or null when the branch
265	* is truly untyped (no type keyword — accepts any value).
266	*
267	* NullModifier sets getType() to PHP null but adds a TypeHintDecorator containing 'null',
268	* so we distinguish null-typed branches (effective types: ['null']) from truly untyped
269	* branches (getType() also null, but no 'null' in the type hint).
270	*
271	* @return string[]\|null null when the branch has no type constraint
272	*/
273	private function getBranchTypeNames(CompositionPropertyDecorator $branch): ?array	16✔
274	{
275	$type = $branch->getType();	16✔
276
277	if ($type !== null) {	16✔
278	return $type->getNames();	16✔
279	}
280
281	// NullModifier-processed branch: getType() is PHP null but typeHint contains 'null'.
282	if (str_contains($branch->getTypeHint(), 'null')) {	2✔
283	return ['null'];	2✔
284	}
285
NEW UNCOV 286	return null;	×
287	}
288	}

wol-soft / php-json-schema-model-generator / 26158351968

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous