From 693963f44ad2d62d858a24ed5dad4698d3835223 Mon Sep 17 00:00:00 2001 From: laurentlb Date: Fri, 29 Nov 2019 08:05:42 -0800 Subject: [PATCH] Delete the depset.union() method It has been deprecated for a long time. https://github.com/bazelbuild/bazel/issues/5817 RELNOTES: None. PiperOrigin-RevId: 283060975 --- .../devtools/build/lib/syntax/Depset.java | 30 +-------- .../devtools/build/lib/syntax/DepsetTest.java | 62 ------------------- .../build/lib/syntax/MethodLibraryTest.java | 6 +- 3 files changed, 5 insertions(+), 93 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Depset.java b/src/main/java/com/google/devtools/build/lib/syntax/Depset.java index 67a1ad4d245f29..43f16e4b8dfc81 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Depset.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Depset.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; @@ -58,7 +57,7 @@ + "

Depsets are immutable. They should be created using their constructor function and merged or augmented with" + " other depsets via the transitive argument. There are other deprecated" - + " methods (| and + operators, union method)" + + " methods (| and + operators)" + " that will eventually go away." + "

The order parameter determines the" + " kind of traversal that is done to convert the depset to an iterable. There are" @@ -192,7 +191,7 @@ static Depset legacyOf(Order order, Object items) throws EvalException { return create(order, SkylarkType.TOP, items, null); } - // implementation of deprecated depset+x, depset.union(x), depset|x + // implementation of deprecated depset+x, depset|x static Depset unionOf(Depset left, Object right) throws EvalException { return create(left.set.getOrder(), left.contentType, right, left); } @@ -397,31 +396,6 @@ public void repr(Printer printer) { printer.append(")"); } - @SkylarkCallable( - name = "union", - doc = - "(Deprecated) Returns a new depset that is the merge " - + "of the given depset and new_elements. Use the " - + "transitive constructor argument instead.", - parameters = { - @Param(name = "new_elements", type = Object.class, doc = "The elements to be added.") - }, - useStarlarkThread = true) - public Depset union(Object newElements, StarlarkThread thread) throws EvalException { - if (thread.getSemantics().incompatibleDepsetUnion()) { - throw new EvalException( - null, - "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 - // Depsets versus a set and another kind of iterable. - // Can't use Starlark#toIterable since that would discard this information. - return Depset.unionOf(this, newElements); - } - @SkylarkCallable( name = "to_list", doc = diff --git a/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java b/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java index 539c76f270088b..d38652f89a5c79 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java @@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows; -import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.syntax.util.EvaluationTestCase; @@ -272,10 +271,6 @@ public void testTransitiveOrderDirect() throws Exception { @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']"); @@ -283,35 +278,6 @@ public void testIncompatibleUnion() throws Exception { .testIfErrorContains("`|` operator on a depset is forbidden", "depset([]) | ['a']"); } - @Test - public void testUnionWithList() throws Exception { - thread = newStarlarkThreadWithSkylarkOptions("--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"); - assertContainsInOrder("depset(['a', 'b', 'c']).union([])", "a", "b", "c"); - } - - @Test - public void testUnionWithDepset() throws Exception { - thread = newStarlarkThreadWithSkylarkOptions("--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"); - assertContainsInOrder("depset(['a', 'b', 'c']).union(depset([]))", "a", "b", "c"); - } - - @Test - public void testUnionDuplicates() throws Exception { - thread = newStarlarkThreadWithSkylarkOptions("--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"); - - assertContainsInOrder("depset(['a', 'b', 'c']).union(depset(['a', 'b', 'c']))", "a", "b", "c"); - assertContainsInOrder("depset(['a', 'a', 'a']).union(depset(['a', 'a']))", "a"); - } - - private void assertContainsInOrder(String statement, Object... expectedElements) throws Exception { assertThat(((Depset) eval(statement)).toCollection()) @@ -342,34 +308,6 @@ public void testUnionIncompatibleOrder() throws Exception { "depset(['a', 'b'], order='postorder') + depset(['c', 'd'], order='topological')"); } - @Test - public void testUnionWithNonsequence() throws Exception { - thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false"); - checkEvalError("cannot union value of type 'int' to a depset", "depset([]).union(5)"); - checkEvalError("cannot union value of type 'string' to a depset", "depset(['a']).union('b')"); - } - - @Test - public void testUnionWrongNumArgs() throws Exception { - new BothModesTest() - .testIfErrorContains( - "parameter 'new_elements' has no default value, " - + "for call to method union(new_elements) of 'depset'", - "depset(['a']).union()"); - } - - @Test - public void testUnionNoSideEffects() throws Exception { - thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false"); - exec( - "def func():", - " s1 = depset(['a'])", - " s2 = s1.union(['b'])", - " return s1", - "s = func()"); - assertThat(((Depset) lookup("s")).toCollection()).isEqualTo(ImmutableList.of("a")); - } - @Test public void testFunctionReturnsDepset() throws Exception { thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false"); diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java index 9fec6d076c02d1..0d55dc1af9a17b 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java @@ -159,7 +159,7 @@ public void testBuiltinFunctionErrorMessage() throws Exception { @Test public void testHasAttr() throws Exception { new SkylarkTest() - .testExpression("hasattr(depset(), 'union')", Boolean.TRUE) + .testExpression("hasattr(depset(), 'to_list')", Boolean.TRUE) .testExpression("hasattr('test', 'count')", Boolean.TRUE) .testExpression("hasattr(dict(a = 1, b = 2), 'items')", Boolean.TRUE) .testExpression("hasattr({}, 'items')", Boolean.TRUE); @@ -738,7 +738,7 @@ public void testLegacyNamed() throws Exception { .testEval("enumerate(list=[40, 41])", "[(0, 40), (1, 41)]") .testExpression("hash(value='hello')", "hello".hashCode()) .testEval("range(start_or_stop=3, stop_or_none=9, step=2)", "range(3, 9, 2)") - .testExpression("hasattr(x=depset(), name='union')", Boolean.TRUE) + .testExpression("hasattr(x=depset(), name='to_list')", Boolean.TRUE) .testExpression("bool(x=3)", Boolean.TRUE) .testExpression("getattr(x='hello', name='cnt', default='default')", "default") .testEval( @@ -749,7 +749,7 @@ public void testLegacyNamed() throws Exception { .testEval("str(depset(items=[0,1]))", "'depset([0, 1])'") .testIfErrorContains("hello", "fail(msg='hello', attr='someattr')") // Parameters which may be None but are not explicitly 'noneable' - .testExpression("hasattr(x=None, name='union')", Boolean.FALSE) + .testExpression("hasattr(x=None, name='to_list')", Boolean.FALSE) .testEval("getattr(x=None, name='count', default=None)", "None") .testEval("dir(None)", "[]") .testIfErrorContains("None", "fail(msg=None)")