Skip to content

openapi/merge: equal bounds past big.Rat's range are reported as a conflict #287

Description

@OmarAlJarrah

merge.bigValEqual compares two numeric literals by parsing each into a big.Rat and falls back to exact string equality when either refuses to parse. The comment above it says the fallback is unreachable in practice:

Both are already-validated BigVals, so parsing succeeds; an unparseable pair falls back to exact string equality.

The first clause is false. ir.NewBigVal validates through big.ParseFloat, which holds the exponent separately and accepts magnitudes up to a binary exponent of int32; big.Rat.SetString has to materialize the value and refuses outright once the decimal exponent (less the fraction digits) exceeds 1e6. 1e1000000 parses as a rational; 1e1000001 does not. Both are legal in a spec and both survive into the IR intact.

Reproduction

openapi: 3.1.0
info: {title: t, version: "1"}
paths: {}
components:
  schemas:
    Same:
      allOf:
        - type: object
          properties: {n: {type: number, minimum: 1e1000001}}
        - type: object
          properties: {n: {type: number, minimum: 10e1000000}}

1e1000001 and 10e1000000 are the same number. Compiling reports:

warning openapi/conflicting-redeclaration /components/schemas/Same/allOf/1/properties/n:
  declarations of field "n" disagree: conflicting minimum (1e1000001 and 10e1000000); kept the first declaration

The same pair written as 1e2 and 100 is correctly silent, because a rational holds those.

Impact

A spurious conflicting-redeclaration warning on a document with no conflict — and, for anyone running the CLI with -fail-on warning, a failed compile. The magnitudes involved are unusual, but the fallback is silent about being a fallback: nothing distinguishes "these differ" from "these could not be compared".

There is also a cost side to the same call. A rational for 1e1000000 is roughly 400 KB of digits and takes ~14 ms to build, and this comparison runs per redeclared bound.

Expected

Compare the two literals exactly, without materializing either. compilers/openapi/internal/annotation/decimal.go (added with #274) already does this for the co-declared-bound reconciliation: it splits a literal into sign, significant digits and the power of ten the leading digit carries, and orders those. merge may import annotation today, so the comparison can be shared rather than written twice — it would need exporting.

Related: #10 wants these branches intersected rather than first-declaration-wins, which needs the same exact ordering to decide which bound is the tighter.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions