Delete the depset.union() method

It has been deprecated for a long time. #5817 RELNOTES: None. PiperOrigin-RevId: 283060975
bazelbuild · Nov 29, 2019 · 693963f · 693963f
1 parent 95f6a94
commit 693963f
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 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
@@ -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 @@
             + "<p>Depsets are immutable. They should be created using their <a"
             + " href=\"globals.html#depset\">constructor function</a> and merged or augmented with"
             + " other depsets via the <code>transitive</code> argument. There are other deprecated"
-            + " methods (<code>|</code> and <code>+</code> operators, <code>union</code> method)"
+            + " methods (<code>|</code> and <code>+</code> operators)"
             + " that will eventually go away."
             + "<p>The <code>order</code> 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 =
-          "<i>(Deprecated)</i> Returns a new <a href=\"depset.html\">depset</a> that is the merge "
-              + "of the given depset and <code>new_elements</code>. Use the "
-              + "<code>transitive</code> 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
@@ -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,46 +271,13 @@ 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']");
 
     new BothModesTest("--incompatible_depset_union=true")
         .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
@@ -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)")