Skip to content

Commit b6f250e

Browse files
committed
Revert "encoding/jsonschema: pass comments to DefineSchema"
This reverts https://cuelang.org/cl/1209259. Unity CI is still broken a full day after we intentionally broke it with this experimental API breaking change affecting the services repo. I need to continue pairing with Roger to be able to update Unity to test the latest version of the services repo without failing. But we don't need to keep CI broken while we do that. Signed-off-by: Daniel Martí <[email protected]> Change-Id: I032ce8dbfa3ffe03706e2b236efe0613b4925b48 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1209359 Reviewed-by: Paul Jolly <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
1 parent 26a698f commit b6f250e

File tree

3 files changed

+14
-29
lines changed

3 files changed

+14
-29
lines changed

Diff for: encoding/jsonschema/decode.go

+4-8
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,6 @@ type definedSchema struct {
101101
// schema holds the actual syntax for the schema. This
102102
// is nil if the entry was created by a reference only.
103103
schema ast.Expr
104-
105-
// comment holds any doc comment associated with the above schema.
106-
comment *ast.CommentGroup
107104
}
108105

109106
// addImport registers
@@ -218,7 +215,7 @@ func (d *decoder) decode(v cue.Value) *ast.File {
218215
// have been mapped to an external location.
219216
for _, def := range d.defs {
220217
if def.schema != nil && def.importPath != "" {
221-
d.cfg.DefineSchema(def.importPath, def.path, def.schema, def.comment)
218+
d.cfg.DefineSchema(def.importPath, def.path, def.schema)
222219
}
223220
}
224221
}
@@ -752,7 +749,7 @@ func (s *state) schema(n cue.Value) ast.Expr {
752749
// schemaState returns a new state value derived from s.
753750
// n holds the JSONSchema node to translate to a schema.
754751
// types holds the set of possible types that the value can hold.
755-
func (s0 *state) schemaState(n cue.Value, types cue.Kind) (expr ast.Expr, info schemaInfo) {
752+
func (s0 *state) schemaState(n cue.Value, types cue.Kind) (expr ast.Expr, ingo schemaInfo) {
756753
s := &state{
757754
up: s0,
758755
schemaInfo: schemaInfo{
@@ -766,7 +763,7 @@ func (s0 *state) schemaState(n cue.Value, types cue.Kind) (expr ast.Expr, info s
766763
}
767764
defer func() {
768765
// Perhaps replace the schema expression with a reference.
769-
expr = s.maybeDefine(expr, info)
766+
expr = s.maybeDefine(expr)
770767
}()
771768
if n.Kind() == cue.BoolKind {
772769
if vfrom(VersionDraft6).contains(s.schemaVersion) {
@@ -850,13 +847,12 @@ func (s0 *state) schemaState(n cue.Value, types cue.Kind) (expr ast.Expr, info s
850847
// it just returns expr itself.
851848
// TODO also report whether the schema has been defined at a place
852849
// where it can be unified with something else?
853-
func (s *state) maybeDefine(expr ast.Expr, info schemaInfo) ast.Expr {
850+
func (s *state) maybeDefine(expr ast.Expr) ast.Expr {
854851
def := s.definedSchemaForNode(s.pos)
855852
if def == nil || len(def.path.Selectors()) == 0 {
856853
return expr
857854
}
858855
def.schema = expr
859-
def.comment = info.comment()
860856
if def.importPath == "" {
861857
// It's a local definition that's not at the root.
862858
if !s.builder.put(def.path, expr, s.comment()) {

Diff for: encoding/jsonschema/decode_test.go

+8-19
Original file line numberDiff line numberDiff line change
@@ -337,11 +337,7 @@ func TestMapRefExternalRefForInternalSchema(t *testing.T) {
337337
v := cuecontext.New().CompileString(`
338338
type: "object"
339339
$id: "https://this.test"
340-
$defs: foo: {
341-
description: "foo can be a number or a string"
342-
type: ["number", "string"]
343-
}
344-
$defs: bar: type: "boolean"
340+
$defs: foo: type: ["number", "string"]
345341
$ref: "#/$defs/foo"
346342
`)
347343
var calls []string
@@ -351,19 +347,14 @@ $ref: "#/$defs/foo"
351347
calls = append(calls, loc.String())
352348
switch loc.ID.String() {
353349
case "https://this.test#/$defs/foo":
354-
return "otherpkg.example/foo", cue.ParsePath("#x"), nil
355-
case "https://this.test#/$defs/bar":
356-
return "otherpkg.example/bar", cue.ParsePath("#x"), nil
350+
return "otherpkg.example/foo", cue.ParsePath("#foo"), nil
357351
case "https://this.test":
358352
return "", cue.Path{}, nil
359353
}
360354
t.Errorf("unexpected ID")
361355
return "", cue.Path{}, fmt.Errorf("unexpected ID %q passed to MapRef", loc.ID)
362356
},
363-
DefineSchema: func(importPath string, path cue.Path, e ast.Expr, c *ast.CommentGroup) {
364-
if c != nil {
365-
ast.AddComment(e, c)
366-
}
357+
DefineSchema: func(importPath string, path cue.Path, e ast.Expr) {
367358
data, err := format.Node(e)
368359
if err != nil {
369360
t.Errorf("cannot format: %v", err)
@@ -377,22 +368,20 @@ $ref: "#/$defs/foo"
377368
if err != nil {
378369
t.Fatal(errors.Details(err, nil))
379370
}
380-
qt.Check(t, qt.DeepEquals(calls, []string{
371+
qt.Assert(t, qt.DeepEquals(calls, []string{
381372
"id=https://this.test#/$defs/foo localPath=$defs.foo",
382-
"id=https://this.test#/$defs/bar localPath=$defs.bar",
383373
"id=https://this.test localPath=",
384374
}))
385-
qt.Check(t, qt.Equals(string(b), `
375+
qt.Assert(t, qt.Equals(string(b), `
386376
import "otherpkg.example/foo"
387377
388378
@jsonschema(id="https://this.test")
389-
foo.#x & {
379+
foo.#foo & {
390380
...
391381
}
392382
`[1:]))
393-
qt.Check(t, qt.DeepEquals(defines, map[string]string{
394-
"otherpkg.example/bar.#x": "bool",
395-
"otherpkg.example/foo.#x": "// foo can be a number or a string\nnumber | string",
383+
qt.Assert(t, qt.DeepEquals(defines, map[string]string{
384+
"otherpkg.example/foo.#foo": "number | string",
396385
}))
397386
}
398387

Diff for: encoding/jsonschema/jsonschema.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -216,15 +216,15 @@ type Config struct {
216216
// DefineSchema is called, if not nil, for any schema that is defined
217217
// within the json schema being converted but is mapped somewhere
218218
// external via [Config.MapRef]. The invoker of [Extract] is
219-
// responsible for defining the schema in the correct place as described
219+
// responsible for defining the schema e in the correct place as described
220220
// by the import path and its relative CUE path.
221221
//
222222
// The importPath and path are exactly as returned by [Config.MapRef].
223223
// If this or [Config.MapRef] is nil this function will never be called.
224224
// Note that importPath will never be empty, because if MapRef
225225
// returns an empty importPath, it's specifying an internal schema
226226
// which will be defined accordingly.
227-
DefineSchema func(importPath string, path cue.Path, e ast.Expr, docComment *ast.CommentGroup)
227+
DefineSchema func(importPath string, path cue.Path, e ast.Expr)
228228

229229
// TODO: configurability to make it compatible with OpenAPI, such as
230230
// - locations of definitions: #/components/schemas, for instance.

0 commit comments

Comments
 (0)