Delete the flag --incompatible_depset_union

#5817

RELNOTES: The flag `--incompatible_depset_union` is removed.
PiperOrigin-RevId: 303325924

site/docs/skylark/depsets.md

@@ -285,13 +285,6 @@ argument is omitted, the depset has the special `default` order, in which case
 there are no guarantees about the order of any of its elements (except that it
 is deterministic).
 
-For safety, depsets with different orders cannot be merged with the `+` operator
-unless one of them uses the default order; the resulting depset’s order is the
-same as the left operand. Note that when two depsets of different order are
-merged in this way, the child may appear to have had its elements rearranged
-when it is traversed via the parent. **The `+` operator is deprecated, anyway;
-use the `transitive` argument instead.**
-
 ## Performance
 
 To see the motivation for using depsets, consider what would have happened if we

src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java

@@ -290,20 +290,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
               + " https://github.com/bazelbuild/bazel/issues/8830 for details.")
   public boolean experimentalAllowTagsPropagation;
 
-  @Option(
-      name = "incompatible_depset_union",
-      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, depset union using `+`, `|` or `.union` are forbidden. "
-              + "Use the `depset` constructor instead.")
-  public boolean incompatibleDepsetUnion;
-
   @Option(
       name = "incompatible_always_check_depset_elements",
       defaultValue = "true",
@@ -676,7 +662,6 @@ public StarlarkSemantics toSkylarkSemantics() {
             .experimentalSiblingRepositoryLayout(experimentalSiblingRepositoryLayout)
             .experimentalExecGroups(experimentalExecGroups)
             .incompatibleApplicableLicenses(incompatibleApplicableLicenses)
-            .incompatibleDepsetUnion(incompatibleDepsetUnion)
             .incompatibleDisableTargetProviderFields(incompatibleDisableTargetProviderFields)
             .incompatibleDisableThirdPartyLicenseChecking(
                 incompatibleDisableThirdPartyLicenseChecking)

src/main/java/com/google/devtools/build/lib/syntax/Depset.java

@@ -59,9 +59,7 @@
             + " simulate one using a dictionary where all keys map to <code>True</code>."
             + "<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)"
-            + " that will eventually go away."
+            + " other depsets via the <code>transitive</code> argument. "
             + "<p>The <code>order</code> parameter determines the"
             + " kind of traversal that is done to convert the depset to an iterable. There are"
             + " four possible values:"
@@ -92,7 +90,7 @@
 public final class Depset implements StarlarkValue {
   private final SkylarkType contentType;
   private final NestedSet<?> set;
-  @Nullable private final ImmutableList<Object> items;
+  @Nullable private final ImmutableList<Object> items; // TODO(laurentlb): Delete field.
   @Nullable private final ImmutableList<NestedSet<?>> transitiveItems;
 
   @AutoCodec.VisibleForSerialization
@@ -107,6 +105,7 @@ public final class Depset implements StarlarkValue {
     this.transitiveItems = transitiveItems;
   }
 
+  // TODO(laurentlb): Remove the left argument once `unionOf` is deleted.
   private static Depset create(
       Order order, SkylarkType contentType, Object item, @Nullable Depset left)
       throws EvalException {
@@ -191,7 +190,7 @@ static Depset legacyOf(Order order, Object items) throws EvalException {
     return create(order, SkylarkType.TOP, items, null);
   }
 
-  // implementation of deprecated depset+x, depset|x
+  // TODO(laurentlb): Delete the method. It's used only in tests.
   static Depset unionOf(Depset left, Object right) throws EvalException {
     return create(left.set.getOrder(), left.contentType, right, left);
   }

src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java

@@ -405,17 +405,6 @@ static Object binaryOp(
             return StarlarkList.concat((StarlarkList<?>) x, (StarlarkList<?>) y, mu);
           }
 
-        } else if (x instanceof Depset) {
-          // depset + any
-          // TODO(bazel-team): Remove deprecated operator.
-          if (semantics.incompatibleDepsetUnion()) {
-            throw Starlark.errorf(
-                "`+` 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 Depset.unionOf((Depset) x, y);
         }
         break;
 
@@ -425,16 +414,6 @@ static Object binaryOp(
             // int | int
             return ((Integer) x) | (Integer) y;
           }
-        } else if (x instanceof Depset) {
-          // depset | any
-          if (semantics.incompatibleDepsetUnion()) {
-            throw Starlark.errorf(
-                "`|` 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 Depset.unionOf((Depset) x, y);
         }
         break;
 

src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -243,8 +243,6 @@ boolean isFeatureEnabledBasedOnTogglingFlags(String enablingFlag, String disabli
 
   public abstract boolean incompatibleApplicableLicenses();
 
-  public abstract boolean incompatibleDepsetUnion();
-
   public abstract boolean incompatibleDisableTargetProviderFields();
 
   public abstract boolean incompatibleDisableThirdPartyLicenseChecking();
@@ -346,7 +344,6 @@ public static Builder builderWithDefaults() {
           .experimentalExecGroups(false)
           .incompatibleAlwaysCheckDepsetElements(true)
           .incompatibleApplicableLicenses(false)
-          .incompatibleDepsetUnion(true)
           .incompatibleDisableTargetProviderFields(false)
           .incompatibleDisableThirdPartyLicenseChecking(true)
           .incompatibleDisableDeprecatedAttrParams(true)
@@ -417,8 +414,6 @@ public abstract static class Builder {
 
     public abstract Builder incompatibleApplicableLicenses(boolean value);
 
-    public abstract Builder incompatibleDepsetUnion(boolean value);
-
     public abstract Builder incompatibleDisableTargetProviderFields(boolean value);
 
     public abstract Builder incompatibleDisableThirdPartyLicenseChecking(boolean value);

src/main/java/com/google/devtools/build/skydoc/SkydocMain.java

@@ -190,7 +190,6 @@ public static void main(String[] args)
             .build();
     parser.parseAndExitUponError(args);
     StarlarkSemanticsOptions semanticsOptions = parser.getOptions(StarlarkSemanticsOptions.class);
-    semanticsOptions.incompatibleDepsetUnion = false;
     semanticsOptions.incompatibleDisableDeprecatedAttrParams = false;
     semanticsOptions.incompatibleNewActionsApi = false;
     SkydocOptions skydocOptions = parser.getOptions(SkydocOptions.class);

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -143,7 +143,6 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
         "--incompatible_always_check_depset_elements=" + rand.nextBoolean(),
         "--incompatible_applicable_licenses=" + rand.nextBoolean(),
         "--incompatible_depset_for_libraries_to_link_getter=" + rand.nextBoolean(),
-        "--incompatible_depset_union=" + rand.nextBoolean(),
         "--incompatible_disable_target_provider_fields=" + rand.nextBoolean(),
         "--incompatible_disable_deprecated_attr_params=" + rand.nextBoolean(),
         "--incompatible_disable_depset_items=" + rand.nextBoolean(),
@@ -197,7 +196,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleAlwaysCheckDepsetElements(rand.nextBoolean())
         .incompatibleApplicableLicenses(rand.nextBoolean())
         .incompatibleDepsetForLibrariesToLinkGetter(rand.nextBoolean())
-        .incompatibleDepsetUnion(rand.nextBoolean())
         .incompatibleDisableTargetProviderFields(rand.nextBoolean())
         .incompatibleDisableDeprecatedAttrParams(rand.nextBoolean())
         .incompatibleDisableDepsetItems(rand.nextBoolean())

src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java

@@ -275,11 +275,11 @@ public void testTransitiveOrderDirect() throws Exception {
 
   @Test
   public void testIncompatibleUnion() throws Exception {
-    new Scenario("--incompatible_depset_union=true")
-        .testIfErrorContains("`+` operator on a depset is forbidden", "depset([]) + ['a']");
+    new Scenario()
+        .testIfErrorContains("unsupported binary operation: depset + list", "depset([]) + ['a']");
 
-    new Scenario("--incompatible_depset_union=true")
-        .testIfErrorContains("`|` operator on a depset is forbidden", "depset([]) | ['a']");
+    new Scenario()
+        .testIfErrorContains("unsupported binary operation: depset | list", "depset([]) | ['a']");
   }
 
   private void assertContainsInOrder(String statement, Object... expectedElements)
@@ -289,128 +289,19 @@ private void assertContainsInOrder(String statement, Object... expectedElements)
         .inOrder();
   }
 
-  @Test
-  public void testUnionOrder() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
-    exec(
-        "def func():",
-        "  s1 = depset()",
-        "  s2 = depset()",
-        "  s1 += ['a']",
-        "  s2 += ['b']",
-        "  s1 += s2",
-        "  return s1",
-        "s = func()");
-    assertThat(get("s").toCollection()).containsExactly("b", "a").inOrder();
-  }
-
-  @Test
-  public void testUnionIncompatibleOrder() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
-    checkEvalError(
-        "Order mismatch: topological != postorder",
-        "depset(['a', 'b'], order='postorder') + depset(['c', 'd'], order='topological')");
-  }
-
-  @Test
-  public void testFunctionReturnsDepset() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
-    exec(
-        "def func():", //
-        "  t = depset()",
-        "  t += ['a']",
-        "  return t",
-        "s = func()");
-    assertThat(get("s")).isInstanceOf(Depset.class);
-    assertThat(get("s").toCollection()).containsExactly("a");
-  }
-
-  @Test
-  public void testPlusEqualsWithList() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
-    exec(
-        "def func():", //
-        "  t = depset()",
-        "  t += ['a', 'b']",
-        "  return t",
-        "s = func()");
-    assertThat(get("s").toCollection()).containsExactly("a", "b").inOrder();
-  }
-
-  @Test
-  public void testPlusEqualsNoSideEffects() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
-    exec(
-        "def func():",
-        "  s1 = depset()",
-        "  s1 += ['a']",
-        "  s2 = s1",
-        "  s2 += ['b']",
-        "  return s1",
-        "s = func()");
-    assertThat(get("s").toCollection()).containsExactly("a");
-  }
-
-  @Test
-  public void testFuncParamNoSideEffects() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
-    exec(
-        "def func1(t):",
-        "  t += ['b']",
-        "def func2():",
-        "  u = depset()",
-        "  u += ['a']",
-        "  func1(u)",
-        "  return u",
-        "s = func2()");
-    assertThat(get("s").toCollection()).containsExactly("a");
-  }
-
-  @Test
-  public void testTransitiveOrdering() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
-    exec(
-        "def func():",
-        "  sa = depset(['a'], order='postorder')",
-        "  sb = depset(['b'], order='postorder')",
-        "  sc = depset(['c'], order='postorder') + sa",
-        "  return depset() + sb + sc",
-        "s = func()");
-    // The iterator lists the Transitive sets first
-    assertThat(get("s").toCollection()).containsExactly("b", "a", "c").inOrder();
-  }
-
-  @Test
-  public void testLeftRightDirectOrdering() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
-    exec(
-        "def func():",
-        "  t = depset()",
-        "  t += [4]",
-        "  t += [2, 4]",
-        "  t += [3, 4, 5]",
-        "  return t",
-        "s = func()");
-    // All elements are direct. The iterator lists them left-to-right.
-    assertThat(get("s").toCollection()).containsExactly(4, 2, 3, 5).inOrder();
-  }
-
   @Test
   public void testToString() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
-    exec(
-        "s = depset() + [2, 4, 6] + [3, 4, 5]", //
-        "x = str(s)");
+    exec("s = depset([3, 4, 5], transitive = [depset([2, 4, 6])])", "x = str(s)");
     assertThat(lookup("x")).isEqualTo("depset([2, 4, 6, 3, 5])");
   }
 
   @Test
   public void testToStringWithOrder() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
     exec(
-        "s = depset(order = 'topological') + [2, 4, 6] + [3, 4, 5]", //
+        "s = depset([3, 4, 5], transitive = [depset([2, 4, 6])], ",
+        "           order = 'topological')",
         "x = str(s)");
-    assertThat(lookup("x")).isEqualTo("depset([2, 4, 6, 3, 5], order = \"topological\")");
+    assertThat(lookup("x")).isEqualTo("depset([3, 5, 6, 4, 2], order = \"topological\")");
   }
 
   private Depset get(String varname) throws Exception {
@@ -419,10 +310,8 @@ private Depset get(String varname) throws Exception {
 
   @Test
   public void testToList() throws Exception {
-    setSemantics("--incompatible_depset_union=false");
-    exec(
-        "s = depset() + [2, 4, 6] + [3, 4, 5]", //
-        "x = s.to_list()");
+    setSemantics();
+    exec("s = depset([3, 4, 5], transitive = [depset([2, 4, 6])])", "x = s.to_list()");
     Object value = lookup("x");
     assertThat(value).isInstanceOf(StarlarkList.class);
     assertThat((Iterable<?>) value).containsExactly(2, 4, 6, 3, 5).inOrder();

src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java

@@ -1448,14 +1448,6 @@ public void testClassObjectAccess() throws Exception {
         .testLookup("v", "a");
   }
 
-  @Test
-  public void testUnionSet() throws Exception {
-    new Scenario("--incompatible_depset_union=false")
-        .testExpression("str(depset([1, 3]) | depset([1, 2]))", "depset([1, 2, 3])")
-        .testExpression("str(depset([1, 2]) | [1, 3])", "depset([1, 2, 3])")
-        .testIfExactError("unsupported binary operation: int | bool", "2 | False");
-  }
-
   @Test
   public void testSetIsNotIterable() throws Exception {
     new Scenario()