Skip to content

Accept filters in nested allOf object branches; widen conditional-only properties to mixed - #192

Open
wol-soft wants to merge 3 commits into
masterfrom
claude/sweet-wright-0c9raw
Open

wol-soft wants to merge 3 commits into
masterfrom
claude/sweet-wright-0c9raw

Conversation

@wol-soft

@wol-soft wol-soft commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

Two related defects showed up with an object schema that puts independent if/then blocks into an allOf and uses filters (e.g. trim) on the then properties.

1. Filter on an object property inside a conditional nested in an allOf branch was rejected

A filter keyword inside a allOf composition branch is not supported was thrown, although the filter sits on a property of a branch that is instantiated as its own class. The identical conditional without the allOf wrapper was accepted.

Cause: CompositionCompatibilityChecker::branchContainsFilter() descended into the nested then before that level had inherited type: object from its parent, so the untyped then looked like a scalar branch.

Fix: the recursion into nested compositions is removed. Every nested composition builds its own branch property and runs the check on its own branches after its own type inheritance, so a filter in a scalar composition is still rejected at every depth (verified for allOf>allOf, three levels, allOf>anyOf, allOf>not, allOf>if/then). The message now names the innermost composition (three pinned messages updated; each row still asserts a rejection). Passing the inherited object-ness down manually was rejected because it would duplicate the inheritance rules.

One shape becomes legal: a composition under properties of a string-typed branch. Those properties can never apply to a string, so the filter can never run (pinned by a test with an explanatory comment).

2. TypeError for properties declared only in then (or only in else)

A typed property that exists only in one branch of a conditional kept a typed slot (?int), although a value of any type is valid while that branch is inactive. Reading it threw a TypeError from the getter; for a conditional nested in an allOf branch the constructor threw it (direct and collect-errors mode), i.e. a raw TypeError instead of a ValidationException for a schema-valid input.

Cause: exclusiveBranchPropertyNeedsWidening() only compares with other existing branches; the missing else was never considered, contradicting the rule documented in crossTypedComposition.rst ("properties exclusive to a single branch are widened to mixed"; anyOf/oneOf already do this).

Fix: a conditional with only one of then/else is treated as having an implicit unconstrained branch, so the property is widened to mixed. A property that is additionally declared at the root keeps its type (guarded by a test). Rejecting wrong-typed values outside the active branch instead was considered and rejected: it would invalidate instances the schema allows.

⚠️ Behaviour change

Generated getter/setter types for properties that exist only in a then or only in an else change from ?T to mixed. This reverses what if.rst and two tests documented before (testCrossTypedThenOnlyProducesNullableHint, an assertion in testIfOnlyPropertyIsTransferredToParentSchema); both were updated with their justification, and the docs were corrected. Worth a release note.

Documentation

filter.rst (filter rule, new section on filters in object branches), if.rst (then-only type hint), crossTypedComposition.rst (conditional with one branch). filter.rst also now documents an existing behaviour that may surprise: a filter declared on a conditional branch property also runs while the branch is inactive, because the property belongs to the merged class.

Test plan

Developed test-first; the commits show it:

  1. Add failing tests ... – tests only. Against the old code 16 of 3245 cases fail, all of them the new/updated tests; everything else passes.
  2. Accept filters in nested allOf object branches and widen ... – the implementation, the docs and the one assertion that depends on the new behaviour.
  3. Remove redundant test schemas – pruning of acceptance rows that the runtime tests already cover with superset schemas.
  • Full suite on PHP 8.4.26 with PHPUnit 13.4.1: 3240 tests, 8289 assertions, OK (baseline before the change: 3230 tests OK)
  • phpcs --standard=phpcs.xml on all changed PHP files: clean
  • Not run locally: PHP 8.5 and coverage (CI matrix)
  • New base test helpers assertMessageMatchesTemplate() / assertInstantiationFailsWithMessage() assert complete exception messages with only generated class names dynamic

Not part of this PR

  • Declaring branch-only properties with else: {"properties": {"x": false}} (or unevaluatedProperties, once available) is a schema-level way to forbid values outside the active branch; additionalProperties: false at the root does not work for this because it rejects the branch properties.
  • The filter of a conditional branch property running while the branch is inactive is documented here, not changed.
  • Unrelated pre-existing observation: a type-specific keyword on a property without type (e.g. {"minimum": 10}) is ignored even without any composition.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y7cwkHFG9yLwdmxU9pPrbR

claude added 2 commits October 9, 2026 11:54
…l-only property types

Tests only; they fail against the current implementation.

Filter in an object-typed allOf branch: a conditional (or anyOf/oneOf/not)
nested in an allOf branch of an object schema is rejected with "A filter
keyword inside a allOf composition branch is not supported" although the
filter sits on an object property of a branch which is instantiated as its
own class. The same shape without the allOf wrapper is accepted.

Detection by the nested level: pin that a filter in a scalar composition is
still rejected when it is nested in allOf/anyOf/not/if, at any depth, and
that the string-typed branch with a filter in a composition under its inert
"properties" is accepted. The messages of three existing rows name the
innermost composition once the enclosing level no longer descends into it.

TypeError: a typed property which is only declared in the then (or else) of a
conditional keeps a typed slot, although a value of any type is valid while
the branch is inactive. Reading it throws a TypeError from the getter; inside
an allOf branch the constructor throws it, in direct and collect-errors mode.
Includes a guard that a property which is also declared at the root keeps its
type.

Adds assertMessageMatchesTemplate()/assertInstantiationFailsWithMessage() to
the base test case to assert complete messages with only the generated class
names being dynamic.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y7cwkHFG9yLwdmxU9pPrbR
…only properties to mixed

Filters in nested compositions of an allOf branch:
branchContainsFilter() descended into nested compositions of a branch before
the nested level had inherited the object type of its parent, so the untyped
"then" of a conditional in an object-typed allOf branch was taken for a scalar
branch and the filter on its object property was rejected. The same schema
without the allOf wrapper was accepted. The recursion is removed: every
nested composition builds its own branch property and runs the check on its
own branches after its own type inheritance, so a filter in a scalar
composition is still rejected at every depth (the message now names the
innermost composition). Passing the inherited object-ness down manually was
rejected as it would duplicate the inheritance rules.

A composition below "properties" of a string-typed branch is no longer
inspected; such properties can never apply to a string, so the filter can
never run.

TypeError for conditional-only properties:
a property declared only in the then (or only in the else) of an
if/then/else kept a typed slot although the missing branch constrains
nothing and a value of any type is valid while the declaring branch is
inactive. Reading it threw a TypeError from the getter, and for a conditional
nested in an allOf branch the constructor threw it. A conditional with only
one branch is now treated like a composition with an implicit unconstrained
branch, so the property is widened to mixed as documented for properties
exclusive to a single branch. A property which is also declared at the root
keeps its type. This changes the generated accessors of then-only/else-only
properties from ?T to mixed.

Updates the documentation of the filter rule, the then-only type hint and
the widening rule.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y7cwkHFG9yLwdmxU9pPrbR
@coveralls

coveralls commented Oct 9, 2026 •

Copy link
Copy Markdown

Coverage Report for CI Build 37928178898

Coverage increased (+0.01%) to 98.892%

Details

  • Coverage increased (+0.01%) from the base build.
  • Patch coverage: 4 of 4 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 8029
Covered Lines: 7940
Line Coverage: 98.89%
Coverage Strength: 597.12 hits per line

💛 - Coveralls

The static acceptance rows for a conditional, anyOf, oneOf and not nested in
an allOf object branch only asserted that generation succeeds. The runtime
tests generate supersets of these schemas (the playlist conditional and a
combined anyOf/oneOf/not schema) and fail with the same SchemaException if
that regresses, naming the branch index of the affected composition.

The allOf nested three levels deep exercises the same per-level check as the
allOf nested two levels deep.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y7cwkHFG9yLwdmxU9pPrbR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants