New flag `--incompatible_depset_union` to disable old-style depset merging.

RELNOTES:
  Using `+`, `|` or `.union` on depsets is now deprecated. Please use the new
  constructor instead (see https://docs.bazel.build/versions/master/skylark/depsets.html).
PiperOrigin-RevId: 178262338

5 files changed, 55 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);