diff options
Diffstat (limited to 'src')
7 files changed, 77 insertions, 5 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java index 1edc800c2b..0c0ece9971 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java @@ -44,6 +44,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics codedOut.writeBoolNoTag(semantics.incompatibleCheckedArithmetic()); codedOut.writeBoolNoTag(semantics.incompatibleComprehensionVariablesDoNotLeak()); codedOut.writeBoolNoTag(semantics.incompatibleDepsetIsNotIterable()); + codedOut.writeBoolNoTag(semantics.incompatibleDepsetUnion()); codedOut.writeBoolNoTag(semantics.incompatibleDictLiteralHasNoDuplicates()); codedOut.writeBoolNoTag(semantics.incompatibleDisallowDictPlus()); codedOut.writeBoolNoTag(semantics.incompatibleDisallowKeywordOnlyArgs()); @@ -68,6 +69,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics builder.incompatibleCheckedArithmetic(codedIn.readBool()); builder.incompatibleComprehensionVariablesDoNotLeak(codedIn.readBool()); builder.incompatibleDepsetIsNotIterable(codedIn.readBool()); + builder.incompatibleDepsetUnion(codedIn.readBool()); builder.incompatibleDictLiteralHasNoDuplicates(codedIn.readBool()); builder.incompatibleDisallowDictPlus(codedIn.readBool()); builder.incompatibleDisallowKeywordOnlyArgs(codedIn.readBool()); diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java index 359cce86f0..22598ce7eb 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java @@ -99,6 +99,19 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable public boolean incompatibleComprehensionVariablesDoNotLeak; @Option( + name = "incompatible_depset_union", + defaultValue = "false", + category = "incompatible changes", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = + "If set to true, depset union using `+`, `|` or `.union` are forbidden. " + + "Use the `depset` constructor instead." + ) + public boolean incompatibleDepsetUnion; + + @Option( name = "incompatible_depset_is_not_iterable", defaultValue = "false", category = "incompatible changes", @@ -265,6 +278,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable .incompatibleCheckedArithmetic(incompatibleCheckedArithmetic) .incompatibleComprehensionVariablesDoNotLeak(incompatibleComprehensionVariablesDoNotLeak) .incompatibleDepsetIsNotIterable(incompatibleDepsetIsNotIterable) + .incompatibleDepsetUnion(incompatibleDepsetUnion) .incompatibleDictLiteralHasNoDuplicates(incompatibleDictLiteralHasNoDuplicates) .incompatibleDisallowDictPlus(incompatibleDisallowDictPlus) .incompatibleDisallowKeywordOnlyArgs(incompatibleDisallowKeywordOnlyArgs) diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java index 110c9e1f8c..c0ef08603e 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java @@ -186,13 +186,23 @@ public class BazelLibrary { @Param(name = "input", type = SkylarkNestedSet.class, doc = "The input depset."), @Param(name = "new_elements", type = Object.class, doc = "The elements to be added.") }, - useLocation = true + useLocation = true, + useEnvironment = true ) private static final BuiltinFunction union = new BuiltinFunction("union") { @SuppressWarnings("unused") - public SkylarkNestedSet invoke(SkylarkNestedSet input, Object newElements, Location loc) + public SkylarkNestedSet invoke( + SkylarkNestedSet input, Object newElements, Location loc, Environment env) throws EvalException { + if (env.getSemantics().incompatibleDepsetUnion()) { + throw new EvalException( + location, + "depset method `.union` has been removed. See " + + "https://docs.bazel.build/versions/master/skylark/depsets.html for " + + "recommendations. Use --incompatible_depset_union=false " + + "to temporarily disable this check."); + } // newElements' type is Object because of the polymorphism on unioning two // SkylarkNestedSets versus a set and another kind of iterable. // Can't use EvalUtils#toIterable since that would discard this information. 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 ab545645b4..450db7728a 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 @@ -198,7 +198,7 @@ public final class BinaryOperatorExpression extends Expression { return plus(lhs, rhs, env, location, isAugmented); case PIPE: - return pipe(lhs, rhs, location); + return pipe(lhs, rhs, env, location); case MINUS: return minus(lhs, rhs, env, location); @@ -325,16 +325,33 @@ public final class BinaryOperatorExpression extends Expression { } } - // TODO(bazel-team): Deprecate + and | on depsets. Needs new API design. + // TODO(bazel-team): Remove deprecated operator. if (lval instanceof SkylarkNestedSet) { + if (env.getSemantics().incompatibleDepsetUnion()) { + throw new EvalException( + location, + "`+` operator on a depset is forbidden. See " + + "https://docs.bazel.build/versions/master/skylark/depsets.html for " + + "recommendations. Use --incompatible_depset_union=false " + + "to temporarily disable this check."); + } return new SkylarkNestedSet((SkylarkNestedSet) lval, rval, location); } throw typeException(lval, rval, Operator.PLUS, location); } /** Implements Operator.PIPE. */ - private static Object pipe(Object lval, Object rval, Location location) throws EvalException { + private static Object pipe(Object lval, Object rval, Environment env, Location location) + throws EvalException { if (lval instanceof SkylarkNestedSet) { + if (env.getSemantics().incompatibleDepsetUnion()) { + throw new EvalException( + location, + "`|` operator on a depset is forbidden. See " + + "https://docs.bazel.build/versions/master/skylark/depsets.html for " + + "recommendations. Use --incompatible_depset_union=false " + + "to temporarily disable this check."); + } return new SkylarkNestedSet((SkylarkNestedSet) lval, rval, location); } throw typeException(lval, rval, Operator.PIPE, location); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java index 709449a4dc..7f6f6c6018 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java @@ -43,6 +43,9 @@ public abstract class SkylarkSemantics { public abstract boolean incompatibleCheckedArithmetic(); public abstract boolean incompatibleComprehensionVariablesDoNotLeak(); public abstract boolean incompatibleDepsetIsNotIterable(); + + public abstract boolean incompatibleDepsetUnion(); + public abstract boolean incompatibleDictLiteralHasNoDuplicates(); public abstract boolean incompatibleDisallowDictPlus(); public abstract boolean incompatibleDisallowKeywordOnlyArgs(); @@ -75,6 +78,7 @@ public abstract class SkylarkSemantics { .incompatibleCheckedArithmetic(true) .incompatibleComprehensionVariablesDoNotLeak(true) .incompatibleDepsetIsNotIterable(false) + .incompatibleDepsetUnion(false) .incompatibleDictLiteralHasNoDuplicates(true) .incompatibleDisallowDictPlus(false) .incompatibleDisallowKeywordOnlyArgs(true) @@ -98,6 +102,9 @@ public abstract class SkylarkSemantics { public abstract Builder incompatibleCheckedArithmetic(boolean value); public abstract Builder incompatibleComprehensionVariablesDoNotLeak(boolean value); public abstract Builder incompatibleDepsetIsNotIterable(boolean value); + + public abstract Builder incompatibleDepsetUnion(boolean value); + public abstract Builder incompatibleDictLiteralHasNoDuplicates(boolean value); public abstract Builder incompatibleDisallowDictPlus(boolean value); public abstract Builder incompatibleDisallowKeywordOnlyArgs(boolean value); diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java index 714f197f38..05d5a3e74e 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java @@ -117,6 +117,7 @@ public class SkylarkSemanticsConsistencyTest { "--incompatible_checked_arithmetic=" + rand.nextBoolean(), "--incompatible_comprehension_variables_do_not_leak=" + rand.nextBoolean(), "--incompatible_depset_is_not_iterable=" + rand.nextBoolean(), + "--incompatible_depset_union=" + rand.nextBoolean(), "--incompatible_dict_literal_has_no_duplicates=" + rand.nextBoolean(), "--incompatible_disallow_dict_plus=" + rand.nextBoolean(), "--incompatible_disallow_keyword_only_args=" + rand.nextBoolean(), @@ -142,6 +143,7 @@ public class SkylarkSemanticsConsistencyTest { .incompatibleCheckedArithmetic(rand.nextBoolean()) .incompatibleComprehensionVariablesDoNotLeak(rand.nextBoolean()) .incompatibleDepsetIsNotIterable(rand.nextBoolean()) + .incompatibleDepsetUnion(rand.nextBoolean()) .incompatibleDictLiteralHasNoDuplicates(rand.nextBoolean()) .incompatibleDisallowDictPlus(rand.nextBoolean()) .incompatibleDisallowKeywordOnlyArgs(rand.nextBoolean()) diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java index 3ba85635d2..9c8f4de230 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java @@ -295,7 +295,21 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { } @Test + public void testIncompatibleUnion() throws Exception { + new BothModesTest("--incompatible_depset_union=true") + .testIfErrorContains( + "depset method `.union` has been removed", "depset([]).union(['a', 'b', 'c'])"); + + new BothModesTest("--incompatible_depset_union=true") + .testIfErrorContains("`+` operator on a depset is forbidden", "depset([]) + ['a']"); + + new BothModesTest("--incompatible_depset_union=true") + .testIfErrorContains("`|` operator on a depset is forbidden", "depset([]) | ['a']"); + } + + @Test public void testUnionWithList() throws Exception { + env = newEnvironmentWithSkylarkOptions("--incompatible_depset_union=false"); assertContainsInOrder("depset([]).union(['a', 'b', 'c'])", "a", "b", "c"); assertContainsInOrder("depset(['a']).union(['b', 'c'])", "a", "b", "c"); assertContainsInOrder("depset(['a', 'b']).union(['c'])", "a", "b", "c"); @@ -304,6 +318,7 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { @Test public void testUnionWithDepset() throws Exception { + env = newEnvironmentWithSkylarkOptions("--incompatible_depset_union=false"); assertContainsInOrder("depset([]).union(depset(['a', 'b', 'c']))", "a", "b", "c"); assertContainsInOrder("depset(['a']).union(depset(['b', 'c']))", "b", "c", "a"); assertContainsInOrder("depset(['a', 'b']).union(depset(['c']))", "c", "a", "b"); @@ -312,6 +327,7 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { @Test public void testUnionDuplicates() throws Exception { + env = newEnvironmentWithSkylarkOptions("--incompatible_depset_union=false"); assertContainsInOrder("depset(['a', 'b', 'c']).union(['a', 'b', 'c'])", "a", "b", "c"); assertContainsInOrder("depset(['a', 'a', 'a']).union(['a', 'a'])", "a"); @@ -329,6 +345,7 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { @Test public void testUnionOrder() throws Exception { + env = newEnvironmentWithSkylarkOptions("--incompatible_depset_union=false"); eval( "def func():", " s1 = depset()", @@ -455,6 +472,7 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { @Test public void testToString() throws Exception { + env = newEnvironmentWithSkylarkOptions("--incompatible_depset_union=false"); eval( "s = depset() + [2, 4, 6] + [3, 4, 5]", "x = str(s)"); @@ -463,6 +481,7 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { @Test public void testToStringWithOrder() throws Exception { + env = newEnvironmentWithSkylarkOptions("--incompatible_depset_union=false"); eval( "s = depset(order = 'topological') + [2, 4, 6] + [3, 4, 5]", "x = str(s)"); @@ -476,6 +495,7 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { @Test public void testToList() throws Exception { + env = newEnvironmentWithSkylarkOptions("--incompatible_depset_union=false"); eval( "s = depset() + [2, 4, 6] + [3, 4, 5]", "x = s.to_list()"); |