Skip to content

Commit

Permalink
Improve handling of type annotations and type parameter bounds
Browse files Browse the repository at this point in the history
- handle explicit type annotations on method results
- interpret annotations on type variables as bound and look for explicit bound first
- inherit annotations for wildcards
- handle intersection bounds
Also centralize annotation interpretation methods in a single class and merge two methods previously introduced to handle annotations for method results back into a single method.
RELNOTES: None.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=231288208
  • Loading branch information
kevin1e100 authored and ronshapiro committed Jan 31, 2019
1 parent 5ca7751 commit 78d34fc
Show file tree
Hide file tree
Showing 9 changed files with 212 additions and 177 deletions.
Expand Up @@ -16,13 +16,6 @@

package com.google.errorprone.dataflow.nullnesspropagation;

import com.google.errorprone.util.MoreAnnotations;
import com.sun.tools.javac.code.Symbol;
import java.util.Collection;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import org.checkerframework.dataflow.analysis.AbstractValue;

/**
Expand Down Expand Up @@ -124,32 +117,6 @@ public Nullness deducedValueWhenNotEqual() {
}
}

// TODO(kmb): Correctly handle JSR 305 @Nonnull(NEVER) etc.
private static final Predicate<String> ANNOTATION_RELEVANT_TO_NULLNESS =
Pattern.compile(
".*\\.((Recently)?Nullable(Decl)?|(Recently)?NotNull(Decl)?|NonNull(Decl)?|Nonnull|"
+ "CheckForNull)$")
.asPredicate();

private static final Predicate<String> NULLABLE_ANNOTATION =
Pattern.compile(".*\\.((Recently)?Nullable(Decl)?|CheckForNull)$").asPredicate();

private static Optional<Nullness> fromAnnotationStream(Stream<String> annotations) {
return annotations
.filter(ANNOTATION_RELEVANT_TO_NULLNESS)
.map(annot -> NULLABLE_ANNOTATION.test(annot) ? NULLABLE : NONNULL)
.reduce(Nullness::greatestLowerBound);
}

public static Optional<Nullness> fromAnnotations(Collection<String> annotations) {
return fromAnnotationStream(annotations.stream());
}

public static Optional<Nullness> fromAnnotationsOn(Symbol sym) {
return fromAnnotationStream(
MoreAnnotations.getDeclarationAndTypeAttributes(sym).map(Object::toString));
}

@Override
public String toString() {
return displayName;
Expand Down
@@ -0,0 +1,129 @@
/*
* Copyright 2014 The Error Prone Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.errorprone.dataflow.nullnesspropagation;

import static javax.lang.model.element.ElementKind.TYPE_PARAMETER;

import com.google.errorprone.util.MoreAnnotations;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Type;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.Parameterizable;
import javax.lang.model.element.TypeParameterElement;
import javax.lang.model.type.IntersectionType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.type.TypeVariable;

/** Utilities to extract {@link Nullness} from annotations. */
public class NullnessAnnotations {
// TODO(kmb): Correctly handle JSR 305 @Nonnull(NEVER) etc.
private static final Predicate<String> ANNOTATION_RELEVANT_TO_NULLNESS =
Pattern.compile(
".*\\.((Recently)?Nullable(Decl)?|(Recently)?NotNull(Decl)?|NonNull(Decl)?|Nonnull|"
+ "CheckForNull)$")
.asPredicate();
private static final Predicate<String> NULLABLE_ANNOTATION =
Pattern.compile(".*\\.((Recently)?Nullable(Decl)?|CheckForNull)$").asPredicate();

private NullnessAnnotations() {} // static methods only

public static Optional<Nullness> fromAnnotations(Collection<String> annotations) {
return fromAnnotationStream(annotations.stream());
}

public static Optional<Nullness> fromAnnotationsOn(@Nullable Symbol sym) {
if (sym != null) {
return fromAnnotationStream(
MoreAnnotations.getDeclarationAndTypeAttributes(sym).map(Object::toString));
}
return Optional.empty();
}

public static Optional<Nullness> fromAnnotationsOn(@Nullable TypeMirror type) {
if (type != null) {
return fromAnnotationList(type.getAnnotationMirrors());
}
return Optional.empty();
}

public static Optional<Nullness> getUpperBound(@Nullable Type type) {
if (type != null && type.getKind() == TypeKind.TYPEVAR) {
return getUpperBound((TypeVariable) type);
}
return Optional.empty();
}

private static Optional<Nullness> getUpperBound(TypeVariable typeVar) {
// Annotations on bounds at type variable declaration
Optional<Nullness> result;
if (typeVar.getUpperBound() instanceof IntersectionType) {
// For intersection types, use the lower bound of any annotations on the individual bounds
result =
fromAnnotationStream(
((IntersectionType) typeVar.getUpperBound())
.getBounds().stream()
.map(TypeMirror::getAnnotationMirrors)
.map(Object::toString));
} else {
result = fromAnnotationsOn(typeVar.getUpperBound());
}
if (result.isPresent()) {
// If upper bound is annotated, return that, ignoring annotations on the type variable itself.
// This gets the upper bound for <T extends @Nullable Object> whether T is annotated or not.
return result;
}

// Only if the bound isn't annotated, look for an annotation on the type variable itself and
// treat that as the upper bound. This handles "interface I<@NonNull|@Nullable T>" as a bound.
if (typeVar.asElement().getKind() == TYPE_PARAMETER) {
Element genericElt = ((TypeParameterElement) typeVar.asElement()).getGenericElement();
if (genericElt.getKind().isClass()
|| genericElt.getKind().isInterface()
|| genericElt.getKind() == ElementKind.METHOD) {
return ((Parameterizable) genericElt)
.getTypeParameters().stream()
.filter(
typeParam ->
typeParam.getSimpleName().equals(typeVar.asElement().getSimpleName()))
.findFirst()
// Annotations at class/interface/method type variable declaration
.flatMap(decl -> fromAnnotationList(decl.getAnnotationMirrors()));
}
}
return Optional.empty();
}

private static Optional<Nullness> fromAnnotationList(List<?> annotations) {
return fromAnnotationStream(annotations.stream().map(Object::toString));
}

private static Optional<Nullness> fromAnnotationStream(Stream<String> annotations) {
return annotations
.filter(ANNOTATION_RELEVANT_TO_NULLNESS)
.map(annot -> NULLABLE_ANNOTATION.test(annot) ? Nullness.NULLABLE : Nullness.NONNULL)
.reduce(Nullness::greatestLowerBound);
}
}
Expand Up @@ -200,7 +200,8 @@ private static class ReturnValueIsNonNull implements Predicate<MethodInfo>, Seri
@Override
public boolean apply(MethodInfo methodInfo) {
// Any method explicitly annotated is trusted to behave as advertised.
Optional<Nullness> fromAnnotations = Nullness.fromAnnotations(methodInfo.annotations());
Optional<Nullness> fromAnnotations =
NullnessAnnotations.fromAnnotations(methodInfo.annotations());
if (fromAnnotations.isPresent()) {
return fromAnnotations.get() == NONNULL;
}
Expand Down Expand Up @@ -255,7 +256,8 @@ public AccessPathStore<Nullness> initialStore(
AccessPathStore.Builder<Nullness> result = AccessPathStore.<Nullness>empty().toBuilder();
for (LocalVariableNode param : parameters) {
Nullness declared =
Nullness.fromAnnotationsOn((Symbol) param.getElement()).orElse(defaultAssumption);
NullnessAnnotations.fromAnnotationsOn((Symbol) param.getElement())
.orElse(defaultAssumption);
result.setInformation(AccessPath.fromLocalVariable(param), declared);
}
return result.build();
Expand Down Expand Up @@ -404,7 +406,7 @@ Nullness visitTypeCast(TypeCastNode node, SubNodeValues inputs) {
node.getType().getAnnotationMirrors().stream()
.map(Object::toString)
.collect(ImmutableList.toImmutableList());
return Nullness.fromAnnotations(annotations)
return NullnessAnnotations.fromAnnotations(annotations)
.orElseGet(
() -> hasPrimitiveType(node) ? NONNULL : inputs.valueOfSubNode(node.getOperand()));
}
Expand Down Expand Up @@ -746,30 +748,25 @@ Nullness fieldNullness(
Nullness standardFieldNullness(
ClassAndField accessed, @Nullable AccessPath path, AccessPathValues<Nullness> store) {
// First, check the store for a dataflow-computed nullness value and return it if it exists
// Otherwise, check for nullness annotations on the field declaration
// If there are none, check for nullness annotations on generic type declarations
// Otherwise, check for nullness annotations on the field's symbol (including type annotations)
// If there are none, check for nullness annotations on generic type bounds, if any
// If there are none, fall back to the defaultAssumption

Nullness dataflowResult = (path == null) ? BOTTOM : store.valueOfAccessPath(path, BOTTOM);
if (dataflowResult != BOTTOM) {
return dataflowResult;
}

Optional<Nullness> declaredNullness = Nullness.fromAnnotationsOn(accessed.symbol);
Optional<Nullness> declaredNullness = NullnessAnnotations.fromAnnotationsOn(accessed.symbol);
return declaredNullness.orElseGet(
() ->
Nullness.fromAnnotations(
MoreAnnotations.inheritedAnnotations(accessed.symbol.type).stream()
.map(Object::toString)
.collect(ImmutableList.toImmutableList()))
.orElse(defaultAssumption));
() -> NullnessAnnotations.getUpperBound(accessed.symbol.type).orElse(defaultAssumption));
}

private Nullness returnValueNullness(MethodInvocationNode node, @Nullable ClassAndMethod callee) {
if (callee == null) {
return defaultAssumption;
}
Optional<Nullness> declaredNullness = Nullness.fromAnnotations(callee.annotations);
Optional<Nullness> declaredNullness = NullnessAnnotations.fromAnnotations(callee.annotations);
if (declaredNullness.isPresent()) {
return declaredNullness.get();
}
Expand Down
Expand Up @@ -125,6 +125,6 @@ public Nullness getFieldInitializerNullness(TreePath fieldDeclPath, Context cont
}

public static boolean hasNullableAnnotation(Symbol symbol) {
return Nullness.fromAnnotationsOn(symbol).orElse(null) == Nullness.NULLABLE;
return NullnessAnnotations.fromAnnotationsOn(symbol).orElse(null) == Nullness.NULLABLE;
}
}
Expand Up @@ -73,7 +73,7 @@ private enum TrustReturnAnnotation implements Predicate<MethodInfo> {

@Override
public boolean apply(MethodInfo input) {
return Nullness.fromAnnotations(input.annotations()).orElse(Nullness.NONNULL)
return NullnessAnnotations.fromAnnotations(input.annotations()).orElse(Nullness.NONNULL)
== Nullness.NONNULL;
}
}
Expand Down

0 comments on commit 78d34fc

Please sign in to comment.