resolve: statically reject duplicate keyword args in a call (google#72)

This complements the dynamic checks, which is already implemented in setArgs and in UnpackArgs.

Updates bazelbuild/starlark#21

doc/spec.md

@@ -991,6 +991,11 @@ Once the parameters have been successfully bound to the arguments
 supplied by the call, the sequence of statements that comprise the
 function body is executed.
 
+It is a static error if a function call has two named arguments of the
+same name, such as `f(x=1, x=2)`. A call that provides a `**kwargs`
+argument may yet have two values for the same name, such as
+`f(x=1, **dict(x=2))`. This results in a dynamic error.
+
 A function call completes normally after the execution of either a
 `return` statement, or of the last statement in the function body.
 The result of the function call is the value of the return statement's

resolve/resolve.go

@@ -668,7 +668,8 @@ func (r *resolver) expr(e syntax.Expr) {
 
 	case *syntax.CallExpr:
 		r.expr(e.Fn)
-		var seenVarargs, seenKwargs, seenNamed bool
+		var seenVarargs, seenKwargs bool
+		var seenName map[string]bool
 		for _, arg := range e.Args {
 			pos, _ := arg.Span()
 			if unop, ok := arg.(*syntax.UnaryExpr); ok && unop.Op == syntax.STARSTAR {
@@ -692,16 +693,23 @@ func (r *resolver) expr(e syntax.Expr) {
 				if seenKwargs {
 					r.errorf(pos, "argument may not follow **kwargs")
 				}
-				// ignore binop.X
+				x := binop.X.(*syntax.Ident)
+				if seenName[x.Name] {
+					r.errorf(x.NamePos, "keyword argument %s repeated", x.Name)
+				} else {
+					if seenName == nil {
+						seenName = make(map[string]bool)
+					}
+					seenName[x.Name] = true
+				}
 				r.expr(binop.Y)
-				seenNamed = true
 			} else {
 				// positional argument
 				if seenVarargs {
 					r.errorf(pos, "argument may not follow *args")
 				} else if seenKwargs {
 					r.errorf(pos, "argument may not follow **kwargs")
-				} else if seenNamed {
+				} else if len(seenName) > 0 {
 					r.errorf(pos, "positional argument may not follow named")
 				}
 				r.expr(arg)

resolve/testdata/resolve.star

@@ -270,3 +270,8 @@ M = 2 # ok (legacy)
 # universal predeclared name
 U = 1 # ok
 U = 1 # ok (legacy)
+
+---
+# https://github.com/bazelbuild/starlark/starlark/issues/21
+def f(**kwargs): pass
+f(a=1, a=1) ### `keyword argument a repeated`

starlark/testdata/dict.star

@@ -52,13 +52,13 @@ small = dict([("a", 0), ("b", 1), ("c", 2)])
 small.update([("d", 4), ("e", 5), ("f", 6), ("g", 7), ("h", 8), ("i", 9), ("j", 10), ("k", 11)])
 assert.eq(small.keys(), ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k"])
 
-# duplicate keys are not permitted in dictionary expressions (see b/35698444).
+# Duplicate keys are not permitted in dictionary expressions (see b/35698444).
+# (Nor in keyword args to function calls---checked by resolver.)
 assert.fails(lambda: {"aa": 1, "bb": 2, "cc": 3, "bb": 4}, 'duplicate key: "bb"')
-# nor in keyword args to dict built-in
-assert.fails(lambda: dict(a=1, a=1), 'dict: duplicate keyword arg: "a"')
-# check that even with many positional args, keyword collisions are detected
-assert.fails(lambda: dict({'b': 3}, a=4, a=5), 'dict: duplicate keyword arg: "a"')
-assert.fails(lambda: dict({'a': 2, 'b': 3}, a=4, a=5), 'dict: duplicate keyword arg: "a"')
+
+# Check that even with many positional args, keyword collisions are detected.
+assert.fails(lambda: dict({'b': 3}, a=4, **dict(a=5)), 'dict: duplicate keyword arg: "a"')
+assert.fails(lambda: dict({'a': 2, 'b': 3}, a=4, **dict(a=5)), 'dict: duplicate keyword arg: "a"')
 # positional/keyword arg key collisions are ok
 assert.eq(dict((['a', 2], ), a=4), {'a': 4})
 assert.eq(dict((['a', 2], ['a', 3]), a=4), {'a': 4})

starlark/testdata/function.star

@@ -175,22 +175,24 @@ assert.fails(lambda: f(
     mm = 100), 'multiple values for keyword argument "mm"')
 
 ---
-# Regression test for github.com/google/starlark-go/issues/21.
+# Regression test for github.com/google/starlark-go/issues/21,
+# which concerns dynamic checks.
+# Related: https://github.com/bazelbuild/starlark/issues/21,
+# which concerns static checks.
 
 load("assert.star", "assert")
 
 def f(*args, **kwargs):
   return args, kwargs
 
 assert.eq(f(x=1, y=2), ((), {"x": 1, "y": 2}))
-assert.fails(lambda: f(x=1, x=2), 'multiple values for keyword argument "x"')
 assert.fails(lambda: f(x=1, **dict(x=2)), 'multiple values for keyword argument "x"')
 
 def g(x, y):
   return x, y
 
 assert.eq(g(1, y=2), (1, 2))
-assert.fails(lambda: g(1, y=2, y=3), 'multiple values for keyword argument "y"')
+assert.fails(lambda: g(1, y=2, **{'y': 3}), 'multiple values for keyword argument "y"')
 
 ---
 # Regression test for a bug in CALL_VAR_KW.