Skip to content

Remove old support for reading annotations from bytecode #1661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
msridhar wants to merge 1 commit into
base: master
Choose a base branch
from
+66 −213
Draft

Remove old support for reading annotations from bytecode #1661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
239 changes: 50 additions & 189 deletions nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@
import com.sun.tools.javac.code.Attribute;
import com.sun.tools.javac.code.Flags;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.TargetType;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.TypeAnnotationPosition;
import com.sun.tools.javac.code.TypeAnnotationPosition.TypePathEntry;
import com.sun.tools.javac.code.TypeTag;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.tree.JCTree;
Expand All @@ -62,7 +59,6 @@
import java.util.stream.Stream;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.AnnotationValue;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
import org.checkerframework.nullaway.javacutil.AnnotationUtils;
import org.jspecify.annotations.Nullable;
Expand Down Expand Up @@ -194,35 +190,6 @@ public static Stream<? extends AnnotationMirror> getAllAnnotations(Symbol symbol
return Stream.concat(symbol.getAnnotationMirrors().stream(), typeUseAnnotations);
}

/**
* Check if any direct annotation a symbol matches a given predicate. Includes code for backward
* compatibility with older JDKs where javac did not place type-use annotations on Symbols from
* bytecodes.
*
* @param symbol the symbol
* @param predicate the predicate to match annotation names against
* @return true if any annotation on the symbol matches the predicate, false otherwise
*/
public static boolean hasAnyAnnotationMatchingBackCompat(
Symbol symbol, Predicate<String> predicate) {
if (hasAnyAnnotationMatching(symbol, predicate)) {
return true;
}
// to handle bytecodes, also check direct type-use annotations stored in attributes
Symbol typeAnnotationOwner =
symbol.getKind().equals(ElementKind.PARAMETER) ? symbol.owner : symbol;
for (Attribute.TypeCompound typeCompound : typeAnnotationOwner.getRawTypeAttributes()) {
if (!targetTypeMatches(symbol, typeCompound.position)
|| !isDirectTypeUseAnnotation(typeCompound, symbol)) {
continue;
}
if (predicate.test(typeCompound.getAnnotationType().toString())) {
return true;
}
}
return false;
}

/**
* Check if any direct annotation a symbol matches a given predicate.
*
Expand All @@ -237,14 +204,24 @@ public static boolean hasAnyAnnotationMatching(Symbol symbol, Predicate<String>
return true;
}
}
// check for type use annotations. For MethodSymbols, look on the return type
Type annotatedType =
symbol instanceof Symbol.MethodSymbol methodSymbol
? methodSymbol.getReturnType()
: symbol.type;
for (AnnotationMirror annotationMirror : annotatedType.getAnnotationMirrors()) {
if (predicate.test(annotationMirror.getAnnotationType().toString())) {
return true;
// Check for type-use annotations. For methods, look on the return type and its enclosing types,
// since NullAway treats an annotation before the outer class of a nested return type as
// applying to the full return type.
if (symbol instanceof Symbol.MethodSymbol methodSymbol) {
for (Type currentType = methodSymbol.getReturnType();
currentType != null && !currentType.hasTag(TypeTag.NONE);
currentType = currentType.getEnclosingType()) {
for (AnnotationMirror annotationMirror : currentType.getAnnotationMirrors()) {
if (predicate.test(annotationMirror.getAnnotationType().toString())) {
return true;
}
}
}
} else {
for (AnnotationMirror annotationMirror : symbol.type.getAnnotationMirrors()) {
if (predicate.test(annotationMirror.getAnnotationType().toString())) {
return true;
}
}
}
return false;
Expand Down Expand Up @@ -353,18 +330,11 @@ public static boolean hasAnyAnnotationMatching(Symbol symbol, Predicate<String>
public static Stream<? extends AnnotationMirror> getAllAnnotationsForParameter(
Symbol.MethodSymbol symbol, int paramInd) {
Symbol.VarSymbol varSymbol = symbol.getParameters().get(paramInd);
// On modern javac versions, type-use annotations are attached directly to the parameter's
// type. Keep reading raw attributes as well for backward compatibility with older javac
// behavior.
Stream<? extends AnnotationMirror> typeUseAnnotations =
Type parameterType = symbol.type.getParameterTypes().get(paramInd);
Stream<Attribute.TypeCompound> typeUseAnnotations =
Stream.concat(
varSymbol.type.getAnnotationMirrors().stream(),
symbol.getRawTypeAttributes().stream()
.filter(
t ->
t.position.type.equals(TargetType.METHOD_FORMAL_PARAMETER)
&& t.position.parameter_index == paramInd
&& NullabilityUtil.isDirectTypeUseAnnotation(t, symbol)))
getTypeUseAnnotationsIncludingEnclosingTypes(varSymbol.type),
getTypeUseAnnotationsIncludingEnclosingTypes(parameterType))
.distinct();
return Stream.concat(varSymbol.getAnnotationMirrors().stream(), typeUseAnnotations);
}
Expand All @@ -374,137 +344,30 @@ public static Stream<? extends AnnotationMirror> getAllAnnotationsForParameter(
* arguments, wildcards, etc.)
*/
public static Stream<Attribute.TypeCompound> getTypeUseAnnotations(Symbol symbol) {
return getTypeUseAnnotations(symbol, /* onlyDirect= */ true);
}

/**
* Gets the type use annotations on a symbol
*
* @param symbol the symbol
* @param onlyDirect if true, only return annotations that are directly on the type, not on
* components of the type (type arguments, wildcards, array contents, etc.)
* @return the type use annotations on the symbol
*/
private static Stream<Attribute.TypeCompound> getTypeUseAnnotations(
Symbol symbol, boolean onlyDirect) {
// Adapted from Error Prone's MoreAnnotations class:
// https://github.com/google/error-prone/blob/5f71110374e63f3c35b661f538295fa15b5c1db2/check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java#L84-L91
Symbol typeAnnotationOwner =
symbol.getKind().equals(ElementKind.PARAMETER) ? symbol.owner : symbol;
Stream<Attribute.TypeCompound> rawTypeAttributes =
typeAnnotationOwner.getRawTypeAttributes().stream();
Type annotatedType =
symbol instanceof Symbol.MethodSymbol methodSymbol
? methodSymbol.getReturnType()
: symbol.type;
if (symbol instanceof Symbol.MethodSymbol) {
// for methods, we want annotations on the return type
return rawTypeAttributes.filter(
(t) ->
t.position.type.equals(TargetType.METHOD_RETURN)
&& (!onlyDirect || isDirectTypeUseAnnotation(t, symbol)));
} else {
// filter for annotations directly on the type
return rawTypeAttributes.filter(
t ->
targetTypeMatches(symbol, t.position)
&& (!onlyDirect || isDirectTypeUseAnnotation(t, symbol)));
}
}

// Adapted from Error Prone MoreAnnotations:
// https://github.com/google/error-prone/blob/5f71110374e63f3c35b661f538295fa15b5c1db2/check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java#L128
private static boolean targetTypeMatches(Symbol sym, TypeAnnotationPosition position) {
switch (sym.getKind()) {
case LOCAL_VARIABLE -> {
return position.type == TargetType.LOCAL_VARIABLE;
}
case FIELD, ENUM_CONSTANT -> {
// treated like a field
return position.type == TargetType.FIELD;
}
case CONSTRUCTOR, METHOD -> {
return position.type == TargetType.METHOD_RETURN;
}
case PARAMETER -> {
if (position.type.equals(TargetType.METHOD_FORMAL_PARAMETER)) {
int parameterIndex = position.parameter_index;
if (position.onLambda != null) {
com.sun.tools.javac.util.List<JCTree.JCVariableDecl> lambdaParams =
position.onLambda.params;
return parameterIndex >= 0
&& parameterIndex < lambdaParams.size()
&& lambdaParams.get(parameterIndex).sym.equals(sym);
} else {
return ((Symbol.MethodSymbol) sym.owner).getParameters().indexOf(sym) == parameterIndex;
}
} else {
return false;
}
}
case CLASS, ENUM, RECORD -> {
// treated like a class
// There are no type annotations on the top-level type of the class/enum/record being
// declared,
// only on other types in the signature (e.g. `class Foo extends Bar<@A Baz> {}`).
return false;
}
default -> {
throw new AssertionError("unsupported element kind " + sym.getKind() + " symbol " + sym);
}
return getTypeUseAnnotationsIncludingEnclosingTypes(annotatedType);
}
return annotatedType.getAnnotationMirrors().stream();
}

/**
* Check whether a type-use annotation should be treated as applying directly to the top-level
* type
* Gets the type-use annotations directly on {@code type} or any of its enclosing types.
*
* <p>For example {@code @Nullable List<T> lst} is a direct type use annotation of {@code lst},
* but {@code List<@Nullable T> lst} is not.
*
* @param t the annotation and its position in the type
* @param symbol the symbol for the annotated element
* @return {@code true} if the annotation should be treated as applying directly to the top-level
* type, false otherwise
* <p>javac models an annotation written before the outer class in a nested type on that enclosing
* type. For method return and parameter nullability, NullAway treats such an annotation as
* applying to the full nested type.
*/
private static boolean isDirectTypeUseAnnotation(Attribute.TypeCompound t, Symbol symbol) {
// location is a list of TypePathEntry objects, indicating whether the annotation is
// on an array, inner type, wildcard, or type argument. If it's empty, then the
// annotation is directly on the type.
// Annotations on array dimensions, wildcards, or type arguments do not apply to the top-level
// type. For nested classes, an annotation applies directly only when placed on the innermost
// type.
int innerTypeCount = 0;
for (TypePathEntry entry : t.position.location) {
switch (entry.tag) {
case INNER_TYPE -> {
innerTypeCount++;
}
case ARRAY -> {
return false;
}
default -> {
// Wildcard or type argument!
return false;
}
}
}
// For non-nested classes annotations apply to the innermost type.
if (!isTypeOfNestedClass(symbol.type)) {
return true;
}
// For nested classes the annotation is only valid if it is on the innermost type.
return innerTypeCount == getNestingDepth(symbol.type) - 1;
}

private static int getNestingDepth(Type type) {
int depth = 0;
for (Type curr = type;
curr != null && !curr.hasTag(TypeTag.NONE);
curr = curr.getEnclosingType()) {
depth++;
}
return depth;
}

private static boolean isTypeOfNestedClass(Type type) {
return type.tsym != null && type.tsym.owner instanceof Symbol.ClassSymbol;
private static Stream<Attribute.TypeCompound> getTypeUseAnnotationsIncludingEnclosingTypes(
Type type) {
Stream<Attribute.TypeCompound> annotations = type.getAnnotationMirrors().stream();
Type enclosingType = type.getEnclosingType();
return enclosingType == null || enclosingType.hasTag(TypeTag.NONE)
? annotations
: Stream.concat(annotations, getTypeUseAnnotationsIncludingEnclosingTypes(enclosingType));
}

/**
Expand Down Expand Up @@ -644,19 +507,17 @@ private static boolean checkArrayElementAnnotations(
Config config,
BiPredicate<String, Config> typeUseCheck,
BiPredicate<Symbol, Config> declarationCheck) {
if (getTypeUseAnnotations(arraySymbol, /* onlyDirect= */ false)
.anyMatch(
t -> {
// the location list should be of length 1 and the entry tag should be ARRAY
com.sun.tools.javac.util.List<TypePathEntry> location = t.position.location;
TypePathEntry head = location.head;
boolean singleElementList = head != null && location.tail.isEmpty();
if (singleElementList && head.tag == TypeAnnotationPosition.TypePathEntryKind.ARRAY) {
return typeUseCheck.test(t.type.toString(), config);
}
return false;
})) {
return true;
Type annotatedType =
arraySymbol instanceof Symbol.MethodSymbol methodSymbol
? methodSymbol.getReturnType()
: arraySymbol.type;
if (annotatedType instanceof Type.ArrayType arrayType) {
for (AnnotationMirror annotationMirror :
arrayType.getComponentType().getAnnotationMirrors()) {
if (typeUseCheck.test(annotationMirror.getAnnotationType().toString(), config)) {
return true;
}
}
}
// For varargs symbols we also check for declaration annotations on the parameter
// NOTE this flag check does not work for the varargs parameter of a method defined in bytecodes
Expand Down
10 changes: 5 additions & 5 deletions nullaway/src/main/java/com/uber/nullaway/Nullness.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public static boolean isMonotonicNonNullAnnotation(String annotName) {
* {@code symbol}. Used to reason whether a field may be null.
*/
public static boolean hasNullableOrMonotonicNonNullAnnotation(Symbol symbol, Config config) {
return NullabilityUtil.hasAnyAnnotationMatchingBackCompat(
return NullabilityUtil.hasAnyAnnotationMatching(
symbol,
annot -> isNullableAnnotation(annot, config) || isMonotonicNonNullAnnotation(annot));
}
Expand Down Expand Up @@ -208,7 +208,7 @@ public static boolean isNonNullAnnotation(String annotName, Config config) {
* Config)}
*/
public static boolean hasNonNullAnnotation(Symbol symbol, Config config) {
return NullabilityUtil.hasAnyAnnotationMatchingBackCompat(
return NullabilityUtil.hasAnyAnnotationMatching(
symbol, annot -> isNonNullAnnotation(annot, config));
}

Expand All @@ -220,7 +220,7 @@ public static boolean hasNonNullAnnotation(Symbol symbol, Config config) {
* Config)}
*/
public static boolean hasNullableAnnotation(Symbol symbol, Config config) {
return NullabilityUtil.hasAnyAnnotationMatchingBackCompat(
return NullabilityUtil.hasAnyAnnotationMatching(
symbol, annot -> isNullableAnnotation(annot, config));
}

Expand Down Expand Up @@ -319,7 +319,7 @@ public static boolean varargsArrayIsNonNull(Symbol paramSymbol, Config config) {

/** Checks if the symbol has a {@code @Nullable} declaration annotation */
public static boolean hasNullableDeclarationAnnotation(Symbol symbol, Config config) {
for (AnnotationMirror annotationMirror : symbol.getRawAttributes()) {
for (AnnotationMirror annotationMirror : symbol.getAnnotationMirrors()) {
if (isNullableAnnotation(annotationMirror.getAnnotationType().toString(), config)) {
return true;
}
Expand All @@ -329,7 +329,7 @@ public static boolean hasNullableDeclarationAnnotation(Symbol symbol, Config con

/** Checks if the symbol has a {@code @NonNull} declaration annotation */
public static boolean hasNonNullDeclarationAnnotation(Symbol symbol, Config config) {
for (AnnotationMirror annotationMirror : symbol.getRawAttributes()) {
for (AnnotationMirror annotationMirror : symbol.getAnnotationMirrors()) {
if (isNonNullAnnotation(annotationMirror.getAnnotationType().toString(), config)) {
return true;
}
Expand Down
17 changes: 0 additions & 17 deletions nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import com.sun.tools.javac.code.BoundKind;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.TargetType;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.tree.JCTree;
Expand Down Expand Up @@ -296,22 +295,6 @@ private boolean[] getTypeParamsWithNullableUpperBound(Type type) {
result[i] = true;
}
}
// For handling types declared in bytecode rather than source code.
// Due to a bug in javac versions before JDK 22 (https://bugs.openjdk.org/browse/JDK-8225377),
// the above code does not work for types declared in bytecode. We need to read the raw type
// attributes instead.
com.sun.tools.javac.util.List<Attribute.TypeCompound> rawTypeAttributes =
tsym.getRawTypeAttributes();
if (rawTypeAttributes != null) {
for (Attribute.TypeCompound typeCompound : rawTypeAttributes) {
if (typeCompound.position.type.equals(TargetType.CLASS_TYPE_PARAMETER_BOUND)
&& Nullness.isNullableAnnotation(
typeCompound.type.tsym.getQualifiedName().toString(), config)) {
int index = typeCompound.position.parameter_index;
result[index] = true;
}
}
}
return result;
}

Expand Down
3 changes: 2 additions & 1 deletion nullaway/src/test/java/com/uber/nullaway/ErrorProneCLIFlagsConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ public void bothAnnotatedPackagesAndOnlyNullMarkedFails() {
public void missingTypeAnnotationSymbolFlagForJSpecifyModeOnOlderJDK() {
Assume.assumeTrue(Runtime.version().feature() < 22);
CompilationTestHelper compilationTestHelper =
makeTestHelperWithArgs(
CompilationTestHelper.newInstance(NullAway.class, getClass())
.setArgs(
List.of("-XepOpt:NullAway:OnlyNullMarked", "-XepOpt:NullAway:JSpecifyMode=true"))
.addSourceLines("Stub.java", "package com.uber; class Stub {}");
AssertionError e = assertThrows(AssertionError.class, () -> compilationTestHelper.doTest());
Expand Down
Loading
Loading