diff --git a/CHANGELOG.md b/CHANGELOG.md index 39277045af..4889978048 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ Changelog ========= +Version 0.12.4 +--------------- +* Better `@MonotonicNonNull` support (#1149) +* Add support for local variables for arrays. (#1146) +* Ignore Spring Framework 6.2 `@MockitoBean`, `@MockitoSpyBean` fields (#1147) +* JSpecify: preserve explicit nullability annotations on type variables when performing substitutions (#1143) +* Always acknowledge restrictive annotations in JSpecify mode (#1144) +* Fix printing of array types in JSpecify errors (#1145) +* Remove need to use JSpecify's @Nullable annotation (#1142) +* Handle calls to generic constructors in JSpecify mode (#1141) +* Properly handle conditional expression within parens as RHS of assignment (#1140) +* Skip checks involving wildcard generic type arguments (#1137) +* Update to Gradle 8.12.1 (#1133) + Version 0.12.3 --------------- * Remove InferredJARModelsHandler (#1079) diff --git a/annotations/src/main/java/com/uber/nullaway/annotations/MonotonicNonNull.java b/annotations/src/main/java/com/uber/nullaway/annotations/MonotonicNonNull.java new file mode 100644 index 0000000000..e177eea76a --- /dev/null +++ b/annotations/src/main/java/com/uber/nullaway/annotations/MonotonicNonNull.java @@ -0,0 +1,29 @@ +package com.uber.nullaway.annotations; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Indicates that once the field becomes non-null, it never becomes null again. Inspired by the + * identically-named annotation from the Checker Framework. A {@code @MonotonicNonNull} field can + * only be assigned non-null values. The key reason to use this annotation with NullAway is to + * enable reasoning about field non-nullness in nested lambdas / anonymous classes, e.g.: + * + * 
+ * class Foo {
+ *   {@literal @}MonotonicNonNull Object theField;
+ *   void foo() {
+ *     theField = new Object();
+ *     Runnable r = () -> {
+ *       // No error, NullAway knows theField is non-null after assignment
+ *       theField.toString();
+ *     }
+ *   }
+ * }
+ *
+ */ +@Retention(RetentionPolicy.CLASS) +@Target(ElementType.FIELD) +public @interface MonotonicNonNull {} diff --git a/gradle.properties b/gradle.properties index 04565dd890..0493e3a262 100644 --- a/gradle.properties +++ b/gradle.properties @@ -12,7 +12,7 @@ org.gradle.caching=true org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=512m GROUP=com.uber.nullaway -VERSION_NAME=0.12.3 +VERSION_NAME=0.12.4 POM_DESCRIPTION=A fast annotation-based null checker for Java diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 82dd18b204..d71047787f 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,7 +1,7 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionSha256Sum=57dafb5c2622c6cc08b993c85b7c06956a2f53536432a30ead46166dbca0f1e9 -distributionUrl=https\://services.gradle.org/distributions/gradle-8.11-bin.zip +distributionSha256Sum=8d97a97984f6cbd2b85fe4c60a743440a347544bf18818048e611f5288d46c94 +distributionUrl=https\://services.gradle.org/distributions/gradle-8.12.1-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME diff --git a/gradlew b/gradlew index f5feea6d6b..f3b75f3b0d 100755 --- a/gradlew +++ b/gradlew @@ -86,8 +86,7 @@ done # shellcheck disable=SC2034 APP_BASE_NAME=${0##*/} # Discard cd standard output in case $CDPATH is set (https://github.com/gradle/gradle/issues/25036) -APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s -' "$PWD" ) || exit +APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s\n' "$PWD" ) || exit # Use the maximum available, or set MAX_FD != -1 to use that value. MAX_FD=maximum diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java index bc82e58cab..c211f0f7cc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java @@ -175,10 +175,11 @@ final class ErrorProneCLIFlagsConfig implements Config { "jakarta.inject.Inject", // no explicit initialization when there is dependency injection "javax.inject.Inject", // no explicit initialization when there is dependency injection "com.google.errorprone.annotations.concurrent.LazyInit", - "org.checkerframework.checker.nullness.qual.MonotonicNonNull", "org.springframework.beans.factory.annotation.Autowired", "org.springframework.boot.test.mock.mockito.MockBean", - "org.springframework.boot.test.mock.mockito.SpyBean"); + "org.springframework.boot.test.mock.mockito.SpyBean", + "org.springframework.test.context.bean.override.mockito.MockitoBean", + "org.springframework.test.context.bean.override.mockito.MockitoSpyBean"); private static final String DEFAULT_URL = "http://t.uber.com/nullaway"; @@ -481,9 +482,14 @@ public boolean isKnownInitializerMethod(Symbol.MethodSymbol methodSymbol) { return knownInitializers.contains(classAndName); } + /** + * NOTE: this checks not only for excluded field annotations according to the config, but also for + * a {@code @Nullable} annotation or a {@code @MonotonicNonNull} annotation. + */ @Override public boolean isExcludedFieldAnnotation(String annotationName) { return Nullness.isNullableAnnotation(annotationName, this) + || Nullness.isMonotonicNonNullAnnotation(annotationName) || (fieldAnnotPattern != null && fieldAnnotPattern.matcher(annotationName).matches()); } diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index b9d42df44c..00c36ba772 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -1509,7 +1509,7 @@ public Description matchVariable(VariableTree tree, VisitorState state) { } ExpressionTree initializer = tree.getInitializer(); if (initializer != null) { - if (!symbol.type.isPrimitive() && !skipDueToFieldAnnotation(symbol)) { + if (!symbol.type.isPrimitive() && !skipFieldInitializationCheckingDueToAnnotation(symbol)) { if (mayBeNullExpr(state, initializer)) { ErrorMessage errorMessage = new ErrorMessage( @@ -2398,7 +2398,8 @@ private FieldInitEntities collectEntities(ClassTree tree, VisitorState state) { // field declaration VariableTree varTree = (VariableTree) memberTree; Symbol fieldSymbol = ASTHelpers.getSymbol(varTree); - if (fieldSymbol.type.isPrimitive() || skipDueToFieldAnnotation(fieldSymbol)) { + if (fieldSymbol.type.isPrimitive() + || skipFieldInitializationCheckingDueToAnnotation(fieldSymbol)) { continue; } if (varTree.getInitializer() != null) { @@ -2462,7 +2463,13 @@ private boolean isInitializerMethod(VisitorState state, Symbol.MethodSymbol symb return isInitializerMethod(state, closestOverriddenMethod); } - private boolean skipDueToFieldAnnotation(Symbol fieldSymbol) { + /** + * Checks if the field has an annotation indicating that we should skip initialization checking + * + * @param fieldSymbol the field symbol + * @return true if the field has an annotation indicating that we should skip initialization + */ + private boolean skipFieldInitializationCheckingDueToAnnotation(Symbol fieldSymbol) { return NullabilityUtil.getAllAnnotations(fieldSymbol, config) .map(anno -> anno.getAnnotationType().toString()) .anyMatch(config::isExcludedFieldAnnotation); diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index acdfd60b49..7a226f6ecf 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java @@ -480,7 +480,7 @@ public static boolean mayBeNullFieldFromType( return !(symbol.getSimpleName().toString().equals("class") || symbol.isEnum() || codeAnnotationInfo.isSymbolUnannotated(symbol, config, null)) - && Nullness.hasNullableAnnotation(symbol, config); + && Nullness.hasNullableOrMonotonicNonNullAnnotation(symbol, config); } /** diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java index 158c8e33a9..7db7abf50a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java +++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java @@ -54,6 +54,31 @@ public enum Nullness implements AbstractValue { this.displayName = displayName; } + /** + * Check whether an annotation should be treated as equivalent to @MonotonicNonNull. + * For now checks if the simple name of the annotation is {@code MonotonicNonNull}, from any + * package. + */ + public static boolean isMonotonicNonNullAnnotation(String annotName) { + return annotName.endsWith(".MonotonicNonNull"); + } + + /** + * Check for either a {@code @Nullable} annotation or a {@code @MonotonicNonNull} annotation on + * {@code symbol}. Used to reason whether a field may be null. + */ + public static boolean hasNullableOrMonotonicNonNullAnnotation(Symbol symbol, Config config) { + return hasNullableOrMonotonicNonNullAnnotation( + NullabilityUtil.getAllAnnotations(symbol, config), config); + } + + private static boolean hasNullableOrMonotonicNonNullAnnotation( + Stream annotations, Config config) { + return annotations + .map(anno -> anno.getAnnotationType().toString()) + .anyMatch(anno -> isNullableAnnotation(anno, config) || isMonotonicNonNullAnnotation(anno)); + } + // The following leastUpperBound and greatestLowerBound methods were created by handwriting a // truth table and then encoding the values into these functions. A better approach would be to // represent the lattice directly and compute these functions from the lattice. diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java index 6487e5d34a..f4ecb899b1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java @@ -21,6 +21,7 @@ import static com.uber.nullaway.NullabilityUtil.castToNonNull; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.errorprone.VisitorState; import com.google.errorprone.dataflow.nullnesspropagation.NullnessAnalysis; import com.sun.source.tree.Tree; @@ -211,12 +212,27 @@ public NullnessStore getNullnessInfoBeforeNestedMethodNode( return store.filterAccessPaths( (ap) -> { boolean allAPNonRootElementsAreFinalFields = true; - for (AccessPathElement ape : ap.getElements()) { + ImmutableList elements = ap.getElements(); + for (int i = 0; i < elements.size(); i++) { + AccessPathElement ape = elements.get(i); Element e = ape.getJavaElement(); - if (!e.getKind().equals(ElementKind.FIELD) - || !e.getModifiers().contains(Modifier.FINAL)) { - allAPNonRootElementsAreFinalFields = false; - break; + if (i != elements.size() - 1) { // "inner" elements of the access path + if (!e.getKind().equals(ElementKind.FIELD) + || !e.getModifiers().contains(Modifier.FINAL)) { + allAPNonRootElementsAreFinalFields = false; + break; + } + } else { // last element + // must be a field that is final or annotated with @MonotonicNonNull + if (!e.getKind().equals(ElementKind.FIELD) + || (!e.getModifiers().contains(Modifier.FINAL) + && !e.getAnnotationMirrors().stream() + .anyMatch( + am -> + Nullness.isMonotonicNonNullAnnotation( + am.getAnnotationType().toString())))) { + allAPNonRootElementsAreFinalFields = false; + } } } if (allAPNonRootElementsAreFinalFields) { diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java index f61272168d..715ba66498 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/CoreNullnessStoreInitializer.java @@ -107,7 +107,7 @@ private static NullnessStore lambdaInitialStore( // annotations in case of generic type arguments. Only used in JSpecify mode. List overridenMethodParamTypeList = TypeSubstitutionUtils.memberType( - types, castToNonNull(ASTHelpers.getType(code)), fiMethodSymbol) + types, castToNonNull(ASTHelpers.getType(code)), fiMethodSymbol, config) .getParameterTypes(); // If fiArgumentPositionNullness[i] == null, parameter position i is unannotated Nullness[] fiArgumentPositionNullness = new Nullness[fiMethodParameters.size()]; diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/LocalVariableLocation.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/LocalVariableLocation.java new file mode 100644 index 0000000000..ff46a34c7d --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/LocalVariableLocation.java @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2024 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.fixserialization.location; + +import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.fixserialization.Serializer; +import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; +import javax.lang.model.element.ElementKind; + +/** subtype of {@link AbstractSymbolLocation} targeting local variables. */ +public class LocalVariableLocation extends AbstractSymbolLocation { + + /** Symbol of the targeted local variable. */ + private final Symbol.VarSymbol localVariableSymbol; + + public LocalVariableLocation(Symbol target) { + super(ElementKind.LOCAL_VARIABLE, target); + this.localVariableSymbol = (Symbol.VarSymbol) target; + } + + @Override + public String tabSeparatedToString(SerializationAdapter adapter) { + Symbol.MethodSymbol enclosingMethod = (Symbol.MethodSymbol) localVariableSymbol.owner; + return String.join( + "\t", + type.toString(), + Serializer.serializeSymbol(enclosingClass, adapter), + Serializer.serializeSymbol(enclosingMethod, adapter), + Serializer.serializeSymbol(localVariableSymbol, adapter), + "null", + path != null ? path.toString() : "null"); + } +} diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/SymbolLocation.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/SymbolLocation.java index 262df711e1..e3491aef8c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/SymbolLocation.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/SymbolLocation.java @@ -62,6 +62,21 @@ static SymbolLocation createLocationFromSymbol(Symbol target) { return new MethodLocation(target); case FIELD: return new FieldLocation(target); + // The case where a local variable is declared inside a lambda expression is currently not + // handled. This will require changes to how LocalVariableLocation is created. + // An example of the case : + // void shadowInLambda() { + // Object[] l = new Object[12]; + // a.exec( + // () -> { + // Object[] l = new Object[10]; + // // BUG: Diagnostic contains: Writing @Nullable expression into array with + // @NonNull contents. + // l[0] = null; + // }); + // } + case LOCAL_VARIABLE: + return new LocalVariableLocation(target); default: throw new IllegalArgumentException("Cannot locate node: " + target); } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java index d567c2e028..66b9451c46 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java @@ -3,6 +3,7 @@ import com.google.errorprone.VisitorState; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; +import com.uber.nullaway.Config; import java.util.List; import javax.lang.model.type.NullType; import javax.lang.model.type.TypeKind; @@ -14,9 +15,11 @@ */ public class CheckIdenticalNullabilityVisitor extends Types.DefaultTypeVisitor { private final VisitorState state; + private final Config config; - CheckIdenticalNullabilityVisitor(VisitorState state) { + CheckIdenticalNullabilityVisitor(VisitorState state, Config config) { this.state = state; + this.config = config; } @Override @@ -55,8 +58,13 @@ public Boolean visitClassType(Type.ClassType lhsType, Type rhsType) { for (int i = 0; i < lhsTypeArguments.size(); i++) { Type lhsTypeArgument = lhsTypeArguments.get(i); Type rhsTypeArgument = rhsTypeArguments.get(i); - boolean isLHSNullableAnnotated = GenericsChecks.isNullableAnnotated(lhsTypeArgument, state); - boolean isRHSNullableAnnotated = GenericsChecks.isNullableAnnotated(rhsTypeArgument, state); + if (lhsTypeArgument.getKind().equals(TypeKind.WILDCARD) + || rhsTypeArgument.getKind().equals(TypeKind.WILDCARD)) { + // TODO Handle wildcard types + continue; + } + boolean isLHSNullableAnnotated = GenericsChecks.isNullableAnnotated(lhsTypeArgument, config); + boolean isRHSNullableAnnotated = GenericsChecks.isNullableAnnotated(rhsTypeArgument, config); if (isLHSNullableAnnotated != isRHSNullableAnnotated) { return false; } @@ -86,8 +94,8 @@ public Boolean visitArrayType(Type.ArrayType lhsType, Type rhsType) { Type.ArrayType arrRhsType = (Type.ArrayType) rhsType; Type lhsComponentType = lhsType.getComponentType(); Type rhsComponentType = arrRhsType.getComponentType(); - boolean isLHSNullableAnnotated = GenericsChecks.isNullableAnnotated(lhsComponentType, state); - boolean isRHSNullableAnnotated = GenericsChecks.isNullableAnnotated(rhsComponentType, state); + boolean isLHSNullableAnnotated = GenericsChecks.isNullableAnnotated(lhsComponentType, config); + boolean isRHSNullableAnnotated = GenericsChecks.isNullableAnnotated(rhsComponentType, config); if (isRHSNullableAnnotated != isLHSNullableAnnotated) { return false; } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java index f0f5f5b4cc..f2da7fe687 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java @@ -73,8 +73,13 @@ public String visitCapturedType(Type.CapturedType t, Void s) { @Override public String visitArrayType(Type.ArrayType t, Void unused) { - // TODO properly print cases like int @Nullable[] - return t.elemtype.accept(this, null) + "[]"; + StringBuilder sb = new StringBuilder(); + sb.append(t.elemtype.accept(this, null)); + for (Attribute.TypeCompound compound : t.getAnnotationMirrors()) { + sb.append(" @"); + sb.append(compound.type.accept(this, null)); + } + return sb.append(" []").toString(); } @Override diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 2205c66854..3318763a44 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -4,8 +4,6 @@ import static com.uber.nullaway.NullabilityUtil.castToNonNull; import com.google.errorprone.VisitorState; -import com.google.errorprone.suppliers.Supplier; -import com.google.errorprone.suppliers.Suppliers; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AnnotatedTypeTree; import com.sun.source.tree.AnnotationTree; @@ -19,6 +17,7 @@ import com.sun.source.tree.NewArrayTree; import com.sun.source.tree.NewClassTree; import com.sun.source.tree.ParameterizedTypeTree; +import com.sun.source.tree.ParenthesizedTree; import com.sun.source.tree.Tree; import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; @@ -47,14 +46,6 @@ /** Methods for performing checks related to generic types and nullability. */ public final class GenericsChecks { - /** - * Supplier for the JSpecify {@code @Nullable} annotation. Required since for now, certain checks - * related to generics specifically look for {@code @org.jspecify.annotations.Nullable} - * annotations and do not apply to other {@code @Nullable} annotations. - */ - static final Supplier JSPECIFY_NULLABLE_TYPE_SUPPLIER = - Suppliers.typeFromString("org.jspecify.annotations.Nullable"); - /** Do not instantiate; all methods should be static */ private GenericsChecks() {} @@ -102,7 +93,7 @@ public static void checkInstantiationForParameterizedTypedTree( return; } boolean[] typeParamsWithNullableUpperBound = - getTypeParamsWithNullableUpperBound(baseType, config, state, handler); + getTypeParamsWithNullableUpperBound(baseType, config, handler); com.sun.tools.javac.util.List baseTypeArgs = baseType.tsym.type.getTypeArguments(); for (int i = 0; i < baseTypeArgs.size(); i++) { if (nullableTypeArguments.containsKey(i) && !typeParamsWithNullableUpperBound[i]) { @@ -115,7 +106,7 @@ public static void checkInstantiationForParameterizedTypedTree( } private static boolean[] getTypeParamsWithNullableUpperBound( - Type type, Config config, VisitorState state, Handler handler) { + Type type, Config config, Handler handler) { Symbol.TypeSymbol tsym = type.tsym; com.sun.tools.javac.util.List baseTypeArgs = tsym.type.getTypeArguments(); boolean[] result = new boolean[baseTypeArgs.size()]; @@ -138,8 +129,8 @@ private static boolean[] getTypeParamsWithNullableUpperBound( if (rawTypeAttributes != null) { for (Attribute.TypeCompound typeCompound : rawTypeAttributes) { if (typeCompound.position.type.equals(TargetType.CLASS_TYPE_PARAMETER_BOUND) - && ASTHelpers.isSameType( - typeCompound.type, JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state), state)) { + && Nullness.isNullableAnnotation( + typeCompound.type.tsym.getQualifiedName().toString(), config)) { int index = typeCompound.position.parameter_index; result[index] = true; } @@ -160,12 +151,21 @@ private static boolean[] getTypeParamsWithNullableUpperBound( */ public static void checkGenericMethodCallTypeArguments( Tree tree, VisitorState state, NullAway analysis, Config config, Handler handler) { - List typeArguments = ((MethodInvocationTree) tree).getTypeArguments(); + List typeArguments; + switch (tree.getKind()) { + case METHOD_INVOCATION: + typeArguments = ((MethodInvocationTree) tree).getTypeArguments(); + break; + case NEW_CLASS: + typeArguments = ((NewClassTree) tree).getTypeArguments(); + break; + default: + throw new RuntimeException("Unexpected tree kind: " + tree.getKind()); + } if (typeArguments.isEmpty()) { return; } // get Nullable annotated type arguments - MethodInvocationTree methodTree = (MethodInvocationTree) tree; Map nullableTypeArguments = new HashMap<>(); for (int i = 0; i < typeArguments.size(); i++) { Tree curTypeArg = typeArguments.get(i); @@ -181,7 +181,8 @@ public static void checkGenericMethodCallTypeArguments( } } } - Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodTree); + Symbol.MethodSymbol methodSymbol = + castToNonNull((Symbol.MethodSymbol) ASTHelpers.getSymbol(tree)); // check if type variables are allowed to be Nullable Type baseType = methodSymbol.asType(); @@ -357,10 +358,10 @@ private static void reportInvalidOverridingMethodParamTypeError( * Foo<@Nullable A>}). * * @param tree A tree for which we need the type with preserved annotations. - * @param state the visitor state + * @param config the analysis config * @return Type of the tree with preserved annotations. */ - private static @Nullable Type getTreeType(Tree tree, VisitorState state) { + private static @Nullable Type getTreeType(Tree tree, Config config) { if (tree instanceof NewClassTree && ((NewClassTree) tree).getIdentifier() instanceof ParameterizedTypeTree) { ParameterizedTypeTree paramTypedTree = @@ -370,10 +371,10 @@ private static void reportInvalidOverridingMethodParamTypeError( // TODO: support diamond operators return null; } - return typeWithPreservedAnnotations(paramTypedTree, state); + return typeWithPreservedAnnotations(paramTypedTree, config); } else if (tree instanceof NewArrayTree && ((NewArrayTree) tree).getType() instanceof AnnotatedTypeTree) { - return typeWithPreservedAnnotations(tree, state); + return typeWithPreservedAnnotations(tree, config); } else { Type result; if (tree instanceof VariableTree || tree instanceof IdentifierTree) { @@ -414,10 +415,11 @@ private static void reportInvalidOverridingMethodParamTypeError( */ public static void checkTypeParameterNullnessForAssignability( Tree tree, NullAway analysis, VisitorState state) { - if (!analysis.getConfig().isJSpecifyMode()) { + Config config = analysis.getConfig(); + if (!config.isJSpecifyMode()) { return; } - Type lhsType = getTreeType(tree, state); + Type lhsType = getTreeType(tree, config); Tree rhsTree; if (tree instanceof VariableTree) { VariableTree varTree = (VariableTree) tree; @@ -431,10 +433,10 @@ public static void checkTypeParameterNullnessForAssignability( if (rhsTree == null || rhsTree.getKind().equals(Tree.Kind.NULL_LITERAL)) { return; } - Type rhsType = getTreeType(rhsTree, state); + Type rhsType = getTreeType(rhsTree, config); if (lhsType != null && rhsType != null) { - boolean isAssignmentValid = subtypeParameterNullability(lhsType, rhsType, state); + boolean isAssignmentValid = subtypeParameterNullability(lhsType, rhsType, state, config); if (!isAssignmentValid) { reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis); } @@ -455,7 +457,8 @@ public static void checkTypeParameterNullnessForFunctionReturnType( Symbol.MethodSymbol methodSymbol, NullAway analysis, VisitorState state) { - if (!analysis.getConfig().isJSpecifyMode()) { + Config config = analysis.getConfig(); + if (!config.isJSpecifyMode()) { return; } @@ -464,10 +467,10 @@ public static void checkTypeParameterNullnessForFunctionReturnType( // bail out of any checking involving raw types for now return; } - Type returnExpressionType = getTreeType(retExpr, state); + Type returnExpressionType = getTreeType(retExpr, config); if (formalReturnType != null && returnExpressionType != null) { boolean isReturnTypeValid = - subtypeParameterNullability(formalReturnType, returnExpressionType, state); + subtypeParameterNullability(formalReturnType, returnExpressionType, state, config); if (!isReturnTypeValid) { reportInvalidReturnTypeError( retExpr, formalReturnType, returnExpressionType, state, analysis); @@ -487,20 +490,20 @@ public static void checkTypeParameterNullnessForFunctionReturnType( * @param state the visitor state */ private static boolean identicalTypeParameterNullability( - Type lhsType, Type rhsType, VisitorState state) { - return lhsType.accept(new CheckIdenticalNullabilityVisitor(state), rhsType); + Type lhsType, Type rhsType, VisitorState state, Config config) { + return lhsType.accept(new CheckIdenticalNullabilityVisitor(state, config), rhsType); } /** - * Like {@link #identicalTypeParameterNullability(Type, Type, VisitorState)}, but allows for - * covariant array subtyping at the top level. + * Like {@link #identicalTypeParameterNullability(Type, Type, VisitorState, Config)}, but allows + * for covariant array subtyping at the top level. * * @param lhsType type for the lhs of the assignment * @param rhsType type for the rhs of the assignment * @param state the visitor state */ private static boolean subtypeParameterNullability( - Type lhsType, Type rhsType, VisitorState state) { + Type lhsType, Type rhsType, VisitorState state, Config config) { if (lhsType.getKind().equals(TypeKind.ARRAY) && rhsType.getKind().equals(TypeKind.ARRAY)) { // for array types we must allow covariance, i.e., an array of @NonNull references is a // subtype of an array of @Nullable references; see @@ -509,15 +512,15 @@ private static boolean subtypeParameterNullability( Type.ArrayType rhsArrayType = (Type.ArrayType) rhsType; Type lhsComponentType = lhsArrayType.getComponentType(); Type rhsComponentType = rhsArrayType.getComponentType(); - boolean isLHSNullableAnnotated = isNullableAnnotated(lhsComponentType, state); - boolean isRHSNullableAnnotated = isNullableAnnotated(rhsComponentType, state); + boolean isLHSNullableAnnotated = isNullableAnnotated(lhsComponentType, config); + boolean isRHSNullableAnnotated = isNullableAnnotated(rhsComponentType, config); // an array of @Nullable references is _not_ a subtype of an array of @NonNull references if (isRHSNullableAnnotated && !isLHSNullableAnnotated) { return false; } - return identicalTypeParameterNullability(lhsComponentType, rhsComponentType, state); + return identicalTypeParameterNullability(lhsComponentType, rhsComponentType, state, config); } else { - return identicalTypeParameterNullability(lhsType, rhsType, state); + return identicalTypeParameterNullability(lhsType, rhsType, state, config); } } @@ -527,11 +530,10 @@ private static boolean subtypeParameterNullability( * Type of the tree with the annotations. * * @param tree A parameterized typed tree for which we need class type with preserved annotations. - * @param state the visitor state * @return A Type with preserved annotations. */ - private static Type typeWithPreservedAnnotations(Tree tree, VisitorState state) { - return tree.accept(new PreservedAnnotationTreeVisitor(state), null); + private static Type typeWithPreservedAnnotations(Tree tree, Config config) { + return tree.accept(new PreservedAnnotationTreeVisitor(config), null); } /** @@ -551,28 +553,29 @@ private static Type typeWithPreservedAnnotations(Tree tree, VisitorState state) */ public static void checkTypeParameterNullnessForConditionalExpression( ConditionalExpressionTree tree, NullAway analysis, VisitorState state) { - if (!analysis.getConfig().isJSpecifyMode()) { + Config config = analysis.getConfig(); + if (!config.isJSpecifyMode()) { return; } Tree truePartTree = tree.getTrueExpression(); Tree falsePartTree = tree.getFalseExpression(); - Type condExprType = getConditionalExpressionType(tree, state); - Type truePartType = getTreeType(truePartTree, state); - Type falsePartType = getTreeType(falsePartTree, state); + Type condExprType = getConditionalExpressionType(tree, state, config); + Type truePartType = getTreeType(truePartTree, config); + Type falsePartType = getTreeType(falsePartTree, config); // The condExpr type should be the least-upper bound of the true and false part types. To check // the nullability annotations, we check that the true and false parts are assignable to the // type of the whole expression if (condExprType != null) { if (truePartType != null) { - if (!subtypeParameterNullability(condExprType, truePartType, state)) { + if (!subtypeParameterNullability(condExprType, truePartType, state, config)) { reportMismatchedTypeForTernaryOperator( truePartTree, condExprType, truePartType, state, analysis); } } if (falsePartType != null) { - if (!subtypeParameterNullability(condExprType, falsePartType, state)) { + if (!subtypeParameterNullability(condExprType, falsePartType, state, config)) { reportMismatchedTypeForTernaryOperator( falsePartTree, condExprType, falsePartType, state, analysis); } @@ -581,14 +584,19 @@ public static void checkTypeParameterNullnessForConditionalExpression( } private static @Nullable Type getConditionalExpressionType( - ConditionalExpressionTree tree, VisitorState state) { + ConditionalExpressionTree tree, VisitorState state, Config config) { // hack: sometimes array nullability doesn't get computed correctly for a conditional expression // on the RHS of an assignment. So, look at the type of the assignment tree. - Tree parent = state.getPath().getParentPath().getLeaf(); + TreePath parentPath = state.getPath().getParentPath(); + Tree parent = parentPath.getLeaf(); + while (parent instanceof ParenthesizedTree) { + parentPath = parentPath.getParentPath(); + parent = parentPath.getLeaf(); + } if (parent instanceof AssignmentTree || parent instanceof VariableTree) { - return getTreeType(parent, state); + return getTreeType(parent, config); } - return getTreeType(tree, state); + return getTreeType(tree, config); } /** @@ -609,7 +617,8 @@ public static void compareGenericTypeParameterNullabilityForCall( boolean isVarArgs, NullAway analysis, VisitorState state) { - if (!analysis.getConfig().isJSpecifyMode()) { + Config config = analysis.getConfig(); + if (!config.isJSpecifyMode()) { return; } Type invokedMethodType = methodSymbol.type; @@ -618,20 +627,21 @@ public static void compareGenericTypeParameterNullabilityForCall( ExpressionTree methodSelect = ((MethodInvocationTree) tree).getMethodSelect(); Type enclosingType; if (methodSelect instanceof MemberSelectTree) { - enclosingType = getTreeType(((MemberSelectTree) methodSelect).getExpression(), state); + enclosingType = getTreeType(((MemberSelectTree) methodSelect).getExpression(), config); } else { // implicit this parameter enclosingType = methodSymbol.owner.type; } if (enclosingType != null) { invokedMethodType = - TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, methodSymbol); + TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, methodSymbol, config); } } // substitute type arguments for generic methods if (tree instanceof MethodInvocationTree && methodSymbol.type instanceof Type.ForAll) { invokedMethodType = - substituteTypeArgsInGenericMethodType((MethodInvocationTree) tree, methodSymbol, state); + substituteTypeArgsInGenericMethodType( + (MethodInvocationTree) tree, methodSymbol, state, config); } List formalParamTypes = invokedMethodType.getParameterTypes(); int n = formalParamTypes.size(); @@ -646,9 +656,9 @@ public static void compareGenericTypeParameterNullabilityForCall( // bail out of any checking involving raw types for now return; } - Type actualParameter = getTreeType(actualParams.get(i), state); + Type actualParameter = getTreeType(actualParams.get(i), config); if (actualParameter != null) { - if (!subtypeParameterNullability(formalParameter, actualParameter, state)) { + if (!subtypeParameterNullability(formalParameter, actualParameter, state, config)) { reportInvalidParametersNullabilityError( formalParameter, actualParameter, actualParams.get(i), state, analysis); } @@ -659,12 +669,13 @@ public static void compareGenericTypeParameterNullabilityForCall( (Type.ArrayType) formalParamTypes.get(formalParamTypes.size() - 1); Type varargsElementType = varargsArrayType.elemtype; for (int i = formalParamTypes.size() - 1; i < actualParams.size(); i++) { - Type actualParameterType = getTreeType(actualParams.get(i), state); + Type actualParameterType = getTreeType(actualParams.get(i), config); // If the actual parameter type is assignable to the varargs array type, then the call site // is passing the varargs directly in an array, and we should skip our check. if (actualParameterType != null && !state.getTypes().isAssignable(actualParameterType, varargsArrayType)) { - if (!subtypeParameterNullability(varargsElementType, actualParameterType, state)) { + if (!subtypeParameterNullability( + varargsElementType, actualParameterType, state, config)) { reportInvalidParametersNullabilityError( varargsElementType, actualParameterType, actualParams.get(i), state, analysis); } @@ -696,7 +707,7 @@ public static void checkTypeParameterNullnessForMethodOverriding( // method's class Type methodWithTypeParams = TypeSubstitutionUtils.memberType( - state.getTypes(), overridingMethod.owner.type, overriddenMethod); + state.getTypes(), overridingMethod.owner.type, overriddenMethod, analysis.getConfig()); checkTypeParameterNullnessForOverridingMethodReturnType( tree, methodWithTypeParams, analysis, state); @@ -736,7 +747,7 @@ public static void checkTypeParameterNullnessForMethodOverriding( */ public static Nullness getGenericMethodReturnTypeNullness( Symbol.MethodSymbol method, Symbol enclosingSymbol, VisitorState state, Config config) { - Type enclosingType = getTypeForSymbol(enclosingSymbol, state); + Type enclosingType = getTypeForSymbol(enclosingSymbol, state, config); return getGenericMethodReturnTypeNullness(method, enclosingType, state, config); } @@ -745,9 +756,10 @@ public static Nullness getGenericMethodReturnTypeNullness( * * @param symbol the symbol * @param state the visitor state + * @param config the analysis config * @return the type for {@code symbol} */ - private static @Nullable Type getTypeForSymbol(Symbol symbol, VisitorState state) { + private static @Nullable Type getTypeForSymbol(Symbol symbol, VisitorState state, Config config) { if (symbol.isAnonymous()) { // For anonymous classes, symbol.type does not contain annotations on generic type parameters. // So, we get a correct type from the enclosing NewClassTree. @@ -757,7 +769,7 @@ public static Nullness getGenericMethodReturnTypeNullness( throw new RuntimeException( "method should be inside a NewClassTree " + state.getSourceForNode(path.getLeaf())); } - Type typeFromTree = getTreeType(newClassTree, state); + Type typeFromTree = getTreeType(newClassTree, config); if (typeFromTree != null) { verify(state.getTypes().isAssignable(symbol.type, typeFromTree)); } @@ -775,7 +787,7 @@ public static Nullness getGenericMethodReturnTypeNullness( return Nullness.NONNULL; } Type overriddenMethodType = - TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, method); + TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, method, config); verify( overriddenMethodType instanceof ExecutableType, "expected ExecutableType but instead got %s", @@ -824,7 +836,8 @@ public static Nullness getGenericReturnNullnessAtInvocation( if (!invokedMethodSymbol.getTypeParameters().isEmpty()) { // Substitute type arguments inside the return type Type substitutedReturnType = - substituteTypeArgsInGenericMethodType(tree, invokedMethodSymbol, state).getReturnType(); + substituteTypeArgsInGenericMethodType(tree, invokedMethodSymbol, state, config) + .getReturnType(); // If this condition evaluates to false, we fall through to the subsequent logic, to handle // type variables declared on the enclosing class if (substitutedReturnType != null @@ -837,7 +850,7 @@ public static Nullness getGenericReturnNullnessAtInvocation( return Nullness.NONNULL; } Type methodReceiverType = - getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state); + getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), config); if (methodReceiverType == null) { return Nullness.NONNULL; } else { @@ -864,12 +877,14 @@ private static com.sun.tools.javac.util.List convertTreesToTypes( * @param methodInvocationTree the method invocation tree * @param methodSymbol symbol for the invoked generic method * @param state the visitor state + * @param config the NullAway config * @return the substituted method type for the generic method */ private static Type substituteTypeArgsInGenericMethodType( MethodInvocationTree methodInvocationTree, Symbol.MethodSymbol methodSymbol, - VisitorState state) { + VisitorState state, + Config config) { List typeArgumentTrees = methodInvocationTree.getTypeArguments(); com.sun.tools.javac.util.List explicitTypeArgs = convertTreesToTypes(typeArgumentTrees); @@ -877,7 +892,7 @@ private static Type substituteTypeArgsInGenericMethodType( Type.ForAll forAllType = (Type.ForAll) methodSymbol.type; Type.MethodType underlyingMethodType = (Type.MethodType) forAllType.qtype; return TypeSubstitutionUtils.subst( - state.getTypes(), underlyingMethodType, forAllType.tvars, explicitTypeArgs); + state.getTypes(), underlyingMethodType, forAllType.tvars, explicitTypeArgs, config); } /** @@ -925,7 +940,7 @@ public static Nullness getGenericParameterNullnessAtInvocation( // Substitute the argument types within the MethodType // NOTE: if explicitTypeArgs is empty, this is a noop List substitutedParamTypes = - substituteTypeArgsInGenericMethodType(tree, invokedMethodSymbol, state) + substituteTypeArgsInGenericMethodType(tree, invokedMethodSymbol, state, config) .getParameterTypes(); // If this condition evaluates to false, we fall through to the subsequent logic, to handle // type variables declared on the enclosing class @@ -940,7 +955,7 @@ public static Nullness getGenericParameterNullnessAtInvocation( return Nullness.NONNULL; } Type enclosingType = - getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state); + getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), config); return getGenericMethodParameterNullness( paramIndex, invokedMethodSymbol, enclosingType, state, config); } @@ -982,7 +997,7 @@ public static Nullness getGenericMethodParameterNullness( Symbol enclosingSymbol, VisitorState state, Config config) { - Type enclosingType = getTypeForSymbol(enclosingSymbol, state); + Type enclosingType = getTypeForSymbol(enclosingSymbol, state, config); return getGenericMethodParameterNullness(parameterIndex, method, enclosingType, state, config); } @@ -1011,7 +1026,8 @@ public static Nullness getGenericMethodParameterNullness( // @Nullable annotation is handled elsewhere) return Nullness.NONNULL; } - Type methodType = TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, method); + Type methodType = + TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, method, config); Type paramType = methodType.getParameterTypes().get(parameterIndex); return getTypeNullness(paramType, config); } @@ -1031,12 +1047,13 @@ private static void checkTypeParameterNullnessForOverridingMethodParameterType( List methodParameters = tree.getParameters(); List overriddenMethodParameterTypes = overriddenMethodType.getParameterTypes(); for (int i = 0; i < methodParameters.size(); i++) { - Type overridingMethodParameterType = getTreeType(methodParameters.get(i), state); + Config config = analysis.getConfig(); + Type overridingMethodParameterType = getTreeType(methodParameters.get(i), config); Type overriddenMethodParameterType = overriddenMethodParameterTypes.get(i); if (overriddenMethodParameterType != null && overridingMethodParameterType != null) { // allow contravariant subtyping if (!subtypeParameterNullability( - overridingMethodParameterType, overriddenMethodParameterType, state)) { + overridingMethodParameterType, overriddenMethodParameterType, state, config)) { reportInvalidOverridingMethodParamTypeError( methodParameters.get(i), overriddenMethodParameterType, @@ -1069,7 +1086,7 @@ private static void checkTypeParameterNullnessForOverridingMethodReturnType( } // allow covariant subtyping if (!subtypeParameterNullability( - overriddenMethodReturnType, overridingMethodReturnType, state)) { + overriddenMethodReturnType, overridingMethodReturnType, state, analysis.getConfig())) { reportInvalidOverridingMethodReturnTypeError( tree, overriddenMethodReturnType, overridingMethodReturnType, analysis, state); } @@ -1140,18 +1157,7 @@ public static boolean passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode( return callingUnannotated; } - public static boolean isNullableAnnotated(Type type, VisitorState state) { - boolean result = false; - // To ensure that we are checking only jspecify nullable annotations - Type jspecifyNullableType = JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state); - List lhsAnnotations = type.getAnnotationMirrors(); - for (Attribute.TypeCompound annotation : lhsAnnotations) { - if (ASTHelpers.isSameType( - (Type) annotation.getAnnotationType(), jspecifyNullableType, state)) { - result = true; - break; - } - } - return result; + public static boolean isNullableAnnotated(Type type, Config config) { + return Nullness.hasNullableAnnotation(type.getAnnotationMirrors().stream(), config); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java index 51d5224951..adb83f8d8e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java @@ -1,8 +1,8 @@ package com.uber.nullaway.generics; import static com.uber.nullaway.NullabilityUtil.castToNonNull; +import static com.uber.nullaway.generics.TypeMetadataBuilder.TYPE_METADATA_BUILDER; -import com.google.errorprone.VisitorState; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AnnotatedTypeTree; import com.sun.source.tree.AnnotationTree; @@ -15,10 +15,8 @@ import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.TypeMetadata; -import com.sun.tools.javac.util.ListBuffer; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.MethodType; +import com.uber.nullaway.Config; +import com.uber.nullaway.Nullness; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -31,10 +29,10 @@ */ public class PreservedAnnotationTreeVisitor extends SimpleTreeVisitor { - private final VisitorState state; + private final Config config; - PreservedAnnotationTreeVisitor(VisitorState state) { - this.state = state; + PreservedAnnotationTreeVisitor(Config config) { + this.config = config; } @Override @@ -57,7 +55,8 @@ public Type visitParameterizedType(ParameterizedTypeTree tree, Void p) { for (int i = 0; i < typeArguments.size(); i++) { newTypeArgs.add(typeArguments.get(i).accept(this, null)); } - Type finalType = TYPE_METADATA_BUILDER.createWithBaseTypeAndTypeArgs(baseType, newTypeArgs); + Type finalType = + TYPE_METADATA_BUILDER.createClassType(baseType, baseType.getEnclosingType(), newTypeArgs); return finalType; } @@ -65,11 +64,15 @@ public Type visitParameterizedType(ParameterizedTypeTree tree, Void p) { public Type visitAnnotatedType(AnnotatedTypeTree annotatedType, Void unused) { List annotations = annotatedType.getAnnotations(); boolean hasNullableAnnotation = false; - Type nullableType = GenericsChecks.JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state); + Type nullableType = null; for (AnnotationTree annotation : annotations) { - if (ASTHelpers.isSameType( - nullableType, ASTHelpers.getType(annotation.getAnnotationType()), state)) { + Symbol annotSymbol = ASTHelpers.getSymbol(annotation.getAnnotationType()); + if (annotSymbol != null + && Nullness.isNullableAnnotation(annotSymbol.getQualifiedName().toString(), config)) { hasNullableAnnotation = true; + // save the type of the nullable annotation, so that we can use it when constructing the + // TypeMetadata object below + nullableType = castToNonNull(ASTHelpers.getType(annotation)); break; } } @@ -92,181 +95,4 @@ public Type visitAnnotatedType(AnnotatedTypeTree annotatedType, Void unused) { protected Type defaultAction(Tree node, Void unused) { return castToNonNull(ASTHelpers.getType(node)); } - - /** - * Abstracts over the different APIs for building {@link TypeMetadata} objects in different JDK - * versions. - */ - private interface TypeMetadataBuilder { - TypeMetadata create(com.sun.tools.javac.util.List attrs); - - Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metaData); - - Type createWithBaseTypeAndTypeArgs(Type baseType, List typeArgs); - } - - /** - * Provides implementations for methods under TypeMetadataBuilder compatible with JDK 17 and - * earlier versions. - */ - private static class JDK17AndEarlierTypeMetadataBuilder implements TypeMetadataBuilder { - - @Override - public TypeMetadata create(com.sun.tools.javac.util.List attrs) { - return new TypeMetadata(new TypeMetadata.Annotations(attrs)); - } - - /** - * Clones the given type with the specified Metadata for getting the right nullability - * annotations. - * - * @param typeToBeCloned The Type we want to clone with the required Nullability Metadata - * @param metadata The required Nullability metadata which is lost from the type - * @return Type after it has been cloned by applying the required Nullability metadata - */ - @Override - public Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metadata) { - return typeToBeCloned.cloneWithMetadata(metadata); - } - - @Override - public Type createWithBaseTypeAndTypeArgs(Type baseType, List typeArgs) { - return new Type.ClassType( - baseType.getEnclosingType(), - com.sun.tools.javac.util.List.from(typeArgs), - baseType.tsym, - baseType.getMetadata()); - } - } - - /** - * Provides implementations for methods under TypeMetadataBuilder compatible with the updates made - * to the library methods for Jdk 21. The implementation calls the logic specific to JDK 21 - * indirectly using MethodHandles since we still need the code to compile on earlier versions. - */ - private static class JDK21TypeMetadataBuilder implements TypeMetadataBuilder { - - private static final MethodHandle typeMetadataConstructorHandle = createHandle(); - private static final MethodHandle addMetadataHandle = - createVirtualMethodHandle(Type.class, TypeMetadata.class, Type.class, "addMetadata"); - private static final MethodHandle dropMetadataHandle = - createVirtualMethodHandle(Type.class, Class.class, Type.class, "dropMetadata"); - private static final MethodHandle getMetadataHandler = createGetMetadataHandle(); - private static final MethodHandle classTypeConstructorHandle = - createClassTypeConstructorHandle(); - - private static MethodHandle createHandle() { - MethodHandles.Lookup lookup = MethodHandles.lookup(); - MethodType mt = MethodType.methodType(void.class, com.sun.tools.javac.util.ListBuffer.class); - try { - return lookup.findConstructor(TypeMetadata.Annotations.class, mt); - } catch (NoSuchMethodException e) { - throw new RuntimeException(e); - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } - } - - private static MethodHandle createGetMetadataHandle() { - MethodHandles.Lookup lookup = MethodHandles.lookup(); - MethodType mt = MethodType.methodType(com.sun.tools.javac.util.List.class); - try { - return lookup.findVirtual(Type.class, "getMetadata", mt); - } catch (NoSuchMethodException | IllegalAccessException e) { - throw new RuntimeException(e); - } - } - - private static MethodHandle createClassTypeConstructorHandle() { - try { - MethodHandles.Lookup lookup = MethodHandles.lookup(); - MethodType methodType = - MethodType.methodType( - void.class, // return type for a constructor is void - Type.class, - com.sun.tools.javac.util.List.class, - Symbol.TypeSymbol.class, - com.sun.tools.javac.util.List.class); - return lookup.findConstructor(Type.ClassType.class, methodType); - } catch (NoSuchMethodException | IllegalAccessException e) { - throw new RuntimeException(e); - } - } - - /** - * Used to get a MethodHandle for a virtual method from the specified class - * - * @param retTypeClass Class to indicate the return type of the desired method - * @param paramTypeClass Class to indicate the parameter type of the desired method - * @param refClass Class within which the desired method is contained - * @param methodName Name of the desired method - * @return The appropriate MethodHandle for the virtual method - */ - private static MethodHandle createVirtualMethodHandle( - Class retTypeClass, Class paramTypeClass, Class refClass, String methodName) { - MethodHandles.Lookup lookup = MethodHandles.lookup(); - MethodType mt = MethodType.methodType(retTypeClass, paramTypeClass); - try { - return lookup.findVirtual(refClass, methodName, mt); - } catch (NoSuchMethodException e) { - throw new RuntimeException(e); - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } - } - - @Override - public TypeMetadata create(com.sun.tools.javac.util.List attrs) { - ListBuffer b = new ListBuffer<>(); - b.appendList(attrs); - try { - return (TypeMetadata) typeMetadataConstructorHandle.invoke(b); - } catch (Throwable e) { - throw new RuntimeException(e); - } - } - - /** - * Calls dropMetadata and addMetadata using MethodHandles for JDK 21, which removed the previous - * cloneWithMetadata method. - * - * @param typeToBeCloned The Type we want to clone with the required Nullability metadata - * @param metadata The required Nullability metadata - * @return Cloned Type with the necessary Nullability metadata - */ - @Override - public Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metadata) { - try { - // In JDK 21 addMetadata works if there is no metadata associated with the type, so we - // create a copy without the existing metadata first and then add it - Type clonedTypeWithoutMetadata = - (Type) dropMetadataHandle.invoke(typeToBeCloned, metadata.getClass()); - return (Type) addMetadataHandle.invoke(clonedTypeWithoutMetadata, metadata); - } catch (Throwable e) { - throw new RuntimeException(e); - } - } - - @Override - public Type createWithBaseTypeAndTypeArgs(Type baseType, List typeArgs) { - try { - com.sun.tools.javac.util.List metadata = - (com.sun.tools.javac.util.List) getMetadataHandler.invoke(baseType); - return (Type) - classTypeConstructorHandle.invoke( - baseType.getEnclosingType(), - com.sun.tools.javac.util.List.from(typeArgs), - baseType.tsym, - metadata); - } catch (Throwable e) { - throw new RuntimeException(e); - } - } - } - - /** The TypeMetadataBuilder to be used for the current JDK version. */ - private static final TypeMetadataBuilder TYPE_METADATA_BUILDER = - Runtime.version().feature() >= 21 - ? new JDK21TypeMetadataBuilder() - : new JDK17AndEarlierTypeMetadataBuilder(); } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/TypeMetadataBuilder.java b/nullaway/src/main/java/com/uber/nullaway/generics/TypeMetadataBuilder.java new file mode 100644 index 0000000000..b9e96f782c --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/generics/TypeMetadataBuilder.java @@ -0,0 +1,262 @@ +package com.uber.nullaway.generics; + +import com.sun.tools.javac.code.Attribute; +import com.sun.tools.javac.code.BoundKind; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.code.TypeMetadata; +import com.sun.tools.javac.util.ListBuffer; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.util.List; + +/** + * Abstracts over the different APIs for building {@link TypeMetadata} objects in different JDK + * versions. + */ +public interface TypeMetadataBuilder { + /** The TypeMetadataBuilder to be used for the current JDK version. */ + TypeMetadataBuilder TYPE_METADATA_BUILDER = + Runtime.version().feature() >= 21 + ? new JDK21TypeMetadataBuilder() + : new JDK17AndEarlierTypeMetadataBuilder(); + + TypeMetadata create(com.sun.tools.javac.util.List attrs); + + Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metaData); + + Type.ClassType createClassType(Type baseType, Type enclosingType, List typeArgs); + + Type.ArrayType createArrayType(Type.ArrayType baseType, Type elementType); + + Type.WildcardType createWildcardType(Type.WildcardType baseType, Type boundType); + + /** + * Provides implementations for methods under TypeMetadataBuilder compatible with JDK 17 and + * earlier versions. + */ + class JDK17AndEarlierTypeMetadataBuilder implements TypeMetadataBuilder { + + @Override + public TypeMetadata create(com.sun.tools.javac.util.List attrs) { + return new TypeMetadata(new TypeMetadata.Annotations(attrs)); + } + + /** + * Clones the given type with the specified Metadata for getting the right nullability + * annotations. + * + * @param typeToBeCloned The Type we want to clone with the required Nullability Metadata + * @param metadata The required Nullability metadata which is lost from the type + * @return Type after it has been cloned by applying the required Nullability metadata + */ + @Override + public Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metadata) { + return typeToBeCloned.cloneWithMetadata(metadata); + } + + @Override + public Type.ClassType createClassType(Type baseType, Type enclosingType, List typeArgs) { + return new Type.ClassType( + enclosingType, + com.sun.tools.javac.util.List.from(typeArgs), + baseType.tsym, + baseType.getMetadata()); + } + + @Override + public Type.ArrayType createArrayType(Type.ArrayType baseType, Type elementType) { + return new Type.ArrayType(elementType, baseType.tsym, baseType.getMetadata()); + } + + @Override + public Type.WildcardType createWildcardType(Type.WildcardType baseType, Type boundType) { + return new Type.WildcardType(boundType, baseType.kind, baseType.tsym, baseType.getMetadata()); + } + } + + /** + * Provides implementations for methods under TypeMetadataBuilder compatible with the updates made + * to the library methods for Jdk 21. The implementation calls the logic specific to JDK 21 + * indirectly using MethodHandles since we still need the code to compile on earlier versions. + */ + class JDK21TypeMetadataBuilder implements TypeMetadataBuilder { + + private static final MethodHandle typeMetadataConstructorHandle = createHandle(); + private static final MethodHandle addMetadataHandle = + createVirtualMethodHandle(Type.class, TypeMetadata.class, Type.class, "addMetadata"); + private static final MethodHandle dropMetadataHandle = + createVirtualMethodHandle(Type.class, Class.class, Type.class, "dropMetadata"); + private static final MethodHandle getMetadataHandler = createGetMetadataHandle(); + private static final MethodHandle classTypeConstructorHandle = + createClassTypeConstructorHandle(); + private static final MethodHandle arrayTypeConstructorHandle = + createArrayTypeConstructorHandle(); + private static final MethodHandle wildcardTypeConstructorHandle = + createWildcardTypeConstructorHandle(); + + private static MethodHandle createHandle() { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodType mt = MethodType.methodType(void.class, com.sun.tools.javac.util.ListBuffer.class); + try { + return lookup.findConstructor(TypeMetadata.Annotations.class, mt); + } catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + private static MethodHandle createGetMetadataHandle() { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodType mt = MethodType.methodType(com.sun.tools.javac.util.List.class); + try { + return lookup.findVirtual(Type.class, "getMetadata", mt); + } catch (NoSuchMethodException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + private static MethodHandle createClassTypeConstructorHandle() { + try { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodType methodType = + MethodType.methodType( + void.class, // return type for a constructor is void + Type.class, + com.sun.tools.javac.util.List.class, + Symbol.TypeSymbol.class, + com.sun.tools.javac.util.List.class); + return lookup.findConstructor(Type.ClassType.class, methodType); + } catch (NoSuchMethodException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + private static MethodHandle createArrayTypeConstructorHandle() { + try { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodType methodType = + MethodType.methodType( + void.class, // return type for a constructor is void + Type.class, + Symbol.TypeSymbol.class, + com.sun.tools.javac.util.List.class); + return lookup.findConstructor(Type.ArrayType.class, methodType); + } catch (NoSuchMethodException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + private static MethodHandle createWildcardTypeConstructorHandle() { + try { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodType methodType = + MethodType.methodType( + void.class, // return type for a constructor is void + Type.class, + BoundKind.class, + Symbol.TypeSymbol.class, + com.sun.tools.javac.util.List.class); + return lookup.findConstructor(Type.WildcardType.class, methodType); + } catch (NoSuchMethodException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + /** + * Used to get a MethodHandle for a virtual method from the specified class + * + * @param retTypeClass Class to indicate the return type of the desired method + * @param paramTypeClass Class to indicate the parameter type of the desired method + * @param refClass Class within which the desired method is contained + * @param methodName Name of the desired method + * @return The appropriate MethodHandle for the virtual method + */ + private static MethodHandle createVirtualMethodHandle( + Class retTypeClass, Class paramTypeClass, Class refClass, String methodName) { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodType mt = MethodType.methodType(retTypeClass, paramTypeClass); + try { + return lookup.findVirtual(refClass, methodName, mt); + } catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + @Override + public TypeMetadata create(com.sun.tools.javac.util.List attrs) { + ListBuffer b = new ListBuffer<>(); + b.appendList(attrs); + try { + return (TypeMetadata) typeMetadataConstructorHandle.invoke(b); + } catch (Throwable e) { + throw new RuntimeException(e); + } + } + + /** + * Calls dropMetadata and addMetadata using MethodHandles for JDK 21, which removed the previous + * cloneWithMetadata method. + * + * @param typeToBeCloned The Type we want to clone with the required Nullability metadata + * @param metadata The required Nullability metadata + * @return Cloned Type with the necessary Nullability metadata + */ + @Override + public Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metadata) { + try { + // In JDK 21 addMetadata works if there is no metadata associated with the type, so we + // create a copy without the existing metadata first and then add it + Type clonedTypeWithoutMetadata = + (Type) dropMetadataHandle.invoke(typeToBeCloned, metadata.getClass()); + return (Type) addMetadataHandle.invoke(clonedTypeWithoutMetadata, metadata); + } catch (Throwable e) { + throw new RuntimeException(e); + } + } + + @Override + public Type.ClassType createClassType(Type baseType, Type enclosingType, List typeArgs) { + try { + com.sun.tools.javac.util.List metadata = + (com.sun.tools.javac.util.List) getMetadataHandler.invoke(baseType); + return (Type.ClassType) + classTypeConstructorHandle.invoke( + enclosingType, + com.sun.tools.javac.util.List.from(typeArgs), + baseType.tsym, + metadata); + } catch (Throwable e) { + throw new RuntimeException(e); + } + } + + @Override + public Type.ArrayType createArrayType(Type.ArrayType baseType, Type elementType) { + try { + com.sun.tools.javac.util.List metadata = + (com.sun.tools.javac.util.List) getMetadataHandler.invoke(baseType); + return (Type.ArrayType) + arrayTypeConstructorHandle.invoke(elementType, baseType.tsym, metadata); + } catch (Throwable e) { + throw new RuntimeException(e); + } + } + + @Override + public Type.WildcardType createWildcardType(Type.WildcardType baseType, Type boundType) { + try { + com.sun.tools.javac.util.List metadata = + (com.sun.tools.javac.util.List) getMetadataHandler.invoke(baseType); + return (Type.WildcardType) + wildcardTypeConstructorHandle.invoke(boundType, baseType.kind, baseType.tsym, metadata); + } catch (Throwable e) { + throw new RuntimeException(e); + } + } + } +} diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java b/nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java index 158264f78d..4fe96814d1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java @@ -1,9 +1,18 @@ package com.uber.nullaway.generics; +import static com.uber.nullaway.generics.TypeMetadataBuilder.TYPE_METADATA_BUILDER; + +import com.sun.tools.javac.code.Attribute; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.code.TypeMetadata; import com.sun.tools.javac.code.Types; import com.sun.tools.javac.util.List; +import com.sun.tools.javac.util.ListBuffer; +import com.uber.nullaway.Config; +import com.uber.nullaway.Nullness; +import java.util.Collections; +import org.jspecify.annotations.Nullable; /** Utility method related to substituting type arguments for type variables. */ public class TypeSubstitutionUtils { @@ -14,10 +23,174 @@ public class TypeSubstitutionUtils { * @param types the {@link Types} instance * @param t the enclosing type * @param sym the symbol + * @param config the NullAway config * @return the type of {@code sym} as a member of {@code t} */ - public static Type memberType(Types types, Type t, Symbol sym) { - return types.memberType(t, sym); + public static Type memberType(Types types, Type t, Symbol sym, Config config) { + Type origType = sym.type; + Type memberType = types.memberType(t, sym); + return restoreExplicitNullabilityAnnotations(origType, memberType, config); + } + + /** + * Restores explicit nullability annotations on type variables in {@code origType} to {@code + * newType}. + * + * @param origType the original type + * @param newType the new type, a result of applying some substitution to {@code origType} + * @param config the NullAway config + * @return the new type with explicit nullability annotations restored + */ + private static Type restoreExplicitNullabilityAnnotations( + Type origType, Type newType, Config config) { + return new RestoreNullnessAnnotationsVisitor(config).visit(newType, origType); + } + + /** + * A visitor that restores explicit nullability annotations on type variables from another type to + * the corresponding positions in the visited type. If no annotations need to be restored, returns + * the visited type object itself. + */ + @SuppressWarnings("ReferenceEquality") + private static class RestoreNullnessAnnotationsVisitor extends Types.MapVisitor { + + private final Config config; + + RestoreNullnessAnnotationsVisitor(Config config) { + this.config = config; + } + + @Override + public Type visitMethodType(Type.MethodType t, Type other) { + Type.MethodType otherMethodType = (Type.MethodType) other; + List argtypes = t.argtypes; + Type restype = t.restype; + List thrown = t.thrown; + List argtypes1 = visitTypeLists(argtypes, otherMethodType.argtypes); + Type restype1 = visit(restype, otherMethodType.restype); + List thrown1 = visitTypeLists(thrown, otherMethodType.thrown); + if (argtypes1 == argtypes && restype1 == restype && thrown1 == thrown) { + return t; + } else { + return new Type.MethodType(argtypes1, restype1, thrown1, t.tsym); + } + } + + @Override + public Type visitClassType(Type.ClassType t, Type other) { + if (other instanceof Type.TypeVar) { + Type updated = updateNullabilityAnnotationsForType(t, (Type.TypeVar) other); + if (updated != null) { + return updated; + } + } + if (!(other instanceof Type.ClassType)) { + return t; + } + Type outer = t.getEnclosingType(); + Type outer1 = visit(outer, other.getEnclosingType()); + List typarams = t.getTypeArguments(); + List typarams1 = visitTypeLists(typarams, other.getTypeArguments()); + if (outer1 == outer && typarams1 == typarams) { + return t; + } else { + return TYPE_METADATA_BUILDER.createClassType(t, outer1, typarams1); + } + } + + @Override + public Type visitWildcardType(Type.WildcardType wt, Type other) { + if (!(other instanceof Type.WildcardType)) { + return wt; + } + Type t = wt.type; + if (t != null) { + t = visit(t, ((Type.WildcardType) other).type); + } + if (t == wt.type) { + return wt; + } else { + return TYPE_METADATA_BUILDER.createWildcardType(wt, t); + } + } + + @Override + public Type visitTypeVar(Type.TypeVar t, Type other) { + Type updated = updateNullabilityAnnotationsForType(t, (Type.TypeVar) other); + return updated != null ? updated : t; + } + + /** + * Updates the nullability annotations on a type {@code t} based on the nullability annotations + * on a type variable {@code other}. + * + * @param t the type to update + * @param other the type variable to update from + * @return the updated type, or {@code null} if no updates were made + */ + private @Nullable Type updateNullabilityAnnotationsForType(Type t, Type.TypeVar other) { + for (Attribute.TypeCompound annot : other.getAnnotationMirrors()) { + if (annot.type.tsym == null) { + continue; + } + String qualifiedName = annot.type.tsym.getQualifiedName().toString(); + if (Nullness.isNullableAnnotation(qualifiedName, config) + || Nullness.isNonNullAnnotation(qualifiedName, config)) { + // Construct and return an updated version of t with annotation annot. + List annotationCompound = + List.from( + Collections.singletonList( + new Attribute.TypeCompound(annot.type, List.nil(), null))); + TypeMetadata typeMetadata = TYPE_METADATA_BUILDER.create(annotationCompound); + return TYPE_METADATA_BUILDER.cloneTypeWithMetadata(t, typeMetadata); + } + } + return null; + } + + @Override + public Type visitArrayType(Type.ArrayType t, Type other) { + if (other instanceof Type.TypeVar) { + Type updated = updateNullabilityAnnotationsForType(t, (Type.TypeVar) other); + if (updated != null) { + return updated; + } + } + if (!(other instanceof Type.ArrayType)) { + return t; + } + Type.ArrayType otherArrayType = (Type.ArrayType) other; + Type elemtype = t.elemtype; + Type newElemType = visit(elemtype, otherArrayType.elemtype); + if (newElemType == elemtype) { + return t; + } else { + return TYPE_METADATA_BUILDER.createArrayType(t, newElemType); + } + } + + /** + * Visits each corresponding pair in two lists of types. Returns a list of the updated types, or + * {@code newtypes} itself if no updates were made. + * + * @param newtypes list of new types to be updated + * @param origtypes list of original types to update from + * @return the updated list of types, or {@code newtypes} itself if no updates were made + */ + private List visitTypeLists(List newtypes, List origtypes) { + ListBuffer buf = new ListBuffer<>(); + boolean changed = false; + for (List l = newtypes, l1 = origtypes; l.nonEmpty(); l = l.tail, l1 = l1.tail) { + Type t = l.head; + Type t1 = l1.head; + Type t2 = visit(t, t1); + buf.append(t2); + if (t2 != t) { + changed = true; + } + } + return changed ? buf.toList() : newtypes; + } } /** @@ -27,9 +200,11 @@ public static Type memberType(Types types, Type t, Symbol sym) { * @param t the type to which to perform the substitution * @param from the types that will be substituted out * @param to the types that will be substituted in + * @param config the NullAway config * @return the type resulting from the substitution */ - public static Type subst(Types types, Type t, List from, List to) { - return types.subst(t, from, to); + public static Type subst(Types types, Type t, List from, List to, Config config) { + Type substResult = types.subst(t, from, to); + return restoreExplicitNullabilityAnnotations(t, substResult, config); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java index 168c4dc18e..fc68ca37d9 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java @@ -46,7 +46,11 @@ public static Handler buildDefault(Config config) { ImmutableList.Builder handlerListBuilder = ImmutableList.builder(); MethodNameUtil methodNameUtil = new MethodNameUtil(); - if (config.acknowledgeRestrictiveAnnotations()) { + // Acknowledge restrictive annotations if in JSpecify mode or if the user has explicitly + // requested it + boolean acknowledgeRestrictive = + config.acknowledgeRestrictiveAnnotations() || config.isJSpecifyMode(); + if (acknowledgeRestrictive) { // This runs before LibraryModelsHandler, so that library models can override third-party // bytecode annotations handlerListBuilder.add(new RestrictiveAnnotationHandler(config)); diff --git a/nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java b/nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java index e364edc989..9a367c7697 100644 --- a/nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java @@ -388,6 +388,8 @@ public void springTestAutowiredFieldTest() { defaultCompilationHelper .addSourceFile("testdata/springboot-annotations/MockBean.java") .addSourceFile("testdata/springboot-annotations/SpyBean.java") + .addSourceFile("testdata/springboot-annotations/MockitoBean.java") + .addSourceFile("testdata/springboot-annotations/MockitoSpyBean.java") .addSourceLines( "Foo.java", "package com.uber;", @@ -406,7 +408,13 @@ public void springTestAutowiredFieldTest() { "import org.junit.jupiter.api.Test;", "import org.springframework.boot.test.mock.mockito.SpyBean;", "import org.springframework.boot.test.mock.mockito.MockBean;", + "import org.springframework.test.context.bean.override.mockito.MockitoBean;", + "import org.springframework.test.context.bean.override.mockito.MockitoSpyBean;", "public class TestCase {", + " @MockitoSpyBean", + " private Foo sf62Spy;", // Initialized by spring test (via Mockito). + " @MockitoBean", + " private Foo sf62Mock;", // Initialized by spring test (via Mockito). " @SpyBean", " private Foo spy;", // Initialized by spring test (via Mockito). " @MockBean", @@ -415,6 +423,8 @@ public void springTestAutowiredFieldTest() { " void springTest() {", " spy.setBar(\"hello\");", " mock.setBar(\"hello\");", + " sf62Spy.setBar(\"hello\");", + " sf62Mock.setBar(\"hello\");", " }", "}") .doTest(); diff --git a/nullaway/src/test/java/com/uber/nullaway/MonotonicNonNullTests.java b/nullaway/src/test/java/com/uber/nullaway/MonotonicNonNullTests.java new file mode 100644 index 0000000000..3c0ed559da --- /dev/null +++ b/nullaway/src/test/java/com/uber/nullaway/MonotonicNonNullTests.java @@ -0,0 +1,186 @@ +package com.uber.nullaway; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class MonotonicNonNullTests extends NullAwayTestsBase { + + @Test + public void initializerExpression() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import com.uber.nullaway.annotations.MonotonicNonNull;", + "class Test {", + " // this is fine; same as implicit initialization", + " @MonotonicNonNull Object f1 = null;", + " @MonotonicNonNull Object f2 = new Object();", + "}") + .doTest(); + } + + @Test + public void assignments() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import com.uber.nullaway.annotations.MonotonicNonNull;", + "class Test {", + " @MonotonicNonNull Object f1;", + " void testPositive() {", + " // BUG: Diagnostic contains: assigning @Nullable expression", + " f1 = null;", + " }", + " void testNegative() {", + " f1 = new Object();", + " }", + "}") + .doTest(); + } + + @Test + public void lambdas() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import com.uber.nullaway.annotations.MonotonicNonNull;", + "class Test {", + " @MonotonicNonNull Object f1;", + " void testPositive() {", + " Runnable r = () -> {", + " // BUG: Diagnostic contains: dereferenced expression f1", + " f1.toString();", + " };", + " }", + " void testNegative() {", + " f1 = new Object();", + " Runnable r = () -> {", + " f1.toString();", + " };", + " }", + "}") + .doTest(); + } + + @Test + public void anonymousClasses() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import com.uber.nullaway.annotations.MonotonicNonNull;", + "class Test {", + " @MonotonicNonNull Object f1;", + " void testPositive() {", + " Runnable r = new Runnable() {", + " @Override", + " public void run() {", + " // BUG: Diagnostic contains: dereferenced expression f1", + " f1.toString();", + " }", + " };", + " }", + " void testNegative() {", + " f1 = new Object();", + " Runnable r = new Runnable() {", + " @Override", + " public void run() {", + " f1.toString();", + " }", + " };", + " }", + "}") + .doTest(); + } + + @Test + public void nestedObjects() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import com.uber.nullaway.annotations.MonotonicNonNull;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " class Foo {", + " @MonotonicNonNull Object x;", + " }", + " final Foo f1 = new Foo();", + " Foo f2 = new Foo(); // not final", + " @Nullable Foo f3;", + " void testPositive1() {", + " f2.x = new Object();", + " Runnable r = () -> {", + " // report a bug since f2 may be overwritten", + " // BUG: Diagnostic contains: dereferenced expression f2.x", + " f2.x.toString();", + " };", + " }", + " void testPositive2() {", + " f3 = new Foo();", + " f3.x = new Object();", + " Runnable r = () -> {", + " // report a bug since f3 may be overwritten", + " // BUG: Diagnostic contains: dereferenced expression f3.x", + " f3.x.toString();", + " };", + " }", + " void testNegative() {", + " f1.x = new Object();", + " Runnable r = () -> {", + " f1.x.toString();", + " };", + " }", + "}") + .doTest(); + } + + @Test + public void accessPathsWithMethodCalls() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import com.uber.nullaway.annotations.MonotonicNonNull;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " class Foo {", + " @MonotonicNonNull Object x;", + " }", + " Foo f1 = new Foo();", + " final Foo getF1() {", + " return f1;", + " }", + " final @Nullable Foo getOther() {", + " return null;", + " }", + " void testPositive1() {", + " getF1().x = new Object();", + " Runnable r = () -> {", + " // BUG: Diagnostic contains: dereferenced expression", + " getF1().x.toString();", + " };", + " }", + " void testPositive2() {", + " if (getOther() != null) {", + " getOther().x = new Object();", + " Runnable r1 = () -> {", + " // getOther() should be treated as @Nullable in the lambda", + " // BUG: Diagnostic contains: dereferenced expression", + " getOther().toString();", + " };", + " Runnable r2 = () -> {", + " // BUG: Diagnostic contains: dereferenced expression", + " getOther().x.toString();", + " };", + " }", + " }", + "}") + .doTest(); + } +} diff --git a/nullaway/src/test/java/com/uber/nullaway/SerializationTest.java b/nullaway/src/test/java/com/uber/nullaway/SerializationTest.java index d828e9cf30..e91c1b0480 100644 --- a/nullaway/src/test/java/com/uber/nullaway/SerializationTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/SerializationTest.java @@ -2168,4 +2168,173 @@ public void errorSerializationTestArrayComponentNull() { .setOutputFileNameAndHeader(ERROR_FILE_NAME, ERROR_FILE_HEADER) .doTest(); } + + @Test + public void errorSerializationTestArrayComponentNullLocalVariable() { + SerializationTestHelper tester = new SerializationTestHelper<>(root); + tester + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:JSpecifyMode=true", + "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) + .addSourceLines( + "com/uber/A.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "public class A {", + " void spin() {", + " String [] foo = {\"SomeRandomWords\"};", + " // BUG: Diagnostic contains: Writing @Nullable expression into array with @NonNull contents.", + " foo[1] = null;", + " }", + "}") + .setExpectedOutputs( + new ErrorDisplay( + "ASSIGN_NULLABLE_TO_NONNULL_ARRAY", + "Writing @Nullable expression into array with @NonNull contents.", + "com.uber.A", + "spin()", + 235, + "com/uber/A.java", + "LOCAL_VARIABLE", + "com.uber.A", + "spin()", + "foo", + "null", + "com/uber/A.java")) + .setFactory(errorDisplayFactory) + .setOutputFileNameAndHeader(ERROR_FILE_NAME, ERROR_FILE_HEADER) + .doTest(); + } + + @Test + public void errorSerializationTestArrayComponentNullLocalVariableLambda() { + SerializationTestHelper tester = new SerializationTestHelper<>(root); + tester + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:JSpecifyMode=true", + "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) + .addSourceLines( + "com/uber/A.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "public class A {", + " A a = new A();", + " public void f() {", + " final Object[] l = new Object[10];", + " class B {", + " void b() {", + " // BUG: Diagnostic contains: Writing @Nullable expression into array with @NonNull contents.", + " l[0] = null;", + " }", + " void shadowInLambda() {", + " a.exec(", + " () -> {", + " Object[] l = new Object[10];", + " // BUG: Diagnostic contains: Writing @Nullable expression into array with @NonNull contents.", + " l[0] = null;", + " });", + " }", + " void useFieldInLambda(A a) {", + " Object[] l = new Object[10];", + " a.exec(", + " () -> {", + " // BUG: Diagnostic contains: Writing @Nullable expression into array with @NonNull contents.", + " l[0] = null;", + " });", + " }", + " }", + " a.exec(new Runnable() {", + " @Override", + " public void run() {", + " // BUG: Diagnostic contains: Writing @Nullable expression into array with @NonNull contents.", + " l[0] = null;", + " }", + " });", + " // BUG: Diagnostic contains: Writing @Nullable expression into array with @NonNull contents.", + " l[0] = null;", + " }", + " void exec(Runnable runnable) {", + " runnable.run();", + " }", + "}") + .setExpectedOutputs( + new ErrorDisplay( + "ASSIGN_NULLABLE_TO_NONNULL_ARRAY", + "Writing @Nullable expression into array with @NonNull contents.", + "com.uber.A$1B", + "b()", + 293, + "com/uber/A.java", + "LOCAL_VARIABLE", + "com.uber.A", + "f()", + "l", + "null", + "com/uber/A.java"), + new ErrorDisplay( + "ASSIGN_NULLABLE_TO_NONNULL_ARRAY", + "Writing @Nullable expression into array with @NonNull contents.", + "com.uber.A$1B", + "shadowInLambda()", + 560, + "com/uber/A.java", + "LOCAL_VARIABLE", + "com.uber.A$1B", + "shadowInLambda()", + "l", + "null", + "com/uber/A.java"), + new ErrorDisplay( + "ASSIGN_NULLABLE_TO_NONNULL_ARRAY", + "Writing @Nullable expression into array with @NonNull contents.", + "com.uber.A$1B", + "useFieldInLambda(com.uber.A)", + 842, + "com/uber/A.java", + "LOCAL_VARIABLE", + "com.uber.A$1B", + "useFieldInLambda(com.uber.A)", + "l", + "null", + "com/uber/A.java"), + new ErrorDisplay( + "ASSIGN_NULLABLE_TO_NONNULL_ARRAY", + "Writing @Nullable expression into array with @NonNull contents.", + "com.uber.A$1", + "run()", + 1068, + "com/uber/A.java", + "LOCAL_VARIABLE", + "com.uber.A", + "f()", + "l", + "null", + "com/uber/A.java"), + new ErrorDisplay( + "ASSIGN_NULLABLE_TO_NONNULL_ARRAY", + "Writing @Nullable expression into array with @NonNull contents.", + "com.uber.A", + "f()", + 1198, + "com/uber/A.java", + "LOCAL_VARIABLE", + "com.uber.A", + "f()", + "l", + "null", + "com/uber/A.java")) + .setFactory(errorDisplayFactory) + .setOutputFileNameAndHeader(ERROR_FILE_NAME, ERROR_FILE_HEADER) + .doTest(); + } } diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index 7bbe2c4ddf..6f37df6b8e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -190,6 +190,58 @@ public void issue1035() { .doTest(); } + @Test + public void issue1138() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.Nullable;", + "@NullMarked", + "class Foo {", + " Foo(T source) {", + " }", + " static Foo createNoTypeArgs(T in) {", + " return new Foo(in);", + " }", + " static Foo createWithTypeArgNegative(String s) {", + " return new Foo(s);", + " }", + " static Foo createWithTypeArgPositive() {", + " // BUG: Diagnostic contains: Type argument cannot be @Nullable, as method Foo(T)'s type variable T is not @Nullable", + " return new <@Nullable String>Foo(null);", + " }", + "}") + .doTest(); + } + + @Test + public void nullableAnnotOnMethodTypeVarUse() { + makeHelper() + .addSourceLines( + "GenericMethod.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.function.Function;", + "public abstract class GenericMethod {", + " abstract @Nullable V foo(", + " Function<@Nullable V, @Nullable V> f);", + " void testNegative(Function<@Nullable String, @Nullable String> f) {", + " this.foo(f);", + " }", + " void testPositive(Function f) {", + " // BUG: Diagnostic contains: Cannot pass parameter of type Function, as formal parameter has type", + " this.foo(f);", + " }", + " void testPositive2(Function<@Nullable String, @Nullable String> f) {", + " // BUG: Diagnostic contains: dereferenced expression this.foo(f) is @Nullable", + " this.foo(f).hashCode();", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java index ac749a7afc..b391ff60b4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -303,7 +303,8 @@ public void genericsChecksForAssignmentsWithNonJSpecifyAnnotations() { "class Test {", " static class NullableTypeParam {}", " static void testNoWarningForMismatch(NullableTypeParam<@Nullable String> t1) {", - " // no error here since we only do our checks for JSpecify @Nullable annotations", + " // we still get an error here as we are not forcing use of JSpecify's @Nullable", + " // BUG: Diagnostic contains: Cannot assign from type NullableTypeParam<@Nullable String>", " NullableTypeParam t2 = t1;", " }", " static void testNegative(NullableTypeParam<@Nullable String> t1) {", @@ -1071,7 +1072,7 @@ public void genericPrimitiveArrayTypeAssignment() { "class Test {", " static class A { }", " static void testPositive() {", - " // BUG: Diagnostic contains: Cannot assign from type A", + " // BUG: Diagnostic contains: Cannot assign from type A", " A x = new A();", " }", " static void testNegative() {", @@ -1326,6 +1327,41 @@ public void overrideAnonymousNestedClass() { .doTest(); } + @Test + public void otherTypeUseNullableAnnotation() { + makeHelper() + .addSourceLines( + "Nullable.java", + "package com.other;", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Retention;", + "import java.lang.annotation.RetentionPolicy;", + "import java.lang.annotation.Target;", + "@Target(ElementType.TYPE_USE)", + "@Retention(RetentionPolicy.CLASS)", + "public @interface Nullable {}") + .addSourceLines( + "Foo.java", + "package com.uber;", + "import org.jspecify.annotations.NullMarked;", + "import com.other.Nullable;", + "@NullMarked", + "class Foo {", + " static abstract class MyClass {", + " abstract T doThing(T value);", + " }", + " static void repro() {", + " new MyClass<@Nullable Object>() {", + " @Override", + " @Nullable Object doThing(@Nullable Object value) {", + " return value;", + " }", + " }.doThing(null);", + " }", + "}") + .doTest(); + } + @Test public void nullableVoidGenericsLambda() { makeHelper() @@ -2027,6 +2063,108 @@ public void issue1082() { .doTest(); } + @Test + public void nullableAnnotOnClassTypeVarUse() { + makeHelper() + .addSourceLines( + "Generics.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.function.Function;", + "public abstract class Generics {", + " abstract void foo(", + " Function<@Nullable V, @Nullable V> f);", + " void testNegative(Function<@Nullable V, @Nullable V> f) {", + " foo(f);", + " }", + " void testPositive(Function f) {", + " // BUG: Diagnostic contains: Cannot pass parameter of type Function, as formal parameter has type", + " foo(f);", + " }", + " abstract void takesArray(Function<@Nullable V, @Nullable V>[] f);", + " void testNegativeArray(Function<@Nullable V, @Nullable V>[] f) {", + " takesArray(f);", + " }", + " void testPositiveArray(Function[] f) {", + " // BUG: Diagnostic contains: Cannot pass parameter of type Function [], as formal parameter has type", + " takesArray(f);", + " }", + "}") + .doTest(); + } + + @Test + public void nonnullAnnotOnClassTypeVarUse() { + makeHelper() + .addSourceLines( + "Generics.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import org.jspecify.annotations.NonNull;", + "import java.util.function.Function;", + "public abstract class Generics {", + " abstract void foo(", + " Function<@NonNull V, @NonNull V> f);", + " void testPositive(Function<@Nullable String, @Nullable String> f) {", + " // BUG: Diagnostic contains: Cannot pass parameter of type Function<@Nullable String, @Nullable String>, as formal parameter has type", + " this.<@Nullable String>foo(f);", + " }", + " void testPositiveArray(Function f) {", + " // BUG: Diagnostic contains: Cannot pass parameter of type Function, as formal parameter has type", + " this.foo(f);", + " }", + "}") + .doTest(); + } + + @Test + public void nullableAnnotOnClassTypeVarUseMixed() { + makeHelper() + .addSourceLines( + "Generics.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.function.Function;", + "public abstract class Generics {", + " abstract void foo(", + " Function f);", + " void testNegative(Function f) {", + " foo(f);", + " }", + " void testPositive1(Function<@Nullable V, V> f) {", + " // BUG: Diagnostic contains: Cannot pass parameter of type Function<@org.jspecify.annotations.Nullable V, V>", + " foo(f);", + " }", + " void testPositive2(Function f) {", + " // BUG: Diagnostic contains: Cannot pass parameter of type Function, as formal parameter has type", + " foo(f);", + " }", + " void testPositive3(Function<@Nullable V, @Nullable V> f) {", + " // BUG: Diagnostic contains: Cannot pass parameter of type Function<@org.jspecify.annotations.Nullable V, @org.jspecify.annotations.Nullable V>, as formal parameter has type", + " foo(f);", + " }", + "}") + .doTest(); + } + + @Test + public void nullableAnnotOnClassTypeVarWildcardUse() { + makeHelper() + .addSourceLines( + "Generics.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.function.Function;", + "public abstract class Generics {", + " abstract void foo(", + " Function f);", + " void test(Function f) {", + " foo(f);", + " }", + "}") + .doTest(); + } + @Test public void issue1093() { makeHelper() @@ -2045,6 +2183,21 @@ public void issue1093() { .doTest(); } + @Test + public void issue1127() { + makeHelper() + .addSourceLines( + "Main.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "public class Main {", + " void arrayAssign(boolean b, @Nullable String @Nullable [] vals) {", + " @Nullable String[] arr = (b ? vals : null);", + " }", + "}") + .doTest(); + } + @Test public void nullUnmarkedGenericField() { makeHelper() @@ -2069,6 +2222,42 @@ public void nullUnmarkedGenericField() { .doTest(); } + @Test + public void issue1126() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.function.Supplier;", + "public class Test {", + " static class K {}", + " void foo(K<@Nullable Object> k) {", + " K k2 = k;", + " Supplier s = () -> null;", + " }", + "}") + .addSourceLines( + "Test2.java", + "package com.uber;", + "import java.util.HashMap;", + "import java.util.Map;", + "import org.jspecify.annotations.Nullable;", + "import org.jetbrains.annotations.Contract;", + "public class Test2 {", + " @Contract(\"null -> true\")", + " public static boolean isEmpty(@Nullable Map map) {", + " return (map == null || map.isEmpty());", + " }", + " static void foo() {", + " Map variables = new HashMap<>();", + " if (isEmpty(variables)) { /* do nothing */ }", + " variables.toString();", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyArrayTests.java index 35560acae2..e5acf8866f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyArrayTests.java @@ -250,7 +250,7 @@ public void arraySubtyping() { " @Nullable Integer[] x2 = nullableIntArr;", " // legal (covariant array subtypes)", " x2 = nonnullIntArr;", - " // BUG: Diagnostic contains: Cannot assign from type @Nullable Integer[] to type Integer[]", + " // BUG: Diagnostic contains: Cannot assign from type @Nullable Integer [] to type Integer []", " x1 = nullableIntArr;", " }", "}") @@ -272,7 +272,7 @@ public void arraySubtypingWithNewExpression() { " @Nullable Integer[] x2 = new Integer[0];", " // legal", " x2 = new @Nullable Integer[0];", - " // BUG: Diagnostic contains: Cannot assign from type @Nullable Integer[] to type Integer[]", + " // BUG: Diagnostic contains: Cannot assign from type @Nullable Integer [] to type Integer []", " x1 = new @Nullable Integer[0];", " }", "}") @@ -290,7 +290,7 @@ public void arraysAndGenerics() { "class Test {", " void foo(List<@Nullable Integer[]> l) {}", " void testPositive(List p) {", - " // BUG: Diagnostic contains: Cannot pass parameter of type List", + " // BUG: Diagnostic contains: Cannot pass parameter of type List", " foo(p);", " }", " void testNegative(List<@Nullable Integer[]> p) {", @@ -312,7 +312,7 @@ public void genericArraysReturnedAndPassed() { " static class Bar {", " Foo[] getFoosPositive() {", " @Nullable Foo[] result = new Foo[0];", - " // BUG: Diagnostic contains: Cannot return expression of type @Nullable Foo[] from method", + " // BUG: Diagnostic contains: Cannot return expression of type @Nullable Foo [] from method", " return result;", " }", " Foo[] getFoosNegative() {", @@ -321,7 +321,7 @@ public void genericArraysReturnedAndPassed() { " }", " void takeFoos(Foo[] foos) {}", " void callTakeFoosPositive(@Nullable Foo[] p) {", - " // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo[]", + " // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo []", " takeFoos(p);", " }", " void callTakeFoosNegative(Foo[] p) {", @@ -331,9 +331,9 @@ public void genericArraysReturnedAndPassed() { " void callTakeFoosVarargsPositive(@Nullable Foo[] p, Foo[] p2) {", " // Under the hood, a @Nullable Foo[][] is passed, which is not a subtype", " // of the formal parameter type Foo[][]", - " // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo[]", + " // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo []", " takeFoosVarargs(p);", - " // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo[]", + " // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo []", " takeFoosVarargs(p2, p);", " }", " void callTakeFoosVarargsNegative(Foo[] p) {", @@ -367,7 +367,7 @@ public void overridesReturnType() { " @Override", " Integer[] foo() { return new Integer[0]; }", " @Override", - " // BUG: Diagnostic contains: Method returns @Nullable Integer[], but overridden method returns Integer[]", + " // BUG: Diagnostic contains: Method returns @Nullable Integer [], but overridden method returns Integer []", " @Nullable Integer[] bar() { return new @Nullable Integer[0]; }", " }", "}") @@ -389,7 +389,7 @@ public void overridesParameterType() { " }", " class Sub extends Super {", " @Override", - " // BUG: Diagnostic contains: Parameter has type Integer[], but overridden method has parameter type @Nullable Integer[]", + " // BUG: Diagnostic contains: Parameter has type Integer [], but overridden method has parameter type @Nullable Integer []", " void foo(Integer[] p) { }", " @Override", " void bar(@Nullable Integer[] p) { }", @@ -407,7 +407,7 @@ public void ternaryOperator() { "import org.jspecify.annotations.Nullable;", "class Test {", " static Integer[] testPositive(Integer[] p, boolean t) {", - " // BUG: Diagnostic contains: Conditional expression must have type Integer[]", + " // BUG: Diagnostic contains: Conditional expression must have type Integer []", " Integer[] t1 = t ? new Integer[0] : new @Nullable Integer[0];", " // BUG: Diagnostic contains: Conditional expression must have type", " return t ? new @Nullable Integer[0] : new @Nullable Integer[0];", diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java index b08feb77e7..19e19a6e33 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java @@ -535,14 +535,13 @@ public void varargsOverride() { " }", " static class NullableVarargsContentsImpl2 implements NullableVarargsContents {", " @Override", - " // BUG: Diagnostic contains: Parameter has type Object[], but overridden method", + " // BUG: Diagnostic contains: Parameter has type Object [], but overridden method", " public void varargs(Object... params) {", " }", " }", " static class NullableVarargsContentsImpl3 implements NullableVarargsContents {", " @Override", - // TODO open an issue to improve the error message in a follow up - " // BUG: Diagnostic contains: Parameter has type Object[]", + " // BUG: Diagnostic contains: Parameter has type Object @Nullable []", " public void varargs(Object @Nullable... params) {", " }", " }", @@ -596,7 +595,7 @@ public void varargsOverride() { " }", " static class NullableVarargsBothImpl3 implements NullableVarargsBoth {", " @Override", - " // BUG: Diagnostic contains: Parameter has type Object[]", + " // BUG: Diagnostic contains: Parameter has type Object @Nullable []", " public void varargs(Object @Nullable... params) {", " }", " }", diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java index 34feb6f7e5..4a7470ec53 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java @@ -866,6 +866,68 @@ public void nullUnmarkedMethodLevel() { .doTest(); } + @Test + public void nullUnmarkedMethodWithNonNullParamJSpecifyMode() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:OnlyNullMarked=true", + "-XepOpt:NullAway:JSpecifyMode=true")) + .addSourceLines( + "Foo.java", + "package com.uber;", + "import org.jspecify.annotations.NullUnmarked;", + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.NonNull;", + "@NullMarked", + "public class Foo {", + " @NullUnmarked", + " public static void callee(@NonNull Object o) {", + " }", + " @NullUnmarked", + " public static void callee2(Object o) {", + " }", + " public static void caller() {", + " // Error due to explicit @NonNull annotation", + " // BUG: Diagnostic contains: passing @Nullable parameter", + " callee(null);", + " // this is fine", + " callee2(null);", + " }", + "}") + .doTest(); + } + + @Test + public void nullUnmarkedMethodWithNullableReturnJSpecifyMode() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:OnlyNullMarked=true", + "-XepOpt:NullAway:JSpecifyMode=true")) + .addSourceLines( + "Foo.java", + "package com.uber;", + "import org.jspecify.annotations.NullUnmarked;", + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.Nullable;", + "@NullMarked", + "public class Foo {", + " @NullUnmarked", + " public static @Nullable String callee() {", + " return null;", + " }", + " public static void caller() {", + " // Error due to explicit @Nullable annotation", + " // BUG: Diagnostic contains: dereferenced expression callee() is @Nullable", + " callee().toString();", + " }", + "}") + .doTest(); + } + @Test public void nullUnmarkedOuterMethodLevelWithLocalClass() { defaultCompilationHelper diff --git a/nullaway/src/test/resources/com/uber/nullaway/testdata/CheckFieldInitNegativeCases.java b/nullaway/src/test/resources/com/uber/nullaway/testdata/CheckFieldInitNegativeCases.java index 9ef64f8f70..9f2461c73b 100644 --- a/nullaway/src/test/resources/com/uber/nullaway/testdata/CheckFieldInitNegativeCases.java +++ b/nullaway/src/test/resources/com/uber/nullaway/testdata/CheckFieldInitNegativeCases.java @@ -326,6 +326,8 @@ static class MonotonicNonNullUsage { @MonotonicNonNull Object f; + @com.uber.nullaway.annotations.MonotonicNonNull Object g; + MonotonicNonNullUsage() {} } diff --git a/nullaway/src/test/resources/com/uber/nullaway/testdata/springboot-annotations/MockitoBean.java b/nullaway/src/test/resources/com/uber/nullaway/testdata/springboot-annotations/MockitoBean.java new file mode 100644 index 0000000000..2482bcd56d --- /dev/null +++ b/nullaway/src/test/resources/com/uber/nullaway/testdata/springboot-annotations/MockitoBean.java @@ -0,0 +1,13 @@ +package org.springframework.test.context.bean.override.mockito; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ElementType.TYPE, ElementType.FIELD}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +public @interface MockitoBean { +} diff --git a/nullaway/src/test/resources/com/uber/nullaway/testdata/springboot-annotations/MockitoSpyBean.java b/nullaway/src/test/resources/com/uber/nullaway/testdata/springboot-annotations/MockitoSpyBean.java new file mode 100644 index 0000000000..a036107923 --- /dev/null +++ b/nullaway/src/test/resources/com/uber/nullaway/testdata/springboot-annotations/MockitoSpyBean.java @@ -0,0 +1,13 @@ +package org.springframework.test.context.bean.override.mockito; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ElementType.TYPE, ElementType.FIELD}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +public @interface MockitoSpyBean { +}