Skip to content

Commit

Permalink
Revert "JSpecify: initial checks for generic type compatibility at as…
Browse files Browse the repository at this point in the history 
…signments (uber#715)"

This reverts commit 14d3693.
  • Loading branch information
@msridhar
msridhar committed Jul 19, 2023
1 parent ce7e084 commit 5d781e8
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 577 deletions.
1 change: 0 additions & 1 deletion nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public enum MessageTypes {
WRONG_OVERRIDE_POSTCONDITION,
WRONG_OVERRIDE_PRECONDITION,
TYPE_PARAMETER_CANNOT_BE_NULLABLE,
ASSIGN_GENERIC_NULLABLE,
}

public String getMessage() {
Expand Down
220 changes: 4 additions & 216 deletions nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java
View file
Original file line number Diff line number Diff line change
@@ -1,44 +1,22 @@
package com.uber.nullaway;

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

import com.google.common.base.Preconditions;
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;
import com.sun.source.tree.AssignmentTree;
import com.sun.source.tree.NewClassTree;
import com.sun.source.tree.ParameterizedTypeTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import com.sun.tools.javac.code.Attribute;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.TypeMetadata;
import com.sun.tools.javac.code.Types;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/** Methods for performing checks related to generic types and nullability. */
public final class GenericsChecks {

private static final String NULLABLE_NAME = "org.jspecify.annotations.Nullable";

private static final Supplier<Type> NULLABLE_TYPE_SUPPLIER =
Suppliers.typeFromString(NULLABLE_NAME);
private VisitorState state;
private Config config;
private NullAway analysis;

public GenericsChecks(VisitorState state, Config config, NullAway analysis) {
this.state = state;
this.config = config;
this.analysis = analysis;
private GenericsChecks() {
// just utility methods
}

/**
Expand Down Expand Up @@ -92,14 +70,14 @@ public static void checkInstantiationForParameterizedTypedTree(
// if base type argument does not have @Nullable annotation then the instantiation is
// invalid
if (!hasNullableAnnotation) {
reportInvalidInstantiationError(
invalidInstantiationError(
nullableTypeArguments.get(i), baseType, typeVariable, state, analysis);
}
}
}
}

private static void reportInvalidInstantiationError(
private static void invalidInstantiationError(
Tree tree, Type baseType, Type baseTypeVariable, VisitorState state, NullAway analysis) {
ErrorBuilder errorBuilder = analysis.getErrorBuilder();
ErrorMessage errorMessage =
Expand All @@ -112,194 +90,4 @@ private static void reportInvalidInstantiationError(
errorBuilder.createErrorDescription(
errorMessage, analysis.buildDescription(tree), state, null));
}

private static void reportInvalidAssignmentInstantiationError(
Tree tree, Type lhsType, Type rhsType, VisitorState state, NullAway analysis) {
ErrorBuilder errorBuilder = analysis.getErrorBuilder();
ErrorMessage errorMessage =
new ErrorMessage(
ErrorMessage.MessageTypes.ASSIGN_GENERIC_NULLABLE,
String.format(
"Cannot assign from type "
+ rhsType
+ " to type "
+ lhsType
+ " due to mismatched nullability of type parameters"));
state.reportMatch(
errorBuilder.createErrorDescription(
errorMessage, analysis.buildDescription(tree), state, null));
}

/**
* For a tree representing an assignment, ensures that from the perspective of type parameter
* nullability, the type of the right-hand side is assignable to (a subtype of) the type of the
* left-hand side. This check ensures that for every parameterized type nested in each of the
* types, the type parameters have identical nullability.
*
* @param tree the tree to check, which must be either an {@link AssignmentTree} or a {@link
* VariableTree}
*/
public void checkTypeParameterNullnessForAssignability(Tree tree) {
if (!config.isJSpecifyMode()) {
return;
}
Tree lhsTree;
Tree rhsTree;
if (tree instanceof VariableTree) {
VariableTree varTree = (VariableTree) tree;
lhsTree = varTree.getType();
rhsTree = varTree.getInitializer();
} else {
AssignmentTree assignmentTree = (AssignmentTree) tree;
lhsTree = assignmentTree.getVariable();
rhsTree = assignmentTree.getExpression();
}
// rhsTree can be null for a VariableTree. Also, we don't need to do a check
// if rhsTree is the null literal
if (rhsTree == null || rhsTree.getKind().equals(Tree.Kind.NULL_LITERAL)) {
return;
}
Type lhsType = ASTHelpers.getType(lhsTree);
Type rhsType = ASTHelpers.getType(rhsTree);
// For NewClassTrees with annotated type parameters, javac does not preserve the annotations in
// its computed type for the expression. As a workaround, we construct a replacement Type
// object with the appropriate annotations.
if (rhsTree instanceof NewClassTree
&& ((NewClassTree) rhsTree).getIdentifier() instanceof ParameterizedTypeTree) {
ParameterizedTypeTree paramTypedTree =
(ParameterizedTypeTree) ((NewClassTree) rhsTree).getIdentifier();
if (paramTypedTree.getTypeArguments().isEmpty()) {
// no explicit type parameters
return;
}
rhsType = typeWithPreservedAnnotations(paramTypedTree);
}
if (lhsType instanceof Type.ClassType && rhsType instanceof Type.ClassType) {
compareNullabilityAnnotations((Type.ClassType) lhsType, (Type.ClassType) rhsType, tree);
}
}

/**
* Compare two types from an assignment for identical type parameter nullability, recursively
* checking nested generic types. See <a
* href="https://jspecify.dev/docs/spec/#nullness-delegating-subtyping">the JSpecify
* specification</a> and <a
* href="https://docs.oracle.com/javase/specs/jls/se14/html/jls-4.html#jls-4.10.2">the JLS
* subtyping rules for class and interface types</a>.
*
* @param lhsType type for the lhs of the assignment
* @param rhsType type for the rhs of the assignment
* @param tree tree representing the assignment
*/
private void compareNullabilityAnnotations(
Type.ClassType lhsType, Type.ClassType rhsType, Tree tree) {
Types types = state.getTypes();
// The base type of rhsType may be a subtype of lhsType's base type. In such cases, we must
// compare lhsType against the supertype of rhsType with a matching base type.
rhsType = (Type.ClassType) types.asSuper(rhsType, lhsType.tsym);
// This is impossible, considering the fact that standard Java subtyping succeeds before running
// NullAway
if (rhsType == null) {
throw new RuntimeException("Did not find supertype of " + rhsType + " matching " + lhsType);
}
List<Type> lhsTypeArguments = lhsType.getTypeArguments();
List<Type> rhsTypeArguments = rhsType.getTypeArguments();
// This is impossible, considering the fact that standard Java subtyping succeeds before running
// NullAway
if (lhsTypeArguments.size() != rhsTypeArguments.size()) {
throw new RuntimeException(
"Number of types arguments in " + rhsType + " does not match " + lhsType);
}
for (int i = 0; i < lhsTypeArguments.size(); i++) {
Type lhsTypeArgument = lhsTypeArguments.get(i);
Type rhsTypeArgument = rhsTypeArguments.get(i);
boolean isLHSNullableAnnotated = false;
List<Attribute.TypeCompound> lhsAnnotations = lhsTypeArgument.getAnnotationMirrors();
// To ensure that we are checking only jspecify nullable annotations
for (Attribute.TypeCompound annotation : lhsAnnotations) {
if (annotation.getAnnotationType().toString().equals(NULLABLE_NAME)) {
isLHSNullableAnnotated = true;
break;
}
}
boolean isRHSNullableAnnotated = false;
List<Attribute.TypeCompound> rhsAnnotations = rhsTypeArgument.getAnnotationMirrors();
// To ensure that we are checking only jspecify nullable annotations
for (Attribute.TypeCompound annotation : rhsAnnotations) {
if (annotation.getAnnotationType().toString().equals(NULLABLE_NAME)) {
isRHSNullableAnnotated = true;
break;
}
}
if (isLHSNullableAnnotated != isRHSNullableAnnotated) {
reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis);
return;
}
// nested generics
if (lhsTypeArgument.getTypeArguments().length() > 0) {
compareNullabilityAnnotations(
(Type.ClassType) lhsTypeArgument, (Type.ClassType) rhsTypeArgument, tree);
}
}
}

/**
* For the Parameterized typed trees, ASTHelpers.getType(tree) does not return a Type with
* preserved annotations. This method takes a Parameterized typed tree as an input and returns the
* Type of the tree with the annotations.
*
* @param tree A parameterized typed tree for which we need class type with preserved annotations.
* @return A Type with preserved annotations.
*/
private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) {
Type.ClassType type = (Type.ClassType) ASTHelpers.getType(tree);
Preconditions.checkNotNull(type);
Type nullableType = NULLABLE_TYPE_SUPPLIER.get(state);
List<? extends Tree> typeArguments = tree.getTypeArguments();
List<Type> newTypeArgs = new ArrayList<>();
boolean hasNullableAnnotation = false;
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();
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 =
new TypeMetadata(new TypeMetadata.Annotations(nullableAnnotationCompound));
Type currentTypeArgType = castToNonNull(ASTHelpers.getType(curTypeArg));
if (currentTypeArgType.getTypeArguments().size() > 0) {
// nested generic type; recursively preserve its nullability type argument annotations
currentTypeArgType = typeWithPreservedAnnotations((ParameterizedTypeTree) curTypeArg);
}
Type.ClassType newTypeArgType =
(Type.ClassType) currentTypeArgType.cloneWithMetadata(typeMetadata);
newTypeArgs.add(newTypeArgType);
}
Type.ClassType finalType =
new Type.ClassType(
type.getEnclosingType(), com.sun.tools.javac.util.List.from(newTypeArgs), type.tsym);
return finalType;
}
}
9 changes: 0 additions & 9 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,11 +466,6 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) {
if (lhsType != null && lhsType.isPrimitive()) {
doUnboxingCheck(state, tree.getExpression());
}
// generics check
if (lhsType != null && lhsType.getTypeArguments().length() > 0) {
new GenericsChecks(state, config, this).checkTypeParameterNullnessForAssignability(tree);
}

Symbol assigned = ASTHelpers.getSymbol(tree.getVariable());
if (assigned == null || assigned.getKind() != ElementKind.FIELD) {
// not a field of nullable type
Expand Down Expand Up @@ -1338,10 +1333,6 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
return Description.NO_MATCH;
}
VarSymbol symbol = ASTHelpers.getSymbol(tree);
if (tree.getInitializer() != null) {
new GenericsChecks(state, config, this).checkTypeParameterNullnessForAssignability(tree);
}

if (symbol.type.isPrimitive() && tree.getInitializer() != null) {
doUnboxingCheck(state, tree.getInitializer());
}
Expand Down
Loading

0 comments on commit 5d781e8

Please sign in to comment.