resolve: allow if/for/while statements at toplevel

[alandonovan]

These constructs are still disallowed in the Bazel build language,
but it is easy to check for them as a separate pass over the syntax tree,
so there is no need to complicate the spec with this restriction,
which is a nuisance in other dialects.

Clients that set the -globalreassign flag to suppress the check
need do so no longer.

[alandonovan]

@laurentlb

[jayconrod]

The code here seems sound, but I'm not sure I understand the premise. if and for are still forbidden at top-level in the canonical Starlark spec. It's part of the language, right, not Bazel's validation? It seems like this should still be behind a dialect flag.

[alandonovan]

Correct. I am suggesting that the Starlark spec should say the same thing, and we specify the restrictions in to the Bazel manual. Ping @laurentlb

[alandonovan]

It seems like this should still be behind a dialect flag.

Bazel already needs to inspect the AST for other reasons, for example, it disallows f(**kwargs) in a BUILD file. So it already has a place to do this check, and it already has (I assume) a place in its manual to explain the notion of syntax restrictions.

[laurentlb]

Can you have it behind a flag for now?

[alandonovan]

Can you have it behind a flag for now?

Have what behind a flag, exactly? A Bazel-like tool using Starlark-go already must inspect the syntax tree to reject def and f(**kwargs) in BUILD files, so it is trivial for it to also check for toplevel if/for/while at that point by adding these 8 lines of code:

	for _, stmt := range f.Stmts {
		switch stmt := stmt.(type) {
		case *syntax.IfStmt:
			// error: if-statements are not allowed at top level
		case *syntax.ForStmt:
			// error: for loops are not allowed at top level
		}
	}

No other client besides Bazel seems to need or want this behavior.

[laurentlb]

As it doesn't follow the specification, I think it's preferable to have a flag to allow if/for at top-level. Users should explicitly opt in if they prefer this behavior.

An if statement is permitted only within a function definition. An if statement at top level results in a static error. (https://github.com/bazelbuild/starlark/blob/master/spec.md#if-statements)
In Starlark, a for loop is permitted only within a function definition. A for loop at top level results in a static error.

[alandonovan]

As it doesn't follow the specification, I think it's preferable...

Laurent, we own the specification and it is wrong. Let's change it.

[AlekSi]

  -globalreassign
    	allow reassignment of globals, and if/for/while statements at top level

Isn't that supported already? Is this PR stale?

[alandonovan]

This PR is really a request to change the spec to match reality: top-level if/for are permitted in all dialects except for Bazel, which imposes various unique restrictions on BUILD and .bzl files, this being one of them.