diff options
Diffstat (limited to 'src/main/java/com/google/devtools')
26 files changed, 128 insertions, 280 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java index 1e092d1b81..5cdaaa9476 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java @@ -76,8 +76,6 @@ public abstract class AbstractComprehension extends Expression { void eval(Environment env, OutputCollector collector, int step) throws EvalException, InterruptedException; - void validate(ValidationEnvironment env, Location loc) throws EvalException; - /** * The LValue defined in Clause, i.e. the loop variables for ForClause and null for * IfClause. This is needed for SyntaxTreeVisitor. @@ -130,12 +128,6 @@ public abstract class AbstractComprehension extends Expression { } @Override - public void validate(ValidationEnvironment env, Location loc) throws EvalException { - lvalue.validate(env, loc); - iterable.validate(env); - } - - @Override public LValue getLValue() { return lvalue; } @@ -190,11 +182,6 @@ public abstract class AbstractComprehension extends Expression { } @Override - public void validate(ValidationEnvironment env, Location loc) throws EvalException { - condition.validate(env); - } - - @Override public LValue getLValue() { return null; } @@ -307,27 +294,6 @@ public abstract class AbstractComprehension extends Expression { return result; } - @Override - void validate(ValidationEnvironment env) throws EvalException { - // Create a new scope so that loop variables do not leak outside the comprehension. - if (env.getSemantics().incompatibleComprehensionVariablesDoNotLeak) { - env.openScope(); - } - - for (Clause clause : clauses) { - clause.validate(env, getLocation()); - } - - // Clauses have to be validated before expressions in order to introduce the variable names. - for (Expression expr : outputExpressions) { - expr.validate(env); - } - - if (env.getSemantics().incompatibleComprehensionVariablesDoNotLeak) { - env.closeScope(); - } - } - /** * Evaluate the clause indexed by step, or elementExpression. When we evaluate the * comprehension, step is 0 and we evaluate the first clause. Each clause may diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java index 9b74f30b60..436384a43a 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java @@ -66,10 +66,4 @@ public final class AssignmentStatement extends Statement { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - expression.validate(env); - lvalue.validate(env, getLocation()); - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java index 04f54e799b..902b73acdb 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java @@ -67,10 +67,4 @@ public final class AugmentedAssignmentStatement extends Statement { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - lvalue.validate(env, getLocation()); - expression.validate(env); - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java index 6678978ea2..65565ad7fa 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java @@ -259,12 +259,6 @@ public final class BinaryOperatorExpression extends Expression { visitor.visit(this); } - @Override - void validate(ValidationEnvironment env) throws EvalException { - lhs.validate(env); - rhs.validate(env); - } - /** Implements Operator.PLUS. */ private static Object plus( Object lval, Object rval, Environment env, Location location, boolean isAugmented) diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java index 4abc12ade7..c2d1c5de95 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java @@ -65,11 +65,4 @@ public final class ConditionalExpression extends Expression { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - condition.validate(env); - thenCase.validate(env); - elseCase.validate(env); - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java index 8795fe0192..7a5abd2818 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java @@ -102,12 +102,4 @@ public final class DictionaryLiteral extends Expression { public ImmutableList<DictionaryEntryLiteral> getEntries() { return entries; } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - for (DictionaryEntryLiteral entry : entries) { - entry.key.validate(env); - entry.value.validate(env); - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java index e04bdf0538..056fca42ca 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java @@ -124,9 +124,4 @@ public final class DotExpression extends Expression { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - object.validate(env); - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Expression.java b/src/main/java/com/google/devtools/build/lib/syntax/Expression.java index c92eebbd0f..ae1707c678 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Expression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Expression.java @@ -59,16 +59,6 @@ public abstract class Expression extends ASTNode { */ abstract Object doEval(Environment env) throws EvalException, InterruptedException; - /** - * Returns the inferred type of the result of the Expression. - * - * <p>Checks the semantics of the Expression using the {@link Environment} according to - * the rules of the Skylark language, throws {@link EvalException} in case of a semantical error. - * - * @see Statement - */ - abstract void validate(ValidationEnvironment env) throws EvalException; - @Override public final void prettyPrint(Appendable buffer, int indentLevel) throws IOException { prettyPrint(buffer); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java index c739dfaf1e..f0a5fa39ad 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java @@ -47,9 +47,4 @@ public final class ExpressionStatement extends Statement { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - expression.validate(env); - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java index a1639c95eb..187c4b83e0 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java @@ -52,9 +52,6 @@ public final class FlowStatement extends Statement { } @Override - void validate(ValidationEnvironment env) throws EvalException {} - - @Override public void prettyPrint(Appendable buffer, int indentLevel) throws IOException { printIndent(buffer, indentLevel); buffer.append(kind.name); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java index bb640ed295..381995d250 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java @@ -96,15 +96,4 @@ public final class ForStatement extends Statement { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - // TODO(bazel-team): validate variable. Maybe make it temporarily readonly. - collection.validate(env); - variable.validate(env, getLocation()); - - for (Statement stmt : block) { - stmt.validate(env); - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java index 55ae49d49b..e034b16bd3 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java @@ -761,14 +761,6 @@ public final class FuncallExpression extends Expression { } @Override - void validate(ValidationEnvironment env) throws EvalException { - function.validate(env); - for (Argument.Passed arg : arguments) { - arg.getValue().validate(env); - } - } - - @Override protected boolean isNewScope() { return true; } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java index 994f6f2b3d..31b16f0eba 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java @@ -109,37 +109,4 @@ public final class FunctionDefStatement extends Statement { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(final ValidationEnvironment env) throws EvalException { - FunctionSignature sig = signature.getSignature(); - FunctionSignature.Shape shape = sig.getShape(); - ImmutableList<String> names = sig.getNames(); - List<Expression> defaultExpressions = signature.getDefaultValues(); - - int positionals = shape.getPositionals(); - int mandatoryPositionals = shape.getMandatoryPositionals(); - int namedOnly = shape.getNamedOnly(); - int mandatoryNamedOnly = shape.getMandatoryNamedOnly(); - boolean starArg = shape.hasStarArg(); - boolean kwArg = shape.hasKwArg(); - int named = positionals + namedOnly; - int args = named + (starArg ? 1 : 0) + (kwArg ? 1 : 0); - int startOptionals = mandatoryPositionals; - int endOptionals = named - mandatoryNamedOnly; - - env.openScope(); - int j = 0; // index for the defaultExpressions - for (int i = 0; i < args; i++) { - String name = names.get(i); - if (startOptionals <= i && i < endOptionals) { - defaultExpressions.get(j++).validate(env); - } - env.declare(name, getLocation()); - } - for (Statement stmts : statements) { - stmts.validate(env); - } - env.closeScope(); - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java index a404bfa79f..419427c9b1 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java @@ -85,14 +85,7 @@ public final class Identifier extends Expression { visitor.visit(this); } - @Override - void validate(ValidationEnvironment env) throws EvalException { - if (!env.hasSymbolInEnvironment(name)) { - throw createInvalidIdentifierException(env.getAllSymbols()); - } - } - - private EvalException createInvalidIdentifierException(Set<String> symbols) { + EvalException createInvalidIdentifierException(Set<String> symbols) { if (name.equals("$error$")) { return new EvalException(getLocation(), "contains syntax error(s)", true); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java index c96f001c09..089f9c113a 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java @@ -26,8 +26,8 @@ public final class IfStatement extends Statement { /** * Syntax node for an [el]if statement. * - * <p>This extends Statement because it implements {@code doExec} and {@code validate}, but it - * is not actually an independent statement in the grammar. + * <p>This extends Statement because it implements {@code doExec}, but it is not actually an + * independent statement in the grammar. */ public static final class ConditionalStatements extends Statement { @@ -69,14 +69,6 @@ public final class IfStatement extends Statement { public ImmutableList<Statement> getStatements() { return statements; } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - condition.validate(env); - for (Statement stmt : statements) { - stmt.validate(env); - } - } } /** "if" or "elif" clauses. Must be non-empty. */ @@ -141,14 +133,4 @@ public final class IfStatement extends Statement { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - for (ConditionalStatements stmts : thenBlocks) { - stmts.validate(env); - } - for (Statement stmt : elseBlock) { - stmt.validate(env); - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java index b341b33cc1..6597f9b742 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java @@ -83,9 +83,4 @@ public final class IndexExpression extends Expression { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - object.validate(env); - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java index b940f50dc2..68192c82c2 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java @@ -43,8 +43,4 @@ public final class IntegerLiteral extends Expression { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java index bd2160e320..b964a2750f 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java @@ -229,25 +229,6 @@ public final class LValue extends ASTNode { visitor.visit(this); } - void validate(ValidationEnvironment env, Location loc) throws EvalException { - validate(env, loc, expr); - } - - private static void validate(ValidationEnvironment env, Location loc, Expression expr) - throws EvalException { - if (expr instanceof Identifier) { - env.declare(((Identifier) expr).getName(), loc); - } else if (expr instanceof IndexExpression) { - expr.validate(env); - } else if (expr instanceof ListLiteral) { - for (Expression e : ((ListLiteral) expr).getElements()) { - validate(env, loc, e); - } - } else { - throw new EvalException(loc, "cannot assign to '" + expr + "'"); - } - } - @Override public void prettyPrint(Appendable buffer, int indentLevel) throws IOException { expr.prettyPrint(buffer); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java index 9e9b04d136..c1734011ef 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java @@ -112,11 +112,4 @@ public final class ListLiteral extends Expression { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - for (Expression element : elements) { - element.validate(env); - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java index d451333501..21c0888200 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java @@ -107,11 +107,4 @@ public final class LoadStatement extends Statement { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - for (Identifier symbol : symbolMap.keySet()) { - env.declare(symbol.getName(), getLocation()); - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java index ed5ffabc5a..db4b4de107 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java @@ -78,12 +78,4 @@ public final class ReturnStatement extends Statement { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - if (env.isTopLevel()) { - throw new EvalException(getLocation(), "Return statements must be inside a function"); - } - returnExpression.validate(env); - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java index 520dcbd954..71b637630c 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java @@ -118,9 +118,4 @@ public final class SliceExpression extends Expression { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - object.validate(env); - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Statement.java b/src/main/java/com/google/devtools/build/lib/syntax/Statement.java index cd49a45a0a..58130b6e87 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Statement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Statement.java @@ -44,20 +44,4 @@ public abstract class Statement extends ASTNode { * @throws InterruptedException may be thrown in a sub class. */ abstract void doExec(Environment env) throws EvalException, InterruptedException; - - /** - * Checks the semantics of the Statement using the Environment according to - * the rules of the Skylark language. The Environment can be used e.g. to check - * variable type collision, read only variables, detecting recursion, existence of - * built-in variables, functions, etc. - * - * <p>The semantical check should be performed after the Skylark extension is loaded - * (i.e. is syntactically correct) and before is executed. The point of the semantical check - * is to make sure (as much as possible) that no error can occur during execution (Skylark - * programmers get a "compile time" error). It should also check execution branches (e.g. in - * if statements) that otherwise might never get executed. - * - * @throws EvalException if the Statement has a semantical error. - */ - abstract void validate(ValidationEnvironment env) throws EvalException; } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java index 19d00b9b8d..61856e2c64 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java @@ -43,8 +43,4 @@ public final class StringLiteral extends Expression { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java index a76abb18d8..e55acdd305 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java @@ -99,9 +99,4 @@ public final class UnaryOperatorExpression extends Expression { public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - operand.validate(env); - } } 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 68268e0774..8634bcdb08 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 @@ -24,14 +24,11 @@ import java.util.Set; import javax.annotation.Nullable; /** - * An Environment for the semantic checking of Skylark files. - * - * @see Statement#validate - * @see Expression#validate + * A class for doing static checks on files, before evaluating them. */ -public final class ValidationEnvironment { +public final class ValidationEnvironment extends SyntaxTreeVisitor { - private class Scope { + private static class Scope { private final Set<String> variables = new HashSet<>(); private final Set<String> readOnlyVariables = new HashSet<>(); @Nullable private final Scope parent; @@ -41,6 +38,26 @@ public final class ValidationEnvironment { } } + /** + * We use an unchecked exception around EvalException because the SyntaxTreeVisitor doesn't let + * visit methods throw checked exceptions. We might change that later. + */ + private static class ValidationException extends RuntimeException { + EvalException exception; + + ValidationException(EvalException e) { + exception = e; + } + + ValidationException(Location location, String message, String url) { + exception = new EvalException(location, message, url); + } + + ValidationException(Location location, String message) { + exception = new EvalException(location, message); + } + } + private final SkylarkSemanticsOptions semantics; private Scope scope; @@ -54,17 +71,91 @@ public final class ValidationEnvironment { semantics = env.getSemantics(); } - /** Returns true if this ValidationEnvironment is top level i.e. has no parent. */ - boolean isTopLevel() { - return scope.parent == null; + @Override + public void visit(LoadStatement node) { + for (Identifier symbol : node.getSymbols()) { + declare(symbol.getName(), node.getLocation()); + } + } + + @Override + public void visit(Identifier node) { + if (!hasSymbolInEnvironment(node.getName())) { + throw new ValidationException(node.createInvalidIdentifierException(getAllSymbols())); + } + } + + private void validateLValue(Location loc, Expression expr) { + if (expr instanceof Identifier) { + declare(((Identifier) expr).getName(), loc); + } else if (expr instanceof IndexExpression) { + visit(expr); + } else if (expr instanceof ListLiteral) { + for (Expression e : ((ListLiteral) expr).getElements()) { + validateLValue(loc, e); + } + } else { + throw new ValidationException(loc, "cannot assign to '" + expr + "'"); + } } - SkylarkSemanticsOptions getSemantics() { - return semantics; + @Override + public void visit(LValue node) { + validateLValue(node.getLocation(), node.getExpression()); + } + + @Override + public void visit(ReturnStatement node) { + if (isTopLevel()) { + throw new ValidationException( + node.getLocation(), "Return statements must be inside a function"); + } + super.visit(node); + } + + @Override + public void visit(DotExpression node) { + visit(node.getObject()); + // Do not visit the field. + } + + @Override + public void visit(AbstractComprehension node) { + if (semantics.incompatibleComprehensionVariablesDoNotLeak) { + openScope(); + super.visit(node); + closeScope(); + } else { + super.visit(node); + } + } + + @Override + public void visit(FunctionDefStatement node) { + for (Parameter<Expression, Expression> param : node.getParameters()) { + if (param.isOptional()) { + visit(param.getDefaultValue()); + } + } + openScope(); + for (Parameter<Expression, Expression> param : node.getParameters()) { + if (param.hasName()) { + declare(param.getName(), param.getLocation()); + } + } + for (Statement stmt : node.getStatements()) { + visit(stmt); + } + closeScope(); + } + + /** Returns true if this ValidationEnvironment is top level i.e. has no parent. */ + private boolean isTopLevel() { + return scope.parent == null; } /** Declare a variable and add it to the environment. */ - void declare(String varname, Location location) throws EvalException { + private void declare(String varname, Location location) { checkReadonly(varname, location); if (scope.parent == null) { // top-level values are immutable scope.readOnlyVariables.add(varname); @@ -72,9 +163,9 @@ public final class ValidationEnvironment { scope.variables.add(varname); } - private void checkReadonly(String varname, Location location) throws EvalException { + private void checkReadonly(String varname, Location location) { if (scope.readOnlyVariables.contains(varname)) { - throw new EvalException( + throw new ValidationException( location, String.format("Variable %s is read only", varname), "https://bazel.build/versions/master/docs/skylark/errors/read-only-variable.html"); @@ -82,7 +173,7 @@ public final class ValidationEnvironment { } /** Returns true if the symbol exists in the validation environment (or a parent). */ - boolean hasSymbolInEnvironment(String varname) { + private boolean hasSymbolInEnvironment(String varname) { for (Scope s = scope; s != null; s = s.parent) { if (s.variables.contains(varname)) { return true; @@ -92,7 +183,7 @@ public final class ValidationEnvironment { } /** Returns the set of all accessible symbols (both local and global) */ - Set<String> getAllSymbols() { + private Set<String> getAllSymbols() { Set<String> all = new HashSet<>(); for (Scope s = scope; s != null; s = s.parent) { all.addAll(s.variables); @@ -100,8 +191,8 @@ public final class ValidationEnvironment { return all; } - /** Throws EvalException if a load() appears after another kind of statement. */ - private static void checkLoadAfterStatement(List<Statement> statements) throws EvalException { + /** Throws ValidationException if a load() appears after another kind of statement. */ + private static void checkLoadAfterStatement(List<Statement> statements) { Location firstStatement = null; for (Statement statement : statements) { @@ -115,7 +206,7 @@ public final class ValidationEnvironment { if (firstStatement == null) { continue; } - throw new EvalException( + throw new ValidationException( statement.getLocation(), "load() statements must be called before any other statement. " + "First non-load() statement appears at " @@ -130,11 +221,11 @@ public final class ValidationEnvironment { } } - /** Throws EvalException if a `if` statement appears at the top level. */ - private static void checkToplevelIfStatement(List<Statement> statements) throws EvalException { + /** Throws ValidationException if a `if` statement appears at the top level. */ + private static void checkToplevelIfStatement(List<Statement> statements) { for (Statement statement : statements) { if (statement instanceof IfStatement) { - throw new EvalException( + throw new ValidationException( 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). " @@ -145,7 +236,7 @@ public final class ValidationEnvironment { } /** Validates the AST and runs static checks. */ - void validateAst(List<Statement> statements) throws EvalException { + private void validateAst(List<Statement> statements) { // Check that load() statements are on top. if (semantics.incompatibleBzlDisallowLoadAfterStatement) { checkLoadAfterStatement(statements); @@ -167,15 +258,19 @@ public final class ValidationEnvironment { } for (Statement statement : statements) { - statement.validate(this); + this.visit(statement); } } public static void validateAst(Environment env, List<Statement> statements) throws EvalException { - ValidationEnvironment venv = new ValidationEnvironment(env); - venv.validateAst(statements); - // Check that no closeScope was forgotten. - Preconditions.checkState(venv.scope.parent == null); + try { + ValidationEnvironment venv = new ValidationEnvironment(env); + venv.validateAst(statements); + // Check that no closeScope was forgotten. + Preconditions.checkState(venv.scope.parent == null); + } catch (ValidationException e) { + throw e.exception; + } } public static boolean validateAst( @@ -192,12 +287,12 @@ public final class ValidationEnvironment { } /** Open a new scope that will contain the future declarations. */ - public void openScope() { + private void openScope() { this.scope = new Scope(this.scope); } /** Close a scope (and lose all declarations it contained). */ - public void closeScope() { + private void closeScope() { this.scope = Preconditions.checkNotNull(this.scope.parent); } } |