Skip to content

Commit

Permalink
encoding/jsonschema: simplify closedness
Browse files Browse the repository at this point in the history
This removes redundant `ellipses` and `close` to simplify structs
closedness of output of JSONSchema encoder.

Fixes #3433

Change-Id: I3bb77d00a94f5bbcd49ce32b8d3fbf75e281829e
Signed-off-by: haoqixu <hq.xu0o0@gmail.com>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1203727
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
haoqixu authored and rogpeppe committed Nov 15, 2024
1 parent e3e1c9f commit 9b22a3c
Show file tree
Hide file tree
Showing 8 changed files with 424 additions and 9 deletions.
96 changes: 96 additions & 0 deletions cmd/cue/cmd/testdata/script/def_jsonschema.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ cmp stdout expect-stdout
exec cue def schema.json -p schema -l '#Person:'
cmp stdout expect-stdout

exec cue def schema.json -p schema -l 'Person:'
cmp stdout expect-stdout2

exec cue def schema-closed.json -p schema -l '#Person:'
cmp stdout expect-stdout3

exec cue def schema-closed.json -p schema -l 'Person:'
cmp stdout expect-stdout4

exec cue def jsonschema: bad.json

# The default strictness modes should apply even when
Expand Down Expand Up @@ -51,6 +60,26 @@ import "strings"
age?: >=0 & int
...
}
-- expect-stdout2 --
package schema

import "strings"

Person: {
// Person
@jsonschema(schema="http://json-schema.org/draft-07/schema#")
@jsonschema(id="https://example.com/person.schema.json")

// The person's first name.
firstName?: string

// The person's last name.
lastName?: strings.MinRunes(1)

// Age in years which must be equal to or greater than zero.
age?: >=0 & int
...
}
-- schema.json --
{
"$id": "https://example.com/person.schema.json",
Expand All @@ -75,6 +104,73 @@ import "strings"
}
}

-- schema-closed.json --
{
"$id": "https://example.com/person.schema.json",
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "Person",
"type": "object",
"properties": {
"firstName": {
"type": "string",
"description": "The person's first name."
},
"lastName": {
"type": "string",
"description": "The person's last name.",
"minLength": 1
},
"age": {
"description": "Age in years which must be equal to or greater than zero.",
"type": "integer",
"minimum": 0
}
},
"additionalProperties": false
}

-- expect-stdout3 --
package schema

import "strings"

#Person: {
// Person
@jsonschema(schema="http://json-schema.org/draft-07/schema#")
close({
@jsonschema(id="https://example.com/person.schema.json")

// The person's first name.
firstName?: string

// The person's last name.
lastName?: strings.MinRunes(1)

// Age in years which must be equal to or greater than zero.
age?: >=0 & int
})
}
-- expect-stdout4 --
package schema

import "strings"

Person: {
// Person
@jsonschema(schema="http://json-schema.org/draft-07/schema#")
close({
@jsonschema(id="https://example.com/person.schema.json")

// The person's first name.
firstName?: string

// The person's last name.
lastName?: strings.MinRunes(1)

// Age in years which must be equal to or greater than zero.
age?: >=0 & int
})
}
-- bad.json --
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
Expand Down
2 changes: 2 additions & 0 deletions encoding/jsonschema/jsonschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"cuelang.org/go/cue"
"cuelang.org/go/cue/ast"
"cuelang.org/go/cue/token"
"cuelang.org/go/internal"
)

// Extract converts JSON Schema data into an equivalent CUE representation.
Expand Down Expand Up @@ -64,6 +65,7 @@ func Extract(data cue.InstanceOrValue, cfg *Config) (f *ast.File, err error) {
if d.errs != nil {
return nil, d.errs
}
f = internal.SimplifyClosedness(f).(*ast.File)

return f, nil
}
Expand Down
1 change: 0 additions & 1 deletion encoding/jsonschema/testdata/txtar/id_in_oneOf.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,4 @@ matchN(1, [{
string
}, {
@jsonschema(id="https://2.test.example/object")
...
}])
4 changes: 1 addition & 3 deletions encoding/jsonschema/testdata/txtar/issue3176.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@
-- out/decode/extract --
import "strings"

matchN(1, [{
...
}, strings.MaxRunes(
matchN(1, [{}, strings.MaxRunes(
3)]) & (string | {
{[=~"^x-"]: string}
...
Expand Down
2 changes: 0 additions & 2 deletions encoding/jsonschema/testdata/txtar/typeexcluded.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,7 @@ e5?: int & >=0
e6?: [...=~"^[A-Za-z0-9 _.-]+$"]
e7?: matchN(>=1, [true, {
disableFix?: bool
...
} & {
ignoreMediaFeatureNames?: list.UniqueItems() & [_, ...] & [...string]
...
}])
...
4 changes: 1 addition & 3 deletions encoding/jsonschema/testdata/txtar/unsupported.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@
@jsonschema(schema="http://json-schema.org/draft-07/schema#")
_

#ref: matchN(1, [matchN(0, [string]) & {
...
}, null]) & (null | {
#ref: matchN(1, [matchN(0, [string]) & {}, null]) & (null | {
branches?: {
...
}
Expand Down
132 changes: 132 additions & 0 deletions internal/closedness.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// Copyright 2024 CUE Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package internal

import (
"cuelang.org/go/cue/ast"
"cuelang.org/go/cue/ast/astutil"
)

// SimplifyClosedness updates the AST to remove redundant ellipses and `close` calls.
func SimplifyClosedness(n ast.Node) ast.Node {
sc := closednessSimplifier{}
return sc.simplify(n)
}

type frame struct {
inDefinition bool
inCloseCall bool
inParams bool
inBinaryExpr bool
}

type closednessSimplifier struct {
stack []frame
current frame
}

func (sc *closednessSimplifier) simplify(root ast.Node) ast.Node {
return astutil.Apply(root, func(c astutil.Cursor) bool {
switch n := c.Node().(type) {
case *ast.BinaryExpr:
sc.pushStack()
sc.current.inBinaryExpr = true
case *ast.Field:
sc.pushStack()
sc.current.inDefinition = sc.current.inDefinition || IsDefinition(n.Label)
case *ast.CallExpr:
sc.pushStack()
if fn, ok := n.Fun.(*ast.Ident); ok {
if fn.Name == "close" {
sc.current.inCloseCall = true
} else if fn.Name == "matchN" {
// If a function returns a value containing the struct literals it received, we can't
// omit ellipses from its parameters. Although functions in CUE can be renamed, making it
// hard to tell if omitting ellipses is safe, doing so for a JSONSchema encoder's
// output is considered safe.
// For now, we only do so for `matchN`.
sc.current.inParams = true
sc.current.inDefinition = false
sc.current.inBinaryExpr = false
}
}
case *ast.StructLit:
f := sc.current
sc.pushStack()

sc.current.inCloseCall = f.inCloseCall
case *ast.Ellipsis:
parent := c.Parent()
if parent != nil {
switch parent.Node().(type) {
case *ast.StructLit, *ast.File:
if sc.current.inParams && !sc.current.inDefinition {
for _, comm := range ast.Comments(c.Node()) {
ast.AddComment(parent.Node(), comm)
}
c.Delete()
}
}
}
}
return true
}, func(c astutil.Cursor) bool {
switch n := c.Node().(type) {
case *ast.BinaryExpr:
sc.popStack()
case *ast.Field:
sc.popStack()
case *ast.CallExpr:
cur := sc.current
sc.popStack()
if fn, ok := n.Fun.(*ast.Ident); ok && fn.Name == "close" {
// Remove close when
// 1. it's used as a field value inside a definition or inside nested close calls.
// 2. and there is no change to used in BinaryExpr.
parent := c.Parent()
if parent != nil {
switch parent.Node().(type) {
case *ast.Field, *ast.CallExpr:
if (cur.inDefinition || sc.current.inCloseCall) && !cur.inBinaryExpr {
c.Replace(n.Args[0])
}
}
}
}
case *ast.StructLit:
sc.popStack()
}
return true
})
}

func (sc *closednessSimplifier) pushStack() {
sc.stack = append(sc.stack, sc.current)
sc.current.inCloseCall = false
}

func (sc *closednessSimplifier) popStack() {
sc.current = sc.stack[len(sc.stack)-1]
sc.stack = sc.stack[:len(sc.stack)-1]
}

func containsEllipsis(elts []ast.Decl) bool {
for _, elt := range elts {
if _, ok := elt.(*ast.Ellipsis); ok {
return true
}
}
return false
}
Loading

0 comments on commit 9b22a3c

Please sign in to comment.