Skip to content

Commit

Permalink
Allow expansion flags to have values.
Browse files Browse the repository at this point in the history
This lets us change what it expands based on the argument passed to the flag.

RELNOTES: Allows flags that expand to take values.
PiperOrigin-RevId: 1602984
  • Loading branch information
Googler authored and hlopko committed Jun 28, 2017
1 parent 1b2e451 commit 7c7255e
Show file tree
Hide file tree
Showing 16 changed files with 709 additions and 127 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.common.collect.ImmutableList;
import com.google.devtools.common.options.Converter;
import com.google.devtools.common.options.ExpansionContext;
import com.google.devtools.common.options.ExpansionFunction;
import com.google.devtools.common.options.IsolatedOptionsData;
import com.google.devtools.common.options.Option;
Expand Down Expand Up @@ -153,11 +154,11 @@ private static void validateIncompatibleChange(Field field, Option annotation) {
}

@Override
public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) {
public ImmutableList<String> getExpansion(ExpansionContext context) {
// Grab all registered options that are identified as incompatible changes by either name or
// by category. Ensure they satisfy our requirements.
ArrayList<String> incompatibleChanges = new ArrayList<>();
for (Map.Entry<String, Field> entry : optionsData.getAllNamedFields()) {
for (Map.Entry<String, Field> entry : context.getOptionsData().getAllNamedFields()) {
Field field = entry.getValue();
Option annotation = field.getAnnotation(Option.class);
if (annotation.name().startsWith(INCOMPATIBLE_NAME_PREFIX)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ public ExitCode exec(CommandEnvironment env, OptionsProvider options) {

// Print out the canonical invocation policy if requested.
if (canonicalizeOptions.canonicalizePolicy) {
List<FlagPolicy> effectiveFlagPolicies =
InvocationPolicyEnforcer.getEffectivePolicy(policy, parser, commandName);
ImmutableList<FlagPolicy> effectiveFlagPolicies =
InvocationPolicyEnforcer.getEffectivePolicies(policy, parser, commandName);
InvocationPolicy effectivePolicy =
InvocationPolicy.newBuilder().addAllFlagPolicies(effectiveFlagPolicies).build();
env.getReporter().getOutErr().printOutLn(effectivePolicy.toString());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright 2017 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.devtools.common.options;

import java.lang.reflect.Field;
import javax.annotation.Nullable;
import javax.annotation.concurrent.ThreadSafe;

/**
* Encapsulates the data given to {@link ExpansionFunction} objects. This lets {@link
* ExpansionFunction} objects change how it expands flags based on the arguments given to the {@link
* OptionsParser}.
*/
@ThreadSafe
public final class ExpansionContext {
private final IsolatedOptionsData optionsData;
private final Field field;
@Nullable private final String unparsedValue;

public ExpansionContext(
IsolatedOptionsData optionsData, Field field, @Nullable String unparsedValue) {
this.optionsData = optionsData;
this.field = field;
this.unparsedValue = unparsedValue;
}

/** Metadata for the option that is being expanded. */
public IsolatedOptionsData getOptionsData() {
return optionsData;
}

/** {@link Field} object for option that is being expanded. */
public Field getField() {
return field;
}

/** Argument given to this flag during options parsing. Will be null if no argument was given. */
@Nullable
public String getUnparsedValue() {
return unparsedValue;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public interface ExpansionFunction {
*
* @param optionsData the parser's indexed information about its own options, before expansion
* information is computed
* @return An expansion to use for all occurrences of this option in this parser
* @return An expansion to use on an empty list
*/
ImmutableList<String> getExpansion(IsolatedOptionsData optionsData);
ImmutableList<String> getExpansion(ExpansionContext context) throws OptionsParsingException;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2017 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.devtools.common.options;

/**
* Exception specific to evaluating {@link ExpansionFunction} objects. Used when expansion isn't
* possible because of a missing input.
*/
public final class ExpansionNeedsValueException extends OptionsParsingException {

public ExpansionNeedsValueException(String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ public void enforce(OptionsParser parser, @Nullable String command)

// The effective policy returned is expanded, filtered for applicable commands, and cleaned of
// redundancies and conflicts.
List<FlagPolicy> effectivePolicy = getEffectivePolicy(invocationPolicy, parser, command);
List<FlagPolicy> effectivePolicies = getEffectivePolicies(invocationPolicy, parser, command);

for (FlagPolicy flagPolicy : effectivePolicy) {
for (FlagPolicy flagPolicy : effectivePolicies) {
String flagName = flagPolicy.getFlagName();

OptionValueDescription valueDescription;
Expand Down Expand Up @@ -190,7 +190,7 @@ private static boolean policyApplies(FlagPolicy policy, ImmutableSet<String> app
*
* <p>Expands any policies on expansion flags.
*/
public static List<FlagPolicy> getEffectivePolicy(
public static ImmutableList<FlagPolicy> getEffectivePolicies(
InvocationPolicy invocationPolicy, OptionsParser parser, String command)
throws OptionsParsingException {
if (invocationPolicy == null) {
Expand Down Expand Up @@ -220,7 +220,71 @@ public static List<FlagPolicy> getEffectivePolicy(
effectivePolicy.put(flagName, expandedPolicy);
}

return new ArrayList<>(effectivePolicy.values());
return ImmutableList.copyOf(effectivePolicy.values());
}

private static void throwAllowValuesOnExpansionFlagException(String flagName)
throws OptionsParsingException {
throw new OptionsParsingException(
String.format("Allow_Values on expansion flags like %s is not allowed.", flagName));
}

private static void throwDisallowValuesOnExpansionFlagException(String flagName)
throws OptionsParsingException {
throw new OptionsParsingException(
String.format("Disallow_Values on expansion flags like %s is not allowed.", flagName));
}

private static ImmutableList<OptionValueDescription> getExpansionsFromFlagPolicy(
FlagPolicy expansionPolicy, OptionDescription optionDescription, OptionsParser parser)
throws OptionsParsingException {
if (!optionDescription.isExpansion()) {
return ImmutableList.of();
}

String expansionFlagName = expansionPolicy.getFlagName();

ImmutableList.Builder<OptionValueDescription> resultsBuilder =
ImmutableList.<OptionValueDescription>builder();
switch (expansionPolicy.getOperationCase()) {
case SET_VALUE:
{
SetValue setValue = expansionPolicy.getSetValue();
if (setValue.getFlagValueCount() > 0) {
for (String value : setValue.getFlagValueList()) {
resultsBuilder.addAll(
parser.getExpansionOptionValueDescriptions(expansionFlagName, value));
}
} else {
resultsBuilder.addAll(
parser.getExpansionOptionValueDescriptions(expansionFlagName, null));
}
}
break;
case USE_DEFAULT:
resultsBuilder.addAll(parser.getExpansionOptionValueDescriptions(expansionFlagName, null));
break;
case ALLOW_VALUES:
// All expansions originally given to the parser have been expanded by now, so these two
// cases aren't necessary (the values given in the flag policy shouldn't need to be
// checked). If you care about blocking specific flag values you should block the behavior
// on the specific ones, not the expansion that contains them.
throwAllowValuesOnExpansionFlagException(expansionPolicy.getFlagName());
break;
case DISALLOW_VALUES:
throwDisallowValuesOnExpansionFlagException(expansionPolicy.getFlagName());
break;
case OPERATION_NOT_SET:
throw new PolicyOperationNotSetException(expansionPolicy.getFlagName());
default:
log.warning(
String.format(
"Unknown operation '%s' from invocation policy for flag '%s'",
expansionPolicy.getOperationCase(), expansionFlagName));
break;
}

return resultsBuilder.build();
}

/**
Expand All @@ -234,21 +298,24 @@ private static List<FlagPolicy> expandPolicy(
FlagPolicy originalPolicy,
OptionsParser parser)
throws OptionsParsingException {
List<FlagPolicy> expandedPolicy = new ArrayList<>();
List<FlagPolicy> expandedPolicies = new ArrayList<>();

OptionDescription originalDesc = parser.getOptionDescription(originalPolicy.getFlagName());
if (originalDesc == null) {
OptionDescription originalOptionDescription =
parser.getOptionDescription(originalPolicy.getFlagName());
if (originalOptionDescription == null) {
// InvocationPolicy ignores policy on non-existing flags by design, for version compatibility.
return expandedPolicy;
return expandedPolicies;
}

ImmutableList<OptionValueDescription> expansions =
getExpansionsFromFlagPolicy(originalPolicy, originalOptionDescription, parser);
ImmutableList.Builder<OptionValueDescription> subflagBuilder = new ImmutableList.Builder<>();
ImmutableList<OptionValueDescription> subflags =
subflagBuilder
.addAll(originalDesc.getImplicitRequirements())
.addAll(originalDesc.getExpansions())
.addAll(originalOptionDescription.getImplicitRequirements())
.addAll(expansions)
.build();
boolean isExpansion = !originalDesc.getExpansions().isEmpty();
boolean isExpansion = originalOptionDescription.isExpansion();

if (!subflags.isEmpty() && log.isLoggable(Level.FINE)) {
// Log the expansion. Since this is logged regardless of user provided command line, it is
Expand Down Expand Up @@ -283,10 +350,10 @@ private static List<FlagPolicy> expandPolicy(
&& originalPolicy.getOperationCase().equals(OperationCase.SET_VALUE)) {
repeatableSubflagsInSetValues.put(currentSubflag.getName(), currentSubflag);
} else {
FlagPolicy subflagAsPolicy = getSubflagAsPolicy(
currentSubflag, originalPolicy, originalDesc);
FlagPolicy subflagAsPolicy =
getSubflagAsPolicy(currentSubflag, originalPolicy, isExpansion);
// In case any of the expanded flags are themselves expansions, recurse.
expandedPolicy.addAll(expandPolicy(subflagAsPolicy, parser));
expandedPolicies.addAll(expandPolicy(subflagAsPolicy, parser));
}
}

Expand All @@ -299,22 +366,18 @@ private static List<FlagPolicy> expandPolicy(
for (OptionValueDescription setValue : repeatableSubflagsInSetValues.get(repeatableFlag)) {
newValues.add(setValue.getOriginalValueString());
}
expandedPolicy.add(
expandedPolicies.add(
getSetValueSubflagAsPolicy(
repeatableFlag,
newValues,
/* allowMultiple */ true,
originalPolicy));

repeatableFlag, newValues, /* allowMultiple */ true, originalPolicy));
}

// Don't add the original policy if it was an expansion flag, which have no value, but do add
// it if there was either no expansion or if it was a valued flag with implicit requirements.
if (!isExpansion) {
expandedPolicy.add(originalPolicy);
expandedPolicies.add(originalPolicy);
}

return expandedPolicy;
return expandedPolicies;
}

/**
Expand Down Expand Up @@ -365,19 +428,18 @@ private static FlagPolicy getSetValueSubflagAsPolicy(
* corresponding policy.
*/
private static FlagPolicy getSubflagAsPolicy(
OptionValueDescription currentSubflag,
FlagPolicy originalPolicy,
OptionDescription originalDesc) throws OptionsParsingException {
boolean isExpansion = !originalDesc.getExpansions().isEmpty();
OptionValueDescription currentSubflag, FlagPolicy originalPolicy, boolean isExpansion)
throws OptionsParsingException {
FlagPolicy subflagAsPolicy = null;
switch (originalPolicy.getOperationCase()) {
case SET_VALUE:
assert(!currentSubflag.getAllowMultiple());
subflagAsPolicy = getSetValueSubflagAsPolicy(
currentSubflag.getName(),
ImmutableList.of(currentSubflag.getOriginalValueString()),
/* allowMultiple */ false,
originalPolicy);
assert (!currentSubflag.getAllowMultiple());
subflagAsPolicy =
getSetValueSubflagAsPolicy(
currentSubflag.getName(),
ImmutableList.of(currentSubflag.getOriginalValueString()),
/*allowMultiple=*/ false,
originalPolicy);
break;

case USE_DEFAULT:
Expand All @@ -394,10 +456,7 @@ private static FlagPolicy getSubflagAsPolicy(

case ALLOW_VALUES:
if (isExpansion) {
throw new OptionsParsingException(
String.format(
"Allow_Values on expansion flags like %s is not allowed.",
originalPolicy.getFlagName()));
throwAllowValuesOnExpansionFlagException(originalPolicy.getFlagName());
}
// If this flag is an implicitRequirement, and some values for the parent flag are
// allowed, nothing needs to happen on the implicitRequirement that is set for all
Expand All @@ -406,10 +465,7 @@ private static FlagPolicy getSubflagAsPolicy(

case DISALLOW_VALUES:
if (isExpansion) {
throw new OptionsParsingException(
String.format(
"Disallow_Values on expansion flags like %s is not allowed.",
originalPolicy.getFlagName()));
throwDisallowValuesOnExpansionFlagException(originalPolicy.getFlagName());
}
// If this flag is an implicitRequirement, and some values for the parent flag are
// disallowed, that implies that all others are allowed, so nothing needs to happen
Expand Down Expand Up @@ -684,7 +740,7 @@ void checkUserValue(
OptionDescription optionDescription,
Set<Object> convertedPolicyValues)
throws OptionsParsingException {

if (optionDescription.getAllowMultiple()) {
// allowMultiple requires that the type of the option be List<T>, so cast from Object
// to List<?>.
Expand Down
Loading

0 comments on commit 7c7255e

Please sign in to comment.