diff options
6 files changed, 30 insertions, 16 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java index 3eb1c062cf..f565442c22 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java @@ -496,7 +496,8 @@ public class SkylarkRuleImplementationFunctions { // Also, allow empty set for init @Param(name = "transitive_files", type = SkylarkNestedSet.class, generic1 = Artifact.class, noneable = true, defaultValue = "None", - doc = "The (transitive) set of files to be added to the runfiles."), + doc = "The (transitive) set of files to be added to the runfiles. The depset should " + + "use the `default` order (which, as the name implies, is the default)."), @Param(name = "collect_data", type = Boolean.class, defaultValue = "False", doc = "Whether to collect the data " + "runfiles from the dependencies in srcs, data and deps attributes."), 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 e4e4b01970..459858aad4 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 @@ -273,7 +273,7 @@ public final class BinaryOperatorExpression extends Expression { } } - // TODO(bazel-team): Remove this case. Union of sets should use '|' instead of '+'. + // TODO(bazel-team): Deprecate + and | on depsets. Needs new API design. if (lval instanceof SkylarkNestedSet) { return new SkylarkNestedSet((SkylarkNestedSet) lval, rval, location); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java index 9b17cead89..38cbb7c35f 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java @@ -940,16 +940,28 @@ public final class Environment implements Freezable { } } + /** An exception thrown by {@link #FAIL_FAST_HANDLER}. */ + // TODO(bazel-team): Possibly extend RuntimeException instead of IllegalArgumentException. + public static class FailFastException extends IllegalArgumentException { + public FailFastException(String s) { + super(s); + } + } /** - * The fail fast handler, which throws an {@link IllegalArgumentException} whenever an error or - * warning occurs. + * A handler that immediately throws {@link FailFastException} whenever an error or warning + * occurs. + * + * We do not reuse an existing unchecked exception type, because callers (e.g., test assertions) + * need to be able to distinguish between organically occurring exceptions and exceptions thrown + * by this handler. */ public static final EventHandler FAIL_FAST_HANDLER = new EventHandler() { - @Override - public void handle(Event event) { - Preconditions.checkArgument( - !EventKind.ERRORS_AND_WARNINGS.contains(event.getKind()), event); + @Override + public void handle(Event event) { + if (EventKind.ERRORS_AND_WARNINGS.contains(event.getKind())) { + throw new FailFastException(event.toString()); } - }; + } + }; } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java index 12a01326da..c38fe89b6a 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java @@ -159,7 +159,8 @@ public final class SkylarkNestedSet implements SkylarkValue, SkylarkQueryable { for (NestedSet<?> nestedSet : transitiveItems) { builder.addTransitive(nestedSet); } - } catch (IllegalStateException e) { + } catch (IllegalArgumentException e) { + // Order mismatch between item and builder. throw new EvalException(loc, e.getMessage()); } this.set = builder.build(); diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java index 1cd8a98c62..c9b8d1c4e0 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java @@ -635,7 +635,7 @@ public class SkylarkEvaluationTest extends EvaluationTest { private void flowStatementInsideFunction(String statement) throws Exception { checkEvalErrorContains(statement + " statement must be inside a for loop", "def foo():", - " " + statement + "", + " " + statement, "x = foo()"); } @@ -644,7 +644,7 @@ public class SkylarkEvaluationTest extends EvaluationTest { "def foo2():", " for i in range(0, 3):", " pass", - " " + statement + "", + " " + statement, "y = foo2()"); } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java index 70604c6349..0fa14bcec0 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.events.util.EventCollectionApparatus; import com.google.devtools.build.lib.syntax.BazelLibrary; import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.Environment; +import com.google.devtools.build.lib.syntax.Environment.FailFastException; import com.google.devtools.build.lib.syntax.Environment.Phase; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.Expression; @@ -201,12 +202,11 @@ public class EvaluationTestCase { } public void checkEvalError(String msg, String... input) throws Exception { - setFailFast(true); try { eval(input); fail("Expected error '" + msg + "' but got no error"); - } catch (IllegalArgumentException | EvalException e) { - assertThat(e).hasMessage(msg); + } catch (EvalException | FailFastException e) { + assertThat(e).hasMessageThat().isEqualTo(msg); } } @@ -214,7 +214,7 @@ public class EvaluationTestCase { try { eval(input); fail("Expected error containing '" + msg + "' but got no error"); - } catch (IllegalArgumentException | EvalException e) { + } catch (EvalException | FailFastException e) { assertThat(e).hasMessageThat().contains(msg); } } |