Skip to content

Commit

Permalink
Implement a flag to deprecate old constructor for depsets (set)
Browse files Browse the repository at this point in the history
Usage: --incompatible_depset_constructor=true (the default value is false).
PiperOrigin-RevId: 154971526
  • Loading branch information
vladmos authored and damienmg committed May 4, 2017
1 parent 9688e24 commit a6ad2d5
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 12 deletions.
Expand Up @@ -119,12 +119,21 @@ public SkylarkNestedSet invoke(Object items, String order, Location loc)
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) {
Expand Down
Expand Up @@ -42,4 +42,12 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
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;
}
Expand Up @@ -43,6 +43,17 @@ public void testLegacyConstructor() throws Exception {
assertThat(s.getSet(Object.class)).containsExactly(1, 2, 3);
}

@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')");
Expand Down
Expand Up @@ -92,19 +92,25 @@ public Environment newSkylarkEnvironment() {
}

/**
* 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);
}

/**
Expand Down
24 changes: 19 additions & 5 deletions src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
Expand Up @@ -17,33 +17,47 @@
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();
}
};

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;
}

0 comments on commit a6ad2d5

Please sign in to comment.