diff options
author | 2017-08-03 02:12:48 +0200 | |
---|---|---|
committer | 2017-08-03 12:10:02 +0200 | |
commit | c331f24a3a8af5e19f2d603d0b77ca90923b3dc2 (patch) | |
tree | 43b0d089c7b19e1245defdc93c4e8d0629fa37f0 /src | |
parent | dfa86f48a0ee5f0f56b6602b5274691193fa1862 (diff) |
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
Diffstat (limited to 'src')
-rw-r--r-- | src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java | 23 | ||||
-rw-r--r-- | src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java | 30 |
2 files changed, 46 insertions, 7 deletions
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 562221df20..9d8c13a71d 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 @@ public final class InvocationPolicyEnforcer { 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 @@ public final class InvocationPolicyEnforcer { * 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 430366579a..1bb13cfea4 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 @@ public class InvocationPolicySetValueTest extends InvocationPolicyEnforcerTestBa } @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 |