Skip to content

Commit

Permalink
Merge branch 'master' into generic-type-pretty
Browse files Browse the repository at this point in the history
  • Loading branch information
@msridhar
msridhar committed Apr 27, 2023
2 parents 7c86f30 + 8821567 commit 2689a62
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 63 deletions.
68 changes: 27 additions & 41 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2197,18 +2197,13 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) {
// obviously not null
return false;
}
if (expr.getKind() == Tree.Kind.NULL_LITERAL) {
// obviously null, so same as for above literals we return early without consulting handlers
return true;
}
// the logic here is to avoid doing dataflow analysis whenever possible
Symbol exprSymbol = ASTHelpers.getSymbol(expr);
boolean exprMayBeNull;
// return early for expressions that no handler overrides and will not need dataflow analysis
switch (expr.getKind()) {
case NULL_LITERAL:
// obviously null
return true;
case ARRAY_ACCESS:
// unsound! we cannot check for nullness of array contents yet
exprMayBeNull = false;
break;
case NEW_CLASS:
case NEW_ARRAY:
// for string concatenation, auto-boxing
Expand Down Expand Up @@ -2258,29 +2253,35 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) {
case RIGHT_SHIFT:
case UNSIGNED_RIGHT_SHIFT:
// clearly not null
exprMayBeNull = false;
return false;
default:
break;
}
// the logic here is to avoid doing dataflow analysis whenever possible
Symbol exprSymbol = ASTHelpers.getSymbol(expr);
boolean exprMayBeNull;
switch (expr.getKind()) {
case MEMBER_SELECT:
if (exprSymbol == null) {
throw new IllegalStateException(
"unexpected null symbol for dereference expression " + state.getSourceForNode(expr));
}
exprMayBeNull = mayBeNullFieldAccess(state, expr, exprSymbol);
exprMayBeNull =
NullabilityUtil.mayBeNullFieldFromType(exprSymbol, config, codeAnnotationInfo);
break;
case IDENTIFIER:
if (exprSymbol == null) {
throw new IllegalStateException(
"unexpected null symbol for identifier " + state.getSourceForNode(expr));
}
if (exprSymbol.getKind().equals(ElementKind.FIELD)) {
// Special case: mayBeNullFieldAccess runs handler.onOverrideMayBeNullExpr before
// dataflow.
return mayBeNullFieldAccess(state, expr, exprSymbol);
if (exprSymbol.getKind() == ElementKind.FIELD) {
exprMayBeNull =
NullabilityUtil.mayBeNullFieldFromType(exprSymbol, config, codeAnnotationInfo);
} else {
// Check handler.onOverrideMayBeNullExpr before dataflow.
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, exprSymbol, state, true);
return exprMayBeNull ? nullnessFromDataflow(state, expr) : false;
// rely on dataflow analysis for local variables
exprMayBeNull = true;
}
break;
case METHOD_INVOCATION:
if (!(exprSymbol instanceof Symbol.MethodSymbol)) {
throw new IllegalStateException(
Expand All @@ -2289,36 +2290,30 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) {
+ " for method invocation "
+ state.getSourceForNode(expr));
}
// Special case: mayBeNullMethodCall runs handler.onOverrideMayBeNullExpr before dataflow.
return mayBeNullMethodCall(state, expr, (Symbol.MethodSymbol) exprSymbol);
exprMayBeNull = mayBeNullMethodCall((Symbol.MethodSymbol) exprSymbol);
break;
case CONDITIONAL_EXPRESSION:
case ASSIGNMENT:
exprMayBeNull = nullnessFromDataflow(state, expr);
exprMayBeNull = true;
break;
default:
// match switch expressions by comparing strings, so the code compiles on JDK versions < 12
if (expr.getKind().name().equals("SWITCH_EXPRESSION")) {
exprMayBeNull = nullnessFromDataflow(state, expr);
exprMayBeNull = true;
} else {
throw new RuntimeException(
"whoops, better handle " + expr.getKind() + " " + state.getSourceForNode(expr));
}
}
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, exprSymbol, state, exprMayBeNull);
return exprMayBeNull;
return exprMayBeNull && nullnessFromDataflow(state, expr);
}

private boolean mayBeNullMethodCall(
VisitorState state, ExpressionTree expr, Symbol.MethodSymbol exprSymbol) {
boolean exprMayBeNull = true;
private boolean mayBeNullMethodCall(Symbol.MethodSymbol exprSymbol) {
if (codeAnnotationInfo.isSymbolUnannotated(exprSymbol, config)) {
exprMayBeNull = false;
}
if (!Nullness.hasNullableAnnotation(exprSymbol, config)) {
exprMayBeNull = false;
return false;
}
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, exprSymbol, state, exprMayBeNull);
return exprMayBeNull ? nullnessFromDataflow(state, expr) : false;
return Nullness.hasNullableAnnotation(exprSymbol, config);
}

public boolean nullnessFromDataflow(VisitorState state, ExpressionTree expr) {
Expand All @@ -2336,15 +2331,6 @@ public AccessPathNullnessAnalysis getNullnessAnalysis(VisitorState state) {
return AccessPathNullnessAnalysis.instance(state, nonAnnotatedMethod, config, this.handler);
}

private boolean mayBeNullFieldAccess(VisitorState state, ExpressionTree expr, Symbol exprSymbol) {
boolean exprMayBeNull = true;
if (!NullabilityUtil.mayBeNullFieldFromType(exprSymbol, config, codeAnnotationInfo)) {
exprMayBeNull = false;
}
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, exprSymbol, state, exprMayBeNull);
return exprMayBeNull ? nullnessFromDataflow(state, expr) : false;
}

private Description matchDereference(
ExpressionTree baseExpression, ExpressionTree derefExpression, VisitorState state) {
Symbol baseExpressionSymbol = ASTHelpers.getSymbol(baseExpression);
Expand Down
46 changes: 24 additions & 22 deletions nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,29 +128,31 @@ public boolean onOverrideMayBeNullExpr(
@Nullable Symbol exprSymbol,
VisitorState state,
boolean exprMayBeNull) {
if (expr.getKind().equals(Tree.Kind.METHOD_INVOCATION)
&& exprSymbol instanceof Symbol.MethodSymbol) {
OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context);
Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) exprSymbol;
// When looking up library models of annotated code, we match the exact method signature only;
// overriding methods in subclasses must be explicitly given their own library model.
// When dealing with unannotated code, we default to generality: a model applies to a method
// and any of its overriding implementations.
// see https://github.com/uber/NullAway/issues/445 for why this is needed.
boolean isMethodAnnotated =
!getCodeAnnotationInfo(state.context).isSymbolUnannotated(methodSymbol, this.config);
if (optLibraryModels.hasNullableReturn(methodSymbol, state.getTypes(), !isMethodAnnotated)
|| !optLibraryModels.nullImpliesNullParameters(methodSymbol).isEmpty()) {
// These mean the method might be null, depending on dataflow and arguments. We force
// dataflow to run.
return analysis.nullnessFromDataflow(state, expr) || exprMayBeNull;
} else if (optLibraryModels.hasNonNullReturn(
methodSymbol, state.getTypes(), !isMethodAnnotated)) {
// This means the method can't be null, so we return false outright.
return false;
}
if (!(expr.getKind() == Tree.Kind.METHOD_INVOCATION
&& exprSymbol instanceof Symbol.MethodSymbol)) {
return exprMayBeNull;
}
OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context);
Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) exprSymbol;
// When looking up library models of annotated code, we match the exact method signature only;
// overriding methods in subclasses must be explicitly given their own library model.
// When dealing with unannotated code, we default to generality: a model applies to a method
// and any of its overriding implementations.
// see https://github.com/uber/NullAway/issues/445 for why this is needed.
boolean isMethodUnannotated =
getCodeAnnotationInfo(state.context).isSymbolUnannotated(methodSymbol, this.config);
if (exprMayBeNull) {
// This is the only case in which we may switch the result from @Nullable to @NonNull:
return !optLibraryModels.hasNonNullReturn(
methodSymbol, state.getTypes(), isMethodUnannotated);
}
if (optLibraryModels.hasNullableReturn(methodSymbol, state.getTypes(), isMethodUnannotated)) {
return true;
}
if (!optLibraryModels.nullImpliesNullParameters(methodSymbol).isEmpty()) {
return true;
}
return exprMayBeNull;
return false;
}

@Override
Expand Down

0 comments on commit 2689a62

Please sign in to comment.