Skip to content

Commit

Permalink
JSpecify: initial checks for generic type compatibility at assignments (
Browse files Browse the repository at this point in the history
#715)

This pull request adds the identical type parameter nullability checks for both sides of an assignment. 
for example
`NullableTypeParam<@nullable String> nullableTypeParam = new NullableTypeParam<String>();` should generate an error as the type parameters of both sides of the assignment do not have the same Nullability annotations.
This PR covers the following cases:
- checks for the variable instantiation `A<String> a = new A<@nullable String>();`
- checks for the assignments
`A<@nullable String> t1 = new A<@nullable String>();
A<String> t2;
t2 = t1;`
- LHS and RHS with the exact same types
`A<String> a = new A<@nullable String>();`
- RHS being a subtype of LHS
```java
class Test {
  class D<P extends @nullable Object> {}
  class B<P extends @nullable Object> extends D<P>{}
  void test1() {
      // BUG: Diagnostic contains: Cannot assign from type
      D<String> f = new B<@nullable String>();
  }
}
```
- nested generics 
`A<B<String>, String> a = new A<B<@nullable String>>();`

The PR does not handle pseudo-assignments due to parameter passing / returns; that support will come in a follow-up PR.  Also note that this PR only changes NullAway behavior when `JSpecifyMode` is enabled.

Additionally, we don't currently have functioning support for checking compatibility of method references, lambdas, or inferred generic instantiations, but this PR includes tests to show that those cases do not produce false positive errors (just false negatives / missed detections).
  • Loading branch information
NikitaAware committed Feb 7, 2023
1 parent a2efa6e commit 14d3693
Show file tree
Hide file tree
Showing 4 changed files with 577 additions and 82 deletions.
1 change: 1 addition & 0 deletions nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public enum MessageTypes {
WRONG_OVERRIDE_POSTCONDITION,
WRONG_OVERRIDE_PRECONDITION,
TYPE_PARAMETER_CANNOT_BE_NULLABLE,
ASSIGN_GENERIC_NULLABLE,
}

public String getMessage() {
Expand Down
220 changes: 216 additions & 4 deletions nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java
Original file line number Diff line number Diff line change
@@ -1,22 +1,44 @@
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 GenericsChecks() {
// just utility methods
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;
}

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

private static void invalidInstantiationError(
private static void reportInvalidInstantiationError(
Tree tree, Type baseType, Type baseTypeVariable, VisitorState state, NullAway analysis) {
ErrorBuilder errorBuilder = analysis.getErrorBuilder();
ErrorMessage errorMessage =
Expand All @@ -90,4 +112,194 @@ private static void invalidInstantiationError(
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: 9 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,11 @@ 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 @@ -1333,6 +1338,10 @@ 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 14d3693

Please sign in to comment.