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.

It's still allowed to have `set` in parts of the code that are not executed, this will be deprecated later. You can check if your code is compatible with this future change by using the flag --incompatible_disallow_uncalled_set_constructor (currently the default is "false").

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

PiperOrigin-RevId: 176491641

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,22 @@ guarded behind flags in the current release:
 *   [New actions API](#new-actions-api)
 *   [Checked arithmetic](#checked-arithmetic)
 
+### Set constructor
+
+To maintain a clear distinction between the specialized [`depset`](depsets.md)
+data structure and Python's native `set` datatype (which does not currently
+exist in Skylark), the `set` constructor has been superseded by `depset`. It is
+no longer allowed to run code that calls the old `set` constructor.
+
+However, for a limited time, it will not be an error to reference the `set`
+constructor from code that is not executed (e.g. a function that is never
+called). Enable this flag to confirm that your code does not still refer to the
+old `set` constructor from unexecuted code.
+
+*   Flag: `--incompatible_disallow_uncalled_set_constructor`
+*   Default: `false`
+
+
 ### Keyword-only arguments
 
 Keyword-only parameters are parameters that can be called only using their name.

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

@@ -48,6 +48,7 @@ public void serialize(SkylarkSemantics semantics, CodedOutputStream codedOut)
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowDictPlus());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowKeywordOnlyArgs());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowToplevelIfStatement());
+    codedOut.writeBoolNoTag(semantics.incompatibleDisallowUncalledSetConstructor());
     codedOut.writeBoolNoTag(semantics.incompatibleListPlusEqualsInplace());
     codedOut.writeBoolNoTag(semantics.incompatibleLoadArgumentIsLabel());
     codedOut.writeBoolNoTag(semantics.incompatibleNewActionsApi());
@@ -70,6 +71,7 @@ public SkylarkSemantics deserialize(CodedInputStream codedIn)
     builder.incompatibleDisallowDictPlus(codedIn.readBool());
     builder.incompatibleDisallowKeywordOnlyArgs(codedIn.readBool());
     builder.incompatibleDisallowToplevelIfStatement(codedIn.readBool());
+    builder.incompatibleDisallowUncalledSetConstructor(codedIn.readBool());
     builder.incompatibleListPlusEqualsInplace(codedIn.readBool());
     builder.incompatibleLoadArgumentIsLabel(codedIn.readBool());
     builder.incompatibleNewActionsApi(codedIn.readBool());

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

@@ -158,6 +158,17 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   )
   public boolean incompatibleDisallowToplevelIfStatement;
 
+  @Option(
+    name = "incompatible_disallow_uncalled_set_constructor",
+    defaultValue = "false",
+    category = "incompatible changes",
+    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+    effectTags = {OptionEffectTag.UNKNOWN},
+    metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+    help = "If set to true, it's not allowed to use `set()` even if that code is never executed."
+  )
+  public boolean incompatibleDisallowUncalledSetConstructor;
+
   @Option(
     name = "incompatible_list_plus_equals_inplace",
     defaultValue = "true",
@@ -244,6 +255,7 @@ public SkylarkSemantics toSkylarkSemantics() {
         .incompatibleDisallowDictPlus(incompatibleDisallowDictPlus)
         .incompatibleDisallowKeywordOnlyArgs(incompatibleDisallowKeywordOnlyArgs)
         .incompatibleDisallowToplevelIfStatement(incompatibleDisallowToplevelIfStatement)
+        .incompatibleDisallowUncalledSetConstructor(incompatibleDisallowUncalledSetConstructor)
         .incompatibleListPlusEqualsInplace(incompatibleListPlusEqualsInplace)
         .incompatibleLoadArgumentIsLabel(incompatibleLoadArgumentIsLabel)
         .incompatibleNewActionsApi(incompatibleNewActionsApi)

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

@@ -174,36 +174,6 @@ private static boolean isEmptySkylarkList(Object o) {
     return o instanceof SkylarkList && ((SkylarkList) o).isEmpty();
   }
 
-  @SkylarkSignature(
-    name = "set",
-    returnType = SkylarkNestedSet.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
-  )
-  private static final BuiltinFunction set =
-      new BuiltinFunction("set") {
-        public SkylarkNestedSet invoke(Object items, String order, Location loc)
-            throws EvalException {
-          throw new EvalException(loc, "The function 'set' has been removed in favor of 'depset'.");
-        }
-      };
-
   @SkylarkSignature(
     name = "union",
     objectType = SkylarkNestedSet.class,
@@ -285,7 +255,7 @@ public Object invoke(SkylarkDict<?, ?> dict, String noMatchError, Location loc)
       };
 
   private static Environment.Frame createGlobals() {
-    List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, set, type);
+    List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, type);
 
     try (Mutability mutability = Mutability.create("BUILD")) {
       Environment env = Environment.builder(mutability)

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

@@ -94,6 +94,13 @@ EvalException createInvalidIdentifierException(Set<String> symbols) {
     if (name.equals("$error$")) {
       return new EvalException(getLocation(), "contains syntax error(s)", true);
     }
+    if (name.equals("set")) {
+      //TODO(vladmos): Remove as soon as the flag is removed
+      return new EvalException(getLocation(),
+          "The function 'set' has been removed in favor of 'depset', please use the latter. "
+              + "You can temporarily refer to the old 'set' constructor from unexecuted code "
+              + "by using --incompatible_disallow_uncalled_set_constructor=false");
+    }
     String suggestion = SpellChecker.didYouMean(name, symbols);
     return new EvalException(getLocation(), "name '" + name + "' is not defined" + suggestion);
   }

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

@@ -47,6 +47,7 @@ public abstract class SkylarkSemantics {
   public abstract boolean incompatibleDisallowDictPlus();
   public abstract boolean incompatibleDisallowKeywordOnlyArgs();
   public abstract boolean incompatibleDisallowToplevelIfStatement();
+  public abstract boolean incompatibleDisallowUncalledSetConstructor();
   public abstract boolean incompatibleListPlusEqualsInplace();
   public abstract boolean incompatibleLoadArgumentIsLabel();
   public abstract boolean incompatibleNewActionsApi();
@@ -68,6 +69,7 @@ public static Builder builder() {
       .incompatibleDisallowDictPlus(false)
       .incompatibleDisallowKeywordOnlyArgs(true)
       .incompatibleDisallowToplevelIfStatement(true)
+      .incompatibleDisallowUncalledSetConstructor(false)
       .incompatibleListPlusEqualsInplace(true)
       .incompatibleLoadArgumentIsLabel(false)
       .incompatibleNewActionsApi(false)
@@ -89,6 +91,7 @@ public abstract static class Builder {
     public abstract Builder incompatibleDisallowDictPlus(boolean value);
     public abstract Builder incompatibleDisallowKeywordOnlyArgs(boolean value);
     public abstract Builder incompatibleDisallowToplevelIfStatement(boolean value);
+    public abstract Builder incompatibleDisallowUncalledSetConstructor(boolean value);
     public abstract Builder incompatibleListPlusEqualsInplace(boolean value);
     public abstract Builder incompatibleLoadArgumentIsLabel(boolean value);
     public abstract Builder incompatibleNewActionsApi(boolean value);

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

@@ -68,6 +68,12 @@ private static class ValidationException extends RuntimeException {
     block.variables.addAll(builtinVariables);
     block.readOnlyVariables.addAll(builtinVariables);
     semantics = env.getSemantics();
+
+    // If the flag is set to false, it should be allowed to have `set`
+    // in non-executable parts of the code.
+    if (!env.getSemantics().incompatibleDisallowUncalledSetConstructor()) {
+      block.variables.add("set");
+    }
   }
 
   @Override

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

@@ -113,13 +113,13 @@ private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Ex
         "--incompatible_disallow_dict_plus=" + rand.nextBoolean(),
         "--incompatible_disallow_keyword_only_args=" + rand.nextBoolean(),
         "--incompatible_disallow_toplevel_if_statement=" + rand.nextBoolean(),
+        "--incompatible_disallow_uncalled_set_constructor=" + rand.nextBoolean(),
         "--incompatible_list_plus_equals_inplace=" + rand.nextBoolean(),
         "--incompatible_load_argument_is_label=" + rand.nextBoolean(),
         "--incompatible_new_actions_api=" + rand.nextBoolean(),
         "--incompatible_string_is_not_iterable=" + rand.nextBoolean(),
         "--internal_do_not_export_builtins=" + rand.nextBoolean(),
-        "--internal_skylark_flag_test_canary=" + rand.nextBoolean()
-    );
+        "--internal_skylark_flag_test_canary=" + rand.nextBoolean());
   }
 
   /**
@@ -137,6 +137,7 @@ private static SkylarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleDisallowDictPlus(rand.nextBoolean())
         .incompatibleDisallowKeywordOnlyArgs(rand.nextBoolean())
         .incompatibleDisallowToplevelIfStatement(rand.nextBoolean())
+        .incompatibleDisallowUncalledSetConstructor(rand.nextBoolean())
         .incompatibleListPlusEqualsInplace(rand.nextBoolean())
         .incompatibleLoadArgumentIsLabel(rand.nextBoolean())
         .incompatibleNewActionsApi(rand.nextBoolean())

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

@@ -37,15 +37,27 @@ public class SkylarkNestedSetTest extends EvaluationTestCase {
 
   @Test
   public void testLegacyConstructorNotCalled() throws Exception {
-    env = newEnvironmentWithSkylarkOptions();
+    env =
+        newEnvironmentWithSkylarkOptions("--incompatible_disallow_uncalled_set_constructor=false");
     eval("s = set([1, 2]) if False else depset([3, 4])");
     SkylarkNestedSet s = get("s");
     assertThat(s.getSet(Object.class)).containsExactly(3, 4);
+
+    // Static check are only enabled in .bzl files
+    new SkylarkTest("--incompatible_disallow_uncalled_set_constructor=true")
+        .testIfErrorContains("The function 'set' has been removed in favor of 'depset'",
+            "s = set([1, 2]) if False else depset([3, 4])");
+    new BuildTest("--incompatible_disallow_uncalled_set_constructor=true")
+        .testEval("s = set([1, 2]) if False else depset([3, 4]); s.to_list()", "[3, 4]");
   }
 
   @Test
   public void testLegacyConstructorCalled() throws Exception {
-    new BothModesTest()
+    new BothModesTest("--incompatible_disallow_uncalled_set_constructor=false")
+        .testIfErrorContains("The function 'set' has been removed in favor of 'depset'",
+            "s = set([1, 2])");
+
+    new BothModesTest("--incompatible_disallow_uncalled_set_constructor=true")
         .testIfErrorContains("The function 'set' has been removed in favor of 'depset'",
             "s = set([1, 2])");
   }