Skip to content

Commit

Permalink
New flag --incompatible_bzl_disallow_load_after_statement
Browse files Browse the repository at this point in the history
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
  • Loading branch information
laurentlb authored and kchodorow committed May 10, 2017
1 parent ed33278 commit a0fd766
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,14 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public final class ValidationEnvironment {

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<>();
Expand All @@ -54,6 +56,7 @@ public ValidationEnvironment(Environment env) {
Set<String> builtinVariables = env.getVariableNames();
variables.addAll(builtinVariables);
readOnlyVariables.addAll(builtinVariables);
semantics = env.getSemantics();
}

/**
Expand All @@ -62,6 +65,7 @@ public ValidationEnvironment(Environment env) {
public ValidationEnvironment(ValidationEnvironment parent) {
// Don't copy readOnlyVariables: Variables may shadow global values.
this.parent = parent;
semantics = parent.semantics;
}

/**
Expand Down Expand Up @@ -146,10 +150,45 @@ public void finishTemporarilyDisableReadonlyCheckBranch() {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,22 @@ public void testReturnOutsideFunction() throws Exception {
checkError("Return statements must be inside a function", "return 2\n");
}

@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(
Expand Down

0 comments on commit a0fd766

Please sign in to comment.