From 38e91d0423ba7c312438ad71986f8cd6a742be22 Mon Sep 17 00:00:00 2001 From: Luca Di Grazia Date: Sun, 4 Sep 2022 18:10:25 +0200 Subject: [PATCH] Delete the flag incompatible_bzl_disallow_load_after_statement https://github.com/bazelbuild/bazel/issues/5815 RELNOTES: The flag `incompatible_bzl_disallow_load_after_statement` is removed. PiperOrigin-RevId: 302473246 --- .../packages/StarlarkSemanticsOptions.java | 15 + .../build/lib/syntax/StarlarkSemantics.java | 9 + .../lib/syntax/ValidationEnvironment.java | 590 +++++++++++++----- .../SkylarkSemanticsConsistencyTest.java | 2 + .../build/lib/syntax/ValidationTest.java | 450 +++++++------ 5 files changed, 673 insertions(+), 393 deletions(-) diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java index dfb51397e79..a5403796227 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java @@ -482,6 +482,20 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl + "2019-10-24-file-visibility.md") public boolean incompatibleNoImplicitFileExport; + @Option( + name = "incompatible_no_output_attr_default", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = + "If set to true, disables the `default` parameter of the `attr.output` and " + + "`attr.output_list` attribute definition functions.") + public boolean incompatibleNoOutputAttrDefault; + @Option( name = "incompatible_no_rule_outputs_param", defaultValue = "false", @@ -690,6 +704,7 @@ public StarlarkSemantics toSkylarkSemantics() { .incompatibleNewActionsApi(incompatibleNewActionsApi) .incompatibleNoAttrLicense(incompatibleNoAttrLicense) .incompatibleNoImplicitFileExport(incompatibleNoImplicitFileExport) + .incompatibleNoOutputAttrDefault(incompatibleNoOutputAttrDefault) .incompatibleNoRuleOutputsParam(incompatibleNoRuleOutputsParam) .incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs) .incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup) diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java index 41ef0d794f6..28c7dd71c33 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java @@ -74,6 +74,8 @@ private FlagIdentifier() {} // uninstantiable "incompatible_applicable_licenses"; public static final String INCOMPATIBLE_DISABLE_DEPSET_INPUTS = "incompatible_disable_depset_inputs"; + public static final String INCOMPATIBLE_NO_OUTPUT_ATTR_DEFAULT = + "incompatible_no_output_attr_default"; public static final String INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM = "incompatible_no_rule_outputs_param"; public static final String INCOMPATIBLE_NO_TARGET_OUTPUT_GROUP = @@ -125,6 +127,8 @@ public boolean flagValue(String flag) { return incompatibleApplicableLicenses(); case FlagIdentifier.INCOMPATIBLE_DISABLE_DEPSET_INPUTS: return incompatibleDisableDepsetItems(); + case FlagIdentifier.INCOMPATIBLE_NO_OUTPUT_ATTR_DEFAULT: + return incompatibleNoOutputAttrDefault(); case FlagIdentifier.INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM: return incompatibleNoRuleOutputsParam(); case FlagIdentifier.INCOMPATIBLE_NO_TARGET_OUTPUT_GROUP: @@ -236,6 +240,8 @@ boolean isFeatureEnabledBasedOnTogglingFlags(String enablingFlag, String disabli public abstract boolean incompatibleNoImplicitFileExport(); + public abstract boolean incompatibleNoOutputAttrDefault(); + public abstract boolean incompatibleNoRuleOutputsParam(); public abstract boolean incompatibleNoSupportToolsInActionInputs(); @@ -330,6 +336,7 @@ public static Builder builderWithDefaults() { .incompatibleNewActionsApi(true) .incompatibleNoAttrLicense(true) .incompatibleNoImplicitFileExport(false) + .incompatibleNoOutputAttrDefault(true) .incompatibleNoRuleOutputsParam(false) .incompatibleNoSupportToolsInActionInputs(true) .incompatibleNoTargetOutputGroup(true) @@ -410,6 +417,8 @@ public abstract static class Builder { public abstract Builder incompatibleNoImplicitFileExport(boolean value); + public abstract Builder incompatibleNoOutputAttrDefault(boolean value); + public abstract Builder incompatibleNoRuleOutputsParam(boolean value); public abstract Builder incompatibleNoSupportToolsInActionInputs(boolean value); diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java index 6afb96a0fb2..15afd2acf5e 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java @@ -1,4 +1,4 @@ -// Copyright 2014 Google Inc. All rights reserved. +// Copyright 2014 The Bazel Authors. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -15,230 +15,506 @@ package com.google.devtools.build.lib.syntax; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.collect.CollectionUtils; +import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Location; -import com.google.devtools.build.lib.syntax.SkylarkType.SkylarkFunctionType; - +import com.google.devtools.build.lib.util.SpellChecker; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; -import java.util.Stack; +import javax.annotation.Nullable; /** - * An Environment for the semantic checking of Skylark files. + * A class for doing static checks on files, before evaluating them. + * + *

We implement the semantics discussed in + * https://github.com/bazelbuild/proposals/blob/master/docs/2018-06-18-name-resolution.md * - * @see Statement#validate - * @see Expression#validate + *

When a variable is defined, it is visible in the entire block. For example, a global variable + * is visible in the entire file; a variable in a function is visible in the entire function block + * (even on the lines before its first assignment). + * + *

Validation is a mutation of the syntax tree, as it attaches scope information to Identifier + * nodes. (In the future, it will attach additional information to functions to support lexical + * scope, and even compilation of the trees to bytecode.) Validation errors are reported in the + * analogous manner to scan/parse errors: for a StarlarkFile, they are appended to {@code + * StarlarkFile.errors}; for an expression they are reported by an SyntaxError exception. It is + * legal to validate a file that already contains scan/parse errors, though it may lead to secondary + * validation errors. */ -public class ValidationEnvironment { +// TODO(adonovan): make this class private. Call it through the EvalUtils facade. +public final class ValidationEnvironment extends NodeVisitor { - private final ValidationEnvironment parent; + enum Scope { + /** Symbols defined inside a function or a comprehension. */ + Local("local"), + /** Symbols defined at a module top-level, e.g. functions, loaded symbols. */ + Module("global"), + /** Predefined symbols (builtins) */ + Universe("builtin"); - private Map> variableTypes = new HashMap<>(); + private final String qualifier; - private Map variableLocations = new HashMap<>(); + private Scope(String qualifier) { + this.qualifier = qualifier; + } - private Set readOnlyVariables = new HashSet<>(); + String getQualifier() { + return qualifier; + } + } - // A stack of variable-sets which are read only but can be assigned in different - // branches of if-else statements. - private Stack> futureReadOnlyVariables = new Stack<>(); + /** + * Module is a static abstraction of a Starlark module. It describes the set of variable names for + * use during name resolution. + */ + public interface Module { - // The function we are currently validating. - private SkylarkFunctionType currentFunction; + // TODO(adonovan): opt: for efficiency, turn this into a predicate, not an enumerable set, + // and look up bindings as they are needed, not preemptively. + // Otherwise we must do work proportional to the number of bindings in the + // environment, not the number of free variables of the file/expression. + // + // A single method will then suffice: + // Scope resolve(String name) throws Undeclared + // This requires that the Module retain its semantics. - // Whether this validation environment is not modified therefore clonable or not. - private boolean clonable; + /** Returns the set of names defined by this module. The caller must not modify the set. */ + Set getNames(); - public ValidationEnvironment( - ImmutableMap> builtinVariableTypes) { - parent = null; - variableTypes = CollectionUtils.copyOf(builtinVariableTypes); - readOnlyVariables.addAll(builtinVariableTypes.get(SkylarkType.GLOBAL).keySet()); - clonable = true; + /** + * Returns (optionally) a more specific error for an undeclared name than the generic message. + * This hook allows the module to implement "semantics-restricted" names without any knowledge + * in this file. + */ + @Nullable + String getUndeclaredNameError(StarlarkSemantics semantics, String name); } - private ValidationEnvironment(Map> builtinVariableTypes, - Set readOnlyVariables) { - parent = null; - this.variableTypes = CollectionUtils.copyOf(builtinVariableTypes); - this.readOnlyVariables = new HashSet<>(readOnlyVariables); - clonable = false; + private static final Identifier PREDECLARED = new Identifier(""); + + private static class Block { + private final Map variables = new HashMap<>(); + private final Scope scope; + @Nullable private final Block parent; + + Block(Scope scope, @Nullable Block parent) { + this.scope = scope; + this.parent = parent; + } } - @Override - public ValidationEnvironment clone() { - Preconditions.checkState(clonable); - return new ValidationEnvironment(variableTypes, readOnlyVariables); + private final List errors; + private final StarlarkSemantics semantics; + private final Module module; + private Block block; + private int loopCount; + + // In BUILD files, we have a slightly different behavior for legacy reasons. + // TODO(adonovan): eliminate isBuildFile. It is necessary because the prelude is implemented + // by inserting shared Statements, which must not be mutated, into each StarlarkFile. + // Instead, we should implement the prelude by executing it like a .bzl module + // and putting its members in the initial environment of the StarlarkFile. + // In the meantime, let's move this flag into Module (GlobalFrame). + private final boolean isBuildFile; + + private ValidationEnvironment( + List errors, Module module, StarlarkSemantics semantics, boolean isBuildFile) { + this.errors = errors; + this.module = module; + this.semantics = semantics; + this.isBuildFile = isBuildFile; + block = new Block(Scope.Universe, null); + for (String name : module.getNames()) { + block.variables.put(name, PREDECLARED); + } + } + + void addError(Location loc, String message) { + errors.add(Event.error(loc, message)); } /** - * Creates a local ValidationEnvironment to validate user defined function bodies. + * First pass: add all definitions to the current block. This is done because symbols are + * sometimes used before their definition point (e.g. a functions are not necessarily declared in + * order). */ - public ValidationEnvironment(ValidationEnvironment parent, SkylarkFunctionType currentFunction) { - this.parent = parent; - this.variableTypes.put(SkylarkType.GLOBAL, new HashMap()); - this.currentFunction = currentFunction; - for (String var : parent.readOnlyVariables) { - if (!parent.variableLocations.containsKey(var)) { - // Mark built in global vars readonly. Variables defined in Skylark may be shadowed locally. - readOnlyVariables.add(var); - } + private void collectDefinitions(Iterable stmts) { + for (Statement stmt : stmts) { + collectDefinitions(stmt); } - this.clonable = false; } - /** - * Returns true if this ValidationEnvironment is top level i.e. has no parent. - */ - public boolean isTopLevel() { - return parent == null; + private void collectDefinitions(Statement stmt) { + switch (stmt.kind()) { + case ASSIGNMENT: + collectDefinitions(((AssignmentStatement) stmt).getLHS()); + break; + case IF: + IfStatement ifStmt = (IfStatement) stmt; + collectDefinitions(ifStmt.getThenBlock()); + if (ifStmt.getElseBlock() != null) { + collectDefinitions(ifStmt.getElseBlock()); + } + break; + case FOR: + ForStatement forStmt = (ForStatement) stmt; + collectDefinitions(forStmt.getLHS()); + collectDefinitions(forStmt.getBlock()); + break; + case DEF: + DefStatement def = (DefStatement) stmt; + declare(def.getIdentifier()); + break; + case LOAD: + LoadStatement load = (LoadStatement) stmt; + + // The global reassignment check is not yet enabled for BUILD files, + // but we apply it to load statements as a special case. + // Because (for now) its error message is better than the general + // message emitted by 'declare', we'll apply it to non-BUILD files too. + Set names = new HashSet<>(); + for (LoadStatement.Binding b : load.getBindings()) { + if (!names.add(b.getLocalName().getName())) { + addError( + b.getLocalName().getStartLocation(), + String.format( + "load statement defines '%s' more than once", b.getLocalName().getName())); + } + } + + for (LoadStatement.Binding b : load.getBindings()) { + declare(b.getLocalName()); + } + break; + case EXPRESSION: + case FLOW: + case RETURN: + // nothing to declare + } } - /** - * Updates the variable type if the new type is "stronger" then the old one. - * The old and the new vartype has to be compatible, otherwise an EvalException is thrown. - * The new type is stronger if the old one doesn't exist or unknown. - */ - public void update(String varname, SkylarkType newVartype, Location location) - throws EvalException { - checkReadonly(varname, location); - if (parent == null) { // top-level values are immutable - readOnlyVariables.add(varname); - if (!futureReadOnlyVariables.isEmpty()) { - // Currently validating an if-else statement - futureReadOnlyVariables.peek().add(varname); + private void collectDefinitions(Expression lhs) { + for (Identifier id : Identifier.boundIdentifiers(lhs)) { + declare(id); + } + } + + private void assign(Expression lhs) { + if (lhs instanceof Identifier) { + if (!isBuildFile) { + ((Identifier) lhs).setScope(block.scope); } + // no-op + } else if (lhs instanceof IndexExpression) { + visit(lhs); + } else if (lhs instanceof ListExpression) { + for (Expression elem : ((ListExpression) lhs).getElements()) { + assign(elem); + } + } else { + addError(lhs.getStartLocation(), "cannot assign to '" + lhs + "'"); } - SkylarkType oldVartype = variableTypes.get(SkylarkType.GLOBAL).get(varname); - if (oldVartype != null) { - newVartype = oldVartype.infer(newVartype, "variable '" + varname + "'", - location, variableLocations.get(varname)); + } + + @Override + public void visit(Identifier node) { + String name = node.getName(); + @Nullable Block b = blockThatDefines(name); + if (b == null) { + // The identifier might not exist because it was restricted (hidden) by the current semantics. + // If this is the case, output a more helpful error message than 'not found'. + String error = module.getUndeclaredNameError(semantics, name); + if (error == null) { + // generic error + error = createInvalidIdentifierException(node.getName(), getAllSymbols()); + } + addError(node.getStartLocation(), error); + return; + } + // TODO(laurentlb): In BUILD files, calling setScope will throw an exception. This happens + // because some AST nodes are shared across multipe ASTs (due to the prelude file). + if (!isBuildFile) { + node.setScope(b.scope); } - variableTypes.get(SkylarkType.GLOBAL).put(varname, newVartype); - variableLocations.put(varname, location); - clonable = false; } - private void checkReadonly(String varname, Location location) throws EvalException { - if (readOnlyVariables.contains(varname)) { - throw new EvalException(location, String.format("Variable %s is read only", varname)); + private static String createInvalidIdentifierException(String name, Set candidates) { + if (name.equals("$error$")) { + return "contains syntax error(s)"; + } + + String error = getErrorForObsoleteThreadLocalVars(name); + if (error != null) { + return error; } + + String suggestion = SpellChecker.didYouMean(name, candidates); + return "name '" + name + "' is not defined" + suggestion; } - public void checkIterable(SkylarkType type, Location loc) throws EvalException { - if (type == SkylarkType.UNKNOWN) { - // Until all the language is properly typed, we ignore Object types. - return; + static String getErrorForObsoleteThreadLocalVars(String name) { + if (name.equals("PACKAGE_NAME")) { + return "The value 'PACKAGE_NAME' has been removed in favor of 'package_name()', " + + "please use the latter (" + + "https://docs.bazel.build/versions/master/skylark/lib/native.html#package_name). "; } - if (!Iterable.class.isAssignableFrom(type.getType()) - && !Map.class.isAssignableFrom(type.getType()) - && !String.class.equals(type.getType())) { - throw new EvalException(loc, - "type '" + EvalUtils.getDataTypeNameFromClass(type.getType()) + "' is not iterable"); + if (name.equals("REPOSITORY_NAME")) { + return "The value 'REPOSITORY_NAME' has been removed in favor of 'repository_name()', please" + + " use the latter (" + + "https://docs.bazel.build/versions/master/skylark/lib/native.html#repository_name)."; } + return null; } - /** - * Returns true if the symbol exists in the validation environment. - */ - public boolean hasSymbolInEnvironment(String varname) { - return variableTypes.get(SkylarkType.GLOBAL).containsKey(varname) - || topLevel().variableTypes.get(SkylarkType.GLOBAL).containsKey(varname); + @Override + public void visit(ReturnStatement node) { + if (block.scope != Scope.Local) { + addError(node.getStartLocation(), "return statements must be inside a function"); + } + super.visit(node); } - /** - * Returns the type of the existing variable. - */ - public SkylarkType getVartype(String varname) { - SkylarkType type = variableTypes.get(SkylarkType.GLOBAL).get(varname); - if (type == null && parent != null) { - type = parent.getVartype(varname); + @Override + public void visit(ForStatement node) { + if (block.scope != Scope.Local) { + addError( + node.getStartLocation(), + "for loops are not allowed at the top level. You may move it inside a function " + + "or use a comprehension, [f(x) for x in sequence]"); } - return Preconditions.checkNotNull(type, - String.format("Variable %s is not found in the validation environment", varname)); + loopCount++; + visit(node.getCollection()); + assign(node.getLHS()); + visitBlock(node.getBlock()); + Preconditions.checkState(loopCount > 0); + loopCount--; } - public SkylarkFunctionType getCurrentFunction() { - return currentFunction; + @Override + public void visit(LoadStatement node) { + if (block.scope == Scope.Local) { + addError(node.getStartLocation(), "load statement not at top level"); + } + super.visit(node); } - /** - * Returns the return type of the function. - */ - public SkylarkType getReturnType(String funcName, Location loc) throws EvalException { - return getReturnType(SkylarkType.GLOBAL, funcName, loc); + @Override + public void visit(FlowStatement node) { + if (node.getKind() != TokenKind.PASS && loopCount <= 0) { + addError(node.getStartLocation(), node.getKind() + " statement must be inside a for loop"); + } + super.visit(node); } - /** - * Returns the return type of the object function. - */ - public SkylarkType getReturnType(SkylarkType objectType, String funcName, Location loc) - throws EvalException { - // All functions are registered in the top level ValidationEnvironment. - Map functions = topLevel().variableTypes.get(objectType); - // TODO(bazel-team): eventually not finding the return type should be a validation error, - // because it means the function doesn't exist. First we have to make sure that we register - // every possible function before. - if (functions != null) { - SkylarkType functionType = functions.get(funcName); - if (functionType != null && functionType != SkylarkType.UNKNOWN) { - if (!(functionType instanceof SkylarkFunctionType)) { - throw new EvalException(loc, (objectType == SkylarkType.GLOBAL ? "" : objectType + ".") - + funcName + " is not a function"); - } - return ((SkylarkFunctionType) functionType).getReturnType(); + @Override + public void visit(DotExpression node) { + visit(node.getObject()); + // Do not visit the field. + } + + @Override + public void visit(Comprehension node) { + openBlock(Scope.Local); + for (Comprehension.Clause clause : node.getClauses()) { + if (clause instanceof Comprehension.For) { + Comprehension.For forClause = (Comprehension.For) clause; + collectDefinitions(forClause.getVars()); } } - return SkylarkType.UNKNOWN; + // TODO(adonovan): opt: combine loops + for (Comprehension.Clause clause : node.getClauses()) { + if (clause instanceof Comprehension.For) { + Comprehension.For forClause = (Comprehension.For) clause; + visit(forClause.getIterable()); + assign(forClause.getVars()); + } else { + Comprehension.If ifClause = (Comprehension.If) clause; + visit(ifClause.getCondition()); + } + } + visit(node.getBody()); + closeBlock(); } - private ValidationEnvironment topLevel() { - return Preconditions.checkNotNull(parent == null ? this : parent); + @Override + public void visit(DefStatement node) { + if (block.scope == Scope.Local) { + addError( + node.getStartLocation(), + "nested functions are not allowed. Move the function to the top level."); + } + for (Parameter param : node.getParameters()) { + if (param instanceof Parameter.Optional) { + visit(param.getDefaultValue()); + } + } + openBlock(Scope.Local); + for (Parameter param : node.getParameters()) { + if (param.getIdentifier() != null) { + declare(param.getIdentifier()); + } + } + collectDefinitions(node.getStatements()); + visitAll(node.getStatements()); + closeBlock(); } - /** - * Adds a user defined function to the validation environment is not exists. - */ - public void updateFunction(String name, SkylarkFunctionType type, Location loc) - throws EvalException { - checkReadonly(name, loc); - if (variableTypes.get(SkylarkType.GLOBAL).containsKey(name)) { - throw new EvalException(loc, "function " + name + " already exists"); + @Override + public void visit(IfStatement node) { + if (block.scope != Scope.Local) { + addError( + node.getStartLocation(), + "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)."); } - variableTypes.get(SkylarkType.GLOBAL).put(name, type); - clonable = false; + super.visit(node); } - /** - * Starts a session with temporarily disabled readonly checking for variables between branches. - * 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() { - futureReadOnlyVariables.add(new HashSet()); - clonable = false; + @Override + public void visit(AssignmentStatement node) { + visit(node.getRHS()); + + // Disallow: [e, ...] += rhs + // Other bad cases are handled in assign. + if (node.isAugmented() && node.getLHS() instanceof ListExpression) { + addError( + node.getStartLocation(), + "cannot perform augmented assignment on a list or tuple expression"); + } + + assign(node.getLHS()); + } + + /** Declare a variable and add it to the environment. */ + private void declare(Identifier id) { + Identifier prev = block.variables.putIfAbsent(id.getName(), id); + + // Symbols defined in the module scope cannot be reassigned. + // TODO(laurentlb): Forbid reassignment in BUILD files too. + if (prev != null && block.scope == Scope.Module && !isBuildFile) { + addError( + id.getStartLocation(), + String.format( + "cannot reassign global '%s' (read more at" + + " https://bazel.build/versions/master/docs/skylark/errors/read-only-variable.html)", + id.getName())); + if (prev != PREDECLARED) { + addError( + prev.getStartLocation(), String.format("'%s' previously declared here", id.getName())); + } + } + } + + /** Returns the nearest Block that defines a symbol. */ + private Block blockThatDefines(String varname) { + for (Block b = block; b != null; b = b.parent) { + if (b.variables.containsKey(varname)) { + return b; + } + } + return null; + } + + /** Returns the set of all accessible symbols (both local and global) */ + private Set getAllSymbols() { + Set all = new HashSet<>(); + for (Block b = block; b != null; b = b.parent) { + all.addAll(b.variables.keySet()); + } + return all; + } + + // Report an error if a load statement appears after another kind of statement. + private void checkLoadAfterStatement(List statements) { + 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; + } + addError( + statement.getStartLocation(), + "load() statements must be called before any other statement. " + + "First non-load() statement appears at " + + firstStatement); + } + + if (firstStatement == null) { + firstStatement = statement.getStartLocation(); + } + } + } + + private void validateToplevelStatements(List statements) { + // Check that load() statements are on top. + if (!isBuildFile) { + checkLoadAfterStatement(statements); + } + + openBlock(Scope.Module); + + // Add each variable defined by statements, not including definitions that appear in + // sub-scopes of the given statements (function bodies and comprehensions). + collectDefinitions(statements); + + // Second pass: ensure that all symbols have been defined. + visitAll(statements); + closeBlock(); } /** - * Finishes the session with temporarily disabled readonly checking. + * Performs static checks, including resolution of identifiers in {@code file} in the environment + * defined by {@code module}. The StarlarkFile is mutated. Errors are appended to {@link + * StarlarkFile#errors}. {@code isBuildFile} enables Bazel's legacy mode for BUILD files in which + * reassignment at top-level is permitted. */ - public void finishTemporarilyDisableReadonlyCheckSession() { - Set variables = futureReadOnlyVariables.pop(); - readOnlyVariables.addAll(variables); - if (!futureReadOnlyVariables.isEmpty()) { - futureReadOnlyVariables.peek().addAll(variables); + public static void validateFile( + StarlarkFile file, Module module, StarlarkSemantics semantics, boolean isBuildFile) { + ValidationEnvironment venv = + new ValidationEnvironment(file.errors, module, semantics, isBuildFile); + if (semantics.incompatibleRestrictStringEscapes()) { + file.addStringEscapeEvents(); } - clonable = false; + venv.validateToplevelStatements(file.getStatements()); + // Check that no closeBlock was forgotten. + Preconditions.checkState(venv.block.parent == null); } /** - * Finishes a branch of temporarily disabled readonly checking. + * Performs static checks, including resolution of identifiers in {@code expr} in the environment + * defined by {@code module}. This operation mutates the Expression. */ - public void finishTemporarilyDisableReadonlyCheckBranch() { - readOnlyVariables.removeAll(futureReadOnlyVariables.peek()); - clonable = false; + public static void validateExpr(Expression expr, Module module, StarlarkSemantics semantics) + throws SyntaxError { + List errors = new ArrayList<>(); + ValidationEnvironment venv = + new ValidationEnvironment(errors, module, semantics, /*isBuildFile=*/ false); + + venv.visit(expr); + + if (!errors.isEmpty()) { + throw new SyntaxError(errors); + } + } + + /** Open a new lexical block that will contain the future declarations. */ + private void openBlock(Scope scope) { + block = new Block(scope, block); } + + /** Close a lexical block (and lose all declarations it contained). */ + private void closeBlock() { + block = Preconditions.checkNotNull(block.parent); + } + } diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java index 6a183e80328..e80bf34f90d 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java @@ -154,6 +154,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E "--incompatible_new_actions_api=" + rand.nextBoolean(), "--incompatible_no_attr_license=" + rand.nextBoolean(), "--incompatible_no_implicit_file_export=" + rand.nextBoolean(), + "--incompatible_no_output_attr_default=" + rand.nextBoolean(), "--incompatible_no_rule_outputs_param=" + rand.nextBoolean(), "--incompatible_no_support_tools_in_action_inputs=" + rand.nextBoolean(), "--incompatible_no_target_output_group=" + rand.nextBoolean(), @@ -208,6 +209,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .incompatibleNewActionsApi(rand.nextBoolean()) .incompatibleNoAttrLicense(rand.nextBoolean()) .incompatibleNoImplicitFileExport(rand.nextBoolean()) + .incompatibleNoOutputAttrDefault(rand.nextBoolean()) .incompatibleNoRuleOutputsParam(rand.nextBoolean()) .incompatibleNoSupportToolsInActionInputs(rand.nextBoolean()) .incompatibleNoTargetOutputGroup(rand.nextBoolean()) diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java index 5f9a71208cb..efadf48482c 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java @@ -14,246 +14,283 @@ package com.google.devtools.build.lib.syntax; import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.testutil.MoreAsserts.assertContainsEvent; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.RuleConfiguredTarget; import com.google.devtools.build.lib.events.Event; -import com.google.devtools.build.lib.syntax.SkylarkList.MutableList; -import com.google.devtools.build.lib.syntax.SkylarkList.Tuple; -import com.google.devtools.build.lib.syntax.util.EvaluationTestCase; - +import com.google.devtools.build.lib.events.EventCollector; +import com.google.devtools.build.lib.packages.StarlarkSemanticsOptions; // TODO(adonovan): break! +import com.google.devtools.common.options.Options; +import com.google.devtools.common.options.OptionsParsingException; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** - * Tests for the validation process of Skylark files. - */ +/** Tests of the Starlark validator. */ @RunWith(JUnit4.class) -public class ValidationTest extends EvaluationTestCase { +public class ValidationTest { - @Test - public void testAssignmentNotValidLValue() { - checkError("can only assign to variables and tuples, not to ''a''", "'a' = 1"); + private StarlarkSemantics semantics = StarlarkSemantics.DEFAULT_SEMANTICS; + + private void setSemantics(String... options) throws OptionsParsingException { + this.semantics = + Options.parse(StarlarkSemanticsOptions.class, options).getOptions().toSkylarkSemantics(); } - @Test - public void testTopLevelForStatement() throws Exception { - checkError("'For' is not allowed as a top level statement", "for i in [1,2,3]: a = i\n"); + // Validates a file using the current semantics. + private StarlarkFile validateFile(String... lines) throws SyntaxError { + ParserInput input = ParserInput.fromLines(lines); + Module module = Module.createForBuiltins(Starlark.UNIVERSE); + return EvalUtils.parseAndValidate(input, module, semantics); } - @Test - public void testReturnOutsideFunction() throws Exception { - checkError("Return statements must be inside a function", "return 2\n"); + // Assertions that parsing and validation succeeds. + private void assertValid(String... lines) throws SyntaxError { + StarlarkFile file = validateFile(lines); + if (!file.ok()) { + throw new SyntaxError(file.errors()); + } } - @Test - public void testTwoFunctionsWithTheSameName() throws Exception { - checkError( - "Variable foo is read only", "def foo():", " return 1", "def foo(x, y):", " return 1"); + // Asserts that parsing of the program succeeds but validation fails + // with at least the specified error. + private void assertInvalid(String expectedError, String... lines) throws SyntaxError { + EventCollector errors = getValidationErrors(lines); + assertContainsEvent(errors, expectedError); } - @Test - public void testFunctionLocalVariable() throws Exception { - checkError( - "name 'a' is not defined", - "def func2(b):", - " c = b", - " c = a", - "def func1():", - " a = 1", - " func2(2)"); + // Returns the non-empty list of validation errors of the program. + private EventCollector getValidationErrors(String... lines) throws SyntaxError { + StarlarkFile file = validateFile(lines); + if (file.ok()) { + throw new AssertionError("validation succeeded unexpectedly"); + } + EventCollector errors = new EventCollector(); + Event.replayEventsOn(errors, file.errors()); + return errors; } @Test - public void testFunctionLocalVariableDoesNotEffectGlobalValidationEnv() throws Exception { - checkError("name 'a' is not defined", "def func1():", " a = 1", "def func2(b):", " b = a"); + public void testAssignmentNotValidLValue() throws Exception { + assertInvalid("cannot assign to '\"a\"'", "'a' = 1"); } @Test - public void testFunctionParameterDoesNotEffectGlobalValidationEnv() throws Exception { - checkError("name 'a' is not defined", "def func1(a):", " return a", "def func2():", " b = a"); + public void testAugmentedAssignmentWithMultipleLValues() throws Exception { + assertInvalid( + "cannot perform augmented assignment on a list or tuple expression", // + "a, b += 2, 3"); } @Test - public void testLocalValidationEnvironmentsAreSeparated() throws Exception { - parse("def func1():", " a = 1", "def func2():", " a = 'abc'\n"); + public void testReturnOutsideFunction() throws Exception { + assertInvalid( + "return statements must be inside a function", // + "return 2\n"); } @Test - public void testBuiltinSymbolsAreReadOnly() throws Exception { - checkError("Variable repr is read only", "repr = 1"); + public void testLoadAfterStatement() throws Exception { + assertInvalid( + "load() statements must be called before any other statement", // + "a = 5", + "load(':b.bzl', 'c')"); } @Test - public void testSkylarkGlobalVariablesAreReadonly() throws Exception { - checkError("Variable a is read only", "a = 1", "a = 2"); + public void testLoadDuplicateSymbols() throws Exception { + assertInvalid( + "load statement defines 'x' more than once", // + "load('module', 'x', 'x')"); + assertInvalid( + "load statement defines 'x' more than once", // + "load('module', 'x', x='y')"); + + // Eventually load bindings will be local, + // at which point these errors will need adjusting. + assertInvalid( + "cannot reassign global 'x'", // + "x=1; load('module', 'x')"); + assertInvalid( + "cannot reassign global 'x'", // + "load('module', 'x'); x=1"); } @Test - public void testFunctionDefRecursion() throws Exception { - parse("def func():", " func()\n"); + public void testForbiddenToplevelIfStatement() throws Exception { + assertInvalid( + "if statements are not allowed at the top level", // + "if True: a = 2"); } @Test - public void testMutualRecursion() throws Exception { - parse("def foo(i):", " bar(i)", "def bar(i):", " foo(i)", "foo(4)"); + public void testFunctionLocalVariable() throws Exception { + assertInvalid( + "name 'a' is not defined", // + "def func2(b):", + " c = b", + " c = a", + "def func1():", + " a = 1", + " func2(2)"); } @Test - public void testFunctionDefinedBelow() { - parse("def bar(): a = foo() + 'a'", "def foo(): return 1\n"); + public void testFunctionLocalVariableDoesNotEffectGlobalValidationEnv() throws Exception { + assertInvalid( + "name 'a' is not defined", // + "def func1():", + " a = 1", + "def func2(b):", + " b = a"); } @Test - public void testFunctionDoesNotExist() { - checkError("function 'foo' does not exist", "def bar(): a = foo() + 'a'"); + public void testFunctionParameterDoesNotEffectGlobalValidationEnv() throws Exception { + assertInvalid( + "name 'a' is not defined", // + "def func1(a):", + " return a", + "def func2():", + " b = a"); } @Test - public void testStructMembersAreImmutable() { - checkError( - "can only assign to variables and tuples, not to 's.x'", - "s = struct(x = 'a')", - "s.x = 'b'\n"); + public void testDefinitionByItself() throws Exception { + // Variables are assumed to be statically visible in the block (even if they might not be + // initialized). + assertValid("a = a"); + assertValid("a += a"); + assertValid("[[] for a in a]"); + assertValid("def f():", " for a in a: pass"); } @Test - public void testStructDictMembersAreImmutable() { - checkError( - "can only assign to variables and tuples, not to 's.x['b']'", - "s = struct(x = {'a' : 1})", - "s.x['b'] = 2\n"); + public void testLocalValidationEnvironmentsAreSeparated() throws Exception { + assertValid( + "def func1():", // + " a = 1", + "def func2():", + " a = 'abc'"); } @Test - public void testTupleLiteralWorksForDifferentTypes() throws Exception { - parse("('a', 1)"); + public void testBuiltinsCanBeShadowed() throws Exception { + assertValid("repr = 1"); } @Test - public void testDictLiteralDifferentValueTypeWorks() throws Exception { - parse("{'a': 1, 'b': 'c'}"); + public void testNoGlobalReassign() throws Exception { + EventCollector errors = getValidationErrors("a = 1", "a = 2"); + assertContainsEvent(errors, ":2:1: cannot reassign global 'a'"); + assertContainsEvent(errors, ":1:1: 'a' previously declared here"); } @Test - public void testNoneAssignment() throws Exception { - parse("def func():", " a = None", " a = 2", " a = None\n"); + public void testTwoFunctionsWithTheSameName() throws Exception { + EventCollector errors = getValidationErrors("def foo(): pass", "def foo(): pass"); + assertContainsEvent(errors, ":2:5: cannot reassign global 'foo'"); + assertContainsEvent(errors, ":1:5: 'foo' previously declared here"); } @Test - public void testNoneIsAnyType() throws Exception { - parse("None + None"); - parse("2 == None"); - parse("None > 'a'"); - parse("[] in None"); - parse("5 * None"); + public void testFunctionDefRecursion() throws Exception { + assertValid("def func():", " func()\n"); } - // Skylark built-in functions specific tests + @Test + public void testMutualRecursion() throws Exception { + assertValid("def foo(i):", " bar(i)", "def bar(i):", " foo(i)", "foo(4)"); + } @Test - public void testFuncReturningDictAssignmentAsLValue() throws Exception { - checkError( - "can only assign to variables and tuples, not to 'my_dict()['b']'", - "def my_dict():", - " return {'a': 1}", - "def func():", - " my_dict()['b'] = 2", - " return d\n"); + public void testFunctionDefinedBelow() throws Exception { + assertValid("def bar(): a = foo() + 'a'", "def foo(): return 1\n"); } @Test - public void testEmptyLiteralGenericIsSetInLaterConcatWorks() { - parse("def func():", " s = {}", " s['a'] = 'b'\n"); + public void testGlobalDefinedBelow() throws Exception { + assertValid("def bar(): return x", "x = 5\n"); } @Test - public void testReadOnlyWorksForSimpleBranching() { - parse("if 1:", " v = 'a'", "else:", " v = 'b'"); + public void testLocalVariableDefinedBelow() throws Exception { + assertValid( + "def bar():", + " for i in range(5):", + " if i > 2: return x", + " x = i" // x is visible in the entire function block + ); } @Test - public void testReadOnlyWorksForNestedBranching() { - parse( - "if 1:", - " if 0:", - " v = 'a'", - " else:", - " v = 'b'", - "else:", - " if 0:", - " v = 'c'", - " else:", - " v = 'd'\n"); + public void testFunctionDoesNotExist() throws Exception { + assertInvalid( + "name 'foo' is not defined", // + "def bar(): a = foo() + 'a'"); } @Test - public void testReadOnlyWorksForDifferentLevelBranches() { - checkError("Variable v is read only", "if 1:", " if 1:", " v = 'a'", " v = 'b'\n"); + public void testTupleLiteralWorksForDifferentTypes() throws Exception { + assertValid("('a', 1)"); } @Test - public void testReadOnlyWorksWithinSimpleBranch() { - checkError( - "Variable v is read only", "if 1:", " v = 'a'", "else:", " v = 'b'", " v = 'c'\n"); + public void testDictExpressionDifferentValueTypeWorks() throws Exception { + assertValid("{'a': 1, 'b': 'c'}"); } @Test - public void testReadOnlyWorksWithinNestedBranch() { - checkError( - "Variable v is read only", - "if 1:", - " v = 'a'", - "else:", - " if 1:", - " v = 'b'", - " else:", - " v = 'c'", - " v = 'd'\n"); + public void testNoneAssignment() throws Exception { + assertValid("def func():", " a = None", " a = 2", " a = None\n"); } @Test - public void testReadOnlyWorksAfterSimpleBranch() { - checkError("Variable v is read only", "if 1:", " v = 'a'", "else:", " w = 'a'", "v = 'b'"); + public void testNoneIsAnyType() throws Exception { + assertValid("None + None"); + assertValid("2 == None"); + assertValid("None > 'a'"); + assertValid("[] in None"); + assertValid("5 * None"); } + // Starlark built-in functions specific tests + @Test - public void testReadOnlyWorksAfterNestedBranch() { - checkError("Variable v is read only", "if 1:", " if 1:", " v = 'a'", "v = 'b'"); + public void testFuncReturningDictAssignmentAsLValue() throws Exception { + assertValid( + "def my_dict():", // + " return {'a': 1}", + "def func():", + " my_dict()['b'] = 2"); } @Test - public void testReadOnlyWorksAfterNestedBranch2() { - checkError( - "Variable v is read only", - "if 1:", - " v = 'a'", - "else:", - " if 0:", - " w = 1", - "v = 'b'\n"); + public void testEmptyLiteralGenericIsSetInLaterConcatWorks() throws Exception { + assertValid( + "def func():", // + " s = {}", + " s['a'] = 'b'"); } @Test - public void testModulesReadOnlyInFuncDefBody() { - parse("def func():", " cmd_helper = set()"); + public void testModulesReadOnlyInFuncDefBody() throws Exception { + assertValid("def func():", " cmd_helper = depset()"); } @Test - public void testBuiltinGlobalFunctionsReadOnlyInFuncDefBody() { - parse("def func():", " rule = 'abc'"); + public void testBuiltinGlobalFunctionsReadOnlyInFuncDefBody() throws Exception { + assertValid("def func():", " rule = 'abc'"); } @Test - public void testBuiltinGlobalFunctionsReadOnlyAsFuncDefArg() { - parse("def func(rule):", " return rule"); + public void testBuiltinGlobalFunctionsReadOnlyAsFuncDefArg() throws Exception { + assertValid("def func(rule):", " return rule"); } @Test - public void testFunctionReturnsFunction() { - parse( - "def rule(*, implementation): return None", + public void testFunctionReturnsFunction() throws Exception { + assertValid( + "def rule(*, implementation): return None", // "def impl(ctx): return None", "", "skylark_rule = rule(implementation = impl)", @@ -263,126 +300,67 @@ public void testFunctionReturnsFunction() { } @Test - public void testTypeForBooleanLiterals() { - parse("len([1, 2]) == 0 and True"); - parse("len([1, 2]) == 0 and False"); - } - - @Test - public void testLoadRelativePathOneSegment() throws Exception { - parse("load('extension', 'a')\n"); - } - - @Test - public void testLoadAbsolutePathMultipleSegments() throws Exception { - parse("load('/pkg/extension', 'a')\n"); - } - - @Test - public void testLoadRelativePathMultipleSegments() throws Exception { - checkError( - "Path 'pkg/extension.bzl' is not valid. It should either start with " - + "a slash or refer to a file in the current directory.", - "load('pkg/extension', 'a')\n"); + public void testTypeForBooleanLiterals() throws Exception { + assertValid("len([1, 2]) == 0 and True"); + assertValid("len([1, 2]) == 0 and False"); } @Test public void testDollarErrorDoesNotLeak() throws Exception { - setFailFast(false); - parseFile( - "def GenerateMapNames():", " a = 2", " b = [3, 4]", " if a not b:", " print(a)"); - assertContainsEvent("syntax error at 'b': expected in"); + EventCollector errors = + getValidationErrors( + "def GenerateMapNames():", // + " a = 2", + " b = [3, 4]", + " if a not b:", + " print(a)"); + assertContainsEvent(errors, "syntax error at 'b': expected 'in'"); // Parser uses "$error" symbol for error recovery. // It should not be used in error messages. - for (Event event : getEventCollector()) { + for (Event event : errors) { assertThat(event.getMessage()).doesNotContain("$error$"); } } @Test - public void testParentWithSkylarkModule() throws Exception { - Class emptyTupleClass = Tuple.EMPTY.getClass(); - Class tupleClass = Tuple.of(1, "a", "b").getClass(); - Class mutableListClass = new MutableList(Tuple.of(1, 2, 3), env).getClass(); - - assertThat(mutableListClass).isEqualTo(MutableList.class); - assertThat(MutableList.class.isAnnotationPresent(SkylarkModule.class)).isTrue(); - assertThat(EvalUtils.getParentWithSkylarkModule(MutableList.class)) - .isEqualTo(MutableList.class); - assertThat(EvalUtils.getParentWithSkylarkModule(emptyTupleClass)).isEqualTo(Tuple.class); - assertThat(EvalUtils.getParentWithSkylarkModule(tupleClass)).isEqualTo(Tuple.class); - // TODO(bazel-team): make a tuple not a list anymore. - assertThat(EvalUtils.getParentWithSkylarkModule(tupleClass)).isEqualTo(Tuple.class); - - // TODO(bazel-team): fix that? - assertThat(ClassObject.class.isAnnotationPresent(SkylarkModule.class)).isFalse(); - assertThat(ClassObject.SkylarkClassObject.class.isAnnotationPresent(SkylarkModule.class)) - .isTrue(); - assertThat( - EvalUtils.getParentWithSkylarkModule(ClassObject.SkylarkClassObject.class) - == ClassObject.SkylarkClassObject.class) - .isTrue(); - assertThat(EvalUtils.getParentWithSkylarkModule(ClassObject.class)).isNull(); + public void testPositionalAfterStarArg() throws Exception { + assertInvalid( + "positional argument is misplaced (positional arguments come first)", // + "def fct(*args, **kwargs): pass", + "fct(1, *[2], 3)"); } @Test - public void testSkylarkTypeEquivalence() throws Exception { - // All subclasses of SkylarkList are made equivalent - Class emptyTupleClass = Tuple.EMPTY.getClass(); - Class tupleClass = Tuple.of(1, "a", "b").getClass(); - Class mutableListClass = new MutableList(Tuple.of(1, 2, 3), env).getClass(); - - assertThat(SkylarkType.of(mutableListClass)).isEqualTo(SkylarkType.LIST); - assertThat(SkylarkType.of(emptyTupleClass)).isEqualTo(SkylarkType.TUPLE); - assertThat(SkylarkType.of(tupleClass)).isEqualTo(SkylarkType.TUPLE); - assertThat(SkylarkType.TUPLE).isNotEqualTo(SkylarkType.LIST); - - // Also for ClassObject - assertThat(SkylarkType.of(ClassObject.SkylarkClassObject.class)).isEqualTo(SkylarkType.STRUCT); - // TODO(bazel-team): fix that? - assertThat(SkylarkType.of(ClassObject.class)).isNotEqualTo(SkylarkType.STRUCT); - - // Also test for these bazel classes, to avoid some regression. - // TODO(bazel-team): move to some other place to remove dependency of syntax tests on Artifact? - assertThat(SkylarkType.of(Artifact.SpecialArtifact.class)) - .isEqualTo(SkylarkType.of(Artifact.class)); - assertThat(SkylarkType.of(RuleConfiguredTarget.class)).isNotEqualTo(SkylarkType.STRUCT); + public void testTwoStarArgs() throws Exception { + assertInvalid( + "*arg argument is misplaced", // + "def fct(*args, **kwargs):", + " pass", + "fct(1, 2, 3, *[], *[])"); } @Test - public void testSkylarkTypeInclusion() throws Exception { - assertThat(SkylarkType.INT.includes(SkylarkType.BOTTOM)).isTrue(); - assertThat(SkylarkType.BOTTOM.includes(SkylarkType.INT)).isFalse(); - assertThat(SkylarkType.TOP.includes(SkylarkType.INT)).isTrue(); - - SkylarkType combo1 = SkylarkType.Combination.of(SkylarkType.LIST, SkylarkType.INT); - assertThat(SkylarkType.LIST.includes(combo1)).isTrue(); - - SkylarkType union1 = - SkylarkType.Union.of(SkylarkType.MAP, SkylarkType.LIST, SkylarkType.STRUCT); - assertThat(union1.includes(SkylarkType.MAP)).isTrue(); - assertThat(union1.includes(SkylarkType.STRUCT)).isTrue(); - assertThat(union1.includes(combo1)).isTrue(); - assertThat(union1.includes(SkylarkType.STRING)).isFalse(); - - SkylarkType union2 = - SkylarkType.Union.of( - SkylarkType.LIST, SkylarkType.MAP, SkylarkType.STRING, SkylarkType.INT); - SkylarkType inter1 = SkylarkType.intersection(union1, union2); - assertThat(inter1.includes(SkylarkType.MAP)).isTrue(); - assertThat(inter1.includes(SkylarkType.LIST)).isTrue(); - assertThat(inter1.includes(combo1)).isTrue(); - assertThat(inter1.includes(SkylarkType.INT)).isFalse(); + public void testKeywordArgAfterStarArg() throws Exception { + assertInvalid( + "keyword argument is misplaced (keyword arguments must be before any *arg or **kwarg)", // + "def fct(*args, **kwargs): pass", + "fct(1, *[2], a=3)"); } - private void parse(String... lines) { - parseFile(lines); - assertNoEvents(); + @Test + public void testTopLevelForFails() throws Exception { + assertInvalid( + "for loops are not allowed at the top level", // + "for i in []: 0\n"); } - private void checkError(String errorMsg, String... lines) { - setFailFast(false); - parseFile(lines); - assertContainsEvent(errorMsg); + @Test + public void testNestedFunctionFails() throws Exception { + assertInvalid( + "nested functions are not allowed. Move the function to the top level", // + "def func(a):", + " def bar(): return 0", + " return bar()", + ""); } }