Cleanup in ValidationEnvironment, provide static methods, reduce visibility.
Call sites were creating a ValidationEnvironment object with no other purpose
than calling validateAst(). Simplify the code so that callers don't have to
worry about it.
RELNOTES: None.
PiperOrigin-RevId: 158705853
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 b66e0d6..f06c15f 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
@@ -47,10 +47,8 @@
// branches of if-else statements.
private final Stack<Set<String>> futureReadOnlyVariables = new Stack<>();
- /**
- * Create a ValidationEnvironment for a given global Environment.
- */
- public ValidationEnvironment(Environment env) {
+ /** Create a ValidationEnvironment for a given global Environment. */
+ ValidationEnvironment(Environment env) {
Preconditions.checkArgument(env.isGlobal());
parent = null;
Set<String> builtinVariables = env.getVariableNames();
@@ -59,27 +57,20 @@
semantics = env.getSemantics();
}
- /**
- * Creates a local ValidationEnvironment to validate user defined function bodies.
- */
- public ValidationEnvironment(ValidationEnvironment parent) {
+ /** Creates a local ValidationEnvironment to validate user defined function bodies. */
+ ValidationEnvironment(ValidationEnvironment parent) {
// Don't copy readOnlyVariables: Variables may shadow global values.
this.parent = parent;
semantics = parent.semantics;
}
- /**
- * Returns true if this ValidationEnvironment is top level i.e. has no parent.
- */
- public boolean isTopLevel() {
+ /** Returns true if this ValidationEnvironment is top level i.e. has no parent. */
+ boolean isTopLevel() {
return parent == null;
}
- /**
- * Declare a variable and add it to the environment.
- */
- public void declare(String varname, Location location)
- throws EvalException {
+ /** Declare a variable and add it to the environment. */
+ void declare(String varname, Location location) throws EvalException {
checkReadonly(varname, location);
if (parent == null) { // top-level values are immutable
readOnlyVariables.add(varname);
@@ -101,16 +92,14 @@
}
}
- /**
- * Returns true if the symbol exists in the validation environment.
- */
- public boolean hasSymbolInEnvironment(String varname) {
+ /** Returns true if the symbol exists in the validation environment. */
+ boolean hasSymbolInEnvironment(String varname) {
return variables.contains(varname)
|| (parent != null && topLevel().variables.contains(varname));
}
/** Returns the set of all accessible symbols (both local and global) */
- public Set<String> getAllSymbols() {
+ Set<String> getAllSymbols() {
Set<String> all = new HashSet<>();
all.addAll(variables);
if (parent != null) {
@@ -128,14 +117,12 @@
* This is useful to validate control flows like if-else when we know that certain parts of the
* code cannot both be executed.
*/
- public void startTemporarilyDisableReadonlyCheckSession() {
+ void startTemporarilyDisableReadonlyCheckSession() {
futureReadOnlyVariables.add(new HashSet<String>());
}
- /**
- * Finishes the session with temporarily disabled readonly checking.
- */
- public void finishTemporarilyDisableReadonlyCheckSession() {
+ /** Finishes the session with temporarily disabled readonly checking. */
+ void finishTemporarilyDisableReadonlyCheckSession() {
Set<String> variables = futureReadOnlyVariables.pop();
readOnlyVariables.addAll(variables);
if (!futureReadOnlyVariables.isEmpty()) {
@@ -143,15 +130,13 @@
}
}
- /**
- * Finishes a branch of temporarily disabled readonly checking.
- */
- public void finishTemporarilyDisableReadonlyCheckBranch() {
+ /** Finishes a branch of temporarily disabled readonly checking. */
+ 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 {
+ private static void checkLoadAfterStatement(List<Statement> statements) throws EvalException {
Location firstStatement = null;
for (Statement statement : statements) {
@@ -181,7 +166,7 @@
}
/** Throws EvalException if a `if` statement appears at the top level. */
- private void checkToplevelIfStatement(List<Statement> statements) throws EvalException {
+ private static void checkToplevelIfStatement(List<Statement> statements) throws EvalException {
for (Statement statement : statements) {
if (statement instanceof IfStatement) {
throw new EvalException(
@@ -194,10 +179,8 @@
}
}
- /**
- * Validates the AST and runs static checks.
- */
- public void validateAst(List<Statement> statements) throws EvalException {
+ /** Validates the AST and runs static checks. */
+ void validateAst(List<Statement> statements) throws EvalException {
// Check that load() statements are on top.
if (semantics.incompatibleBzlDisallowLoadAfterStatement) {
checkLoadAfterStatement(statements);
@@ -223,9 +206,14 @@
}
}
- public boolean validateAst(List<Statement> statements, EventHandler eventHandler) {
+ public static void validateAst(Environment env, List<Statement> statements) throws EvalException {
+ new ValidationEnvironment(env).validateAst(statements);
+ }
+
+ public static boolean validateAst(
+ Environment env, List<Statement> statements, EventHandler eventHandler) {
try {
- validateAst(statements);
+ validateAst(env, statements);
return true;
} catch (EvalException e) {
if (!e.isDueToIncompleteAST()) {
