Introduce --incompatible_disallow_toplevel_if_statement to forbid top-level if statements.
RELNOTES:
In .bzl files, top-level `if` statements are deprecated and will be forbidden
in the future. Move them in a function body instead (or use a conditional
expression instead: `x if condition else y`).
PiperOrigin-RevId: 158276986
diff --git a/site/docs/skylark/concepts.md b/site/docs/skylark/concepts.md
index 8caf553..703fac1 100644
--- a/site/docs/skylark/concepts.md
+++ b/site/docs/skylark/concepts.md
@@ -116,8 +116,8 @@
We are doing this to reduce confusion between the specialized
[depset](depsets.md) data structure and Python's set datatype.
-* Flag: `--incompatible_disallow_set_constructor=true`
-* Introduced in: 0.5.1
+* Flag: `--incompatible_disallow_set_constructor`
+* Default: `false`
### Keyword-only arguments
@@ -139,8 +139,8 @@
compatibility with Python 2, we are removing this feature (which we have never
documented).
-* Flag: `--incompatible_disallow_keyword_only_args=true`
-* Introduced in: 0.5.1
+* Flag: `--incompatible_disallow_keyword_only_args`
+* Default: `false`
### Mutating `+=`
@@ -161,8 +161,8 @@
This change makes Skylark more compatible with Python and avoids performance
issues. The `+=` operator for tuples is unaffected.
-* Flag: `--incompatible_list_plus_equals_inplace=true`
-* Introduced in: 0.5.1
+* Flag: `--incompatible_list_plus_equals_inplace`
+* Default: `false`
### Dictionary concatenation
@@ -170,8 +170,18 @@
where the left-hand side is a dictionary. This is done to improve compatibility
with Python. A possible workaround is to use the `.update` method instead.
-* Flag: `--incompatible_disallow_dict_plus=true`
-* Introduced in: 0.5.1
+* Flag: `--incompatible_disallow_dict_plus`
+* Default: `false`
+
+### Top level `if` statements
+
+This change forbids `if` statements at the top level of `.bzl` files (they are
+already forbidden in `BUILD` files). This change ensures that every global
+value has a single declaration. This restriction is consistent with the idea
+that global values cannot be redefined.
+
+* Flag: `--incompatible_disallow_toplevel_if_statement`
+* Default: `false`
## Upcoming changes
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
index f25ec69..6ce4801 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
@@ -86,4 +86,14 @@
+ "statement."
)
public boolean incompatibleBzlDisallowLoadAfterStatement;
+
+ @Option(
+ name = "incompatible_disallow_toplevel_if_statement",
+ defaultValue = "false",
+ category = "incompatible changes",
+ help =
+ "If set to true, 'if' statements are forbidden at the top-level "
+ + "(outside a function definition)"
+ )
+ public boolean incompatibleDisallowToplevelIfStatement;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
index 84a7cbb..b66e0d6 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
@@ -180,6 +180,20 @@
}
}
+ /** Throws EvalException if a `if` statement appears at the top level. */
+ private void checkToplevelIfStatement(List<Statement> statements) throws EvalException {
+ for (Statement statement : statements) {
+ if (statement instanceof IfStatement) {
+ throw new EvalException(
+ statement.getLocation(),
+ "if statements are not allowed at the top level. You may move it inside a function "
+ + "or use an if expression (x if condition else y). "
+ + "Use --incompatible_disallow_toplevel_if_statement to temporarily disable "
+ + "this check.");
+ }
+ }
+ }
+
/**
* Validates the AST and runs static checks.
*/
@@ -189,6 +203,11 @@
checkLoadAfterStatement(statements);
}
+ // Check that load() statements are on top.
+ if (semantics.incompatibleDisallowToplevelIfStatement) {
+ checkToplevelIfStatement(statements);
+ }
+
// Add every function in the environment before validating. This is
// necessary because functions may call other functions defined
// later in the file.
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
index 0ad0297..10c80c7 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
@@ -60,6 +60,18 @@
}
@Test
+ public void testForbiddenToplevelIfStatement() throws Exception {
+ env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_toplevel_if_statement=true");
+ checkError("if statements are not allowed at the top level", "if True: a = 2");
+ }
+
+ @Test
+ public void testAllowedToplevelIfStatement() throws Exception {
+ env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_toplevel_if_statement=false");
+ parse("if True: a = 5");
+ }
+
+ @Test
public void testTwoFunctionsWithTheSameName() throws Exception {
checkError(
"Variable foo is read only", "def foo():", " return 1", "def foo(x, y):", " return 1");
