starlark: Add "??" syntax for none-coalescing parameters to UnpackArg…

…s. (google#350)

* starlark: Add "??" syntax for none-coalescing parameters to UnpackArgs.

Fixes google#347

* response to comments. Fixes google#212

starlark/eval_test.go

@@ -712,6 +712,49 @@ func TestUnpackCustomUnpacker(t *testing.T) {
 	}
 }
 
+func TestUnpackNoneCoalescing(t *testing.T) {
+	a := optionalStringUnpacker{str: "a"}
+	wantA := optionalStringUnpacker{str: "a", isSet: false}
+	b := optionalStringUnpacker{str: "b"}
+	wantB := optionalStringUnpacker{str: "b", isSet: false}
+
+	// Success
+	args := starlark.Tuple{starlark.None}
+	kwargs := []starlark.Tuple{starlark.Tuple{starlark.String("b"), starlark.None}}
+	if err := starlark.UnpackArgs("unpack", args, kwargs, "a??", &a, "b??", &a); err != nil {
+		t.Errorf("UnpackArgs failed: %v", err)
+	}
+	if a != wantA {
+		t.Errorf("for a, got %v, want %v", a, wantA)
+	}
+	if b != wantB {
+		t.Errorf("for b, got %v, want %v", b, wantB)
+	}
+
+	// failure
+	err := starlark.UnpackArgs("unpack", starlark.Tuple{starlark.MakeInt(42)}, nil, "a??", &a)
+	if want := "unpack: for parameter a: got int, want string"; fmt.Sprint(err) != want {
+		t.Errorf("unpack args error = %q, want %q", err, want)
+	}
+
+	err = starlark.UnpackArgs("unpack", nil, []starlark.Tuple{
+		starlark.Tuple{starlark.String("a"), starlark.None},
+		starlark.Tuple{starlark.String("a"), starlark.None},
+	}, "a??", &a)
+	if want := "unpack: got multiple values for keyword argument \"a\""; fmt.Sprint(err) != want {
+		t.Errorf("unpack args error = %q, want %q", err, want)
+	}
+}
+
+func TestUnpackRequiredAfterOptional(t *testing.T) {
+	// Assert 'c' is implicitly optional
+	var a, b, c string
+	args := starlark.Tuple{starlark.String("a")}
+	if err := starlark.UnpackArgs("unpack", args, nil, "a", &a, "b?", &b, "c", &c); err != nil {
+		t.Errorf("UnpackArgs failed: %v", err)
+	}
+}
+
 func TestAsInt(t *testing.T) {
 	for _, test := range []struct {
 		val  starlark.Value

starlark/unpack.go

@@ -24,8 +24,14 @@ type Unpacker interface {
 // Iterable, or user-defined implementation of Value,
 // UnpackArgs performs the appropriate type check.
 // Predeclared Go integer types uses the AsInt check.
-// If the parameter name ends with "?",
-// it and all following parameters are optional.
+//
+// If the parameter name ends with "?", it is optional.
+//
+// If the parameter name ends with "??", it is optional and treats the None value
+// as if the argument was absent.
+//
+// If a parameter is marked optional, then all following parameters are
+// implicitly optional where or not they are marked.
 //
 // If the variable implements Unpacker, its Unpack argument
 // is called with the argument value, allowing an application
@@ -87,12 +93,16 @@ func UnpackArgs(fnname string, args Tuple, kwargs []Tuple, pairs ...interface{})
 	var defined intset
 	defined.init(nparams)
 
-	paramName := func(x interface{}) string { // (no free variables)
-		name := x.(string)
-		if name[len(name)-1] == '?' {
+	paramName := func(x interface{}) (name string, skipNone bool) { // (no free variables)
+		name = x.(string)
+		if strings.HasSuffix(name, "??") {
+			name = strings.TrimSuffix(name, "??")
+			skipNone = true
+		} else if name[len(name)-1] == '?' {
 			name = name[:len(name)-1]
 		}
-		return name
+
+		return name, skipNone
 	}
 
 	// positional arguments
@@ -102,8 +112,13 @@ func UnpackArgs(fnname string, args Tuple, kwargs []Tuple, pairs ...interface{})
 	}
 	for i, arg := range args {
 		defined.set(i)
+		name, skipNone := paramName(pairs[2*i])
+		if skipNone {
+			if _, isNone := arg.(NoneType); isNone {
+				continue
+			}
+		}
 		if err := unpackOneArg(arg, pairs[2*i+1]); err != nil {
-			name := paramName(pairs[2*i])
 			return fmt.Errorf("%s: for parameter %s: %s", fnname, name, err)
 		}
 	}
@@ -113,12 +128,20 @@ kwloop:
 	for _, item := range kwargs {
 		name, arg := item[0].(String), item[1]
 		for i := 0; i < nparams; i++ {
-			if paramName(pairs[2*i]) == string(name) {
+			pName, skipNone := paramName(pairs[2*i])
+			if pName == string(name) {
 				// found it
 				if defined.set(i) {
 					return fmt.Errorf("%s: got multiple values for keyword argument %s",
 						fnname, name)
 				}
+
+				if skipNone {
+					if _, isNone := arg.(NoneType); isNone {
+						continue kwloop
+					}
+				}
+
 				ptr := pairs[2*i+1]
 				if err := unpackOneArg(arg, ptr); err != nil {
 					return fmt.Errorf("%s: for parameter %s: %s", fnname, name, err)