resolve: disallow augmented assignments at toplevel

...unless -globalreassign is set.

Formerly, the Go implementation permitted x+=y on the basis
that we couldn't statically tell whether x was a list (in which
case x+=y means x.extend(y), which is not a rebinding), or some
other type, in which case it is a forbidden rebinding.
This change makes it match the Java implementation.
See bazelbuild/starlark#20 (comment)

Most non-Bazel-like clients of Starlark set the -globalreassign flag.

Change-Id: I1a21fc6871e51c201529da91dc0a46ad3e99d448

doc/spec.md

@@ -1090,7 +1090,7 @@ Four Starlark constructs bind names, as illustrated in the example below:
 `load` statements (`a` and `b`),
 `def` statements (`c`),
 function parameters (`d`),
-and assignments (`e`, `h`, including the augmented assignment `e += h`).
+and assignments (`e`, `h`, including the augmented assignment `e += 1`).
 Variables may be assigned or re-assigned explicitly (`e`, `h`), or implicitly, as
 in a `for`-loop (`f`) or comprehension (`g`, `i`).
 
@@ -1182,22 +1182,21 @@ x = 2                   # static error: cannot reassign global x declared on lin
 ```
 
 <!-- The above rule, and the rule that forbids if-statements and loops at
-     toplevel, exist to ensure that there is exactly one statement
+     top level, exist to ensure that there is exactly one statement
      that binds each global variable, which makes cross-referenced
      documentation more useful, the designers assure me, but
      I am skeptical that it's worth the trouble. -->
 
 If a name was pre-bound by the application, the Starlark program may
 explicitly bind it, but only once.
 
+An augmented assignment statement such as `x += y` is considered both a
+reference to `x` and a binding use of `x`, so it may not be used at
+top level.
+
 <b>Implementation note:</b>
-An augmented assignment statement such as `x += 1` is considered a
-binding of `x`.
-However, because of the special behavior of `+=` for lists, which acts
-like a non-binding reference, the Go implementation suppresses the
-"cannot reassign" error for all augmented assigments at toplevel,
-whereas the Java implementation reports the error even when the
-statement would apply `+=` to a list.
+The Go implementation of Starlark permits augmented assignments to appear
+at top level if the `-globalreassign` flag is enabled.
 
 A function may refer to variables defined in an enclosing function.
 In this example, the inner function `f` refers to a variable `x`
@@ -2640,7 +2639,7 @@ An `if` statement is permitted only within a function definition.
 An `if` statement at top level results in a static error.
 
 <b>Implementation note:</b>
-The Go implementation of Starlark permits `if`-statements to appear at top-level
+The Go implementation of Starlark permits `if`-statements to appear at top level
 if the `-globalreassign` flag is enabled.
 
 
@@ -2667,7 +2666,7 @@ A `while` statement at top level results in a static error.
 
 <b>Implementation note:</b>
 The Go implementation of Starlark permits `while` loops only if the `-recursion` flag is enabled.
-A `while` statement is permitted at top-level if the `-globalreassign` flag is enabled.
+A `while` statement is permitted at top level if the `-globalreassign` flag is enabled.
 
 
 ### For loops
@@ -2709,7 +2708,7 @@ In Starlark, a `for` loop is permitted only within a function definition.
 A `for` loop at top level results in a static error.
 
 <b>Implementation note:</b>
-The Go implementation of Starlark permits loops to appear at top-level
+The Go implementation of Starlark permits loops to appear at top level
 if the `-globalreassign` flag is enabled.
 
 
@@ -4033,13 +4032,12 @@ See [Starlark spec issue 20](https://github.com/bazelbuild/starlark/issues/20).
 * The `chr` and `ord` built-in functions are supported.
 * The `set` built-in function is provided (option: `-set`).
 * `set & set` and `set | set` compute set intersection and union, respectively.
-* `x += y` rebindings are permitted at top level.
 * `assert` is a valid identifier.
 * The parser accepts unary `+` expressions.
 * A method call `x.f()` may be separated into two steps: `y = x.f; y()`.
 * Dot expressions may appear on the left side of an assignment: `x.f = 1`.
 * `hash` accepts operands besides strings.
 * `sorted` accepts the additional parameters `key` and `reverse`.
 * `type(x)` returns `"builtin_function_or_method"` for built-in functions.
-* `if`, `for`, and `while` are permitted at toplevel (option: `-globalreassign`).
+* `if`, `for`, and `while` are permitted at top level (option: `-globalreassign`).
 * top-level rebindings are permitted (option: `-globalreassign`).

resolve/resolve.go

@@ -519,7 +519,7 @@ func (r *resolver) assign(lhs syntax.Expr, isAugmented bool) {
 	switch lhs := lhs.(type) {
 	case *syntax.Ident:
 		// x = ...
-		allowRebind := isAugmented
+		const allowRebind = false
 		r.bind(lhs, allowRebind)
 
 	case *syntax.IndexExpr:

resolve/testdata/resolve.star

@@ -91,19 +91,20 @@ def g():
   f()
 
 ---
-# It's permitted to rebind a global using a += assignment.
-
+# It is not permitted to rebind a global using a += assignment.
 x = [1]
 x.extend([2]) # ok
-x += [3] # ok (a list mutation, not a global rebinding)
+x += [3] ### `cannot reassign global x`
+
+---
+# It is not permitted to rebind a global using a += assignment.
 
 def f():
    x += [4] # x is local to f
 
 y = 1
-y += 2 # ok (even though it is in fact a global rebinding)
-
-z += 3 # ok (but fails dynamically because z is undefined)
+y += 2 ### `cannot reassign global y`
+z += 3
 
 ---
 def f(a):
@@ -193,15 +194,15 @@ while U: # ok
 ---
 # The parser allows any expression on the LHS of an assignment.
 
-1 = 2 ### "can't assign to literal"
-1+2 = 3 ### "can't assign to binaryexpr"
-f() = 4 ### "can't assign to callexpr"
+1 = 0 ### "can't assign to literal"
+1+2 = 0 ### "can't assign to binaryexpr"
+f() = 0 ### "can't assign to callexpr"
 
-[a, b] = [1, 2]
-[a, b] += [3, 4] ### "can't use list expression in augmented assignment"
-(a, b) += [3, 4] ### "can't use tuple expression in augmented assignment"
-[] = [] ### "can't assign to \\[\\]"
-() = () ### "can't assign to ()"
+[a, b] = 0
+[c, d] += 0 ### "can't use list expression in augmented assignment"
+(e, f) += 0 ### "can't use tuple expression in augmented assignment"
+[] = 0 ### "can't assign to \\[\\]"
+() = 0 ### "can't assign to ()"
 
 ---
 # break and continue statements must appear within a loop

starlark/testdata/assign.star

@@ -288,6 +288,7 @@ assert.eq(g(), {"one": 1, "two": 2})
 
 ---
 # parenthesized LHS in augmented assignment (success)
+# option:globalreassign
 load("assert.star", "assert")
 
 a = 5