internal/core/adt: allow any CUE value in json/yaml.Validate

By default, when calling builtin functions, an error is returned
for each argument that is:

    - non-concrete (i.e. builtin(`string`))
    - an unresolved disjunction (i.e. builtin("a" | "b"))

In addition, all default values are resolved, so that `builtin(1 | *2)`
will be called as `builtin(2)`, losing important data in some cases
where the disjunction could be resolved to `1`.

Some examples in which this behavior is erroneous:

    - `yaml.Validate("a", "a" | "b")` (results in an "unresolved
      disjunction" error)
    - `yaml.Validate("a", string)` (results in a non-concrete value error)
    - `yaml.Validate("a", *int | string)` (even if non-concrete values
      were allowed, this would result in false because the default value
      `int` would be resolved in place of the disjunction, and 'a' is
      not an int)

To fix this, we delegate the responsibility to evaluate disjunctions
and resolve default values to the builtin. This way each builtin can
decide whether its arguments should be concrete, and whether
disjunctions/defaults should be resolved.

Note: these are further original comments from Noam. The corresponding
code has been hoisted to a separate CL
A new CallCtxt.Schema method is added which, as opposed to
CallCtxt.Value, does not resolve arguments to their default values /
enforce concreteness.
All stdlib function registrations, located in pkg.go files, are
auto-generated based on the public functions in the corresponding
stdlib packages. Because of this, a new `internal.pkg.Schema` type
is added, so that stdlib functions that need to use CallCtxt.Schema,
can declare a parameter astype pkg.Schema.

This fixes incorrect failures when passing non-concrete values and
unresolved disjunctions to `yaml.Validate` and `json.Validate`.

A test is added that only works when the new evaluator is enabled. This
is because the old evaluator evaluated the following CUE as incomplete:

    {a: 1} & ({a!: int} | {b!: int})

Which is incorect because the required fields should "force" the
expression to evaluate to `{a: 1}`.

Note that this does not yet fix cases like:

    yaml.Validate("a: 1", close({a: int}) | close({b: int}))

This will require further investigation to see why the "closedness" of
the disjuncts is seemingly lost during the evaluation, resulting in a
non-concrete unification of the yaml with the disjunction - and a false
result.

Updates #2741.

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

encoding/yaml/yaml.go

@@ -23,6 +23,7 @@ import (
 	"cuelang.org/go/cue"
 	"cuelang.org/go/cue/ast"
 	cueyaml "cuelang.org/go/internal/encoding/yaml"
+	"cuelang.org/go/internal/pkg"
 	"cuelang.org/go/internal/source"
 	pkgyaml "cuelang.org/go/pkg/encoding/yaml"
 )
@@ -109,6 +110,6 @@ func EncodeStream(iter cue.Iterator) ([]byte, error) {
 // Validate validates the YAML and confirms it matches the constraints
 // specified by v. For YAML streams, all values must match v.
 func Validate(b []byte, v cue.Value) error {
-	_, err := pkgyaml.Validate(b, v)
+	_, err := pkgyaml.Validate(b, pkg.Schema(v))
 	return err
 }

internal/core/compile/builtin.go

@@ -81,7 +81,7 @@ var closeBuiltin = &adt.Builtin{
 	Name:   "close",
 	Params: []adt.Param{structParam},
 	Result: adt.StructKind,
-	// Noncrete: true, // TODO: should probably be noncrete
+	// NonConcrete: true, // TODO: should probably be noncrete
 	Func: func(c *adt.OpContext, args []adt.Value) adt.Expr {
 		s, ok := args[0].(*adt.Vertex)
 		if !ok {

internal/pkg/types.go

@@ -21,6 +21,8 @@ import (
 
 // A Schema represents an arbitrary cue.Value that can hold non-concrete values.
 // By default function arguments are checked to be concrete.
+//
+// TODO(mvdan,mpvl): consider using type Schema = cue.Value.
 type Schema cue.Value
 
 func (s Schema) Value() cue.Value {

pkg/encoding/json/manual.go

@@ -28,6 +28,7 @@ import (
 	"cuelang.org/go/cue/token"
 	cuejson "cuelang.org/go/encoding/json"
 	internaljson "cuelang.org/go/internal/encoding/json"
+	"cuelang.org/go/internal/pkg"
 )
 
 // Compact generates the JSON-encoded src with insignificant space characters
@@ -130,8 +131,8 @@ func Unmarshal(b []byte) (ast.Expr, error) {
 
 // Validate validates JSON and confirms it matches the constraints
 // specified by v.
-func Validate(b []byte, v cue.Value) (bool, error) {
-	err := cuejson.Validate(b, v)
+func Validate(b []byte, v pkg.Schema) (bool, error) {
+	err := cuejson.Validate(b, v.Value())
 	if err != nil {
 		return false, err
 	}

pkg/encoding/json/testdata/gen.txtar

@@ -125,7 +125,7 @@ validate: {
 		} | {
 			b!: int
 		}
-		result: json.Validate(str, schema)
+		result: true
 	}
 	disjunctionClosed: {
 		str: *"{\"a\":10}" | string
@@ -134,7 +134,7 @@ validate: {
 		} | {
 			b: int
 		}
-		result: json.Validate(str, schema)
+		result: true
 	}
 
 	// Issue #2395
@@ -406,7 +406,7 @@ validate: {
 		} | {
 			b!: int
 		}
-		result: json.Validate(str, schema)
+		result: true
 	}
 	disjunctionClosed: {
 		str: *"{\"a\":10}" | string
@@ -415,7 +415,7 @@ validate: {
 		} | {
 			b: int
 		}
-		result: json.Validate(str, schema)
+		result: true
 	}
 
 	// Issue #2395

pkg/encoding/yaml/manual.go

@@ -84,10 +84,11 @@ func UnmarshalStream(data []byte) (ast.Expr, error) {
 	return ast.NewList(a...), nil
 }
 
-// Validate validates YAML and confirms it is an instance of the schema
-// specified by v. If the YAML source is a stream, every object must match v.
-func Validate(b []byte, v cue.Value) (bool, error) {
+// Validate validates YAML and confirms it is an instance of schema.
+// If the YAML source is a stream, every object must match v.
+func Validate(b []byte, schema pkg.Schema) (bool, error) {
 	d := cueyaml.NewDecoder("yaml.Validate", b)
+	v := schema.Value()
 	r := v.Context()
 	for {
 		expr, err := d.Decode()
@@ -132,8 +133,9 @@ func Validate(b []byte, v cue.Value) (bool, error) {
 // specified by v using unification. This means that b must be consistent with,
 // but does not have to be an instance of v. If the YAML source is a stream,
 // every object must match v.
-func ValidatePartial(b []byte, v cue.Value) (bool, error) {
+func ValidatePartial(b []byte, schema pkg.Schema) (bool, error) {
 	d := cueyaml.NewDecoder("yaml.ValidatePartial", b)
+	v := schema.Value()
 	r := v.Context()
 	for {
 		expr, err := d.Decode()

pkg/encoding/yaml/testdata/validate.txtar

@@ -21,15 +21,119 @@ import "encoding/yaml"
 
 validate: #test & {
 	fn: yaml.Validate
+
+	// TODO: fix this test: the second disjunct should be eliminated, so there
+	// should not be a concreteness error.
+	t1: _
 }
 
 validatePartial: #test & {
 	fn: yaml.ValidatePartial
 }
+-- out/yaml-v3 --
+Errors:
+#test.t1.ok1: cannot call non-function fn (type _):
+    ./in.cue:8:11
+validate.t1.ok1: invalid value "a: 2" (does not satisfy encoding/yaml.Validate({a!:int} | {b!:int})): error in call to encoding/yaml.Validate: incomplete value {a:2} | {a:2,b!:int}:
+    ./in.cue:8:11
+    ./in.cue:6:9
+#test.t1.ok2: cannot call non-function fn (type _):
+    ./in.cue:9:11
+#test.t1.ok3: cannot call non-function fn (type _):
+    ./in.cue:13:11
+#test.t2.ok1: cannot call non-function fn (type _):
+    ./in.cue:17:11
+#test.t2.ok2: cannot call non-function fn (type _):
+    ./in.cue:18:11
+
+Result:
+import "encoding/yaml"
+
+#test: {
+	fn:    _
+	data1: "a: 2"
+	t1: {
+		ok1: _|_ // #test.t1.ok1: cannot call non-function fn (type _)
+		ok2: _|_ // #test.t1.ok2: cannot call non-function fn (type _)
+		ok3: _|_ // #test.t1.ok3: cannot call non-function fn (type _)
+	}
+	#A: {
+		a: int
+	}
+	#B: {
+		b: int
+	}
+	data2: "'foo'"
+	t2: {
+		ok1: _|_ // #test.t2.ok1: cannot call non-function fn (type _)
+		ok2: _|_ // #test.t2.ok2: cannot call non-function fn (type _)
+	}
+}
+validate: {
+	fn:    yaml.Validate
+	data1: "a: 2"
+
+	// TODO: fix this test: the second disjunct should be eliminated, so there
+	// should not be a concreteness error.
+	t1: {
+		ok1: _|_ // validate.t1.ok1: invalid value "a: 2" (does not satisfy encoding/yaml.Validate({a!:int} | {b!:int})): validate.t1.ok1: error in call to encoding/yaml.Validate: incomplete value {a:2} | {a:2,b!:int}
+		ok2: "a: 2"
+		ok3: "a: 2"
+	}
+	#A: {
+		a: int
+	}
+	#B: {
+		b: int
+	}
+	data2: "'foo'"
+	t2: {
+		ok1: "'foo'"
+		ok2: "'foo'"
+	}
+}
+validatePartial: {
+	fn:    yaml.ValidatePartial
+	data1: "a: 2"
+	t1: {
+		ok1: "a: 2"
+		ok2: "a: 2"
+		ok3: "a: 2"
+	}
+	#A: {
+		a: int
+	}
+	#B: {
+		b: int
+	}
+	data2: "'foo'"
+	t2: {
+		ok1: "'foo'"
+		ok2: "'foo'"
+	}
+}
+-- diff/-out/yaml-v3<==>+out/yaml --
+diff old new
+--- old
++++ new
+@@ -4,7 +4,6 @@
+ validate.t1.ok1: invalid value "a: 2" (does not satisfy encoding/yaml.Validate({a!:int} | {b!:int})): error in call to encoding/yaml.Validate: incomplete value {a:2} | {a:2,b!:int}:
+     ./in.cue:8:11
+     ./in.cue:6:9
+-    ./in.cue:7:16
+ #test.t1.ok2: cannot call non-function fn (type _):
+     ./in.cue:9:11
+ #test.t1.ok3: cannot call non-function fn (type _):
+-- diff/todo --
+missing position
 -- out/yaml --
 Errors:
 #test.t1.ok1: cannot call non-function fn (type _):
     ./in.cue:8:11
+validate.t1.ok1: invalid value "a: 2" (does not satisfy encoding/yaml.Validate({a!:int} | {b!:int})): error in call to encoding/yaml.Validate: incomplete value {a:2} | {a:2,b!:int}:
+    ./in.cue:8:11
+    ./in.cue:6:9
+    ./in.cue:7:16
 #test.t1.ok2: cannot call non-function fn (type _):
     ./in.cue:9:11
 #test.t1.ok3: cannot call non-function fn (type _):
@@ -65,18 +169,13 @@ import "encoding/yaml"
 validate: {
 	fn:    yaml.Validate
 	data1: "a: 2"
+
+	// TODO: fix this test: the second disjunct should be eliminated, so there
+	// should not be a concreteness error.
 	t1: {
-		ok1: fn({
-			a!: int
-		} | {
-			b!: int
-		}) & data1
-		ok2: fn(close({
-			a: int
-		}) | close({
-			b: int
-		})) & data1
-		ok3: fn(#A | #B) & data1
+		ok1: _|_ // validate.t1.ok1: invalid value "a: 2" (does not satisfy encoding/yaml.Validate({a!:int} | {b!:int})): validate.t1.ok1: error in call to encoding/yaml.Validate: incomplete value {a:2} | {a:2,b!:int}
+		ok2: "a: 2"
+		ok3: "a: 2"
 	}
 	#A: {
 		a: int
@@ -86,25 +185,17 @@ validate: {
 	}
 	data2: "'foo'"
 	t2: {
-		ok1: fn(*int | string) & data2
-		ok2: fn(string) & data2
+		ok1: "'foo'"
+		ok2: "'foo'"
 	}
 }
 validatePartial: {
 	fn:    yaml.ValidatePartial
 	data1: "a: 2"
 	t1: {
-		ok1: fn({
-			a!: int
-		} | {
-			b!: int
-		}) & data1
-		ok2: fn(close({
-			a: int
-		}) | close({
-			b: int
-		})) & data1
-		ok3: fn(#A | #B) & data1
+		ok1: "a: 2"
+		ok2: "a: 2"
+		ok3: "a: 2"
 	}
 	#A: {
 		a: int
@@ -114,7 +205,7 @@ validatePartial: {
 	}
 	data2: "'foo'"
 	t2: {
-		ok1: fn(*int | string) & data2
-		ok2: fn(string) & data2
+		ok1: "'foo'"
+		ok2: "'foo'"
 	}
 }