cue icon indicating copy to clipboard operation
cue copied to clipboard

encoding/openapi: Sort fields when generating structlits

Open cuematthew opened this issue 1 year ago • 1 comments

As discovered and discussed in https://review.gerrithub.io/c/cue-lang/cue/+/1202688/ the openapi generation seems to be constructing structlits with decls in some unsorted order. The new toposort algorithm always honours the order of decls within StructLit.Decls and so it would seem the openapi generation algs need adjusting, possibly to explicitly call the toposort VertexFeatures.

cuematthew avatar Nov 05 '24 11:11 cuematthew

Essentially this is impossible to fix without major changes to the CUE public Go API, or a reimplementation to the openapi encoder to use internal APIs.

The OpenAPI CUE -> json encoder is currently using the CUE public API. My understanding of what's going on is that it's trying to build a new CUE AST which represents the OpenAPI encoding. This then gets evaluated, and exported as JSON. So really, it's doing a CUE -> CUE conversion. The fact JSON comes out is just a consequence of the export.

Referring to the comment at https://review.gerrithub.io/c/cue-lang/cue/+/1202688/17/encoding/openapi/testdata/openapi-norefs.json#171 The relevant input is this: #YourMessage: ({a: number} | {b: string} | {b: number}) & {a?: string} Now, it's an explicit unfication (i.e. x & y), and the toposort docs show that we should have no ordering between the lhs and rhs of the &, yet inspection of the graph that is built shows we have exactly that: an edge from b to a. Why?

Well, the CUE public API doesn't expose details of whether a conjunct has arrived via explicit or implicit unification. So, the generation of the CUE AST simply appends to a ast.StructLit's Elts (see orderedmap.go) -- it doesn't know to build BinaryExprs to combine, where needed, with existing Elts. Thus, once this built AST has been evaluated, toposort finds no evidence of any BinaryExpr between a and b, so it's forced to treat them as implicitly unified (which is exactly what the built AST contained). Consequently, ordering of the Decls in adt.StructLits is obeyed, which is why we get b before a. If you imagine the input is in fact

#YourMessage: ({a: number} | {b: string} | {b: number})
#YourMessage: {a?: string}

then it makes sense.

cuematthew avatar Jan 31 '25 12:01 cuematthew