encoding/jsonschema: refactor reference handling

This refactors references to use a general system
that maps references through a new `MapRef` function
that supercedes the existing `MapURL` and `Map` functions.
To support this, we use the new `structBuilder` type
to build up the final syntax.

This fixes a bunch of existing reported and unreported issues, including:
- json pointer escaping: JSON pointers were not previously
escaped and unescaped correctly.
- references into internal structure: `$ref` references can now
reference arbitrary internal structure inside the schema
being extracted, including nodes that aren't actually schemas.
- better doc comments: the outer-level doc comment is
now correctly preserved in all circumstances

There are inevitably some changes in the form of the generated schemas:
- field ordering of definitions is now always lexical
- some comments move to new (better) locations
- attribute placement also moves to a (better) location
- by default, only top level `$defs` members are exported
as public definitions.

The last issue could be considered a backward incompatible
change, but in practice
- nested definitions are rare
- the nested definitions were not easily accessible anyway
in most cases (e.g. when inside a property or other expression)
- the new `MapRef` feature can be used to change the
location of any schema, including these.

As yet, the `MapRef` functionality as provided to the API
is not tested other than with `DefaultMapRef`, and the
`DefineSchemas` callback is not wired up. This will
land in a subsequent CL: in the meantime, what we've
got here seems sufficient as an intermediate step.

Fixes #3593
Fixes #3548
Updates #2699
Fixes #2287
Fixes #390

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: Icfcff6e3d9f1d09f0418ddd493e01beb78045d59
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1205706
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>

Author: rogpeppe

cmd/cue/cmd/testdata/script/def_comments.txtar

@@ -85,6 +85,9 @@ stdout 'description of foo'
   }
 }
 -- stdout1 --
+// This is a title
+//
+// top description
 null | bool | number | string | [...] | {
 	// a description
 	a?: {
@@ -95,6 +98,9 @@ null | bool | number | string | [...] | {
 	...
 }
 -- stdout1-def --
+// This is a title
+//
+// top description
 #top: null | bool | number | string | [...] | {
 	// a description
 	a?: {
@@ -126,17 +132,19 @@ null | bool | number | string | [...] | {
 // This is a title
 //
 // top description
+// a description
 a?: {
 	// b description
 	b?: number
 	...
 }
 ...
 -- stdout2-def --
+// This is a title
+//
+// top description
 #top: {
-	// This is a title
-	//
-	// top description
+	// a description
 	a?: {
 		// b description
 		b?: number

cmd/cue/cmd/testdata/script/def_jsonschema.txtar

@@ -32,12 +32,13 @@ cmp stderr expect-stderr2
 cmp stderr expect-stderr3
 
 -- expect-stdout --
+// Person
+
 package schema
 
 import "strings"
 
 #Person: {
-	// Person
 	@jsonschema(schema="http://json-schema.org/draft-07/schema#")
 	@jsonschema(id="https://example.com/person.schema.json")
 

cmd/cue/cmd/testdata/script/import_auto.txtar

@@ -21,15 +21,16 @@ info: {
 	version: *"v1beta1" | string
 }
 
+#Bar: {
+	foo!: #Foo
+	...
+}
+
 #Foo: {
 	a!: int
 	b!: int & <10 & >=0
 	...
 }
-#Bar: {
-	foo!: #Foo
-	...
-}
 -- openapi.yaml --
 openapi: 3.0.0
 info:

encoding/jsonschema/constraints_combinator.go

@@ -33,7 +33,7 @@ func constraintAllOf(key string, n cue.Value, s *state) {
 	}
 	a := make([]ast.Expr, 0, len(items))
 	for _, v := range items {
-		x, sub := s.schemaState(v, s.allowedTypes, nil)
+		x, sub := s.schemaState(v, s.allowedTypes)
 		s.allowedTypes &= sub.allowedTypes
 		if sub.hasConstraints {
 			// This might seem a little odd, since the actual
@@ -79,7 +79,7 @@ func constraintAnyOf(key string, n cue.Value, s *state) {
 	}
 	a := make([]ast.Expr, 0, len(items))
 	for _, v := range items {
-		x, sub := s.schemaState(v, s.allowedTypes, nil)
+		x, sub := s.schemaState(v, s.allowedTypes)
 		if sub.allowedTypes == 0 {
 			// Nothing is allowed; omit.
 			continue
@@ -123,7 +123,7 @@ func constraintOneOf(key string, n cue.Value, s *state) {
 	}
 	a := make([]ast.Expr, 0, len(items))
 	for _, v := range items {
-		x, sub := s.schemaState(v, s.allowedTypes, nil)
+		x, sub := s.schemaState(v, s.allowedTypes)
 		if sub.allowedTypes == 0 {
 			// Nothing is allowed; omit
 			continue
@@ -198,14 +198,14 @@ func constraintIfThenElse(s *state) {
 		return
 	}
 	var ifExpr, thenExpr, elseExpr ast.Expr
-	ifExpr, ifSub := s.schemaState(s.ifConstraint, s.allowedTypes, nil)
+	ifExpr, ifSub := s.schemaState(s.ifConstraint, s.allowedTypes)
 	if hasThen {
 		// The allowed types of the "then" constraint are constrained both
 		// by the current constraints and the "if" constraint.
-		thenExpr, _ = s.schemaState(s.thenConstraint, s.allowedTypes&ifSub.allowedTypes, nil)
+		thenExpr, _ = s.schemaState(s.thenConstraint, s.allowedTypes&ifSub.allowedTypes)
 	}
 	if hasElse {
-		elseExpr, _ = s.schemaState(s.elseConstraint, s.allowedTypes, nil)
+		elseExpr, _ = s.schemaState(s.elseConstraint, s.allowedTypes)
 	}
 	if thenExpr == nil {
 		thenExpr = top()

encoding/jsonschema/constraints_generic.go

@@ -15,51 +15,28 @@
 package jsonschema
 
 import (
+	"errors"
+	"fmt"
+	"net/url"
+	"strings"
+
 	"cuelang.org/go/cue"
 	"cuelang.org/go/cue/ast"
-	"cuelang.org/go/cue/errors"
 	"cuelang.org/go/cue/token"
 )
 
 // Generic constraints
 
 func constraintAddDefinitions(key string, n cue.Value, s *state) {
 	if n.Kind() != cue.StructKind {
-		s.errf(n, `"definitions" expected an object, found %s`, n.Kind())
+		s.errf(n, `%q expected an object, found %s`, key, n.Kind())
 	}
 
 	s.processMap(n, func(key string, n cue.Value) {
-		name := key
-
-		var f *ast.Field
-
-		ident := "#" + name
-		if ast.IsValidIdent(ident) {
-			expr, sub := s.schemaState(n, allTypes, []label{{ident, true}})
-			f = &ast.Field{
-				Label: ast.NewIdent(ident),
-				Value: expr,
-			}
-			sub.doc(f)
-		} else {
-			expr, sub := s.schemaState(n, allTypes, []label{{"#", true}, {name: name}})
-			inner := ast.NewStruct(&ast.Field{
-				Label: ast.NewString(name),
-				Value: expr,
-			})
-			// Ensure that we get `#: foo: ...` not `#: {foo: ...}`
-			inner.Lbrace = token.NoPos
-			ident = "#"
-			f = &ast.Field{
-				Label: ast.NewIdent("#"),
-				Value: inner,
-			}
-			sub.doc(f)
-		}
-
-		ast.SetRelPos(f, token.NewSection)
-		s.definitions = append(s.definitions, f)
-		s.setField(label{name: ident, isDef: true}, f)
+		// Ensure that we are going to make a definition
+		// for this node.
+		s.ensureDefinition(n)
+		s.schema(n)
 	})
 }
 
@@ -122,26 +99,79 @@ func constraintExamples(key string, n cue.Value, s *state) {
 }
 
 func constraintNullable(key string, n cue.Value, s *state) {
-	// TODO: only allow for OpenAPI.
 	null := ast.NewNull()
 	setPos(null, n)
 	s.nullable = null
 }
 
 func constraintRef(key string, n cue.Value, s *state) {
 	u := s.resolveURI(n)
-
-	fragmentParts, err := splitFragment(u)
+	if u == nil {
+		return
+	}
+	schemaRoot := s.schemaRoot()
+	if u.Fragment == "" && schemaRoot.isRoot && sameSchemaRoot(u, schemaRoot.id) {
+		// It's a reference to the root of the schema being
+		// generated. This never maps to something different.
+		s.all.add(n, s.refExpr(n, "", cue.Path{}))
+		return
+	}
+	importPath, path, err := cueLocationForRef(s, n, u, schemaRoot)
 	if err != nil {
-		s.addErr(errors.Newf(n.Pos(), "%v", err))
+		s.errf(n, "%v", err)
 		return
 	}
-	expr := s.makeCUERef(n, u, fragmentParts)
-	if expr == nil {
-		expr = &ast.BadExpr{From: n.Pos()}
+	if e := s.refExpr(n, importPath, path); e != nil {
+		s.all.add(n, e)
 	}
+}
 
-	s.all.add(n, expr)
+func cueLocationForRef(s *state, n cue.Value, u *url.URL, schemaRoot *state) (importPath string, path cue.Path, err error) {
+	if ds, ok := s.defs[u.String()]; ok {
+		// We already know about the schema, so use the information that's stored for it.
+		return ds.importPath, ds.path, nil
+	}
+	loc := SchemaLoc{
+		ID: u,
+	}
+	var base cue.Value
+	isAnchor := u.Fragment != "" && !strings.HasPrefix(u.Fragment, "/")
+	if !isAnchor {
+		// It's a JSON pointer reference.
+		if sameSchemaRoot(u, s.rootID) {
+			base = s.root
+		} else if sameSchemaRoot(u, schemaRoot.id) {
+			// it's within the current schema.
+			base = schemaRoot.pos
+		}
+		if base.Exists() {
+			target, err := lookupJSONPointer(schemaRoot.pos, u.Fragment)
+			if err != nil {
+				if errors.Is(err, errRefNotFound) {
+					return "", cue.Path{}, fmt.Errorf("reference to non-existent schema")
+				}
+				return "", cue.Path{}, fmt.Errorf("invalid JSON Pointer: %v", err)
+			}
+			if ds := s.defForValue.get(target); ds != nil {
+				// There's a definition in place for the value, which gives
+				// us our answer.
+				return ds.importPath, ds.path, nil
+			}
+			s.ensureDefinition(target)
+			loc.IsLocal = true
+			loc.Path = relPath(target, s.root)
+		}
+	}
+	importPath, path, err = s.cfg.MapRef(loc)
+	if err != nil {
+		return "", cue.Path{}, fmt.Errorf("cannot determine CUE location for JSON Schema location %v: %v", loc, err)
+	}
+	// TODO we'd quite like to avoid invoking MapRef many times
+	// for the same reference, but in general we don't necessily know
+	// the canonical URI of the schema until we've done at least one pass.
+	// There are potentially ways to do it, but leave it for now in favor
+	// of simplicity.
+	return importPath, path, nil
 }
 
 func constraintTitle(key string, n cue.Value, s *state) {

encoding/jsonschema/constraints_meta.go

@@ -24,9 +24,9 @@ func constraintID(key string, n cue.Value, s *state) {
 	// URL: https://domain.com/schemas/foo.json
 	// anchors: #identifier
 	//
-	// TODO: mark identifiers.
+	// TODO: mark anchors
 
-	// Resolution must be relative to parent $id
+	// Resolution is relative to parent $id
 	// https://tools.ietf.org/html/draft-handrews-json-schema-02#section-8.2.2
 	u := s.resolveURI(n)
 	if u == nil {

encoding/jsonschema/constraints_object.go

@@ -118,9 +118,11 @@ func constraintProperties(key string, n cue.Value, s *state) {
 	s.processMap(n, func(key string, n cue.Value) {
 		// property?: value
 		name := ast.NewString(key)
-		expr, state := s.schemaState(n, allTypes, []label{{name: key}})
+		expr, state := s.schemaState(n, allTypes)
 		f := &ast.Field{Label: name, Value: expr}
-		state.doc(f)
+		if doc := state.comment(); doc != nil {
+			ast.SetComments(f, []*ast.CommentGroup{doc})
+		}
 		f.Optional = token.Blank.Pos()
 		if len(obj.Elts) > 0 && len(f.Comments()) > 0 {
 			// TODO: change formatter such that either a NewSection on the
@@ -136,13 +138,12 @@ func constraintProperties(key string, n cue.Value, s *state) {
 			}
 		}
 		obj.Elts = append(obj.Elts, f)
-		s.setField(label{name: key}, f)
 	})
 }
 
 func constraintPropertyNames(key string, n cue.Value, s *state) {
 	// [=~pattern]: _
-	if names, _ := s.schemaState(n, cue.StringKind, nil); !isTop(names) {
+	if names, _ := s.schemaState(n, cue.StringKind); !isTop(names) {
 		x := ast.NewStruct(ast.NewList(names), top())
 		s.add(n, objectType, x)
 	}
@@ -154,8 +155,6 @@ func constraintRequired(key string, n cue.Value, s *state) {
 		return
 	}
 
-	// TODO: detect that properties is defined somewhere.
-	// s.errf(n, `"required" without a "properties" field`)
 	obj := s.object(n)
 
 	// Create field map