From 87f16cd08e3ea8f3c2320ab37f8165f6cae63e45 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 7 Jun 2024 17:55:09 -0700 Subject: [PATCH] Remove AbstractConfig class (#974) `AbstractConfig` has only had one subclass for a long time, and I don't see any future scenario in which we add more. Removing `AbstractConfig` makes adding new NullAway options easier. We push down the code from `AbstractConfig` into `ErrorProneCLIFlagsConfig` and make the fields private and final instead of protected. --- .../com/uber/nullaway/AbstractConfig.java | 363 ------------------ .../nullaway/ErrorProneCLIFlagsConfig.java | 314 ++++++++++++++- 2 files changed, 313 insertions(+), 364 deletions(-) delete mode 100644 nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java diff --git a/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java b/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java deleted file mode 100644 index 3f236c7ce5..0000000000 --- a/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java +++ /dev/null @@ -1,363 +0,0 @@ -/* - * Copyright (c) 2017 Uber Technologies, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package com.uber.nullaway; - -import com.google.auto.value.AutoValue; -import com.google.common.base.Joiner; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; -import com.google.errorprone.util.ASTHelpers; -import com.sun.tools.javac.code.Symbol; -import com.uber.nullaway.fixserialization.FixSerializationConfig; -import java.util.regex.Pattern; -import javax.annotation.Nullable; - -/** abstract base class for null checker {@link Config} implementations */ -@SuppressWarnings("NullAway") // TODO: get rid of this class to avoid suppression -public abstract class AbstractConfig implements Config { - - /** - * Packages that we assume have appropriate nullability annotations. - * - *

When we see an invocation to a method of a class outside these packages, we optimistically - * assume all parameters are @Nullable and the return value is @NonNull - */ - protected Pattern annotatedPackages; - - /** - * Sub-packages without appropriate nullability annotations. - * - *

Used to exclude a particular package that contains unannotated code within a larger, - * properly annotated, package. - */ - protected Pattern unannotatedSubPackages; - - /** Source code in these classes will not be analyzed for nullability issues */ - @Nullable protected ImmutableSet sourceClassesToExclude; - - /** - * these classes will be treated as unannotated (don't analyze *and* treat methods as unannotated) - */ - @Nullable protected ImmutableSet unannotatedClasses; - - protected Pattern fieldAnnotPattern; - - protected boolean isExhaustiveOverride; - - protected boolean isSuggestSuppressions; - - protected boolean isAcknowledgeRestrictive; - - protected boolean checkOptionalEmptiness; - - protected boolean checkContracts; - - protected boolean handleTestAssertionLibraries; - - protected ImmutableSet optionalClassPaths; - - protected boolean assertsEnabled; - - protected boolean treatGeneratedAsUnannotated; - - protected boolean acknowledgeAndroidRecent; - - protected boolean jspecifyMode; - - protected ImmutableSet knownInitializers; - - protected ImmutableSet excludedClassAnnotations; - - protected ImmutableSet generatedCodeAnnotations; - - protected ImmutableSet initializerAnnotations; - - protected ImmutableSet externalInitAnnotations; - - protected ImmutableSet contractAnnotations; - - @Nullable protected String castToNonNullMethod; - - protected String autofixSuppressionComment; - - protected ImmutableSet skippedLibraryModels; - - protected ImmutableSet extraFuturesClasses; - - /** --- JarInfer configs --- */ - protected boolean jarInferEnabled; - - protected boolean jarInferUseReturnAnnotations; - - protected String jarInferRegexStripModelJarName; - protected String jarInferRegexStripCodeJarName; - - protected String errorURL; - - /** --- Fully qualified names of custom nonnull/nullable annotation --- */ - protected ImmutableSet customNonnullAnnotations; - - protected ImmutableSet customNullableAnnotations; - - /** - * If active, NullAway will write all reporting errors in output directory. The output directory - * along with the activation status of other serialization features are stored in {@link - * FixSerializationConfig}. - */ - protected boolean serializationActivationFlag; - - protected FixSerializationConfig fixSerializationConfig; - - @Override - public boolean serializationIsActive() { - return serializationActivationFlag; - } - - @Override - public FixSerializationConfig getSerializationConfig() { - Preconditions.checkArgument( - serializationActivationFlag, "Fix Serialization is not active, cannot access it's config."); - return fixSerializationConfig; - } - - protected static Pattern getPackagePattern(ImmutableSet packagePrefixes) { - // noinspection ConstantConditions - String choiceRegexp = - Joiner.on("|") - .join(Iterables.transform(packagePrefixes, input -> input.replaceAll("\\.", "\\\\."))); - return Pattern.compile("^(?:" + choiceRegexp + ")(?:\\..*)?"); - } - - @Override - public boolean fromExplicitlyAnnotatedPackage(String className) { - return annotatedPackages.matcher(className).matches(); - } - - @Override - public boolean fromExplicitlyUnannotatedPackage(String className) { - return unannotatedSubPackages.matcher(className).matches(); - } - - @Override - public boolean treatGeneratedAsUnannotated() { - return treatGeneratedAsUnannotated; - } - - @Override - public boolean isExcludedClass(String className) { - if (sourceClassesToExclude == null) { - return false; - } - for (String classPrefix : sourceClassesToExclude) { - if (className.startsWith(classPrefix)) { - return true; - } - } - return false; - } - - @Override - public boolean isUnannotatedClass(Symbol.ClassSymbol symbol) { - if (unannotatedClasses == null) { - return false; - } - String className = symbol.getQualifiedName().toString(); - for (String classPrefix : unannotatedClasses) { - if (className.startsWith(classPrefix)) { - return true; - } - } - return false; - } - - @Override - public ImmutableSet getExcludedClassAnnotations() { - return excludedClassAnnotations; - } - - @Override - public ImmutableSet getGeneratedCodeAnnotations() { - return generatedCodeAnnotations; - } - - @Override - public boolean isInitializerMethodAnnotation(String annotationName) { - return initializerAnnotations.contains(annotationName); - } - - @Override - public boolean isCustomNullableAnnotation(String annotationName) { - return customNullableAnnotations.contains(annotationName); - } - - @Override - public boolean isCustomNonnullAnnotation(String annotationName) { - return customNonnullAnnotations.contains(annotationName); - } - - @Override - public boolean exhaustiveOverride() { - return isExhaustiveOverride; - } - - @Override - public boolean isKnownInitializerMethod(Symbol.MethodSymbol methodSymbol) { - Symbol.ClassSymbol enclosingClass = ASTHelpers.enclosingClass(methodSymbol); - MethodClassAndName classAndName = - MethodClassAndName.create( - enclosingClass.getQualifiedName().toString(), methodSymbol.getSimpleName().toString()); - return knownInitializers.contains(classAndName); - } - - @Override - public boolean isExcludedFieldAnnotation(String annotationName) { - return Nullness.isNullableAnnotation(annotationName, this) - || (fieldAnnotPattern != null && fieldAnnotPattern.matcher(annotationName).matches()); - } - - @Override - public boolean suggestSuppressions() { - return isSuggestSuppressions; - } - - @Override - public boolean acknowledgeRestrictiveAnnotations() { - return isAcknowledgeRestrictive; - } - - @Override - public boolean checkOptionalEmptiness() { - return checkOptionalEmptiness; - } - - @Override - public boolean checkContracts() { - return checkContracts; - } - - @Override - public boolean handleTestAssertionLibraries() { - return handleTestAssertionLibraries; - } - - @Override - public ImmutableSet getOptionalClassPaths() { - return optionalClassPaths; - } - - @Override - public boolean assertsEnabled() { - return assertsEnabled; - } - - @Override - @Nullable - public String getCastToNonNullMethod() { - return castToNonNullMethod; - } - - @Override - public String getAutofixSuppressionComment() { - if (autofixSuppressionComment.trim().length() > 0) { - return "/* " + autofixSuppressionComment + " */ "; - } else { - return ""; - } - } - - @Override - public boolean isExternalInitClassAnnotation(String annotationName) { - return externalInitAnnotations.contains(annotationName); - } - - @Override - public boolean isContractAnnotation(String annotationName) { - return contractAnnotations.contains(annotationName); - } - - @Override - public boolean isSkippedLibraryModel(String classDotMethod) { - return skippedLibraryModels.contains(classDotMethod); - } - - @Override - public ImmutableSet getExtraFuturesClasses() { - return extraFuturesClasses; - } - - @AutoValue - abstract static class MethodClassAndName { - - static MethodClassAndName create(String enclosingClass, String methodName) { - return new AutoValue_AbstractConfig_MethodClassAndName(enclosingClass, methodName); - } - - static MethodClassAndName fromClassDotMethod(String classDotMethod) { - int lastDot = classDotMethod.lastIndexOf('.'); - String methodName = classDotMethod.substring(lastDot + 1); - String className = classDotMethod.substring(0, lastDot); - return MethodClassAndName.create(className, methodName); - } - - abstract String enclosingClass(); - - abstract String methodName(); - } - - /** --- JarInfer configs --- */ - @Override - public boolean isJarInferEnabled() { - return jarInferEnabled; - } - - @Override - public boolean isJarInferUseReturnAnnotations() { - return jarInferUseReturnAnnotations; - } - - @Override - public String getJarInferRegexStripModelJarName() { - return jarInferRegexStripModelJarName; - } - - @Override - public String getJarInferRegexStripCodeJarName() { - return jarInferRegexStripCodeJarName; - } - - @Override - public String getErrorURL() { - return errorURL; - } - - @Override - public boolean acknowledgeAndroidRecent() { - return acknowledgeAndroidRecent; - } - - @Override - public boolean isJSpecifyMode() { - return jspecifyMode; - } -} diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java index 6f2b578445..90110e5ce3 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java @@ -22,20 +22,28 @@ package com.uber.nullaway; +import com.google.auto.value.AutoValue; +import com.google.common.base.Joiner; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.errorprone.ErrorProneFlags; +import com.google.errorprone.util.ASTHelpers; +import com.sun.tools.javac.code.Symbol; import com.uber.nullaway.fixserialization.FixSerializationConfig; import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; import java.util.Collections; import java.util.LinkedHashSet; import java.util.Optional; import java.util.Set; +import java.util.regex.Pattern; +import javax.annotation.Nullable; /** * provides nullability configuration based on additional flags passed to ErrorProne via * "-XepOpt:[Namespace:]FlagName[=Value]". See. http://errorprone.info/docs/flags */ -final class ErrorProneCLIFlagsConfig extends AbstractConfig { +final class ErrorProneCLIFlagsConfig implements Config { private static final String BASENAME_REGEX = ".*/([^/]+)\\.[ja]ar$"; @@ -164,6 +172,75 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig { private static final String DEFAULT_URL = "http://t.uber.com/nullaway"; + /** + * Packages that we assume have appropriate nullability annotations. + * + *

When we see an invocation to a method of a class outside these packages, we optimistically + * assume all parameters are @Nullable and the return value is @NonNull + */ + private final Pattern annotatedPackages; + + /** + * Sub-packages without appropriate nullability annotations. + * + *

Used to exclude a particular package that contains unannotated code within a larger, + * properly annotated, package. + */ + private final Pattern unannotatedSubPackages; + + /** Source code in these classes will not be analyzed for nullability issues */ + @Nullable private final ImmutableSet sourceClassesToExclude; + + /** + * these classes will be treated as unannotated (don't analyze *and* treat methods as unannotated) + */ + @Nullable private final ImmutableSet unannotatedClasses; + + private final Pattern fieldAnnotPattern; + private final boolean isExhaustiveOverride; + private final boolean isSuggestSuppressions; + private final boolean isAcknowledgeRestrictive; + private final boolean checkOptionalEmptiness; + private final boolean checkContracts; + private final boolean handleTestAssertionLibraries; + private final ImmutableSet optionalClassPaths; + private final boolean assertsEnabled; + private final boolean treatGeneratedAsUnannotated; + private final boolean acknowledgeAndroidRecent; + private final boolean jspecifyMode; + private final ImmutableSet knownInitializers; + private final ImmutableSet excludedClassAnnotations; + private final ImmutableSet generatedCodeAnnotations; + private final ImmutableSet initializerAnnotations; + private final ImmutableSet externalInitAnnotations; + private final ImmutableSet contractAnnotations; + @Nullable private final String castToNonNullMethod; + private final String autofixSuppressionComment; + private final ImmutableSet skippedLibraryModels; + private final ImmutableSet extraFuturesClasses; + + /** --- JarInfer configs --- */ + private final boolean jarInferEnabled; + + private final boolean jarInferUseReturnAnnotations; + private final String jarInferRegexStripModelJarName; + private final String jarInferRegexStripCodeJarName; + private final String errorURL; + + /** --- Fully qualified names of custom nonnull/nullable annotation --- */ + private final ImmutableSet customNonnullAnnotations; + + private final ImmutableSet customNullableAnnotations; + + /** + * If active, NullAway will write all reporting errors in output directory. The output directory + * along with the activation status of other serialization features are stored in {@link + * FixSerializationConfig}. + */ + private final boolean serializationActivationFlag; + + private final FixSerializationConfig fixSerializationConfig; + ErrorProneCLIFlagsConfig(ErrorProneFlags flags) { if (!flags.get(FL_ANNOTATED_PACKAGES).isPresent()) { throw new IllegalStateException( @@ -291,4 +368,239 @@ private static ImmutableSet getFlagStringSet( } return ImmutableSet.copyOf(combined); } + + private static final Pattern getPackagePattern(ImmutableSet packagePrefixes) { + // noinspection ConstantConditions + String choiceRegexp = + Joiner.on("|") + .join(Iterables.transform(packagePrefixes, input -> input.replaceAll("\\.", "\\\\."))); + return Pattern.compile("^(?:" + choiceRegexp + ")(?:\\..*)?"); + } + + @Override + public boolean serializationIsActive() { + return serializationActivationFlag; + } + + @Override + public FixSerializationConfig getSerializationConfig() { + Preconditions.checkArgument( + serializationActivationFlag, "Fix Serialization is not active, cannot access it's config."); + return fixSerializationConfig; + } + + @Override + public boolean fromExplicitlyAnnotatedPackage(String className) { + return annotatedPackages.matcher(className).matches(); + } + + @Override + public boolean fromExplicitlyUnannotatedPackage(String className) { + return unannotatedSubPackages.matcher(className).matches(); + } + + @Override + public boolean treatGeneratedAsUnannotated() { + return treatGeneratedAsUnannotated; + } + + @Override + public boolean isExcludedClass(String className) { + if (sourceClassesToExclude == null) { + return false; + } + for (String classPrefix : sourceClassesToExclude) { + if (className.startsWith(classPrefix)) { + return true; + } + } + return false; + } + + @Override + public boolean isUnannotatedClass(Symbol.ClassSymbol symbol) { + if (unannotatedClasses == null) { + return false; + } + String className = symbol.getQualifiedName().toString(); + for (String classPrefix : unannotatedClasses) { + if (className.startsWith(classPrefix)) { + return true; + } + } + return false; + } + + @Override + public ImmutableSet getExcludedClassAnnotations() { + return excludedClassAnnotations; + } + + @Override + public ImmutableSet getGeneratedCodeAnnotations() { + return generatedCodeAnnotations; + } + + @Override + public boolean isInitializerMethodAnnotation(String annotationName) { + return initializerAnnotations.contains(annotationName); + } + + @Override + public boolean isCustomNullableAnnotation(String annotationName) { + return customNullableAnnotations.contains(annotationName); + } + + @Override + public boolean isCustomNonnullAnnotation(String annotationName) { + return customNonnullAnnotations.contains(annotationName); + } + + @Override + public boolean exhaustiveOverride() { + return isExhaustiveOverride; + } + + @Override + public boolean isKnownInitializerMethod(Symbol.MethodSymbol methodSymbol) { + Symbol.ClassSymbol enclosingClass = ASTHelpers.enclosingClass(methodSymbol); + if (enclosingClass == null) { + return false; + } + MethodClassAndName classAndName = + MethodClassAndName.create( + enclosingClass.getQualifiedName().toString(), methodSymbol.getSimpleName().toString()); + return knownInitializers.contains(classAndName); + } + + @Override + public boolean isExcludedFieldAnnotation(String annotationName) { + return Nullness.isNullableAnnotation(annotationName, this) + || (fieldAnnotPattern != null && fieldAnnotPattern.matcher(annotationName).matches()); + } + + @Override + public boolean suggestSuppressions() { + return isSuggestSuppressions; + } + + @Override + public boolean acknowledgeRestrictiveAnnotations() { + return isAcknowledgeRestrictive; + } + + @Override + public boolean checkOptionalEmptiness() { + return checkOptionalEmptiness; + } + + @Override + public boolean checkContracts() { + return checkContracts; + } + + @Override + public boolean handleTestAssertionLibraries() { + return handleTestAssertionLibraries; + } + + @Override + public ImmutableSet getOptionalClassPaths() { + return optionalClassPaths; + } + + @Override + public boolean assertsEnabled() { + return assertsEnabled; + } + + @Override + @Nullable + public String getCastToNonNullMethod() { + return castToNonNullMethod; + } + + @Override + public String getAutofixSuppressionComment() { + if (autofixSuppressionComment.trim().length() > 0) { + return "/* " + autofixSuppressionComment + " */ "; + } else { + return ""; + } + } + + @Override + public boolean isExternalInitClassAnnotation(String annotationName) { + return externalInitAnnotations.contains(annotationName); + } + + @Override + public boolean isContractAnnotation(String annotationName) { + return contractAnnotations.contains(annotationName); + } + + @Override + public boolean isSkippedLibraryModel(String classDotMethod) { + return skippedLibraryModels.contains(classDotMethod); + } + + @Override + public ImmutableSet getExtraFuturesClasses() { + return extraFuturesClasses; + } + + /** --- JarInfer configs --- */ + @Override + public boolean isJarInferEnabled() { + return jarInferEnabled; + } + + @Override + public boolean isJarInferUseReturnAnnotations() { + return jarInferUseReturnAnnotations; + } + + @Override + public String getJarInferRegexStripModelJarName() { + return jarInferRegexStripModelJarName; + } + + @Override + public String getJarInferRegexStripCodeJarName() { + return jarInferRegexStripCodeJarName; + } + + @Override + public String getErrorURL() { + return errorURL; + } + + @Override + public boolean acknowledgeAndroidRecent() { + return acknowledgeAndroidRecent; + } + + @Override + public boolean isJSpecifyMode() { + return jspecifyMode; + } + + @AutoValue + abstract static class MethodClassAndName { + + static MethodClassAndName create(String enclosingClass, String methodName) { + return new AutoValue_ErrorProneCLIFlagsConfig_MethodClassAndName(enclosingClass, methodName); + } + + static MethodClassAndName fromClassDotMethod(String classDotMethod) { + int lastDot = classDotMethod.lastIndexOf('.'); + String methodName = classDotMethod.substring(lastDot + 1); + String className = classDotMethod.substring(0, lastDot); + return MethodClassAndName.create(className, methodName); + } + + abstract String enclosingClass(); + + abstract String methodName(); + } }