Accept expansion flags expanding to expansion flags in invocation policy.
Fix bug where a null value in the expansion text lead to a NullPointerException.
RELNOTES: None.
PiperOrigin-RevId: 164061496
diff --git a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
index 562221d..9d8c13a 100644
--- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -349,7 +349,7 @@
repeatableSubflagsInSetValues.put(currentSubflag.getName(), currentSubflag);
} else {
FlagPolicy subflagAsPolicy =
- getSubflagAsPolicy(currentSubflag, originalPolicy, isExpansion);
+ getSingleValueSubflagAsPolicy(currentSubflag, originalPolicy, isExpansion);
// In case any of the expanded flags are themselves expansions, recurse.
expandedPolicies.addAll(expandPolicy(subflagAsPolicy, parser));
}
@@ -425,19 +425,28 @@
* For an expansion flag in an invocation policy, each flag it expands to must be given a
* corresponding policy.
*/
- private static FlagPolicy getSubflagAsPolicy(
+ private static FlagPolicy getSingleValueSubflagAsPolicy(
OptionValueDescription currentSubflag, FlagPolicy originalPolicy, boolean isExpansion)
throws OptionsParsingException {
FlagPolicy subflagAsPolicy = null;
switch (originalPolicy.getOperationCase()) {
case SET_VALUE:
- assert (!currentSubflag.getAllowMultiple());
+ if (currentSubflag.getAllowMultiple()) {
+ throw new AssertionError(
+ "SetValue subflags with allowMultiple should have been dealt with separately and "
+ + "accumulated into a single FlagPolicy.");
+ }
+ // Accept null originalValueStrings, they are expected when the subflag is also an expansion
+ // flag.
+ List<String> subflagValue;
+ if (currentSubflag.getOriginalValueString() == null) {
+ subflagValue = ImmutableList.of();
+ } else {
+ subflagValue = ImmutableList.of(currentSubflag.getOriginalValueString());
+ }
subflagAsPolicy =
getSetValueSubflagAsPolicy(
- currentSubflag.getName(),
- ImmutableList.of(currentSubflag.getOriginalValueString()),
- /*allowMultiple=*/ false,
- originalPolicy);
+ currentSubflag.getName(), subflagValue, /*allowMultiple=*/ false, originalPolicy);
break;
case USE_DEFAULT:
diff --git a/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java b/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java
index 4303665..1bb13cf 100644
--- a/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java
+++ b/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java
@@ -232,6 +232,36 @@
}
@Test
+ public void testSetValueWithExpansionFlagOnExpansionFlag() throws Exception {
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_recursive_expansion_top_level")
+ .getSetValueBuilder();
+
+
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+ // Unrelated flag, but --test_expansion is not set
+ parser.parse("--test_string=throwaway value");
+
+ // The flags that --test_expansion expands into should still be their default values
+ TestOptions testOptions = getTestOptions();
+ assertThat(testOptions.expandedA).isEqualTo(TestOptions.EXPANDED_A_DEFAULT);
+ assertThat(testOptions.expandedB).isEqualTo(TestOptions.EXPANDED_B_DEFAULT);
+ assertThat(testOptions.expandedC).isEqualTo(TestOptions.EXPANDED_C_DEFAULT);
+ assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT);
+
+ enforcer.enforce(parser, BUILD_COMMAND);
+
+ // After policy enforcement, the flags should be the values from the expansion flag
+ testOptions = getTestOptions();
+ assertThat(testOptions.expandedA).isEqualTo(TestOptions.EXPANDED_A_TEST_RECURSIVE_EXPANSION);
+ assertThat(testOptions.expandedB).isEqualTo(TestOptions.EXPANDED_B_TEST_RECURSIVE_EXPANSION);
+ assertThat(testOptions.expandedC).isEqualTo(TestOptions.EXPANDED_C_TEST_RECURSIVE_EXPANSION);
+ assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_TEST_RECURSIVE_EXPANSION);
+ }
+
+ @Test
public void testSetValueWithExpansionFunctionFlags() throws Exception {
InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
invocationPolicyBuilder
