Follow-up: broaden object-representability checks and reconsider forced type inheritance in compositions

[wol-soft]

Background

Follow-up from the work that added SchemaProcessor::checkObjectRepresentability() and
GeneratorConfiguration::setImplicitObjectComposition() (rejecting class-defining compositions —
file root, cross-file $ref targets — that cannot resolve to a definite object, unless the value
is guaranteed by an explicit type: object on the schema itself). Several related gaps were
identified during and after that work that are deliberately out of scope for it. Full analysis
below so the reasoning survives independently of the session that found it.

This ships in the same release as #166, so the behaviour changes land as a single migration
for users rather than several consecutive breaks.

Code references below were re-checked against master after the $ref-sibling rework (#185)
landed and #166 merged it.

Part 0 — documentation and API text to extend when Part 1 lands

#166 narrowed all user-facing text to the scope it actually implements, so nothing here is
currently inaccurate. What follows is the list of places describing the current narrow scope
that Part 1 must widen — a to-do for the implementer, not a correction:

• GeneratorConfiguration::setImplicitObjectComposition()'s docblock, which scopes the flag to
  "a schema file's root, or a $ref target parsed as a top-level schema in its own right", and
  additionally states that a root carrying only object keywords with no composition keyword is
  skipped before the check runs and is not affected by the flag at all.
• docs/source/gettingStarted.rst, "Implicit object composition", same narrow wording.
• docs/source/combinedSchemas/impliedObjects.rst, "Class-defining compositions must resolve to a
  definite object". Two .. note:: blocks close that section: one states that a named property,
  array item or schema dependencies value never triggers the rejection, the other that a bare
  object-keyword root is skipped entirely. Both become false when Part 1 lands and are the
  markers to search for.

Part 1 is a further default-behaviour change, so it also needs its own entry in the 1.0 release
notes alongside #166's.

Part 1 — Extend setImplicitObjectComposition() to every object-describing site, not just class-defining compositions

Today the check (and its opt-in flag) only affects the file root and cross-file $ref targets —
because every other site that reaches PropertyFactory::createObjectProperty() always does so with
type: object already forced onto the JSON by its own caller, before
checkObjectRepresentability() would ever get a chance to look at it. Two call sites currently
bypass any flag-aware check entirely.

1. Named object properties, array items, and composition branches

PropertyFactory::rerouteBareObjectValidator() (extracted from create() during the #166 review —
it is no longer inline in create(), so search by method name) handles the "bare object-validator
schema" reroute: no type, no filter, no composition keyword and no $ref, classifying as
ObjectShape::ObjectDescribing.

For a property, array item, or composition branch (each branch of a oneOf/allOf/anyOf is
itself processed through this same create() entry point) written as
{"properties": {"name": {"type": "string"}}} with no type, this always proceeds: it logs a
warning and generates a guarded class — object input is instantiated and validated, non-object
input silently passes through unchanged (the getter/setter stay mixed). This happens
unconditionally, regardless of setImplicitObjectComposition() — the flag is never consulted here.

2. Schema dependencies targets — no check, no warning at all

PropertyDependencyTrait::addDependencyValidator() (this method was called
wireSchemaDependency() before the #185 rework; the mechanism below is unchanged):

$json = $dependencyJsonSchema->getJson();
if (!isset($json['type'])) {
    $dependencyJsonSchema = $dependencyJsonSchema->withJson($json + ['type' => 'object']);
}

$dependencySchema = $schemaProcessor->processSchema($dependencyJsonSchema, ...);

The least-guarded of the two: it force-sets type: object on an untyped dependencies schema
unconditionally, with no ObjectShapeResolver classification and no warning of any kind —
not even the log message the property/array-item path at least emits.

Required change

Route both sites through the same classification checkObjectRepresentability() already uses.
When the resolved shape is ObjectShape::ObjectDescribing and setImplicitObjectComposition() is
false (default), generation should throw a SchemaException instead of the current
silent/warning-only guarded behavior. When the flag is true, today's guarded/pass-through
behavior is preserved unchanged, including the warning. ObjectShape::ObjectAsserting is
unaffected either way.

Note the exemption shape changed in #166. There is no longer a filter early return to
replicate: ObjectShapeResolver classifies a filter-bearing schema as
ObjectShape::Undecidable, and checkObjectRepresentability() returns without throwing for that
verdict, which also covers a filter nested inside a composition branch (the old early return only
ever saw a top-level filter key). Whatever check Part 1 adds at the sites above must likewise let
Undecidable through, or it will mask both filter and unresolved-reference diagnostics the same
way the root check did before it was fixed.

A schema whose dependency/property/branch value is itself a $ref likely stays unaffected the same
way root-level $refs are exempted today — worth confirming during implementation. Note that
sibling handling next to a $ref is now draft-dependent (Draft 07 suppresses siblings via
ExclusiveProducer, Draft 2019-09+ applies them); any new check must derive that from the draft
rather than restate it, exactly as ObjectShapeResolver::classifyReference() does.

This is a default-behavior change for any existing schema that currently relies on the
guarded-object warning without ever setting setImplicitObjectComposition(true) — such schemas
would start throwing at generation time.

Tests expected to change

Found by searching every fixture for a bare (no type, no $ref) properties/required schema
sitting at a named-property, composition-branch, or schema-dependencies site. Each will need
either setImplicitObjectComposition(true) added to its GeneratorConfiguration to keep testing
today's guarded behavior, or be rewritten to assert the new SchemaException:

• tests/Issues/Issue/Issue72Test.php (NestedOneOfBareObjectValidators.json,
  NestedAnyOfBareObjectValidators.json, StandaloneObjectDescribingProperty.json). This file was
  consolidated in Fix crashes and silent misvalidation in object-implied compositions, reject non-representable ones #166, so locate cases by fixture rather than by old method names.
  • the bare-validator oneOf instantiate/reject coverage
  • the bare-validator anyOf coverage, including the non-object-accepted-per-spec case
  • testStandaloneObjectDescribingPropertyWarnsAtGenerationAndRejectsInvalidObjects — special
    case: half its purpose is asserting the warning-only behavior Part 1 replaces by default, so it
    needs to be redesigned (e.g. split into "throws by default" + "still warns when the flag is
    enabled"), not just given the flag.
• tests/Basic/SchemaDependencyTest.php (SchemaDependency.json):
  testValidSchemaDependency, testInvalidSchemaDependency (both data-provider driven)
• tests/Issues/Issue/Issue86Test.php (schemaDependency.json):
  testDifferentSchemaDependenciesForReferencedObject,
  testDifferentSchemaDependenciesForReferencedObjectWithInvalidInput

Checked and confirmed unaffected (already type: object explicit, or $ref-based):
NestedObjectSchemaDependency.json, CompositionSchemaDependency.json,
ReferenceSchemaDependency.json, DependenciesImpliedObjectSimple.json,
DependenciesImpliedObject.json, PropertyDependencyTest/*.json. No existing fixture exercises a
bare object-describing array item, so array items get this behavior for the first time with no
prior test to update.

Base-level $ref parity — resolved in #166, keep it that way

An earlier revision listed a concern that a bare base-level $ref root could not be rescued by
setImplicitObjectComposition(true). Fixed in #166; recorded so Part 1 does not regress it.
Note the code moved during the #185 rework — it now lives in Draft\Producer\RefResolver, not
PropertyFactory:

• RefResolver::resolveBaseReference() no longer requires a single nested schema. A reference to a
  root-level anyOf/oneOf transfers its composition via transferComposedPropertiesToSchema(),
  exactly as the inline form does, and the referenced schema's base validators are transferred so
  object-level keywords are not silently dropped.
• A failure raised while eagerly generating a referenced schema's own class is no longer rewrapped
  as "Unresolved Reference"; it propagates naming the referenced file
  (SchemaException::isReferencedSchemaFailure()).

ComposedObjectShapeValidationTest asserts inline/$ref parity through a shared assertion helper.
Part 1 must keep those passing.

Part 2 — Reconsider inheritPropertyType()'s forced type injection onto composition branches

The current mechanism has two logically distinct parts

PropertyFactory::createBaseProperty() does one line of work with two very different
justifications bundled into it:

$objectJson         = $propertySchema->getJson();
$objectJson['type'] = 'object';
$this->applyModifiers($schemaProcessor, $schema, $property, $propertySchema->withJson($objectJson));

Part A — making the class's own direct keywords get processed at all. Draft::getCoveredTypes()
requires the literal string 'object' to be present in $type for the object Type entry's
validators to run. properties, required, patternProperties, additionalProperties,
propertyNames, minProperties, maxProperties and ObjectModifier are all registered
exclusively under the object Type. Without type: 'object' literally present in the JSON handed
to applyModifiers(), none of these run and the class's own properties would silently never be
parsed. Required, load-bearing infrastructure — do not touch.

Part B — propagating that forced type onto composition branches, via
inheritPropertyType()/inheritIfPropertyType(). This is the part in question.

Question A: can the root's class-or-not decision be driven by ObjectShapeResolver instead of the crude keyword gate?

SchemaProcessor::processSchema()'s gate is a crude syntactic pre-filter that returns null (skip
silently) for anything without type: object or a composition keyword or $ref.
checkObjectRepresentability() runs after it, as a separate semantic pass over the same JSON.
The natural simplification is to compute the classification once and derive both "should we even
attempt a class" and "is it valid" from it.

This is not a lossless merge. The silent-skip branch covers two behaviorally identical but
semantically different cases a plain shape classification cannot distinguish:

1. Genuinely nothing here — a definitions-only library file, an annotation-only fragment.
2. Explicitly, deliberately a different type — a root that is just {"type": "string"}: a
   reusable scalar type-definition file meant only to be $ref-ed elsewhere.

Both must stay a silent skip — RecursiveDirectoryProvider iterates every .json file in a schema
directory, and directories routinely contain non-object reusable type-definition files. But a
schema that has a composition keyword and also lands in ObjectShape::NotObject must not be
a silent skip — that is exactly the new rejection case.

Conclusion: the two mechanisms can be unified into one classification pass, but only by keeping
an explicit pre-check ahead of the ObjectShapeResolver call. This removes the redundant second
JSON walk without changing behavior, but does not eliminate the gate's keyword check, only
consolidate it with the classification that follows.

A third case belongs in this audit. A root carrying object keywords but no composition keyword,
no $ref and no type — e.g. {"properties": {"id": {"type": "string"}}, "required": ["id"]} —
produces no class and no diagnostic at all, under both settings of the flag. Verified identical
on master, so pre-existing rather than a #166 regression. It never reaches
checkObjectRepresentability() because it has no composition keyword.

This is the OpenAPI-inheritance case's other half: #166 fixes Derived.json, but the Base.json
component it inherits from still yields nothing. Question A must state deliberately whether the
flag should cover it, or whether such a root should warn rather than skip. #166 documents the
boundary in the meantime.

Note also that not appears in neither the gate's keyword list nor
ObjectShapeResolver::COMPOSITION_KEYWORDS, so a root {"not": ..., "properties": ...} is silently
skipped today. Pre-existing and arguably correct, but it belongs in the same audit.

Question B: can branch-level inheritPropertyType()/inheritIfPropertyType() be dropped for the object case?

Tracing what happens to a branch without the forced injection, for a schema that has already
passed checkObjectRepresentability():

• Aggregate Asserting via the type: object early-return: every branch is evaluated against
  an instance JSON Schema has already filtered to objects via the sibling type: object constraint.
  Forcing type: object onto a branch changes nothing.
• Aggregate Asserting via oneOf/anyOf's disjunctive combination: combineDisjunctive()
  requires every branch to already be independently Asserting — no branch "needed" the injection.
• Aggregate Asserting via allOf's conjunctive combination: other branches can be
  Describing/Neutral, but since allOf requires every branch to hold simultaneously and the
  Asserting branch alone already restricts accepted values to objects, an untyped sibling is
  evaluated only against values already known to be objects.

In every case the forced branch-level injection is provably redundant for the accepted value set
once representability has been established upfront.

Bug 1: the forced injection manufactures an unsatisfiable branch

{
  "allOf": [
    { "type": "object", "properties": { "name": { "type": "string" } } },
    { "enum": [1, 2, 3] }
  ]
}

Branch 1 → Asserting; branch 2 → Neutral. combineConjunctive → Asserting, which passes
checkObjectRepresentability() correctly. But that classification is computed on the JSON before
inheritPropertyType() mutates branch 2: it force-injects type: object, turning it into
{"type": "object", "enum": [1, 2, 3]} — a branch no value can satisfy. The class becomes silently
uninstantiable via this branch combination.

Confirmed empirically: generates without complaint on master, and every input then throws
AllOfException ... Value for 'Root' must be one of [1,2,3], got {} — the got {} proving branch 2
was force-typed to object and the value instantiated.

#166 closes the silence, not the corruption: it emits a generation-time warning when the
injection forces object onto a branch whose enum/const values contain no object. Deliberately
a warning rather than a SchemaException, because the unsatisfiability is manufactured by the
generator's own mutation step — once the injection is removed the same schema becomes merely odd
(an object can never equal an integer), which is visible in the schema as written. Part 2 should
turn that warning into either correct generation or a SchemaException once the injection is gone.

Bug 2: the forced injection suppresses the vacuous-branch warning

Verified: {"type": "string", "allOf": [{"minLength": 2}, {}]} emits no vacuous-branch warning;
dropping the outer type emits it. The {} branch inherits the outer type, which counts as a
constraint and suppresses the check. Since createBaseProperty() always forces type: object at a
root, a {} branch in a root composition never warns — only a literal true branch does, because
boolean branches skip the injection entirely.

#166 documents the discrepancy in allOf.rst rather than hiding it. Removing the injection fixes it.

Bug 3: inheritIfPropertyType() also force-types the if condition itself

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

if is a pure boolean test — forcing type: object onto it changes which values satisfy the
condition, which can change whether a value is routed through then or else. A distinct
correctness concern from redundancy: only then/else need their own type for merging purposes.
Needs its own investigation.

Why this can't simply be deleted wholesale

inheritPropertyType() is type-agnostic machinery used by every composition, for every declared
outer type, at every site a composition can appear. A scalar example that still needs it:
{"type": "string", "oneOf": [{"enum": ["a", "b"]}]} — the branch has no type of its own;
injecting string is what lets scalar validators apply, and there is no ObjectShapeResolver
equivalent for scalar types. Any change must be scoped specifically to the type: object case.

What needs verification before implementing Question B

1. Empirically audit every existing test exercising an untyped composition branch at a class
   boundary, confirming none rely on the forced injection to reach the object path through a branch
   that would not otherwise independently classify as Asserting/Describing.
2. Confirm the same reasoning holds for Describing aggregates accepted under
   setImplicitObjectComposition(true) — the argument above was made for Asserting.
3. Re-verify the if/then/else case specifically.
4. Decide whether Question A's unification lands alongside Question B or separately — related but
   not dependent (A changes whether a class is attempted; B changes what happens to branches
   after that decision).
5. Check the interaction with the base-validator transfer and composition transfer now living in
   RefResolver::resolveBaseReference(), since PropertyDependencyTrait builds its dependency
   class through the same processSchema() path — so Part 1's dependencies work and this
   mechanism meet in the same code.

Part 3 — $ref to a boolean schema cannot be represented

Separate, small, independent of Parts 1 and 2.

JsonSchema::$json is typed array, so a boolean-valued definition ("definitions": {"yes": true})
cannot round-trip through it. Two consequences:

• ObjectShapeResolver::forDictionary()'s peek fails and the reference is classified as decidably
  Blocking, so {"allOf": [<object branch>, {"$ref": "#/definitions/yes"}]} at a root is rejected
  as non-representable even though true imposes nothing and the allOf genuinely is a definite
  object. The equivalent with an inline true branch is accepted.
• The same schema at a named property fails with an uncaught TypeError (Cannot assign true to property JsonSchema::$json of type array), because AllOfValidatorFactory resolves every branch
  into a real Property regardless of classification.

Fixing only the classification was attempted during the #166 review and deliberately reverted:
classifying a $ref to true faithfully as neutral lets generation proceed past the
representability check and then hit that uncaught TypeError, trading a clean SchemaException for
a PHP type error.

#166 hardened this further. When the Undecidable verdict was introduced, the boolean-leaf case was
deliberately kept out of it: forDictionary() catches the TypeError separately from genuine
resolution failures and maps it to Blocking, precisely so it does not get handed back to the
pipeline and resurface as the TypeError again. That distinction is load-bearing — do not collapse
the two catches when implementing this part.

The real fix is to let JsonSchema carry a boolean (or model boolean schemas explicitly), which
touches JsonSchema, SchemaDefinition and SchemaDefinitionDictionary signatures. Worth doing on
its own. This is the last remaining inline-vs-$ref divergence after #166 closed the composition
ones.

Part 4 — Branch enumeration skips mutable base validators

Composition error messages in direct-exception mode enumerate every branch with its own reason —
except when the composition is a mutable base validator. Both ComposedItem.phptpl and
ComposedItemBody.phptpl gate the enumeration on not isMutableBaseValidator(...).

Verified: the same root oneOf, generated with setImmutable(true), produces the full per-branch
breakdown; with setImmutable(false) it produces only the bare two-line summary. A named-property
composition is unaffected under either setting — the gap is specific to base validators.

Closing it means giving the mutable base-validator template path its own per-branch error registry.
#166 pins the current divergence with a test
(testBranchEnumerationInDirectExceptionModeSkipsMutableBaseValidators) so the eventual fix shows
up as a named failure rather than a silent change, and qualifies its release-notes entry accordingly.

[wol-soft]

Additional evidence for Part 2: the forced type injection also suppresses the vacuous-branch warning

Found while making a literal true composition branch warn like the other three spellings of "this
branch constrains nothing" (landed in #166). Recording it here because it is a second concrete,
reproducible symptom of inheritPropertyType()'s injection — and removing that injection fixes it
for free, exactly like the enum-branch corruption already described above.

The mechanism. warnIfVacuousBranch() inspects the branch JSON after
inheritPropertyType() has run. That step copies the outer schema's type onto any branch that
does not declare its own. type is in MODIFIER_ONLY_VALIDATION_KEYWORDS, so the injected keyword
makes the branch look like it carries a real constraint, and the warning is skipped. Booleans are
exempt from the injection (is_bool guard), so a true branch keeps warning.

Reproduction — not object-specific, a plain string outer type is enough:

{ "type": "object", "properties": { "p": { "allOf": [ { "minLength": 2 }, {} ] } } }

vacuous branch	outer type absent	outer type present
{}	warned	no warning
true	warned	warned

So today, any schema whose property declares a type alongside a composition silently loses the
vacuous-branch diagnostic for every non-boolean vacuous branch — {}, {"example": ...},
{"title": ..., "description": ...} alike. That is precisely the example-only branch shape from
the original issue this work started from.

Which side is wrong. The suppressed spellings are. The author wrote a branch carrying no
constraint; the type that makes it look constrained was manufactured by the generator's own
mutation step. The vacuity check should be reading what the author wrote.

Why it was not fixed alongside the true branch. Doing so requires the check to see the
pre-injection branch JSON, which means plumbing the original schema through — building on the exact
mechanism Question B proposes to delete. Removing the injection makes the check correct with no
extra plumbing.

Add to Question B's verification list: after dropping the branch-level injection for the object
case, confirm all four vacuous spellings warn identically in every context, including when the
outer schema declares a type. Issue72Test's vacuous-branch test already pins true / {} parity
for the non-injected case and is the natural place to extend.

---

Generated by Claude Code

[wol-soft]

Adding a concrete reproduction for Part 3, found while reviewing #166. Part 3 already states that
the same schema at a named property fails with an uncaught TypeError; this pins the exact input,
the exact message, and the root-vs-property divergence.

Reproduction

{
  "type": "object",
  "definitions": {
    "anything": true
  },
  "properties": {
    "p": {
      "allOf": [
        {
          "type": "object",
          "properties": {
            "a": {
              "type": "string"
            }
          }
        },
        {
          "$ref": "#/definitions/anything"
        }
      ]
    }
  }
}

TypeError: Cannot assign true to property PHPModelGenerator\Model\SchemaDefinition\JsonSchema::$json of type array

Moving the identical composition to the file root instead produces the intended clean rejection:

SchemaException: Composition for '<Class>' in file '<file>' does not resolve to a definite object and cannot be represented as a generated class: …

Verified to reproduce identically on master, so it is pre-existing rather than anything #166
introduced.

Why the two sites diverge

• At a root, SchemaProcessor::checkObjectRepresentability() runs first.
  ObjectShapeResolver::forDictionary()'s $ref resolver catches the TypeError from
  JsonSchema::navigate() and maps it to BranchObjectShape::Blocking, which becomes the
  SchemaException above.
• At a named property no representability check runs at all. AllOfValidatorFactory resolves every
  branch into a real Property regardless of classification, so the same navigate() call is
  reached for real and the TypeError escapes uncaught.

That difference is worth recording alongside Part 3's existing note, because it means the fix cannot
be "add the check at property level too" — the check is not what makes the root case safe, the
TypeError catch inside forDictionary() is.

Note for whoever implements this

This was originally going to be pinned with a test asserting the TypeError. That was dropped
deliberately: a test asserting a TypeError reads as a contract rather than a defect, and the
expectation would be the thing to delete once the bug is fixed. So there is no in-suite coverage of
this case today — the reproduction above is the only record, which is why it is spelled out here in
full.

[wol-soft]

One addition to Part 1's "Tests expected to change" list, for a test that did not exist when this
issue was written.

#166 added Issue72Test::testBareObjectDependencyValueIsForceAssertedWithoutADescribingWarning
(fixture tests/Schema/Issues/72/DependenciesBareObjectValue.json), which pins exactly the
behaviour Part 1 item 2 replaces — a bare properties/required dependencies value being
force-typed to object before any classification runs, with no warning:

{
  "type": "object",
  "properties": {
    "creditCard": {
      "type": "string"
    }
  },
  "dependencies": {
    "creditCard": {
      "properties": {
        "billingAddress": {
          "type": "string"
        }
      },
      "required": ["billingAddress"]
    }
  }
}

It needs inverting rather than flag-guarding, unlike the three Issue72Test cases already
listed. Adding setImplicitObjectComposition(true) will not keep it passing, because it asserts two
things that Part 1 changes in opposite directions:

• No describing warning is emitted. Once the site is routed through ObjectShapeResolver, the
  flag-enabled path emits one, so this assertion has to flip to asserting the warning fires.
• Generation succeeds and the dependency's constraints are enforced. Under the default
  (setImplicitObjectComposition(false)) Part 1 makes this throw a SchemaException at generation
  time, so the enforcement half has to move behind the flag.

The natural shape after Part 1 is the same split suggested for
testStandaloneObjectDescribingPropertyWarnsAtGenerationAndRejectsInvalidObjects: "throws by
default" plus "still warns and enforces when the flag is enabled".

Worth flagging because this test is currently the only in-suite record that the dependencies site
behaves differently from the property and array-item sites — that asymmetry is invisible everywhere
else in the suite, which is why it was pinned rather than left implicit.