starlark: permit if/for/while at toplevel if -globalreassign

Also:
- make the option handling work the same across all tests
- be explicit about which options are needed in each test chunk
- improve documentation of options
- rename fp option to float consistently
- rename global_reassign option to globalreassign consistently

Fixes google#36

Change-Id: I8fdf5725ce47c2f4a87aac9d5c24583e503d0f74

cmd/starlark/starlark.go

@@ -29,12 +29,13 @@ var (
 
 // non-standard dialect flags
 func init() {
-	flag.BoolVar(&resolve.AllowFloat, "fp", resolve.AllowFloat, "allow floating-point numbers")
+	flag.BoolVar(&resolve.AllowFloat, "float", resolve.AllowFloat, "allow floating-point numbers")
 	flag.BoolVar(&resolve.AllowSet, "set", resolve.AllowSet, "allow set data type")
 	flag.BoolVar(&resolve.AllowLambda, "lambda", resolve.AllowLambda, "allow lambda expressions")
 	flag.BoolVar(&resolve.AllowNestedDef, "nesteddef", resolve.AllowNestedDef, "allow nested def statements")
 	flag.BoolVar(&resolve.AllowBitwise, "bitwise", resolve.AllowBitwise, "allow bitwise operations (&, |, ^, ~, <<, and >>)")
 	flag.BoolVar(&resolve.AllowRecursion, "recursion", resolve.AllowRecursion, "allow while statements and recursive functions")
+	flag.BoolVar(&resolve.AllowGlobalReassign, "globalreassign", resolve.AllowGlobalReassign, "allow reassignment of globals, and if/for/while statements at top level")
 }
 
 func main() {

doc/spec.md

@@ -16,8 +16,7 @@ used by Bazel.
 We identify places where their behaviors differ, and an
 [appendix](#dialect-differences) provides a summary of those
 differences.
-We plan to converge both implementations on a single specification
-in early 2018.
+We plan to converge both implementations on a single specification.
 
 This document is maintained by Alan Donovan <[email protected]>.
 It was influenced by the Python specification,
@@ -241,11 +240,10 @@ characters are tokens:
 identifiers:
 
 ```text
-and            else           load
-break          for            not
-continue       if             or
-def            in             pass
-elif           lambda         return
+and            elif           in             or
+break          else           lambda         pass
+continue       for            load           return
+def            if             not            while
 ```
 
 The tokens below also may not be used as identifiers although they do not
@@ -254,14 +252,11 @@ appear in the grammar; they are reserved as possible future keywords:
 <!-- and to remain a syntactic subset of Python -->
 
 ```text
-as             import
-assert         is
-class          nonlocal
-del            raise
-except         try
-finally        while
-from           with
-global         yield
+as             finally        nonlocal
+assert         from           raise
+class          global         try
+del            import         with
+except         is             yield
 ```
 
 <b>Implementation note:</b>
@@ -449,7 +444,7 @@ of protocol messages which may contain signed and unsigned 64-bit
 integers.
 The Java implementation currently supports only signed 32-bit integers.
 
-The Go implementation of the Starlark REPL requires the `-bitwise` flag to
+The Go implementation of Starlark requires the `-bitwise` flag to
 enable support for `&`, `|`, `^`, `~`, `<<`, and `>>` operations.
 The Java implementation does not support `^`, `~`, `<<`, and `>>` operations.
 
@@ -503,9 +498,8 @@ float(3) / 2                                    # 1.5
 The Go implementation of Starlark supports floating-point numbers as an
 optional feature, motivated by the need for lossless manipulation of
 protocol messages.
-The Go implementation of the Starlark REPL requires the `-fp` flag to
-enable support for floating-point literals, the `float` built-in
-function, and the real division operator `/`.
+The `-float` flag enables support for floating-point literals,
+the `float` built-in function, and the real division operator `/`.
 The Java implementation does not yet support floating-point numbers.
 
 
@@ -842,7 +836,7 @@ The only method of a set is `union`, which is equivalent to the `|` operator.
 A set used in a Boolean context is considered true if it is non-empty.
 
 <b>Implementation note:</b>
-The Go implementation of the Starlark REPL requires the `-set` flag to
+The Go implementation of Starlark requires the `-set` flag to
 enable support for sets and the `-bitwise` flag to enable support for
 the `&`, `|`, and `^` operators.
 The Java implementation does not support sets.
@@ -1016,7 +1010,7 @@ f(-1)           # returns 1 without printing
 ```
 
 <b>Implementation note:</b>
-The Go implementation of the Starlark REPL requires the `-recursion`
+The Go implementation of Starlark requires the `-recursion`
 flag to allow recursive functions.
 
 
@@ -1074,7 +1068,7 @@ The parameter names serve merely as documentation.
 After a Starlark file is parsed, but before its execution begins, the
 Starlark interpreter checks statically that the program is well formed.
 For example, `break` and `continue` statements may appear only within
-a loop; `if`, `for`, `while`, and `return` statements may appear only within a
+a loop; a `return` statement may appear only within a
 function; and `load` statements may appear only outside any function.
 
 _Name resolution_ is the static checking process that
@@ -1920,7 +1914,7 @@ set([1, 2]) ^ set([2, 3])       # set([1, 3])
 ```
 
 <b>Implementation note:</b>
-The Go implementation of the Starlark REPL requires the `-set` flag to
+The Go implementation of Starlark requires the `-set` flag to
 enable support for sets.
 The Java implementation does not support sets, nor recognize `&` as a
 token, nor support `int | int`.
@@ -2364,7 +2358,7 @@ twice = lambda(x): x * 2
 ```
 
 <b>Implementation note:</b>
-The Go implementation of the Starlark REPL requires the `-lambda` flag
+The Go implementation of Starlark requires the `-lambda` flag
 to enable support for lambda expressions.
 The Java implementation does not support them.
 See Google Issue b/36358844.
@@ -2550,7 +2544,7 @@ current module.
 <!-- this is too implementation-oriented; it's not a spec. -->
 
 <b>Implementation note:</b>
-The Go implementation of the Starlark REPL requires the `-nesteddef`
+The Go implementation of Starlark requires the `-nesteddef`
 flag to enable support for nested `def` statements.
 The Java implementation does not permit a `def` expression to be
 nested within the body of another function.
@@ -2638,6 +2632,11 @@ else:
 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
+if the `-globalreassign` flag is enabled.
+
+
 ### While loops
 
 A `while` loop evaluates an expression (the _condition_) and if the truth
@@ -2659,8 +2658,9 @@ while n > 0:
 A `while` statement is permitted only within a function definition.
 A `while` statement at top level results in a static error.
 
-<b>Implementation note:</b> `while` loops are only allowed when the `-recursion`
-flag is specified.
+<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.
 
 
 ### For loops
@@ -2701,6 +2701,10 @@ iteration.
 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
+if the `-globalreassign` flag is enabled.
+
 
 ### Break and Continue
 
@@ -2939,7 +2943,7 @@ With no arguments, `float()` returns `0.0`.
 
 <b>Implementation note:</b>
 Floating-point numbers are an optional feature.
-The Go implementation of the Starlark REPL requires the `-fp` flag to
+The Go implementation of Starlark requires the `-float` flag to
 enable support for floating-point literals, the `float` built-in
 function, and the real division operator `/`.
 The Java implementation does not yet support floating-point numbers.
@@ -3150,7 +3154,8 @@ set([3, 1, 4, 1, 5, 9])         # set([3, 1, 4, 5, 9])
 ```
 
 <b>Implementation note:</b>
-Sets are an optional feature of the Go implementation of Starlark.
+Sets are an optional feature of the Go implementation of Starlark,
+enabled by the `-set` flag.
 
 
 ### sorted
@@ -4029,3 +4034,5 @@ See [Starlark spec issue 20](https://github.com/bazelbuild/starlark/issues/20).
 * `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`).
+* top-level rebindings are permitted (option: `-globalreassign`).

resolve/resolve.go

@@ -431,7 +431,7 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
 		}
 
 	case *syntax.IfStmt:
-		if r.container().function == nil {
+		if !AllowGlobalReassign && r.container().function == nil {
 			r.errorf(stmt.If, "if statement not within a function")
 		}
 		r.expr(stmt.Cond)
@@ -463,7 +463,7 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
 		r.function(stmt.Def, stmt.Name.Name, &stmt.Function)
 
 	case *syntax.ForStmt:
-		if r.container().function == nil {
+		if !AllowGlobalReassign && r.container().function == nil {
 			r.errorf(stmt.For, "for loop not within a function")
 		}
 		r.expr(stmt.X)
@@ -475,9 +475,9 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
 
 	case *syntax.WhileStmt:
 		if !AllowRecursion {
-			r.errorf(stmt.While, "while loop not allowed")
+			r.errorf(stmt.While, doesnt+"support while loops")
 		}
-		if r.container().function == nil {
+		if !AllowGlobalReassign && r.container().function == nil {
 			r.errorf(stmt.While, "while loop not within a function")
 		}
 		r.expr(stmt.Cond)

resolve/resolve_test.go

@@ -14,7 +14,22 @@ import (
 	"go.starlark.net/syntax"
 )
 
+func setOptions(src string) {
+	resolve.AllowBitwise = option(src, "bitwise")
+	resolve.AllowFloat = option(src, "float")
+	resolve.AllowGlobalReassign = option(src, "globalreassign")
+	resolve.AllowLambda = option(src, "lambda")
+	resolve.AllowNestedDef = option(src, "nesteddef")
+	resolve.AllowRecursion = option(src, "recursion")
+	resolve.AllowSet = option(src, "set")
+}
+
+func option(chunk, name string) bool {
+	return strings.Contains(chunk, "option:"+name)
+}
+
 func TestResolve(t *testing.T) {
+	defer setOptions("")
 	filename := starlarktest.DataFile("resolve", "testdata/resolve.star")
 	for _, chunk := range chunkedfile.Read(filename, t) {
 		f, err := syntax.Parse(filename, chunk.Source, 0)
@@ -24,11 +39,7 @@ func TestResolve(t *testing.T) {
 		}
 
 		// A chunk may set options by containing e.g. "option:float".
-		resolve.AllowNestedDef = option(chunk.Source, "nesteddef")
-		resolve.AllowLambda = option(chunk.Source, "lambda")
-		resolve.AllowFloat = option(chunk.Source, "float")
-		resolve.AllowSet = option(chunk.Source, "set")
-		resolve.AllowGlobalReassign = option(chunk.Source, "global_reassign")
+		setOptions(chunk.Source)
 
 		if err := resolve.File(f, isPredeclared, isUniversal); err != nil {
 			for _, err := range err.(resolve.ErrorList) {
@@ -39,10 +50,6 @@ func TestResolve(t *testing.T) {
 	}
 }
 
-func option(chunk, name string) bool {
-	return strings.Contains(chunk, "option:"+name)
-}
-
 func TestDefVarargsAndKwargsSet(t *testing.T) {
 	source := "def f(*args, **kwargs): pass\n"
 	file, err := syntax.Parse("foo.star", source, 0)

resolve/testdata/resolve.star

@@ -144,15 +144,51 @@ load("foo",
      _e="f") # ok
 
 ---
-# return, if statements and for loops at top-level are forbidden
+# return statements must be within a function
+
+return ### "return statement not within a function"
+
+---
+# if-statements and for-loops at top-level are forbidden
+# (without globalreassign option)
 
 for x in "abc": ### "for loop not within a function"
   pass
 
 if x: ### "if statement not within a function"
   pass
 
-return ### "return statement not within a function"
+---
+# option:globalreassign
+
+for x in "abc": # ok
+  pass
+
+if x: # ok
+  pass
+
+---
+# while loops are forbidden (without -recursion option)
+
+def f():
+  while U: ### "dialect does not support while loops"
+    pass
+
+---
+# option:recursion
+
+def f():
+  while U: # ok
+    pass
+
+while U: ### "while loop not within a function"
+  pass
+
+---
+# option:globalreassign option:recursion
+
+while U: # ok
+  pass
 
 ---
 # The parser allows any expression on the LHS of an assignment.
@@ -247,20 +283,20 @@ b = 1 / 2
 c = 3.141
 
 ---
-# option:global_reassign
+# option:globalreassign
 # Legacy Bazel (and Python) semantics: def must precede use even for globals.
 
 _ = x ### `undefined: x`
 x = 1
 
 ---
-# option:global_reassign
+# option:globalreassign
 # Legacy Bazel (and Python) semantics: reassignment of globals is allowed.
 x = 1
 x = 2 # ok
 
 ---
-# option:global_reassign
+# option:globalreassign
 # Redeclaration of predeclared names is allowed.
 
 # module-specific predeclared name