From ae3987dc1e9a2c26b2346718bc60a17aa9afafda Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Fri, 16 Aug 2024 14:53:43 +0100 Subject: [PATCH] encoding/jsonschema: avoid unnecessary alias to root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #3354 demonstrates that aliases are used when they're not actually needed. When investigating the fix for #2287, I realised where the problem might be, and this is the result. The problem was that all the self-references need to reference the same AST node, but they were not doing so. Fix that by creating the struct node to be embedded when we know that we need a self-reference. We can also use the presence of that node to signal that a self-reference is needed, removing the need for `hasSelfReference`. Fixes #3354 Signed-off-by: Roger Peppe Change-Id: Ie886b5819c612cbd64abca62d3231aedd530e2bf Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1199626 Reviewed-by: Daniel Martí Unity-Result: CUE porcuepine TryBot-Result: CUEcueckoo --- encoding/jsonschema/decode.go | 43 +++++++++++++++----- encoding/jsonschema/ref.go | 27 ++---------- encoding/jsonschema/testdata/issue3351.txtar | 18 ++------ encoding/jsonschema/testdata/refroot.txtar | 4 +- encoding/jsonschema/testdata/refroot2.txtar | 4 +- 5 files changed, 42 insertions(+), 54 deletions(-) diff --git a/encoding/jsonschema/decode.go b/encoding/jsonschema/decode.go index 341a2635240..bd78d7b7c16 100644 --- a/encoding/jsonschema/decode.go +++ b/encoding/jsonschema/decode.go @@ -44,6 +44,10 @@ type decoder struct { errs errors.Error numID int // for creating unique numbers: increment on each use mapURLErrors map[string]bool + // self holds the struct literal that will eventually be embedded + // in the top level file. It is only set when decoder.rootRef is + // called. + self *ast.StructLit } // addImport registers @@ -169,17 +173,35 @@ func (d *decoder) schema(ref []ast.Label, v cue.Value) (a []ast.Decl) { expr = ast.NewStruct(ref[i], expr) } - if root.hasSelfReference { - return []ast.Decl{ - &ast.EmbedDecl{Expr: ast.NewIdent(topSchema)}, - &ast.Field{ - Label: ast.NewIdent(topSchema), - Value: &ast.StructLit{Elts: a}, - }, - } + if root.self == nil { + return a + } + root.self.Elts = a + return []ast.Decl{ + &ast.EmbedDecl{Expr: d.rootRef()}, + &ast.Field{ + Label: d.rootRef(), + Value: root.self, + }, } +} - return a +// rootRef returns a reference to the top of the file. We do this by +// creating a helper schema: +// +// _schema: {...} +// _schema +// +// This is created at the finalization stage, signaled by d.self being +// set, which rootRef does as a side-effect. +func (d *decoder) rootRef() *ast.Ident { + ident := ast.NewIdent("_schema") + if d.self == nil { + d.self = &ast.StructLit{} + } + // Ensure that all self-references refer to the same node. + ident.Node = d.self + return ident } func (d *decoder) errf(n cue.Value, format string, args ...interface{}) ast.Expr { @@ -363,8 +385,7 @@ type state struct { definitions []ast.Decl // Used for inserting definitions, properties, etc. - hasSelfReference bool - obj *ast.StructLit + obj *ast.StructLit // Complete at finalize. fieldRefs map[label]refs diff --git a/encoding/jsonschema/ref.go b/encoding/jsonschema/ref.go index 8f28b28f1f8..4c2338d837f 100644 --- a/encoding/jsonschema/ref.go +++ b/encoding/jsonschema/ref.go @@ -89,8 +89,6 @@ func (s *state) resolveURI(n cue.Value) *url.URL { return u } -const topSchema = "_schema" - // makeCUERef converts a URI into a CUE reference for the current location. // The returned identifier (or first expression in a selection chain), is // hardwired to point to the resolved value. This will allow astutil.Sanitize @@ -136,17 +134,8 @@ func (s *state) makeCUERef(n cue.Value, u *url.URL, fragmentParts []string) (_e case u.Host == "" && u.Path == "", s.id != nil && s.id.Host == u.Host && s.id.Path == u.Path: if len(fragmentParts) == 0 { - // refers to the top of the file. We will allow this by - // creating a helper schema as such: - // _schema: {...} - // _schema - // This is created at the finalization stage if - // hasSelfReference is set. - s.hasSelfReference = true - - ident = ast.NewIdent(topSchema) - ident.Node = s.obj - return ident + // Refers to the top of the file. + return s.rootRef() } ident, fragmentParts = s.getNextIdent(n, fragmentParts) @@ -202,16 +191,8 @@ func (s *state) makeCUERef(n cue.Value, u *url.URL, fragmentParts []string) (_e // state above the root state that we need to update. s = s.up - // refers to the top of the file. We will allow this by - // creating a helper schema as such: - // _schema: {...} - // _schema - // This is created at the finalization stage if - // hasSelfReference is set. - s.hasSelfReference = true - ident = ast.NewIdent(topSchema) - ident.Node = s.obj - return ident + // Refers to the top of the file. + return s.rootRef() } x := s.idRef[0] diff --git a/encoding/jsonschema/testdata/issue3351.txtar b/encoding/jsonschema/testdata/issue3351.txtar index 4424fef30d5..43643059752 100644 --- a/encoding/jsonschema/testdata/issue3351.txtar +++ b/encoding/jsonschema/testdata/issue3351.txtar @@ -70,24 +70,14 @@ _schema: { @jsonschema(id="https://www.sourcemeta.com/schemas/vendor/json-e@1.json") $else?: #["jsone-value"] $let?: [string]: null | bool | number | string | [...] | { - [string]: _schema_1 + [string]: _schema } - $sort?: _schema_5 | [...number] + $sort?: _schema | [...number] {[!~"^($else|$let|$sort)$"]: #["jsone-value"]} - #: "jsone-value": _schema_A | [..._schema_8] + #: "jsone-value": _schema | [..._schema] #: "jsone-array": [...#["jsone-value"]] - #: "jsone-object-array": [..._schema_E] + #: "jsone-object-array": [..._schema] } - -let _schema_1 = _schema - -let _schema_5 = _schema - -let _schema_A = _schema - -let _schema_8 = _schema - -let _schema_E = _schema diff --git a/encoding/jsonschema/testdata/refroot.txtar b/encoding/jsonschema/testdata/refroot.txtar index f3ca5934587..b716b512fde 100644 --- a/encoding/jsonschema/testdata/refroot.txtar +++ b/encoding/jsonschema/testdata/refroot.txtar @@ -19,9 +19,7 @@ _schema: { null | bool | number | string | [...] | { @jsonschema(id="http://cuelang.org/go/encoding/openapi/testdata/order.json") value?: _ - next?: _schema_1 + next?: _schema ... } } - -let _schema_1 = _schema diff --git a/encoding/jsonschema/testdata/refroot2.txtar b/encoding/jsonschema/testdata/refroot2.txtar index 1440aaa8dae..41c696dba28 100644 --- a/encoding/jsonschema/testdata/refroot2.txtar +++ b/encoding/jsonschema/testdata/refroot2.txtar @@ -16,9 +16,7 @@ _schema: { @jsonschema(schema="http://json-schema.org/draft-07/schema#") null | bool | number | string | [...] | { value?: _ - next?: _schema_1 + next?: _schema ... } } - -let _schema_1 = _schema