openapi: a field redeclared across allOf branches does not intersect its constraints #10

Problem

Every allOf branch must hold, so a field two inline branches declare should carry the intersection of their constraints. A keyword only one declaration sets reaches the field where its type is a shared primitive. Two cases still leave a constraint out of the typed IR, and in both the result follows branch order.

Both declarations set a keyword. The first declaration's value is kept, even where the other is tighter, so a consumer validating against the field accepts values the spec rules out. Saved as bounds.yaml:

openapi: 3.0.3
info: {title: t, version: "1"}
paths: {}
components:
  schemas:
    LooserFirst:
      allOf:
        - {type: object, properties: {x: {type: string, maxLength: 20}}}
        - {type: object, properties: {x: {type: string, maxLength: 10}}}
    TighterFirst:
      allOf:
        - {type: object, properties: {x: {type: string, maxLength: 10}}}
        - {type: object, properties: {x: {type: string, maxLength: 20}}}

morphic compile --verify exits 0, each losing declaration is kept verbatim under Unmodeled, and the whole diagnostic list is:

warning openapi/conflicting-redeclaration bounds.yaml#/components/schemas/LooserFirst/allOf/1/properties/x: declarations of field "x" disagree: conflicting maxLength (20 and 10); kept what the declaration at /components/schemas/LooserFirst/allOf/0/properties/x says over the one at /components/schemas/LooserFirst/allOf/1/properties/x, which is kept verbatim under Unmodeled
warning openapi/conflicting-redeclaration bounds.yaml#/components/schemas/TighterFirst/allOf/1/properties/x: declarations of field "x" disagree: conflicting maxLength (10 and 20); kept what the declaration at /components/schemas/TighterFirst/allOf/0/properties/x says over the one at /components/schemas/TighterFirst/allOf/1/properties/x, which is kept verbatim under Unmodeled

minLength, minimum and maximum lower the same way.

The constraint sits on the type's own node. A list, and a string with an unknown format, lower to a node holding their constraints. Two such declarations are reported as an overlap and the first one's node is kept, so a bound the other adds leaves the typed IR, even where the first sets none. Saved as nodes.yaml:

openapi: 3.0.3
info: {title: t, version: "1"}
paths: {}
components:
  schemas:
    BareListFirst:
      allOf:
        - {type: object, properties: {x: {type: array, items: {type: string}}}}
        - {type: object, properties: {x: {type: array, items: {type: string}, maxItems: 5}}}
    BoundedListFirst:
      allOf:
        - {type: object, properties: {x: {type: array, items: {type: string}, maxItems: 5}}}
        - {type: object, properties: {x: {type: array, items: {type: string}}}}
    BareScalarFirst:
      allOf:
        - {type: object, properties: {x: {type: string, format: x-custom}}}
        - {type: object, properties: {x: {type: string, format: x-custom, maxLength: 5}}}
    BothBounds:
      allOf:
        - {type: object, properties: {x: {type: array, items: {type: string}, minItems: 1}}}
        - {type: object, properties: {x: {type: array, items: {type: string}, maxItems: 5}}}

morphic compile --verify exits 0. In each model x's type is the node at allOf/0, the other declaration is kept verbatim under Unmodeled, and there is one info of this form, four in all and no others:

info openapi/degraded-construct nodes.yaml#/components/schemas/BareListFirst/allOf/1/properties/x: declarations of field "x" give types t/anon/components/schemas/BareListFirst/allOf/0/properties/x and t/anon/components/schemas/BareListFirst/allOf/1/properties/x whose values overlap but that the merge does not narrow to one; kept the type the declaration at /components/schemas/BareListFirst/allOf/0/properties/x gives over the one at /components/schemas/BareListFirst/allOf/1/properties/x, which is kept verbatim under Unmodeled

Cause

compilers/openapi/internal/merge @608a5a46.

• (*folding).foldConstraints (fold.go) walks the declarations in order, mergeConstraints (constraints.go) keeps the first value of each keyword, and constraintsConflict reports a differing one through disagree.
• foldConstraints reads Property.Constraints alone, so a bound on a node never reaches it. The type judgment calls the two nodes an overlap rather than one narrower, and drop sends the declaration's constraints with its type.

Direction

A proposal, to be improved when built.

Ordered keywords. foldConstraints takes the tightest value over the declarations it folds, not the first: the largest minimum, exclusiveMinimum, minLength, minItems and minProperties, and the smallest maximum, exclusiveMaximum, maxLength, maxItems and maxProperties. That value is the intersection, so it holds in every order. Whether the looser declaration keeps its warning and its Unmodeled entry is open: the tighter bound implies it, as a narrower type implies a broader one and draws no report, but allof-redeclared-property keeps a losing declaration so that a release changing it still shows in the document.

Keywords with no order. multipleOf takes the larger value where one divides the other; otherwise the intersection is their least common multiple, a value no declaration wrote. ir.Constraints.Pattern holds one string and two patterns have no single form, so pattern stays kept and reported unless the IR gains a home for a second, which is a shape change. patternMessage, precision and scale stay as they are.

Node constraints. The judgment can call a node narrower than one it equals but for bounds it adds, which settles BareListFirst and BareScalarFirst with no new node. BothBounds, where each adds what the other lacks, needs a node holding both, the step #1042 needs for its own shapes, so the two are designed together. The alternative is writing the merged bounds to Property.Constraints, as the primitive case does, which builds no node but leaves a reader of the node alone seeing fewer bounds than the field has.

Each case needs a two-order test of its own, declared looser first. The order-invariance oracle reverses mappings and leaves sequences alone, so it never swaps allOf branches and cannot see either case.

Out of scope

• A $ref branch is not folded. Its declaration stays on its own model, and ir.FlattenedProperties lists what a redeclaration overrides, for a consumer to conjoin (docs/ir/models.md#the-flattening).
• Two enums, or two containers whose elements narrow: openapi: an allOf narrowing between two enums or two lists is resolved by branch order #1042.
• Two different defaults. The first is kept and the other reported and kept. default is an annotation, so there is nothing to intersect.
• A default a merged bound refuses: openapi: a merged field keeps a default that a bound on the property refuses #1288.
• Recording which declarations contributed to a field: openapi: decide whether a merged field records every declaration that contributed to it #1289.
• An allOf at one position rather than a field's declarations lowers its non-object branches as residue on main (openapi: an allOf of primitive branches lowers to an empty model #1043). fix(compilers/openapi)!: lower allOf by what its conjuncts say #1269 lowers them and keeps two differing bounds, citing this issue, so once it lands the same intersection applies there.

Related: #1042, #1043, #1269, #1288, #1289

Priority: P3, rule 3 (off the golden spec, valid-spec functionality). go run ./cmd/morphic validate "$D/openapi.json" 2>&1 | grep -c 'declarations of field' → 0, where bounds.yaml and nodes.yaml above give 2 and 4 (spec @319dc756, morphic @608a5a46).