Focus InvocationPolicyEnforcer API
Remove methods that seem to exist for syntactical convenience in tests, remove
some @Nullable from things that aren't null in practice, update some comments
and javadoc that have become outdated.
I think enforce's @Nullable command argument could also get the non-null
treatment, but deferring for now.
PiperOrigin-RevId: 453669814
Change-Id: I720af40bea89fb1e3fd2307d8ad0fd7108cd4a9b
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 63cd484..a0b21b6 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -184,7 +184,7 @@
private final ProjectFile.Provider projectFileProvider;
private final QueryRuntimeHelper.Factory queryRuntimeHelperFactory;
- @Nullable private final InvocationPolicy moduleInvocationPolicy;
+ private final InvocationPolicy moduleInvocationPolicy;
private final SubscriberExceptionHandler eventBusExceptionHandler;
private final BugReporter bugReporter;
private final String productName;
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 3e88a1f..fdeb901 100644
--- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -15,6 +15,7 @@
import static java.util.stream.Collectors.joining;
+import com.google.common.base.Preconditions;
import com.google.common.base.Verify;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
@@ -52,30 +53,19 @@
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
private static final String INVOCATION_POLICY_SOURCE = "Invocation policy";
- @Nullable private final InvocationPolicy invocationPolicy;
+ private final InvocationPolicy invocationPolicy;
private final Level loglevel;
/**
* Creates an InvocationPolicyEnforcer that enforces the given policy.
*
- * @param invocationPolicy the policy to enforce. A null policy means this enforcer will do
- * nothing in calls to enforce().
- */
- public InvocationPolicyEnforcer(@Nullable InvocationPolicy invocationPolicy) {
- this(invocationPolicy, Level.FINE);
- }
-
- /**
- * Creates an InvocationPolicyEnforcer that enforces the given policy.
- *
- * @param invocationPolicy the policy to enforce. A null policy means this enforcer will do
- * nothing in calls to enforce().
+ * @param invocationPolicy the policy to enforce.
* @param loglevel the level at which to log informational statements. Warnings and errors will
* still be logged at the appropriate level.
*/
- public InvocationPolicyEnforcer(@Nullable InvocationPolicy invocationPolicy, Level loglevel) {
- this.invocationPolicy = invocationPolicy;
- this.loglevel = loglevel;
+ public InvocationPolicyEnforcer(InvocationPolicy invocationPolicy, Level loglevel) {
+ this.invocationPolicy = Preconditions.checkNotNull(invocationPolicy);
+ this.loglevel = Preconditions.checkNotNull(loglevel);
}
private static final class FlagPolicyWithContext {
@@ -91,33 +81,18 @@
}
}
- @Nullable
- public InvocationPolicy getInvocationPolicy() {
- return invocationPolicy;
- }
-
/**
- * Applies this OptionsPolicyEnforcer's policy to the given OptionsParser for all blaze commands.
+ * Applies this instance's policy to the provided options parser.
*
* @param parser The OptionsParser to enforce policy on.
- * @throws OptionsParsingException if any flag policy is invalid.
- */
- public void enforce(OptionsParser parser) throws OptionsParsingException {
- enforce(parser, null);
- }
-
- /**
- * Applies this OptionsPolicyEnforcer's policy to the given OptionsParser.
- *
- * @param parser The OptionsParser to enforce policy on.
- * @param command The current blaze command, for flag policies that apply to only specific
- * commands. Such policies will be enforced only if they contain this command or a command
- * they inherit from
+ * @param command The blaze command to enforce the policy for, or null to apply all policies
+ * regardless of command. Flag policies that apply to specific commands will be enforced only
+ * if they contain this command or a command it inherits from.
* @throws OptionsParsingException if any flag policy is invalid.
*/
public void enforce(OptionsParser parser, @Nullable String command)
throws OptionsParsingException {
- if (invocationPolicy == null || invocationPolicy.getFlagPoliciesCount() == 0) {
+ if (invocationPolicy.getFlagPoliciesCount() == 0) {
return;
}
@@ -203,8 +178,7 @@
}
private static boolean policyApplies(FlagPolicy policy, ImmutableSet<String> applicableCommands) {
- // Skip the flag policy if it doesn't apply to this command. If the commands list is empty,
- // then the policy applies to all commands.
+ // If the commands list is empty, then the policy applies to all commands.
if (policy.getCommandsList().isEmpty() || applicableCommands.isEmpty()) {
return true;
}
@@ -232,7 +206,10 @@
* <p>Expands any policies on expansion flags.
*/
private static ImmutableList<FlagPolicyWithContext> getEffectivePolicies(
- InvocationPolicy invocationPolicy, OptionsParser parser, String command, Level loglevel)
+ InvocationPolicy invocationPolicy,
+ OptionsParser parser,
+ @Nullable String command,
+ Level loglevel)
throws OptionsParsingException {
if (invocationPolicy == null) {
return ImmutableList.of();
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java
index 721b462..d6f37e2 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java
@@ -81,6 +81,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.logging.Level;
/**
* A wrapper for {@link BlazeRuntime} for testing purposes that makes it possible to exercise (most)
@@ -184,9 +185,9 @@
private void enforceTestInvocationPolicy(OptionsParser parser) {
InvocationPolicyEnforcer optionsPolicyEnforcer =
- new InvocationPolicyEnforcer(runtime.getModuleInvocationPolicy());
+ new InvocationPolicyEnforcer(runtime.getModuleInvocationPolicy(), Level.FINE);
try {
- optionsPolicyEnforcer.enforce(parser);
+ optionsPolicyEnforcer.enforce(parser, /*command=*/ null);
} catch (OptionsParsingException e) {
throw new IllegalStateException(e);
}
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 5519fa9..ebc20ff 100644
--- a/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java
+++ b/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java
@@ -596,7 +596,7 @@
InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicy);
parser.parse();
- enforcer.enforce(parser);
+ enforcer.enforce(parser, /*command=*/ null);
assertThat(getTestOptions()).isEqualTo(Options.getDefaults(TestOptions.class));
}
