Implement a flag to deprecate old constructor for depsets (`set`)
Usage: --incompatible_depset_constructor=true (the default value is false).
PiperOrigin-RevId: 154971526
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 d7989b1..70f4b95 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
@@ -119,12 +119,21 @@
doc = "Same as for <a href=\"#depset\">depset</a>."
)
},
- useLocation = true
+ useLocation = true,
+ useEnvironment = true
)
private static final BuiltinFunction set =
new BuiltinFunction("set") {
- public SkylarkNestedSet invoke(Object items, String order, Location loc)
+ public SkylarkNestedSet invoke(Object items, String order, Location loc, Environment env)
throws EvalException {
+ if (env.getSemantics().incompatibleDepsetConstructor) {
+ 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_depset_constructor=false");
+ }
try {
return new SkylarkNestedSet(Order.parse(order), items, loc);
} catch (IllegalArgumentException ex) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
index eb1fd0d..5ed2643 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
@@ -42,4 +42,12 @@
optionUsageRestrictions = OptionUsageRestrictions.UNDOCUMENTED
)
public boolean skylarkFlagTestCanary;
+
+ @Option(
+ name = "incompatible_depset_constructor",
+ defaultValue = "false",
+ category = "incompatible changes",
+ help = "If set to true, disables the deprecated `set` constructor for depsets."
+ )
+ public boolean incompatibleDepsetConstructor;
}
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 8fab5eb..4a4cc0f 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
@@ -44,6 +44,17 @@
}
@Test
+ public void testLegacyConstructorDeprecation() throws Exception {
+ env = newEnvironmentWithSkylarkOptions("--incompatible_depset_constructor=true");
+ try {
+ eval("s = set([1, 2, 3], order='postorder')");
+ Assert.fail("`set` should have failed");
+ } catch (EvalException e) {
+ assertThat(e.getMessage()).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);
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
index d76e2eb..0aa9b0a 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
@@ -92,19 +92,25 @@
}
/**
- * Creates a new Environment suitable for the test case. Subclasses may override it
- * to fit their purpose and e.g. call newBuildEnvironment or newSkylarkEnvironment;
- * or they may play with the testMode to run tests in either or both kinds of Environment.
- * Note that all Environment-s may share the same Mutability, so don't close it.
+ * Creates a new Environment suitable for the test case. Subclasses may override it to fit their
+ * purpose and e.g. call newBuildEnvironment or newSkylarkEnvironment; or they may play with the
+ * testMode to run tests in either or both kinds of Environment. Note that all Environment-s may
+ * share the same Mutability, so don't close it.
+ *
* @return a fresh Environment.
*/
public Environment newEnvironment() throws Exception {
+ return newEnvironmentWithSkylarkOptions();
+ }
+
+ protected Environment newEnvironmentWithSkylarkOptions(String... skylarkOptions)
+ throws Exception {
if (testMode == null) {
throw new IllegalArgumentException(
"TestMode is null. Please set a Testmode via setMode() or set the "
+ "Environment manually by overriding newEnvironment()");
}
- return testMode.createEnvironment(getEventHandler(), null);
+ return testMode.createEnvironment(getEventHandler(), skylarkOptions);
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java b/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
index 21132c2..12b8e6b 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
@@ -17,19 +17,30 @@
import com.google.devtools.build.lib.syntax.BazelLibrary;
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.Mutability;
+import com.google.devtools.build.lib.syntax.SkylarkSemanticsOptions;
+import com.google.devtools.common.options.OptionsParser;
/**
* Describes a particular testing mode by determining how the
* appropriate {@code Environment} has to be created
*/
public abstract class TestMode {
+ private static SkylarkSemanticsOptions parseSkylarkSemanticsOptions(String... skylarkOptions)
+ throws Exception {
+ OptionsParser parser = OptionsParser.newOptionsParser(SkylarkSemanticsOptions.class);
+ parser.parse(skylarkOptions);
+ return parser.getOptions(SkylarkSemanticsOptions.class);
+ }
+
public static final TestMode BUILD =
new TestMode() {
@Override
- public Environment createEnvironment(EventHandler eventHandler, Environment environment) {
+ public Environment createEnvironment(EventHandler eventHandler, String... skylarkOptions)
+ throws Exception {
return Environment.builder(Mutability.create("build test"))
- .setGlobals(environment == null ? BazelLibrary.GLOBALS : environment.getGlobals())
+ .setGlobals(BazelLibrary.GLOBALS)
.setEventHandler(eventHandler)
+ .setSemantics(TestMode.parseSkylarkSemanticsOptions(skylarkOptions))
.build();
}
};
@@ -37,13 +48,16 @@
public static final TestMode SKYLARK =
new TestMode() {
@Override
- public Environment createEnvironment(EventHandler eventHandler, Environment environment) {
+ public Environment createEnvironment(EventHandler eventHandler, String... skylarkOptions)
+ throws Exception {
return Environment.builder(Mutability.create("skylark test"))
- .setGlobals(environment == null ? BazelLibrary.GLOBALS : environment.getGlobals())
+ .setGlobals(BazelLibrary.GLOBALS)
.setEventHandler(eventHandler)
+ .setSemantics(TestMode.parseSkylarkSemanticsOptions(skylarkOptions))
.build();
}
};
- public abstract Environment createEnvironment(EventHandler eventHandler, Environment environment);
+ public abstract Environment createEnvironment(EventHandler eventHandler, String... skylarkOptions)
+ throws Exception;
}
