Skip to content
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

Refactor PreservedAnnotationTreeVisitor #955

Merged
merged 4 commits into from
May 14, 2024
Merged

Refactor PreservedAnnotationTreeVisitor #955

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7bf8484
refactor
msridhar May 13, 2024
7354445
simplify
msridhar May 13, 2024
33d207a
cleanup
msridhar May 13, 2024
0a2d4d5
Merge branch 'master' into refactor-preserved-annotations
msridhar May 14, 2024
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
130 changes: 88 additions & 42 deletions nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static com.uber.nullaway.NullabilityUtil.castToNonNull;

import com.google.common.base.Preconditions;
import com.google.errorprone.VisitorState;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotatedTypeTree;
Expand All @@ -12,6 +11,7 @@
import com.sun.source.tree.Tree;
import com.sun.source.util.SimpleTreeVisitor;
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.util.ListBuffer;
Expand Down Expand Up @@ -44,52 +44,42 @@

@Override
public Type visitParameterizedType(ParameterizedTypeTree tree, Void p) {
Type.ClassType type = (Type.ClassType) ASTHelpers.getType(tree);
Preconditions.checkNotNull(type);
Type nullableType = GenericsChecks.JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state);
Type.ClassType baseType = (Type.ClassType) tree.getType().accept(this, null);
List<? extends Tree> typeArguments = tree.getTypeArguments();
List<Type> newTypeArgs = new ArrayList<>();
for (int i = 0; i < typeArguments.size(); i++) {
AnnotatedTypeTree annotatedType = null;
Tree curTypeArg = typeArguments.get(i);
// If the type argument has an annotation, it will either be an AnnotatedTypeTree, or a
// ParameterizedTypeTree in the case of a nested generic type
if (curTypeArg instanceof AnnotatedTypeTree) {
annotatedType = (AnnotatedTypeTree) curTypeArg;
} else if (curTypeArg instanceof ParameterizedTypeTree
&& ((ParameterizedTypeTree) curTypeArg).getType() instanceof AnnotatedTypeTree) {
annotatedType = (AnnotatedTypeTree) ((ParameterizedTypeTree) curTypeArg).getType();
}
List<? extends AnnotationTree> annotations =
annotatedType != null ? annotatedType.getAnnotations() : Collections.emptyList();
boolean hasNullableAnnotation = false;
for (AnnotationTree annotation : annotations) {
if (ASTHelpers.isSameType(
nullableType, ASTHelpers.getType(annotation.getAnnotationType()), state)) {
hasNullableAnnotation = true;
break;
}
}
// construct a TypeMetadata object containing a nullability annotation if needed
com.sun.tools.javac.util.List<Attribute.TypeCompound> nullableAnnotationCompound =
hasNullableAnnotation
? com.sun.tools.javac.util.List.from(
Collections.singletonList(
new Attribute.TypeCompound(
nullableType, com.sun.tools.javac.util.List.nil(), null)))
: com.sun.tools.javac.util.List.nil();
TypeMetadata typeMetadata = TYPE_METADATA_BUILDER.create(nullableAnnotationCompound);
Type currentTypeArgType = curTypeArg.accept(this, null);
Type newTypeArgType =
TYPE_METADATA_BUILDER.cloneTypeWithMetadata(currentTypeArgType, typeMetadata);
newTypeArgs.add(newTypeArgType);
newTypeArgs.add(typeArguments.get(i).accept(this, null));
}
Type.ClassType finalType =
new Type.ClassType(
type.getEnclosingType(), com.sun.tools.javac.util.List.from(newTypeArgs), type.tsym);
Type finalType = TYPE_METADATA_BUILDER.createWithBaseTypeAndTypeArgs(baseType, newTypeArgs);
return finalType;
}

@Override
public Type visitAnnotatedType(AnnotatedTypeTree annotatedType, Void unused) {
List<? extends AnnotationTree> annotations = annotatedType.getAnnotations();
boolean hasNullableAnnotation = false;
Type nullableType = GenericsChecks.JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state);
for (AnnotationTree annotation : annotations) {
if (ASTHelpers.isSameType(
nullableType, ASTHelpers.getType(annotation.getAnnotationType()), state)) {
hasNullableAnnotation = true;
break;
}
}
// construct a TypeMetadata object containing a nullability annotation if needed
com.sun.tools.javac.util.List<Attribute.TypeCompound> nullableAnnotationCompound =
hasNullableAnnotation
? com.sun.tools.javac.util.List.from(
Collections.singletonList(
new Attribute.TypeCompound(
nullableType, com.sun.tools.javac.util.List.nil(), null)))
: com.sun.tools.javac.util.List.nil();
TypeMetadata typeMetadata = TYPE_METADATA_BUILDER.create(nullableAnnotationCompound);
Type underlyingType = annotatedType.getUnderlyingType().accept(this, null);
Type newType = TYPE_METADATA_BUILDER.cloneTypeWithMetadata(underlyingType, typeMetadata);
return newType;
}

/** By default, just use the type computed by javac */
@Override
protected Type defaultAction(Tree node, Void unused) {
Expand All @@ -104,6 +94,8 @@
TypeMetadata create(com.sun.tools.javac.util.List<Attribute.TypeCompound> attrs);

Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metaData);

Type createWithBaseTypeAndTypeArgs(Type baseType, List<Type> typeArgs);
}

/**
Expand All @@ -129,6 +121,15 @@
public Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metadata) {
return typeToBeCloned.cloneWithMetadata(metadata);
}

@Override
public Type createWithBaseTypeAndTypeArgs(Type baseType, List<Type> typeArgs) {
return new Type.ClassType(
baseType.getEnclosingType(),
com.sun.tools.javac.util.List.from(typeArgs),
baseType.tsym,
baseType.getMetadata());
}
}

/**
Expand All @@ -138,11 +139,14 @@
*/
private static class JDK21TypeMetadataBuilder implements TypeMetadataBuilder {

private static final MethodHandle typeMetadataHandle = createHandle();
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();
Expand All @@ -156,6 +160,32 @@
}
}

private static MethodHandle createGetMetadataHandle() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me, all of these methods with reflection trickery are to avoid having a compile-time dep on JDK21, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bunch of (internal) javac APIs changed between JDK 17 and 21 related to these TypeMetadata objects. On JDK 21+ we use reflection to operate on them since I expect that is used less for compiling than JDK 17 and before. Unfortunately I think we can only drop this reflection approach once we drop support for building on JDK 17. This is hacky, but the alternative that doesn't rely on internal APIs would require building up our own type data structures from scratch, like Checker Framework, which would be a lot more code.

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);

Check warning on line 169 in nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java

View check run for this annotation

Codecov / codecov/patch

nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java#L168-L169 

Added lines #L168 - L169 were not covered by tests
}
}

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);

Check warning on line 185 in nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java

View check run for this annotation

Codecov / codecov/patch

nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java#L184-L185 

Added lines #L184 - L185 were not covered by tests
}
}

/**
* Used to get a MethodHandle for a virtual method from the specified class
*
Expand Down Expand Up @@ -183,7 +213,7 @@
ListBuffer<Attribute.TypeCompound> b = new ListBuffer<>();
b.appendList(attrs);
try {
return (TypeMetadata) typeMetadataHandle.invoke(b);
return (TypeMetadata) typeMetadataConstructorHandle.invoke(b);
} catch (Throwable e) {
throw new RuntimeException(e);
}
Expand All @@ -209,6 +239,22 @@
throw new RuntimeException(e);
}
}

@Override
public Type createWithBaseTypeAndTypeArgs(Type baseType, List<Type> typeArgs) {
try {
com.sun.tools.javac.util.List<TypeMetadata> metadata =
(com.sun.tools.javac.util.List<TypeMetadata>) 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);

Check warning on line 255 in nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java

View check run for this annotation

Codecov / codecov/patch

nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java#L254-L255 

Added lines #L254 - L255 were not covered by tests
}
}
}

/** The TypeMetadataBuilder to be used for the current JDK version. */
Expand Down
Loading