Automated rollback of commit 1b98de65873054b148ced772cfa827a7bfb5ad9a.

*** Reason for rollback ***

If the 'set' function was used in a .bzl file but not called, --incompatible_disallow_set_constructor=True would allow the load of that .bzl file without error, but this change removes the 'set' function so loading that bzl file is an error.
Example failure: https://ci.bazel.io/blue/organizations/jenkins/Global%2FTensorFlow/detail/TensorFlow/245/pipeline/

*** Original change description ***

Remove the deprecated set constructor from Skylark

The `set` constructor used to be deprecated, but it was still possible to use
it by providing --incompatible_disallow_set_constructor=false.

RELNOTES[INC]: The flag --incompatible_disallow_set_constructor is no longer
available, the deprecated `set` constructor is not available anymore.

PiperOrigin-RevId: 173115983

10 files changed, 173 insertions, 11 deletions

diff --git a/site/docs/skylark/backward-compatibility.md b/site/docs/skylark/backward-compatibility.md
index cb9991040d..032588b437 100644
--- a/site/docs/skylark/backward-compatibility.md
+++ b/site/docs/skylark/backward-compatibility.md
@@ -30,6 +30,7 @@ To check if your code will be compatible with future releases you can:
 The following are the backward incompatible changes that are implemented and
 guarded behind flags in the current release:
 
+*   [Set constructor](#set-constructor)
 *   [Keyword-only arguments](#keyword-only-arguments)
 *   [Mutating `+=`](#mutating)
 *   [Dictionary concatenation](#dictionary-concatenation)
@@ -43,6 +44,18 @@ guarded behind flags in the current release:
 *   [New actions API](#new-actions-api)
 *   [Checked arithmetic](#checked-arithmetic)
 
+### Set constructor
+
+We are removing the `set` constructor. Use `depset` instead. `set` and `depset`
+are equivalent, you just need to do search and replace to update the old code.
+
+We are doing this to reduce confusion between the specialized
+[depset](depsets.md) data structure and Python's set datatype.
+
+*   Flag: `--incompatible_disallow_set_constructor`
+*   Default: `true`
+
+
 ### Keyword-only arguments
 
 Keyword-only parameters are parameters that can be called only using their name.
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/Order.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/Order.java
index 6b6fde0927..05cc7a1b1a 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/Order.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/Order.java
@@ -102,20 +102,23 @@ import java.util.HashMap;
  * dependencies-after-parent ordering. Note that the latter is usually more important, so please use
  * LINK_ORDER whenever possible.
  */
-// TODO(bazel-team): Remove deprecated names from the documentation above.
+// TODO(bazel-team): Remove deprecatedSkylarkName and it's associated helpers before Bazel 1.0.
 public enum Order {
-  STABLE_ORDER("default"),
-  COMPILE_ORDER("postorder"),
-  LINK_ORDER("topological"),
-  NAIVE_LINK_ORDER("preorder");
+  STABLE_ORDER("default", "stable"),
+  COMPILE_ORDER("postorder", "compile"),
+  LINK_ORDER("topological", "link"),
+  NAIVE_LINK_ORDER("preorder", "naive_link");
 
   private static final ImmutableMap<String, Order> VALUES;
+  private static final ImmutableMap<String, Order> DEPRECATED_VALUES;
 
   private final String skylarkName;
+  private final String deprecatedSkylarkName;
   private final NestedSet<?> emptySet;
 
-  private Order(String skylarkName) {
+  private Order(String skylarkName, String deprecatedSkylarkName) {
     this.skylarkName = skylarkName;
+    this.deprecatedSkylarkName = deprecatedSkylarkName;
     this.emptySet = new NestedSet<>(this);
   }
 
@@ -131,22 +134,47 @@ public enum Order {
     return skylarkName;
   }
 
+  public String getDeprecatedSkylarkName() {
+    return deprecatedSkylarkName;
+  }
+
   /**
    * Parses the given string as a nested set order
    *
    * @param name unique name of the order
+   * @param forbidDeprecatedOrderNames if true, old style ordering names will be rejected
    * @return the appropriate order instance
    * @throws IllegalArgumentException if the name is not valid
    */
-  public static Order parse(String name) {
+  public static Order parse(String name, boolean forbidDeprecatedOrderNames) {
     if (VALUES.containsKey(name)) {
       return VALUES.get(name);
+    } else if (DEPRECATED_VALUES.containsKey(name)) {
+      if (forbidDeprecatedOrderNames) {
+        throw new IllegalArgumentException(String.format(
+            "Order name '%s' is deprecated, use '%s' instead",
+            name,
+            DEPRECATED_VALUES.get(name).getSkylarkName()
+        ));
+      }
+      return DEPRECATED_VALUES.get(name);
     } else {
       throw new IllegalArgumentException("Invalid order: " + name);
     }
   }
 
   /**
+   * Parses the given string as a nested set order
+   *
+   * @param name unique name of the order
+   * @return the appropriate order instance
+   * @throws IllegalArgumentException if the name is not valid
+   */
+  public static Order parse(String name) {
+    return parse(name, false);
+  }
+
+  /**
    * Determines whether two orders are considered compatible.
    *
    * <p>An order is compatible with itself (reflexivity) and all orders are compatible with
@@ -163,11 +191,14 @@ public enum Order {
     Order[] tmpValues = Order.values();
 
     HashMap<String, Order> entries = Maps.newHashMapWithExpectedSize(tmpValues.length);
+    HashMap<String, Order> deprecatedEntries = Maps.newHashMapWithExpectedSize(tmpValues.length);
 
     for (Order current : tmpValues) {
       entries.put(current.getSkylarkName(), current);
+      deprecatedEntries.put(current.getDeprecatedSkylarkName(), current);
     }
 
     VALUES = ImmutableMap.copyOf(entries);
+    DEPRECATED_VALUES = ImmutableMap.copyOf(deprecatedEntries);
   }
 }
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 52db1ce009..e36eca2770 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
@@ -47,6 +47,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics
     codedOut.writeBoolNoTag(semantics.incompatibleDictLiteralHasNoDuplicates());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowDictPlus());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowKeywordOnlyArgs());
+    codedOut.writeBoolNoTag(semantics.incompatibleDisallowSetConstructor());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowToplevelIfStatement());
     codedOut.writeBoolNoTag(semantics.incompatibleListPlusEqualsInplace());
     codedOut.writeBoolNoTag(semantics.incompatibleLoadArgumentIsLabel());
@@ -69,6 +70,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics
     builder.incompatibleDictLiteralHasNoDuplicates(codedIn.readBool());
     builder.incompatibleDisallowDictPlus(codedIn.readBool());
     builder.incompatibleDisallowKeywordOnlyArgs(codedIn.readBool());
+    builder.incompatibleDisallowSetConstructor(codedIn.readBool());
     builder.incompatibleDisallowToplevelIfStatement(codedIn.readBool());
     builder.incompatibleListPlusEqualsInplace(codedIn.readBool());
     builder.incompatibleLoadArgumentIsLabel(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 0f8e0c4ad2..2a7220de1d 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
@@ -146,6 +146,17 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   public boolean incompatibleDisallowKeywordOnlyArgs;
 
   @Option(
+    name = "incompatible_disallow_set_constructor",
+    defaultValue = "true",
+    category = "incompatible changes",
+    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+    effectTags = {OptionEffectTag.UNKNOWN},
+    metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+    help = "If set to true, disables the deprecated `set` constructor for depsets."
+  )
+  public boolean incompatibleDisallowSetConstructor;
+
+  @Option(
     name = "incompatible_disallow_toplevel_if_statement",
     defaultValue = "true",
     category = "incompatible changes",
@@ -243,6 +254,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
         .incompatibleDictLiteralHasNoDuplicates(incompatibleDictLiteralHasNoDuplicates)
         .incompatibleDisallowDictPlus(incompatibleDisallowDictPlus)
         .incompatibleDisallowKeywordOnlyArgs(incompatibleDisallowKeywordOnlyArgs)
+        .incompatibleDisallowSetConstructor(incompatibleDisallowSetConstructor)
         .incompatibleDisallowToplevelIfStatement(incompatibleDisallowToplevelIfStatement)
         .incompatibleListPlusEqualsInplace(incompatibleListPlusEqualsInplace)
         .incompatibleLoadArgumentIsLabel(incompatibleLoadArgumentIsLabel)
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..20111cedd6 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
@@ -114,7 +114,8 @@ public class BazelLibrary {
         defaultValue = "None"
       )
     },
-    useLocation = true
+    useLocation = true,
+    useEnvironment = true
   )
   private static final BuiltinFunction depset =
       new BuiltinFunction("depset") {
@@ -123,11 +124,13 @@ public class BazelLibrary {
             String orderString,
             Object direct,
             Object transitive,
-            Location loc)
+            Location loc,
+            Environment env)
             throws EvalException {
           Order order;
           try {
-            order = Order.parse(orderString);
+            order = Order.parse(
+                orderString, env.getSemantics().incompatibleDisallowSetConstructor());
           } catch (IllegalArgumentException ex) {
             throw new EvalException(loc, ex);
           }
@@ -175,6 +178,52 @@ public class BazelLibrary {
   }
 
   @SkylarkSignature(
+    name = "set",
+    returnType = SkylarkNestedSet.class,
+    documentationReturnType = SkylarkNestedSet.LegacySet.class,
+    doc =
+        "A temporary alias for <a href=\"#depset\">depset</a>. "
+            + "Deprecated in favor of <code>depset</code>.",
+    parameters = {
+      @Param(
+        name = "items",
+        type = Object.class,
+        defaultValue = "[]",
+        doc = "Same as for <a href=\"#depset\">depset</a>."
+      ),
+      @Param(
+        name = "order",
+        type = String.class,
+        defaultValue = "\"default\"",
+        doc = "Same as for <a href=\"#depset\">depset</a>."
+      )
+    },
+    useLocation = true,
+    useEnvironment = true
+  )
+  private static final BuiltinFunction set =
+      new BuiltinFunction("set") {
+        public SkylarkNestedSet invoke(Object items, String order, Location loc, Environment env)
+            throws EvalException {
+          if (env.getSemantics().incompatibleDisallowSetConstructor()) {
+            throw new EvalException(
+                loc,
+                "The `set` constructor for depsets is deprecated and will be removed. Please use "
+                    + "the `depset` constructor instead. You can temporarily enable the "
+                    + "deprecated `set` constructor by passing the flag "
+                    + "--incompatible_disallow_set_constructor=false");
+          }
+          try {
+            return new SkylarkNestedSet(
+                Order.parse(order, /*forbidDeprecatedOrderNames=*/false),
+                items, loc);
+          } catch (IllegalArgumentException ex) {
+            throw new EvalException(loc, ex);
+          }
+        }
+      };
+
+  @SkylarkSignature(
     name = "union",
     objectType = SkylarkNestedSet.class,
     returnType = SkylarkNestedSet.class,
@@ -255,7 +304,7 @@ public class BazelLibrary {
       };
 
   private static Environment.Frame createGlobals() {
-    List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, type);
+    List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, set, type);
 
     try (Mutability mutability = Mutability.create("BUILD")) {
       Environment env = Environment.builder(mutability)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
index f72f689d57..2915a8db83 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
@@ -92,6 +92,20 @@ public final class SkylarkNestedSet implements SkylarkValue, SkylarkQueryable {
   @Nullable
   private final List<NestedSet> transitiveItems;
 
+  // Dummy class used to create a documentation for the deprecated `set` type
+  // TODO(bazel-team): remove before the end of 2017
+  @SkylarkModule(
+      name = "set",
+      category = SkylarkModuleCategory.BUILTIN,
+      doc = "A deprecated alias for <a href=\"depset.html\">depset</a>. "
+          + "Please use <a href=\"depset.html\">depset</a> instead. "
+          + "If you need a hash set that supports O(1) membership testing "
+          + "consider using a <a href=\"dict.html\">dict</a>."
+  )
+  static final class LegacySet {
+    private LegacySet() {}
+  }
+
   public SkylarkNestedSet(Order order, Object item, Location loc) throws EvalException {
     this(order, SkylarkType.TOP, item, loc, null);
   }
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 105768fd3d..8d1fa59f4e 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
@@ -46,6 +46,7 @@ public abstract class SkylarkSemantics {
   public abstract boolean incompatibleDictLiteralHasNoDuplicates();
   public abstract boolean incompatibleDisallowDictPlus();
   public abstract boolean incompatibleDisallowKeywordOnlyArgs();
+  public abstract boolean incompatibleDisallowSetConstructor();
   public abstract boolean incompatibleDisallowToplevelIfStatement();
   public abstract boolean incompatibleListPlusEqualsInplace();
   public abstract boolean incompatibleLoadArgumentIsLabel();
@@ -67,6 +68,7 @@ public abstract class SkylarkSemantics {
       .incompatibleDictLiteralHasNoDuplicates(true)
       .incompatibleDisallowDictPlus(false)
       .incompatibleDisallowKeywordOnlyArgs(true)
+      .incompatibleDisallowSetConstructor(true)
       .incompatibleDisallowToplevelIfStatement(true)
       .incompatibleListPlusEqualsInplace(true)
       .incompatibleLoadArgumentIsLabel(false)
@@ -88,6 +90,7 @@ public abstract class SkylarkSemantics {
     public abstract Builder incompatibleDictLiteralHasNoDuplicates(boolean value);
     public abstract Builder incompatibleDisallowDictPlus(boolean value);
     public abstract Builder incompatibleDisallowKeywordOnlyArgs(boolean value);
+    public abstract Builder incompatibleDisallowSetConstructor(boolean value);
     public abstract Builder incompatibleDisallowToplevelIfStatement(boolean value);
     public abstract Builder incompatibleListPlusEqualsInplace(boolean value);
     public abstract Builder incompatibleLoadArgumentIsLabel(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/collect/nestedset/OrderTest.java b/src/test/java/com/google/devtools/build/lib/collect/nestedset/OrderTest.java
index 00d8ca7551..30ef309089 100644
--- a/src/test/java/com/google/devtools/build/lib/collect/nestedset/OrderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/collect/nestedset/OrderTest.java
@@ -30,6 +30,7 @@ public class OrderTest {
   public void testParsing() throws Exception {
     for (Order current : Order.values()) {
       assertThat(Order.parse(current.getSkylarkName())).isEqualTo(current);
+      assertThat(Order.parse(current.getDeprecatedSkylarkName())).isEqualTo(current);
     }
   }
 
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 f0f203e142..3e1708f3dd 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
@@ -112,6 +112,7 @@ public class SkylarkSemanticsConsistencyTest {
         "--incompatible_dict_literal_has_no_duplicates=" + rand.nextBoolean(),
         "--incompatible_disallow_dict_plus=" + rand.nextBoolean(),
         "--incompatible_disallow_keyword_only_args=" + rand.nextBoolean(),
+        "--incompatible_disallow_set_constructor=" + rand.nextBoolean(),
         "--incompatible_disallow_toplevel_if_statement=" + rand.nextBoolean(),
         "--incompatible_list_plus_equals_inplace=" + rand.nextBoolean(),
         "--incompatible_load_argument_is_label=" + rand.nextBoolean(),
@@ -136,6 +137,7 @@ public class SkylarkSemanticsConsistencyTest {
         .incompatibleDictLiteralHasNoDuplicates(rand.nextBoolean())
         .incompatibleDisallowDictPlus(rand.nextBoolean())
         .incompatibleDisallowKeywordOnlyArgs(rand.nextBoolean())
+        .incompatibleDisallowSetConstructor(rand.nextBoolean())
         .incompatibleDisallowToplevelIfStatement(rand.nextBoolean())
         .incompatibleListPlusEqualsInplace(rand.nextBoolean())
         .incompatibleLoadArgumentIsLabel(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 c93311d3cd..d61cb8ab16 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
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.syntax;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.expectThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -36,6 +37,25 @@ import org.junit.runners.JUnit4;
 public class SkylarkNestedSetTest extends EvaluationTestCase {
 
   @Test
+  public void testLegacyConstructor() throws Exception {
+    env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_set_constructor=false");
+    eval("s = set([1, 2, 3], order='postorder')");
+    SkylarkNestedSet s = get("s");
+    assertThat(s.getOrder().getSkylarkName()).isEqualTo("postorder");
+    assertThat(s.getSet(Object.class)).containsExactly(1, 2, 3);
+  }
+
+  @Test
+  public void testLegacyConstructorDeprecation() throws Exception {
+    env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_set_constructor=true");
+    EvalException e = expectThrows(
+        EvalException.class,
+        () -> eval("s = set([1, 2, 3], order='postorder')")
+    );
+    assertThat(e).hasMessageThat().contains("The `set` constructor for depsets is deprecated");
+  }
+
+  @Test
   public void testConstructor() throws Exception {
     eval("s = depset(order='default')");
     assertThat(lookup("s")).isInstanceOf(SkylarkNestedSet.class);
@@ -130,6 +150,21 @@ public class SkylarkNestedSetTest extends EvaluationTestCase {
   }
 
   @Test
+  public void testDeprecatedOrder() throws Exception {
+    env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_set_constructor=false");
+    eval("s = depset(['a', 'b'], order='compile')");
+    assertThat(get("s").getSet(String.class).getOrder()).isEqualTo(Order.COMPILE_ORDER);
+
+    env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_set_constructor=true");
+    Exception e = expectThrows(
+        Exception.class,
+        () -> eval("s = depset(['a', 'b'], order='compile')")
+    );
+    assertThat(e).hasMessageThat().contains(
+          "Order name 'compile' is deprecated, use 'postorder' instead");
+  }
+
+  @Test
   public void testBadOrder() throws Exception {
     new BothModesTest().testIfExactError(
         "Invalid order: non_existing",