encoding/jsonschema: avoid unnecessary alias to root

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 <[email protected]>
Change-Id: Ie886b5819c612cbd64abca62d3231aedd530e2bf
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1199626
Reviewed-by: Daniel Martí <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

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
 

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]

encoding/jsonschema/testdata/issue3351.txtar

@@ -70,24 +70,14 @@ _schema: {
 	@jsonschema(id="https://www.sourcemeta.com/schemas/vendor/[email protected]")
 	$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

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

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