Skip to content

Commit 6ca6622

Browse files
committed
Merge remote-tracking branch 'origin/master'
2 parents b249578 + 54cb94f commit 6ca6622

27 files changed

Lines changed: 1326 additions & 22 deletions

CLAUDE.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,23 @@
11
# CLAUDE.md
22

3+
## Responding to review notes
4+
5+
When working through a list of review notes, critically evaluate each note before acting on it:
6+
7+
- **Is the note correct?** The reviewer may be mistaken about what the code does, or may be
8+
operating on a false assumption. If the note is factually wrong, explain why and skip it.
9+
- **Is the proposed fix better than the current approach?** The reviewer's suggestion is a
10+
starting point, not a mandate. If a different solution is clearly superior, propose it.
11+
- **Is there an even better solution?** Think beyond the note. If the reviewer flags a smell,
12+
consider whether the right fix is the one they suggest or a deeper redesign.
13+
- **Document the reasoning.** For each note, produce a summary of what action was taken and why
14+
— including why any note was rejected or handled differently than suggested.
15+
16+
After tackling all notes, provide a summary table: one row per note, action taken, and brief
17+
reasoning.
18+
19+
---
20+
321
## Learning from reviews
422

523
After completing a task that involved responding to code review feedback, scan the reviewer's

docs/source/combinedSchemas/mergedProperty.rst

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,34 @@ This schema will generate three classes as no merged property is created. The ma
102102
public function setName(string $name): static
103103
public function getAge(): ?int
104104
public function setAge(int $age): static
105+
106+
Branch-constraint isolation
107+
---------------------------
108+
109+
When the same property name appears in multiple composition branches with different constraints
110+
(e.g. one ``oneOf`` branch requires an ``enum`` value while another allows a free-form string),
111+
the outer merged class does **not** inherit branch-specific constraints from any individual
112+
branch. Validation constraints such as ``enum``, ``minLength``, or ``pattern`` remain scoped to
113+
their respective branch and are enforced only when that branch is being validated.
114+
115+
This means that the ``EnumPostProcessor`` and similar post-processors will correctly operate on
116+
the branch-level property — not on the outer merged property — so the generated merged class
117+
setter accepts the full union of allowed values from all branches rather than being narrowed to
118+
the constraints of a single branch.
119+
120+
Attributes for shared properties
121+
---------------------------------
122+
123+
When ``JSON_POINTER`` or ``JSON_SCHEMA`` attributes are enabled (see
124+
`Attributes <../gettingStarted.html#attributes>`__), properties that appear in more than one
125+
composition branch receive synthesised attributes:
126+
127+
- **JSON_POINTER**: one ``#[JsonPointer]`` attribute is emitted per branch that defines the
128+
property. When the property is also declared in the root ``properties`` block, the root
129+
pointer is prepended.
130+
131+
- **JSON_SCHEMA**: a single ``#[JsonSchema]`` attribute is emitted whose value is a synthesised
132+
JSON object containing the composition keyword (``allOf``, ``anyOf``, ``oneOf``, or
133+
``if``/``then``/``else`` sub-objects) with only those branch sub-schemas that involve this
134+
property. Root-level constraints are merged into the top-level object when the property is
135+
also root-registered.

docs/source/gettingStarted.rst

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,9 @@ The following attributes are available:
320320
* - JSON_POINTER
321321
- Classes & Properties
322322
- Adds a JSON Pointer with the path in the source Schema. This attribute can't be disabled.
323+
For properties that appear in multiple composition branches (e.g. ``oneOf``, ``allOf``),
324+
one ``#[JsonPointer]`` attribute is emitted per defining branch, plus one for the
325+
root-level definition when the property is also declared in the top-level ``properties`` block.
323326
- Yes
324327
* - SCHEMA_NAME
325328
- Properties
@@ -331,7 +334,11 @@ The following attributes are available:
331334
- No
332335
* - JSON_SCHEMA
333336
- Classes & Properties
334-
- Provides the full JSON Schema used to generate the class or the property
337+
- Provides the full JSON Schema used to generate the class or the property.
338+
For properties transferred from composition branches, the attribute value is
339+
*synthesised*: it contains the composition keyword (e.g. ``oneOf``) with only the
340+
branch-level sub-schemas that define this property, merged with any root-level
341+
constraints when the property is also declared in the top-level ``properties`` block.
335342
- No
336343
* - REQUIRED
337344
- Properties

src/Model/Attributes/AttributesTrait.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,20 @@ trait AttributesTrait
1111
/** @var PhpAttribute[] */
1212
private array $phpAttributes = [];
1313

14+
public function filterAttributes(callable $filter): static
15+
{
16+
$this->phpAttributes = array_values(array_filter($this->phpAttributes, $filter));
17+
18+
return $this;
19+
}
20+
1421
public function addAttribute(
1522
PhpAttribute $attribute,
1623
?GeneratorConfiguration $generatorConfiguration = null,
1724
?int $enablementFlag = null,
1825
): static {
19-
if ($generatorConfiguration
26+
if (
27+
$generatorConfiguration
2028
&& $enablementFlag
2129
&& ($generatorConfiguration->getEnabledAttributes() & $enablementFlag) === 0
2230
) {

src/Model/Property/AbstractProperty.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44

55
namespace PHPModelGenerator\Model\Property;
66

7+
use PHPModelGenerator\Attributes\JsonPointer;
78
use PHPModelGenerator\Exception\SchemaException;
89
use PHPModelGenerator\Model\Attributes\AttributesTrait;
10+
use PHPModelGenerator\Model\Attributes\PhpAttribute;
911
use PHPModelGenerator\Model\SchemaDefinition\JsonSchema;
1012
use PHPModelGenerator\Model\SchemaDefinition\JsonSchemaTrait;
1113
use PHPModelGenerator\Utils\NormalizedName;
@@ -36,6 +38,16 @@ public function __construct(protected string $name, JsonSchema $jsonSchema)
3638
$this->attribute = $this->processAttributeName($this->name);
3739
}
3840

41+
/**
42+
* @inheritdoc
43+
*/
44+
public function setJsonSchema(JsonSchema $jsonSchema): static
45+
{
46+
$this->jsonSchema = $jsonSchema;
47+
48+
return $this;
49+
}
50+
3951
/**
4052
* @inheritdoc
4153
*/
@@ -56,6 +68,18 @@ public function getAttribute(bool $variableName = false): string
5668
return ($this->isInternal() ? '_' : '') . $attribute;
5769
}
5870

71+
/**
72+
* @inheritdoc
73+
*/
74+
public function overrideJsonPointer(PhpAttribute $attribute): static
75+
{
76+
$this->filterAttributes(
77+
static fn(PhpAttribute $existing): bool => $existing->getFqcn() !== JsonPointer::class,
78+
);
79+
80+
return $this->addAttribute($attribute);
81+
}
82+
5983
/**
6084
* Convert a name of a JSON-field into a valid PHP variable name to be used as class attribute
6185
*

src/Model/Property/PropertyInterface.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,12 +169,23 @@ public function getNestedSchema(): ?Schema;
169169
*/
170170
public function getJsonSchema(): JsonSchema;
171171

172+
public function setJsonSchema(JsonSchema $jsonSchema): static;
173+
174+
public function filterAttributes(callable $filter): static;
175+
172176
public function addAttribute(
173177
PhpAttribute $attribute,
174178
?GeneratorConfiguration $generatorConfiguration = null,
175179
?int $enablementFlag = null,
176180
): static;
177181

182+
/**
183+
* Replace the JsonPointer attribute with one carrying the given pointer value.
184+
* Used by processReference to set the reference site's pointer on a resolved property
185+
* rather than the definition's pointer.
186+
*/
187+
public function overrideJsonPointer(PhpAttribute $attribute): static;
188+
178189
/**
179190
* @return PhpAttribute[]
180191
*/

src/Model/Property/PropertyProxy.php

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
namespace PHPModelGenerator\Model\Property;
66

7+
use PHPModelGenerator\Attributes\JsonPointer;
8+
use PHPModelGenerator\Attributes\SchemaName;
79
use PHPModelGenerator\Exception\SchemaException;
810
use PHPModelGenerator\Model\Attributes\PhpAttribute;
911
use PHPModelGenerator\Model\GeneratorConfiguration;
@@ -21,6 +23,9 @@
2123
*/
2224
class PropertyProxy extends AbstractProperty
2325
{
26+
private ?JsonSchema $overrideJsonSchema = null;
27+
private ?PhpAttribute $overrideJsonPointer = null;
28+
2429
/**
2530
* PropertyProxy constructor.
2631
*
@@ -296,7 +301,20 @@ public function getNestedSchema(): ?Schema
296301
*/
297302
public function getJsonSchema(): JsonSchema
298303
{
299-
return $this->getProperty()->getJsonSchema();
304+
return $this->overrideJsonSchema ?? $this->getProperty()->getJsonSchema();
305+
}
306+
307+
/**
308+
* @inheritdoc
309+
*
310+
* Stores a local override rather than delegating to the underlying property, preventing
311+
* mutation of a shared $ref-resolved property when only this proxy's schema must change.
312+
*/
313+
public function setJsonSchema(JsonSchema $jsonSchema): static
314+
{
315+
$this->overrideJsonSchema = $jsonSchema;
316+
317+
return $this;
300318
}
301319

302320
/**
@@ -317,6 +335,16 @@ public function isInternal(): bool
317335
return $this->getProperty()->isInternal();
318336
}
319337

338+
/**
339+
* @inheritdoc
340+
*/
341+
public function filterAttributes(callable $filter): static
342+
{
343+
$this->getProperty()->filterAttributes($filter);
344+
345+
return $this;
346+
}
347+
320348
/**
321349
* @inheritdoc
322350
*/
@@ -330,11 +358,45 @@ public function addAttribute(
330358
return $this;
331359
}
332360

361+
/**
362+
* Store the pointer attribute locally so this proxy can show a different JsonPointer from
363+
* the shared underlying property (each reference site has its own pointer).
364+
*/
365+
public function overrideJsonPointer(PhpAttribute $attribute): static
366+
{
367+
$this->overrideJsonPointer = $attribute;
368+
369+
return $this;
370+
}
371+
333372
/**
334373
* @inheritdoc
374+
*
375+
* Replaces the SchemaName attribute from the underlying shared property with one that
376+
* carries the proxy's own name. Two proxies sharing the same $ref definition would
377+
* otherwise both report the first property's name via the shared underlying attribute.
378+
*
379+
* When a JsonPointer override is set, all existing JsonPointer attributes (there may be
380+
* multiple when the underlying property was synthesised from composition branches) are
381+
* removed and replaced with a single attribute pointing to the reference site.
335382
*/
336383
public function getAttributes(): array
337384
{
338-
return $this->getProperty()->getAttributes();
385+
$attributes = array_map(
386+
fn(PhpAttribute $attribute): PhpAttribute => $attribute->getFqcn() === SchemaName::class
387+
? new PhpAttribute(SchemaName::class, [$this->name])
388+
: $attribute,
389+
$this->getProperty()->getAttributes(),
390+
);
391+
392+
if ($this->overrideJsonPointer !== null) {
393+
$attributes = array_values(array_filter(
394+
$attributes,
395+
static fn(PhpAttribute $attribute): bool => $attribute->getFqcn() !== JsonPointer::class,
396+
));
397+
$attributes[] = $this->overrideJsonPointer;
398+
}
399+
400+
return $attributes;
339401
}
340402
}

src/Model/Schema.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ class Schema
6363
/** @var callable[] */
6464
private array $onAllPropertiesResolvedCallbacks = [];
6565

66+
/** @var array<string, true> */
67+
private array $rootRegisteredPropertyNames = [];
68+
6669
/** @var string[] Maps normalized attribute → raw property name; used to detect property-vs-property collisions */
6770
private array $attributeIndex = [];
6871

@@ -204,7 +207,7 @@ public function addProperty(PropertyInterface $property, ?string $compositionPro
204207
$this->properties[$property->getName()] = $property;
205208

206209
if ($compositionProcessor === null) {
207-
$this->propertyMerger->markRootRegistered($property->getName());
210+
$this->rootRegisteredPropertyNames[$property->getName()] = true;
208211
}
209212

210213
$property->onResolve(function (): void {
@@ -224,11 +227,22 @@ public function addProperty(PropertyInterface $property, ?string $compositionPro
224227
$this->properties[$property->getName()],
225228
$property,
226229
is_a($compositionProcessor, AllOfValidatorFactory::class, true),
230+
$this->isRootRegistered($property->getName()),
227231
);
228232

229233
return $this;
230234
}
231235

236+
public function isRootRegistered(string $name): bool
237+
{
238+
return isset($this->rootRegisteredPropertyNames[$name]);
239+
}
240+
241+
public function getProperty(string $name): ?PropertyInterface
242+
{
243+
return $this->properties[$name] ?? null;
244+
}
245+
232246
/**
233247
* @return PropertyValidatorInterface[]
234248
*/

src/Model/Validator/ConditionalPropertyValidator.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace PHPModelGenerator\Model\Validator;
66

7+
use LogicException;
78
use PHPModelGenerator\Exception\ComposedValue\ConditionalException;
89
use PHPModelGenerator\Model\GeneratorConfiguration;
910
use PHPModelGenerator\Model\Property\CompositionPropertyDecorator;
@@ -20,6 +21,9 @@ class ConditionalPropertyValidator extends AbstractComposedPropertyValidator
2021
/** @var CompositionPropertyDecorator[] */
2122
private array $conditionBranches;
2223

24+
private ?CompositionPropertyDecorator $thenBranch;
25+
private ?CompositionPropertyDecorator $elseBranch;
26+
2327
public function __construct(
2428
GeneratorConfiguration $generatorConfiguration,
2529
PropertyInterface $property,
@@ -41,6 +45,8 @@ public function __construct(
4145
$this->compositionProcessor = IfValidatorFactory::class;
4246
$this->composedProperties = $composedProperties;
4347
$this->conditionBranches = $conditionBranches;
48+
$this->thenBranch = $validatorVariables['thenProperty'] ?? null;
49+
$this->elseBranch = $validatorVariables['elseProperty'] ?? null;
4450
}
4551

4652
/**
@@ -53,6 +59,35 @@ public function getConditionBranches(): array
5359
return $this->conditionBranches;
5460
}
5561

62+
/**
63+
* Returns the if-condition branch.
64+
*
65+
* The if branch is always present — ConditionalPropertyValidator cannot be constructed
66+
* without a valid if schema (IfValidatorFactory throws SchemaException otherwise).
67+
*/
68+
public function getIfBranch(): CompositionPropertyDecorator
69+
{
70+
foreach ($this->composedProperties as $composedProperty) {
71+
if (!in_array($composedProperty, $this->conditionBranches, true)) {
72+
return $composedProperty;
73+
}
74+
}
75+
76+
// @codeCoverageIgnoreStart
77+
throw new LogicException('ConditionalPropertyValidator has no if branch — this is a bug.');
78+
// @codeCoverageIgnoreEnd
79+
}
80+
81+
public function getThenBranch(): ?CompositionPropertyDecorator
82+
{
83+
return $this->thenBranch;
84+
}
85+
86+
public function getElseBranch(): ?CompositionPropertyDecorator
87+
{
88+
return $this->elseBranch;
89+
}
90+
5691
/**
5792
* Initialize variables required by the conditional validator.
5893
*/

0 commit comments

Comments
 (0)