aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/main/java/com/google/devtools/common
diff options
context:
space:
mode:
Diffstat (limited to 'src/main/java/com/google/devtools/common')
-rw-r--r--src/main/java/com/google/devtools/common/options/OptionValueDescription.java137
-rw-r--r--src/main/java/com/google/devtools/common/options/OptionsParserImpl.java41
2 files changed, 106 insertions, 72 deletions
diff --git a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
index ed03e9776a..0d31137fa4 100644
--- a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
+++ b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
@@ -15,7 +15,9 @@
package com.google.devtools.common.options;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ListMultimap;
import com.google.devtools.common.options.OptionsParser.ConstructionException;
import java.util.Collection;
@@ -47,10 +49,29 @@ public abstract class OptionValueDescription {
/** Returns the source(s) of this option, if there were multiple, duplicates are removed. */
public abstract String getSourceString();
- abstract void addOptionInstance(
- ParsedOptionDescription parsedOption,
- List<String> warnings)
- throws OptionsParsingException;
+ /**
+ * Add an instance of the option to this value. The various types of options are in charge of
+ * making sure that the value is correctly stored, with proper tracking of its priority and
+ * placement amongst other options.
+ *
+ * @return a bundle containing arguments that need to be parsed further.
+ */
+ abstract ExpansionBundle addOptionInstance(
+ ParsedOptionDescription parsedOption, List<String> warnings) throws OptionsParsingException;
+
+ /**
+ * Grouping of convenience for the options that expand to other options, to attach an
+ * option-appropriate source string along with the options that need to be parsed.
+ */
+ public static class ExpansionBundle {
+ List<String> expansionArgs;
+ String sourceOfExpansionArgs;
+
+ public ExpansionBundle(List<String> args, String source) {
+ expansionArgs = args;
+ sourceOfExpansionArgs = source;
+ }
+ }
/**
* For the given option, returns the correct type of OptionValueDescription, to which unparsed
@@ -59,11 +80,12 @@ public abstract class OptionValueDescription {
* <p>The categories of option types are non-overlapping, an invariant checked by the
* OptionProcessor at compile time.
*/
- public static OptionValueDescription createOptionValueDescription(OptionDefinition option) {
- if (option.allowsMultiple()) {
+ public static OptionValueDescription createOptionValueDescription(
+ OptionDefinition option, OptionsData optionsData) {
+ if (option.isExpansionOption()) {
+ return new ExpansionOptionValueDescription(option, optionsData);
+ } else if (option.allowsMultiple()) {
return new RepeatableOptionValueDescription(option);
- } else if (option.isExpansionOption()) {
- return new ExpansionOptionValueDescription(option);
} else if (option.hasImplicitRequirements()) {
return new OptionWithImplicitRequirementsValueDescription(option);
} else if (option.isWrapperOption()) {
@@ -98,9 +120,7 @@ public abstract class OptionValueDescription {
}
@Override
- void addOptionInstance(
- ParsedOptionDescription parsedOption,
- List<String> warnings) {
+ ExpansionBundle addOptionInstance(ParsedOptionDescription parsedOption, List<String> warnings) {
throw new IllegalStateException(
"Cannot add values to the default option value. Create a modifiable "
+ "OptionValueDescription using createOptionValueDescription() instead.");
@@ -139,15 +159,13 @@ public abstract class OptionValueDescription {
// Warnings should not end with a '.' because the internal reporter adds one automatically.
@Override
- void addOptionInstance(
- ParsedOptionDescription parsedOption,
- List<String> warnings)
+ ExpansionBundle addOptionInstance(ParsedOptionDescription parsedOption, List<String> warnings)
throws OptionsParsingException {
// This might be the first value, in that case, just store it!
if (effectiveOptionInstance == null) {
effectiveOptionInstance = parsedOption;
effectiveValue = effectiveOptionInstance.getConvertedValue();
- return;
+ return null;
}
// If there was another value, check whether the new one will override it, and if so,
@@ -190,8 +208,10 @@ public abstract class OptionValueDescription {
// Create a warning if an expansion option overrides an explicit option:
warnings.add(
String.format(
- "%s was expanded and now overrides a previous explicitly specified %s",
- expandedFrom, optionDefinition));
+ "%s was expanded and now overrides a previous explicitly specified %s with %s",
+ expandedFrom,
+ effectiveOptionInstance.getCommandLineForm(),
+ parsedOption.getCommandLineForm()));
} else if ((optionThatExpandedToEffectiveValue != null) && (expandedFrom != null)) {
warnings.add(
String.format(
@@ -204,6 +224,7 @@ public abstract class OptionValueDescription {
effectiveOptionInstance = parsedOption;
effectiveValue = newValue;
}
+ return null;
}
@VisibleForTesting
@@ -254,9 +275,7 @@ public abstract class OptionValueDescription {
}
@Override
- void addOptionInstance(
- ParsedOptionDescription parsedOption,
- List<String> warnings)
+ ExpansionBundle addOptionInstance(ParsedOptionDescription parsedOption, List<String> warnings)
throws OptionsParsingException {
// For repeatable options, we allow flags that take both single values and multiple values,
// potentially collapsing them down.
@@ -268,6 +287,7 @@ public abstract class OptionValueDescription {
} else {
optionValues.put(priority, convertedValue);
}
+ return null;
}
}
@@ -277,9 +297,12 @@ public abstract class OptionValueDescription {
* in {@link Option#expansion()} and flags with an {@link Option#expansionFunction()}.
*/
static class ExpansionOptionValueDescription extends OptionValueDescription {
+ private final List<String> expansion;
- private ExpansionOptionValueDescription(OptionDefinition optionDefinition) {
+ private ExpansionOptionValueDescription(
+ OptionDefinition optionDefinition, OptionsData optionsData) {
super(optionDefinition);
+ this.expansion = optionsData.getEvaluatedExpansion(optionDefinition);
if (!optionDefinition.isExpansionOption()) {
throw new ConstructionException(
"Options without expansions can't be tracked using ExpansionOptionValueDescription");
@@ -297,11 +320,22 @@ public abstract class OptionValueDescription {
}
@Override
- void addOptionInstance(
- ParsedOptionDescription parsedOption,
- List<String> warnings) {
- // TODO(b/65540004) Deal with expansion options here instead of in parse(), and track their
- // link to the options they expanded to to.
+ ExpansionBundle addOptionInstance(ParsedOptionDescription parsedOption, List<String> warnings) {
+ if (parsedOption.getUnconvertedValue() != null
+ && !parsedOption.getUnconvertedValue().isEmpty()) {
+ warnings.add(
+ String.format(
+ "%s is an expansion option. It does not accept values, and does not change its "
+ + "expansion based on the value provided. Value '%s' will be ignored.",
+ optionDefinition, parsedOption.getUnconvertedValue()));
+ }
+
+ return new ExpansionBundle(
+ expansion,
+ (parsedOption.getSource() == null)
+ ? String.format("expanded from %s", optionDefinition)
+ : String.format(
+ "expanded from %s (source %s)", optionDefinition, parsedOption.getSource()));
}
}
@@ -318,17 +352,34 @@ public abstract class OptionValueDescription {
}
@Override
- void addOptionInstance(
- ParsedOptionDescription parsedOption,
- List<String> warnings)
+ ExpansionBundle addOptionInstance(ParsedOptionDescription parsedOption, List<String> warnings)
throws OptionsParsingException {
// This is a valued flag, its value is handled the same way as a normal
- // SingleOptionValueDescription.
- super.addOptionInstance(parsedOption, warnings);
+ // SingleOptionValueDescription. (We check at compile time that these flags aren't
+ // "allowMultiple")
+ ExpansionBundle superExpansion = super.addOptionInstance(parsedOption, warnings);
+ Preconditions.checkArgument(
+ superExpansion == null, "SingleOptionValueDescription should not expand to anything.");
+ if (parsedOption.getConvertedValue().equals(optionDefinition.getDefaultValue())) {
+ warnings.add(
+ String.format(
+ "%s sets %s to its default value. Since this option has implicit requirements that "
+ + "are set whenever the option is explicitly provided, regardless of the "
+ + "value, this will behave differently than letting a default be a default. "
+ + "Specifically, this options expands to {%s}.",
+ parsedOption.getCommandLineForm(),
+ optionDefinition,
+ String.join(" ", optionDefinition.getImplicitRequirements())));
+ }
// Now deal with the implicit requirements.
- // TODO(b/65540004) Deal with options with implicit requirements here instead of in parse(),
- // and track their link to the options they implicitly expanded to to.
+ return new ExpansionBundle(
+ ImmutableList.copyOf(optionDefinition.getImplicitRequirements()),
+ (parsedOption.getSource() == null)
+ ? String.format("implicit requirement of %s", optionDefinition)
+ : String.format(
+ "implicit requirement of %s (source %s)",
+ optionDefinition, parsedOption.getSource()));
}
}
@@ -350,12 +401,22 @@ public abstract class OptionValueDescription {
}
@Override
- void addOptionInstance(
- ParsedOptionDescription parsedOption,
- List<String> warnings)
+ ExpansionBundle addOptionInstance(ParsedOptionDescription parsedOption, List<String> warnings)
throws OptionsParsingException {
- // TODO(b/65540004) Deal with options with implicit requirements here instead of in parse(),
- // and track their link to the options they implicitly expanded to to.
+ if (!parsedOption.getUnconvertedValue().startsWith("-")) {
+ throw new OptionsParsingException(
+ String.format(
+ "Invalid value format for %s. You may have meant --%s=--%s",
+ optionDefinition,
+ optionDefinition.getOptionName(),
+ parsedOption.getUnconvertedValue()));
+ }
+ return new ExpansionBundle(
+ ImmutableList.of(parsedOption.getUnconvertedValue()),
+ (parsedOption.getSource() == null)
+ ? String.format("unwrapped from %s", optionDefinition)
+ : String.format(
+ "unwrapped from %s (source %s)", optionDefinition, parsedOption.getSource()));
}
}
}
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
index 78bfa07ac5..b543328c5c 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -24,6 +24,7 @@ import com.google.common.collect.Iterators;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Multimap;
import com.google.devtools.common.options.OptionPriority.PriorityCategory;
+import com.google.devtools.common.options.OptionValueDescription.ExpansionBundle;
import com.google.devtools.common.options.OptionsParser.OptionDescription;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
@@ -381,8 +382,9 @@ class OptionsParserImpl {
// the OptionValueDescription.
OptionValueDescription entry =
optionValues.computeIfAbsent(
- optionDefinition, OptionValueDescription::createOptionValueDescription);
- entry.addOptionInstance(parsedOption, warnings);
+ optionDefinition,
+ def -> OptionValueDescription.createOptionValueDescription(def, optionsData));
+ ExpansionBundle expansionBundle = entry.addOptionInstance(parsedOption, warnings);
@Nullable String unconvertedValue = parsedOption.getUnconvertedValue();
// There are 3 types of flags that expand to other flag values. Expansion flags are the
@@ -406,43 +408,14 @@ class OptionsParserImpl {
}
}
- if (optionDefinition.isExpansionOption()
- || optionDefinition.hasImplicitRequirements()
- || optionDefinition.isWrapperOption()) {
- StringBuilder sourceMessage = new StringBuilder();
- ImmutableList<String> expansionArgs;
- if (optionDefinition.isExpansionOption()) {
- expansionArgs = optionsData.getEvaluatedExpansion(optionDefinition);
- sourceMessage.append("expanded from option ");
- } else if (optionDefinition.hasImplicitRequirements()) {
- expansionArgs = ImmutableList.copyOf(optionDefinition.getImplicitRequirements());
- sourceMessage.append("implicit requirement of option ");
- } else {
- if (!unconvertedValue.startsWith("-")) {
- // Wrapper options are the only "expansion" flag type that expand to other flags
- // according to user input, so a malformed option is a user error in this case.
- throw new OptionsParsingException(
- String.format(
- "Invalid value format for %s. You may have meant --%s=--%s",
- optionDefinition, optionDefinition.getOptionName(), unconvertedValue));
- } else {
- expansionArgs = ImmutableList.of(unconvertedValue);
- sourceMessage.append("unwrapped from wrapper option ");
- }
- }
- String source = parsedOption.getSource();
- sourceMessage.append(
- (source == null)
- ? String.format("--%s", optionDefinition.getOptionName())
- : String.format("--%s from %s", optionDefinition.getOptionName(), source));
-
+ if (expansionBundle != null) {
List<String> unparsed =
parse(
parsedOption.getPriority(),
- o -> sourceMessage.toString(),
+ o -> expansionBundle.sourceOfExpansionArgs,
optionDefinition.hasImplicitRequirements() ? optionDefinition : null,
optionDefinition.isExpansionOption() ? optionDefinition : null,
- expansionArgs);
+ expansionBundle.expansionArgs);
if (!unparsed.isEmpty()) {
if (optionDefinition.isWrapperOption()) {
throw new OptionsParsingException(
generated by cgit v1.2.3 (git 2.39.1) at 2024-06-23 03:33:49 +0000