diff options
Diffstat (limited to 'src/main/java')
3 files changed, 83 insertions, 71 deletions
diff --git a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java index e087dbbb56..406b6cd94b 100644 --- a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java +++ b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java @@ -339,52 +339,6 @@ public class IsolatedOptionsData extends OpaqueOptionsData { booleanAliasMap.put("no" + optionName, optionName); } - private static void checkEffectTagRationality(String optionName, OptionEffectTag[] effectTags) { - // Check that there is at least one OptionEffectTag listed. - if (effectTags.length < 1) { - throw new ConstructionException( - "Option " - + optionName - + " does not list at least one OptionEffectTag. If the option has no effect, " - + "please add NO_OP, otherwise, add a tag representing its effect."); - } else if (effectTags.length > 1) { - // If there are more than 1 tag, make sure that NO_OP and UNKNOWN is not one of them. - // These don't make sense if other effects are listed. - ImmutableList<OptionEffectTag> tags = ImmutableList.copyOf(effectTags); - if (tags.contains(OptionEffectTag.UNKNOWN)) { - throw new ConstructionException( - "Option " - + optionName - + " includes UNKNOWN with other, known, effects. Please remove UNKNOWN from " - + "the list."); - } - if (tags.contains(OptionEffectTag.NO_OP)) { - throw new ConstructionException( - "Option " - + optionName - + " includes NO_OP with other effects. This doesn't make much sense. Please " - + "remove NO_OP or the actual effects from the list, whichever is correct."); - } - } - } - - private static void checkMetadataTagAndCategoryRationality( - String optionName, OptionMetadataTag[] metadataTags, OptionDocumentationCategory category) { - for (OptionMetadataTag tag : metadataTags) { - if (tag == OptionMetadataTag.HIDDEN || tag == OptionMetadataTag.INTERNAL) { - if (category != OptionDocumentationCategory.UNDOCUMENTED) { - throw new ConstructionException( - "Option " - + optionName - + " has metadata tag " - + tag - + " but does not have category UNDOCUMENTED. " - + "Please fix."); - } - } - } - } - /** * Constructs an {@link IsolatedOptionsData} object for a parser that knows about the given * {@link OptionsBase} classes. No inter-option analysis is done. Performs basic sanity checking @@ -437,11 +391,6 @@ public class IsolatedOptionsData extends OpaqueOptionsData { + "\" is disallowed."); } - checkEffectTagRationality(optionName, optionDefinition.getOptionEffectTags()); - checkMetadataTagAndCategoryRationality( - optionName, - optionDefinition.getOptionMetadataTags(), - optionDefinition.getDocumentationCategory()); Type fieldType = getFieldSingularType(optionDefinition); // For simple, static expansions, don't accept non-Void types. if (optionDefinition.getOptionExpansion().length != 0 && !optionDefinition.isVoidField()) { diff --git a/src/main/java/com/google/devtools/common/options/processor/BUILD b/src/main/java/com/google/devtools/common/options/processor/BUILD index 2b05631e3f..4d495e606b 100644 --- a/src/main/java/com/google/devtools/common/options/processor/BUILD +++ b/src/main/java/com/google/devtools/common/options/processor/BUILD @@ -21,5 +21,6 @@ java_plugin( processor_class = "com.google.devtools.common.options.processor.OptionProcessor", deps = [ "//src/main/java/com/google/devtools/common/options:options_internal", + "//third_party:guava", ], ) diff --git a/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java b/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java index a961e92546..5afcc39f48 100644 --- a/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java +++ b/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java @@ -13,7 +13,11 @@ // limitations under the License. package com.google.devtools.common.options.processor; +import com.google.common.collect.ImmutableList; import com.google.devtools.common.options.Option; +import com.google.devtools.common.options.OptionDocumentationCategory; +import com.google.devtools.common.options.OptionEffectTag; +import com.google.devtools.common.options.OptionMetadataTag; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParser; import java.util.Set; @@ -28,6 +32,7 @@ import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.Modifier; import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; import javax.lang.model.type.TypeMirror; import javax.lang.model.util.Elements; import javax.lang.model.util.Types; @@ -59,23 +64,23 @@ public final class OptionProcessor extends AbstractProcessor { private static class OptionProcessorException extends Exception { private Element elementInError; - OptionProcessorException(Element element, String message) { - super(message); + OptionProcessorException(Element element, String message, Object... args) { + super(String.format(message, args)); elementInError = element; } } /** Check that the Option variables only occur in OptionBase-inheriting classes. */ - private void checkInOptionBase(Element annotatedElement) throws OptionProcessorException { - if (annotatedElement.getEnclosingElement().getKind() != ElementKind.CLASS) { - throw new OptionProcessorException(annotatedElement, "The field should belong to a class."); + private void checkInOptionBase(VariableElement optionField) throws OptionProcessorException { + if (optionField.getEnclosingElement().getKind() != ElementKind.CLASS) { + throw new OptionProcessorException(optionField, "The field should belong to a class."); } - TypeMirror thisOptionClass = annotatedElement.getEnclosingElement().asType(); + TypeMirror thisOptionClass = optionField.getEnclosingElement().asType(); TypeMirror optionsBase = elementUtils.getTypeElement("com.google.devtools.common.options.OptionsBase").asType(); if (!typeUtils.isAssignable(thisOptionClass, optionsBase)) { throw new OptionProcessorException( - annotatedElement, + optionField, "@Option annotated fields can only be in classes that inherit from OptionsBase."); } } @@ -88,33 +93,90 @@ public final class OptionProcessor extends AbstractProcessor { * * <p>Static or final fields would cause issue with correct value assigning at the end of parsing. */ - private void checkModifiers(Element annotatedElement) throws OptionProcessorException { - if (!annotatedElement.getModifiers().contains(Modifier.PUBLIC)) { + private void checkModifiers(VariableElement optionField) throws OptionProcessorException { + if (!optionField.getModifiers().contains(Modifier.PUBLIC)) { + throw new OptionProcessorException(optionField, "@Option annotated fields should be public."); + } + if (optionField.getModifiers().contains(Modifier.STATIC)) { throw new OptionProcessorException( - annotatedElement, "@Option annotated fields should be public."); + optionField, "@Option annotated fields should not be static."); } - if (annotatedElement.getModifiers().contains(Modifier.STATIC)) { + if (optionField.getModifiers().contains(Modifier.FINAL)) { throw new OptionProcessorException( - annotatedElement, "@Option annotated fields should not be static."); + optionField, "@Option annotated fields should not be final."); } - if (annotatedElement.getModifiers().contains(Modifier.FINAL)) { + } + + /** + * Check that the option lists at least one effect, and that no nonsensical combinations are + * listed, such as having a known effect listed with UNKNOWN. + */ + private void checkEffectTagRationality(VariableElement optionField) + throws OptionProcessorException { + Option annotation = optionField.getAnnotation(Option.class); + OptionEffectTag[] effectTags = annotation.effectTags(); + // Check that there is at least one OptionEffectTag listed. + if (effectTags.length < 1) { throw new OptionProcessorException( - annotatedElement, "@Option annotated fields should not be final."); + optionField, + "Option does not list at least one OptionEffectTag. If the option has no effect, " + + "please be explicit and add NO_OP. Otherwise, add a tag representing its effect."); + } else if (effectTags.length > 1) { + // If there are more than 1 tag, make sure that NO_OP and UNKNOWN is not one of them. + // These don't make sense if other effects are listed. + ImmutableList<OptionEffectTag> tags = ImmutableList.copyOf(effectTags); + if (tags.contains(OptionEffectTag.UNKNOWN)) { + throw new OptionProcessorException( + optionField, + "Option includes UNKNOWN with other, known, effects. Please remove UNKNOWN from " + + "the list."); + } + if (tags.contains(OptionEffectTag.NO_OP)) { + throw new OptionProcessorException( + optionField, + "Option includes NO_OP with other effects. This doesn't make much sense. Please " + + "remove NO_OP or the actual effects from the list, whichever is correct."); + } + } + } + + /** + * Check that if the metadata tags listed by an option require the option to be unknown by the + * average user, the same option will be omitted from documentation. + */ + private void checkMetadataTagAndCategoryRationality(VariableElement optionField) + throws OptionProcessorException { + Option annotation = optionField.getAnnotation(Option.class); + OptionMetadataTag[] metadataTags = annotation.metadataTags(); + OptionDocumentationCategory category = annotation.documentationCategory(); + + for (OptionMetadataTag tag : metadataTags) { + if (tag == OptionMetadataTag.HIDDEN || tag == OptionMetadataTag.INTERNAL) { + if (category != OptionDocumentationCategory.UNDOCUMENTED) { + throw new OptionProcessorException( + optionField, + "Option has metadata tag %s but does not have category UNDOCUMENTED. Please fix.", + tag); + } + } } } @Override public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) { - try { - for (Element annotatedElement : roundEnv.getElementsAnnotatedWith(Option.class)) { + for (Element annotatedElement : roundEnv.getElementsAnnotatedWith(Option.class)) { + try { // Only fields are annotated with Option, this should already be checked by the // @Target(ElementType.FIELD) annotation. + VariableElement optionField = (VariableElement) annotatedElement; - checkModifiers(annotatedElement); - checkInOptionBase(annotatedElement); + checkModifiers(optionField); + checkInOptionBase(optionField); + checkEffectTagRationality(optionField); + checkMetadataTagAndCategoryRationality(optionField); + } catch (OptionProcessorException e) { + error(e.elementInError, e.getMessage()); } - } catch (OptionProcessorException e) { - error(e.elementInError, e.getMessage()); } // Claim all Option annotated fields. return true; |