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/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");