cue: interpret nil as _ or top

depending on the context.

Do not interpret as a disjuntion of both.

Fixes #220.

Change-Id: If935316e23d12825eda3fa75fea8f95a04d09da3
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/4860
Reviewed-by: Marcel van Lohuizen <mpvl@google.com>

cue/builtin.go

@@ -310,7 +310,7 @@ func (x *builtin) call(ctx *context, src source, args ...evaluated) (ret value)
 	case *valueError:
 		return v.err
 	}
-	return convert(ctx, x, false, call.ret)
+	return convert(ctx, x, true, call.ret)
 }
 
 // callCtxt is passed to builtin implementations.

cue/builtinutil.go

@@ -51,7 +51,7 @@ func fill(v Value, x interface{}, path ...string) Value {
 	for i := len(path) - 1; i >= 0; i-- {
 		x = map[string]interface{}{path[i]: x}
 	}
-	value := convert(ctx, root, true, x)
+	value := convert(ctx, root, false, x)
 	eval := binOp(ctx, baseValue{}, opUnify, root, value)
 	return newValueRoot(ctx, eval)
 }

cue/go.go

@@ -38,18 +38,18 @@ import (
 // optimized.
 
 func init() {
-	internal.FromGoValue = func(runtime, x interface{}, allowDefault bool) interface{} {
-		return convertValue(runtime.(*Runtime), x, allowDefault)
+	internal.FromGoValue = func(runtime, x interface{}, nilIsTop bool) interface{} {
+		return convertValue(runtime.(*Runtime), x, nilIsTop)
 	}
 
 	internal.FromGoType = func(runtime, x interface{}) interface{} {
 		return convertType(runtime.(*Runtime), x)
 	}
 }
 
-func convertValue(r *Runtime, x interface{}, allowDefault bool) Value {
+func convertValue(r *Runtime, x interface{}, nilIsTop bool) Value {
 	ctx := r.index().newContext()
-	v := convert(ctx, baseValue{}, allowDefault, x)
+	v := convert(ctx, baseValue{}, nilIsTop, x)
 	return newValueRoot(ctx, v)
 }
 
@@ -189,23 +189,30 @@ func isZero(v reflect.Value) bool {
 	}
 }
 
-func convert(ctx *context, src source, allowDefault bool, x interface{}) evaluated {
-	v := convertRec(ctx, src, allowDefault, x)
+func convert(ctx *context, src source, nilIsTop bool, x interface{}) evaluated {
+	v := convertRec(ctx, src, nilIsTop, x)
 	if v == nil {
 		return ctx.mkErr(baseValue{}, "unsupported Go type (%v)", v)
 	}
 	return v
 }
 
-func convertRec(ctx *context, src source, allowDefault bool, x interface{}) evaluated {
+func isNil(x reflect.Value) bool {
+	switch x.Kind() {
+	// Only check for supported types; ignore func and chan.
+	case reflect.Ptr, reflect.Map, reflect.Slice, reflect.Interface:
+		return x.IsNil()
+	}
+	return false
+}
+
+func convertRec(ctx *context, src source, nilIsTop bool, x interface{}) evaluated {
 	switch v := x.(type) {
 	case nil:
-		if !allowDefault {
-			return &nullLit{src.base()}
+		if nilIsTop {
+			return &top{src.base()}
 		}
-		// Interpret a nil pointer as an undefined value that is only
-		// null by default, but may still be set: *null | _.
-		return makeNullable(&top{src.base()}, false).(evaluated)
+		return &nullLit{src.base()}
 
 	case ast.Expr:
 		x := newVisitorCtx(ctx, nil, nil, nil, false)
@@ -302,7 +309,7 @@ func convertRec(ctx *context, src source, allowDefault bool, x interface{}) eval
 
 	case reflect.Value:
 		if v.CanInterface() {
-			return convertRec(ctx, src, allowDefault, v.Interface())
+			return convertRec(ctx, src, nilIsTop, v.Interface())
 		}
 
 	default:
@@ -328,19 +335,16 @@ func convertRec(ctx *context, src source, allowDefault bool, x interface{}) eval
 			return toUint(ctx, src, value.Uint())
 
 		case reflect.Float32, reflect.Float64:
-			return convertRec(ctx, src, allowDefault, value.Float())
+			return convertRec(ctx, src, nilIsTop, value.Float())
 
 		case reflect.Ptr:
 			if value.IsNil() {
-				if !allowDefault {
-					return &nullLit{src.base()}
+				if nilIsTop {
+					return &top{src.base()}
 				}
-				// Interpret a nil pointer as an undefined value that is only
-				// null by default, but may still be set: *null | _.
-				elem := goTypeToValue(ctx, false, reflect.TypeOf(v).Elem())
-				return makeNullable(elem, false).(evaluated)
+				return &nullLit{src.base()}
 			}
-			return convertRec(ctx, src, allowDefault, value.Elem().Interface())
+			return convertRec(ctx, src, nilIsTop, value.Elem().Interface())
 
 		case reflect.Struct:
 			obj := newStruct(src)
@@ -351,10 +355,13 @@ func convertRec(ctx *context, src source, allowDefault bool, x interface{}) eval
 					continue
 				}
 				val := value.Field(i)
+				if !nilIsTop && isNil(val) {
+					continue
+				}
 				if isOmitEmpty(&t) && isZero(val) {
 					continue
 				}
-				sub := convertRec(ctx, src, allowDefault, val.Interface())
+				sub := convertRec(ctx, src, nilIsTop, val.Interface())
 				if sub == nil {
 					// mimic behavior of encoding/json: skip fields of unsupported types
 					continue
@@ -388,12 +395,12 @@ func convertRec(ctx *context, src source, allowDefault bool, x interface{}) eval
 				reflect.Uint, reflect.Uint8, reflect.Uint16,
 				reflect.Uint32, reflect.Uint64, reflect.Uintptr:
 				for _, k := range value.MapKeys() {
-					s := fmt.Sprint(k)
-					keys = append(keys, s)
-					sorted = append(sorted, s)
+					val := value.MapIndex(k)
+					// if isNil(val) {
+					// 	continue
+					// }
 
-					val := value.MapIndex(k).Interface()
-					sub := convertRec(ctx, src, allowDefault, val)
+					sub := convertRec(ctx, src, nilIsTop, val.Interface())
 					// mimic behavior of encoding/json: report error of
 					// unsupported type.
 					if sub == nil {
@@ -403,6 +410,10 @@ func convertRec(ctx *context, src source, allowDefault bool, x interface{}) eval
 						return sub
 					}
 
+					s := fmt.Sprint(k)
+					keys = append(keys, s)
+					sorted = append(sorted, s)
+
 					// Set feature later.
 					obj.arcs = append(obj.arcs, arc{feature: 0, v: sub})
 				}
@@ -428,8 +439,8 @@ func convertRec(ctx *context, src source, allowDefault bool, x interface{}) eval
 			list := &list{baseValue: src.base()}
 			arcs := []arc{}
 			for i := 0; i < value.Len(); i++ {
-				val := value.Index(i).Interface()
-				x := convertRec(ctx, src, allowDefault, val)
+				val := value.Index(i)
+				x := convertRec(ctx, src, nilIsTop, val.Interface())
 				if x == nil {
 					return ctx.mkErr(baseValue{}, "unsupported Go type (%v)", val)
 				}

cue/go_test.go

@@ -37,7 +37,7 @@ func TestConvert(t *testing.T) {
 		goVal interface{}
 		want  string
 	}{{
-		nil, "(null | _)",
+		nil, "_",
 	}, {
 		true, "true",
 	}, {
@@ -84,8 +84,18 @@ func TestConvert(t *testing.T) {
 		&n34, "-34",
 	}, {
 		[]int{1, 2, 3, 4}, "[1,2,3,4]",
+	}, {
+		struct {
+			A int
+			B *int
+		}{3, nil},
+		"<0>{A: 3}",
 	}, {
 		[]interface{}{}, "[]",
+	}, {
+		[]interface{}{nil}, "[_]",
+	}, {
+		map[string]interface{}{"a": 1, "x": nil}, "<0>{x: _, a: 1}",
 	}, {
 		map[string][]int{
 			"a": []int{1},
@@ -124,7 +134,7 @@ func TestConvert(t *testing.T) {
 	}, {
 		&struct{ A int }{3}, "<0>{A: 3}",
 	}, {
-		(*struct{ A int })(nil), "(null | <0>{A: (int & >=-9223372036854775808 & int & <=9223372036854775807)})",
+		(*struct{ A int })(nil), "_",
 	}, {
 		reflect.ValueOf(3), "3",
 	}, {

cuego/cuego.go

@@ -171,12 +171,12 @@ func init() {
 }
 
 // fromGoValue converts a Go value to CUE
-func fromGoValue(x interface{}, allowDefault bool) (v cue.Value, err error) {
+func fromGoValue(x interface{}, nilIsNull bool) (v cue.Value, err error) {
 	// TODO: remove the need to have a lock here. We could use a new index (new
 	// Instance) here as any previously unrecognized field can never match an
 	// existing one and can only be merged.
 	mutex.Lock()
-	v = internal.FromGoValue(runtime, x, allowDefault).(cue.Value)
+	v = internal.FromGoValue(runtime, x, nilIsNull).(cue.Value)
 	mutex.Unlock()
 	if err := v.Err(); err != nil {
 		return v, err

encoding/gocode/gocodec/codec_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/kr/pretty"
 
 	"cuelang.org/go/cue"
 )
@@ -219,7 +220,7 @@ func TestComplete(t *testing.T) {
 			err = codec.Complete(v, tc.value)
 			checkErr(t, err, tc.err)
 			if !reflect.DeepEqual(tc.value, tc.result) {
-				t.Errorf("value:\n got: %#v;\nwant: %#v", tc.value, tc.result)
+				t.Error(pretty.Diff(tc.value, tc.result))
 			}
 		})
 	}