Skip to content

Commit

Permalink
Streamline reasoning about method return nullness a bit
Browse files Browse the repository at this point in the history
- remove seemingly unnecessary logic superceded by nullness inference
- be consistent about extracting annotations from symbols (this avoids mis-interpretation of @nonnull(when=NEVER), for instance)
- move predicate query to hopefully make logic easier to follow

RELNOTES: None.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=226569216
  • Loading branch information
kevin1e100 authored and ronshapiro committed Dec 24, 2018
1 parent 651df2c commit dd0183f
Showing 1 changed file with 18 additions and 23 deletions.
Expand Up @@ -269,14 +269,7 @@ public AccessPathStore<Nullness> initialStore(
return result.build(); return result.build();
} }


private Nullness getInferredNullness(MethodInvocationNode node, ClassAndMethod callee) { private Optional<Nullness> getInferredNullness(MethodInvocationNode node) {
// Baseline nullness information about this method, in case inference is unsuccessful.
Nullness baselineNullness = methodReturnsNonNull.apply(callee) ? NONNULL : NULLABLE;
if (!callee.isGenericResult) {
// We only care about inference results for methods that return a type variable.
return baselineNullness;
}

// Method has a generic result, so ask inference to infer a qualifier for that type parameter // Method has a generic result, so ask inference to infer a qualifier for that type parameter
if (inferenceResults == null) { if (inferenceResults == null) {
// inferenceResults are per-procedure; if it is null that means this is the first query within // inferenceResults are per-procedure; if it is null that means this is the first query within
Expand All @@ -302,7 +295,7 @@ private Nullness getInferredNullness(MethodInvocationNode node, ClassAndMethod c
"Call `%s` is not contained in an lambda, initializer or method.", "Call `%s` is not contained in an lambda, initializer or method.",
node)); node));
} }
return inferenceResults.getExprNullness(node.getTree()).orElse(baselineNullness); return inferenceResults.getExprNullness(node.getTree());
} }


/** /**
Expand Down Expand Up @@ -789,19 +782,21 @@ private Nullness returnValueNullness(MethodInvocationNode node, @Nullable ClassA
if (AccessPath.isAutoValueAccessor(node.getTree())) { if (AccessPath.isAutoValueAccessor(node.getTree())) {
return NONNULL; return NONNULL;
} }
// If there is no nullness annotation on the callee method declaration, look for applicable
// annotations inherited from elsewhere. Nullness assumedNullness = methodReturnsNonNull.apply(callee) ? NONNULL : NULLABLE;
ImmutableList<String> annotations = if (!callee.isGenericResult) {
ImmutableList.<String>builder() // We only care about inference results for methods that return a type variable.
.addAll(inheritedAnnotations(node.getTarget().getMethod().getReturnType())) return assumedNullness;
.addAll( }
node.getType().getAnnotationMirrors().stream()
.map(Object::toString) // Look for applicable annotations inherited from elsewhere.
.collect(ImmutableList.toImmutableList())) // TODO(b/121398981): Treat type parameter bounds correctly
.build(); Nullness lowerBound =

Nullness.fromAnnotations(inheritedAnnotations(node.getTarget().getMethod().getReturnType()))
return getInferredNullness(node, callee) .orElse(NULLABLE);
.greatestLowerBound(Nullness.fromAnnotations(annotations).orElse(NULLABLE));
// Method has a generic result, so ask inference to infer a qualifier for that type parameter
return getInferredNullness(node).orElse(assumedNullness).greatestLowerBound(lowerBound);
} }


/** /**
Expand Down Expand Up @@ -1060,7 +1055,7 @@ static ClassAndMethod make(MethodSymbol methodSymbol, @Nullable Types types) {
// TODO(b/71812955): consider just wrapping methodSymbol instead of copying everything out. // TODO(b/71812955): consider just wrapping methodSymbol instead of copying everything out.
ImmutableList<String> annotations = ImmutableList<String> annotations =
MoreAnnotations.getDeclarationAndTypeAttributes(methodSymbol) MoreAnnotations.getDeclarationAndTypeAttributes(methodSymbol)
.map(c -> c.getAnnotationType().asElement().toString()) .map(Object::toString)
.collect(toImmutableList()); .collect(toImmutableList());


ClassSymbol clazzSymbol = (ClassSymbol) methodSymbol.owner; ClassSymbol clazzSymbol = (ClassSymbol) methodSymbol.owner;
Expand Down

0 comments on commit dd0183f

Please sign in to comment.