New flag --incompatible_bzl_disallow_load_after_statement
This disallows to have any statement in a .bzl file before a load() statement.
RELNOTES:
load() statements should be called at the top of .bzl files, before any
other statement. This convention will be enforced in the future.
PiperOrigin-RevId: 155636719
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 8653e83..f25ec69 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
@@ -76,4 +76,14 @@
help = "If set to true, the `+` becomes disabled for dicts."
)
public boolean incompatibleDisallowDictPlus;
+
+ @Option(
+ name = "incompatible_bzl_disallow_load_after_statement",
+ defaultValue = "false",
+ category = "incompatible changes",
+ help =
+ "If set to true, all `load` must be called at the top of .bzl files, before any other "
+ + "statement."
+ )
+ public boolean incompatibleBzlDisallowLoadAfterStatement;
}
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 b0e766d..84a7cbb 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
@@ -41,6 +41,8 @@
private final Set<String> readOnlyVariables = new HashSet<>();
+ private final SkylarkSemanticsOptions semantics;
+
// A stack of variable-sets which are read only but can be assigned in different
// branches of if-else statements.
private final Stack<Set<String>> futureReadOnlyVariables = new Stack<>();
@@ -54,6 +56,7 @@
Set<String> builtinVariables = env.getVariableNames();
variables.addAll(builtinVariables);
readOnlyVariables.addAll(builtinVariables);
+ semantics = env.getSemantics();
}
/**
@@ -62,6 +65,7 @@
public ValidationEnvironment(ValidationEnvironment parent) {
// Don't copy readOnlyVariables: Variables may shadow global values.
this.parent = parent;
+ semantics = parent.semantics;
}
/**
@@ -146,10 +150,45 @@
readOnlyVariables.removeAll(futureReadOnlyVariables.peek());
}
+ /** Throws EvalException if a load() appears after another kind of statement. */
+ private void checkLoadAfterStatement(List<Statement> statements) throws EvalException {
+ Location firstStatement = null;
+
+ for (Statement statement : statements) {
+ // Ignore string literals (e.g. docstrings).
+ if (statement instanceof ExpressionStatement
+ && ((ExpressionStatement) statement).getExpression() instanceof StringLiteral) {
+ continue;
+ }
+
+ if (statement instanceof LoadStatement) {
+ if (firstStatement == null) {
+ continue;
+ }
+ throw new EvalException(
+ statement.getLocation(),
+ "load() statements must be called before any other statement. "
+ + "First non-load() statement appears at "
+ + firstStatement
+ + ". Use --incompatible_bzl_disallow_load_after_statement to temporarily disable "
+ + "this check.");
+ }
+
+ if (firstStatement == null) {
+ firstStatement = statement.getLocation();
+ }
+ }
+ }
+
/**
* Validates the AST and runs static checks.
*/
public void validateAst(List<Statement> statements) throws EvalException {
+ // Check that load() statements are on top.
+ if (semantics.incompatibleBzlDisallowLoadAfterStatement) {
+ checkLoadAfterStatement(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 85a8f8c..9b17d4b 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
@@ -44,6 +44,22 @@
}
@Test
+ public void testLoadAfterStatement() throws Exception {
+ env = newEnvironmentWithSkylarkOptions("--incompatible_bzl_disallow_load_after_statement=true");
+ checkError(
+ "load() statements must be called before any other statement",
+ "a = 5",
+ "load(':b.bzl', 'c')");
+ }
+
+ @Test
+ public void testAllowLoadAfterStatement() throws Exception {
+ env =
+ newEnvironmentWithSkylarkOptions("--incompatible_bzl_disallow_load_after_statement=false");
+ parse("a = 5", "load(':b.bzl', 'c')");
+ }
+
+ @Test
public void testTwoFunctionsWithTheSameName() throws Exception {
checkError(
"Variable foo is read only", "def foo():", " return 1", "def foo(x, y):", " return 1");
