Skip to content

Commit

Permalink
Fix error inside Lombok generated code when dealing with @nullable @B…
Browse files Browse the repository at this point in the history
…uilder.Default fields
  • Loading branch information
lazaroclapp committed May 31, 2023
1 parent c1741b3 commit 2a57c54
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 10 deletions.
13 changes: 11 additions & 2 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
Original file line number Diff line number Diff line change
Expand Up @@ -822,8 +822,17 @@ private Description checkReturnExpression(
// support)
return Description.NO_MATCH;
}
if (codeAnnotationInfo.isSymbolUnannotated(methodSymbol, config)
|| Nullness.hasNullableAnnotation(methodSymbol, config)) {
final boolean isContainingMethodAnnotated =
!codeAnnotationInfo.isSymbolUnannotated(methodSymbol, config);
Nullness containingMethodReturnNullness =
Nullness.NULLABLE; // Permissive default for unannotated code.
if (isContainingMethodAnnotated && !Nullness.hasNullableAnnotation(methodSymbol, config)) {
containingMethodReturnNullness = Nullness.NONNULL;
}
containingMethodReturnNullness =
handler.onOverrideMethodInvocationReturnNullability(
methodSymbol, state, isContainingMethodAnnotated, containingMethodReturnNullness);
if (containingMethodReturnNullness.equals(Nullness.NULLABLE)) {
return Description.NO_MATCH;
}
if (mayBeNullExpr(state, retExpr)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public static Handler buildDefault(Config config) {
if (config.checkContracts()) {
handlerListBuilder.add(new ContractCheckHandler(config));
}
handlerListBuilder.add(new LombokHandler(config));

return new CompositeHandler(handlerListBuilder.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ public Nullness onOverrideMethodInvocationReturnNullability(
Nullness returnNullness) {
OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context);
if (optLibraryModels.hasNonNullReturn(methodSymbol, state.getTypes(), !isAnnotated)) {
return Nullness.NONNULL;
return NONNULL;
} else if (optLibraryModels.hasNullableReturn(methodSymbol, state.getTypes(), !isAnnotated)) {
return NULLABLE;
}
return returnNullness;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package com.uber.nullaway.handlers;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.VisitorState;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Symbol;
import com.uber.nullaway.Config;
import com.uber.nullaway.NullAway;
import com.uber.nullaway.Nullness;
import java.util.stream.StreamSupport;
import javax.annotation.Nullable;
import javax.lang.model.element.ElementKind;

/**
* A general handler for Lombok generated code and its internal semantics.
*
* <p>Currently used to propagate @Nullable in cases where the Lombok annotation processor fails to
* do so consistently.
*/
public class LombokHandler extends BaseNoOpHandler {

private static String LOMBOK_GENERATED_ANNOTATION_NAME = "lombok.Generated";
private static String LOMBOK_BUILDER_DEFAULT_METHOD_PREFIX = "$default$";

private final Config config;

public LombokHandler(Config config) {
this.config = config;
}

private boolean isLombokMethodWithMissingNullableAnnotation(
Symbol.MethodSymbol methodSymbol, VisitorState state) {
if (!ASTHelpers.hasAnnotation(methodSymbol, LOMBOK_GENERATED_ANNOTATION_NAME, state)) {
return false;
}
String methodNameString = methodSymbol.name.toString();
if (!methodNameString.startsWith(LOMBOK_BUILDER_DEFAULT_METHOD_PREFIX)) {
return false;
}
String originalFieldName =
methodNameString.substring(LOMBOK_BUILDER_DEFAULT_METHOD_PREFIX.length());
ImmutableList<Symbol> matchingMembers =
StreamSupport.stream(
ASTHelpers.scope(methodSymbol.enclClass().members())
.getSymbols(
sym ->
sym.name.contentEquals(originalFieldName)
&& sym.getKind().equals(ElementKind.FIELD))
.spliterator(),
false)
.collect(ImmutableList.toImmutableList());
Preconditions.checkArgument(
matchingMembers.size() == 1,
String.format(
"Found %d fields matching Lombok generated builder default method %s",
matchingMembers.size(), methodNameString));
return Nullness.hasNullableAnnotation(matchingMembers.get(0), config);
}

@Override
public boolean onOverrideMayBeNullExpr(
NullAway analysis,
ExpressionTree expr,
@Nullable Symbol exprSymbol,
VisitorState state,
boolean exprMayBeNull) {
if (exprMayBeNull) {
return true;
}
Tree.Kind exprKind = expr.getKind();
if (exprSymbol != null && exprKind == Tree.Kind.METHOD_INVOCATION) {
Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) exprSymbol;
return isLombokMethodWithMissingNullableAnnotation(methodSymbol, state);
}
return false;
}

@Override
public Nullness onOverrideMethodInvocationReturnNullability(
Symbol.MethodSymbol methodSymbol,
VisitorState state,
boolean isAnnotated,
Nullness returnNullness) {
if (isLombokMethodWithMissingNullableAnnotation(methodSymbol, state)) {
return Nullness.NULLABLE;
}
return returnNullness;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,22 @@ public void allowLibraryModelsOverrideAnnotations() {
temporaryFolder.getRoot().getAbsolutePath(),
"-XepOpt:NullAway:AnnotatedPackages=com.uber"))
.addSourceLines(
"Foo.java",
"AnnotatedWithModels.java",
"package com.uber;",
"public class Foo {",
"public class AnnotatedWithModels {",
" Object field = new Object();",
" Object bar() {",
" return new Object();",
" // implicitly @Nullable due to library model",
" Object returnsNullFromModel() {",
" // null is valid here only because of the library model",
" return null;",
" }",
" Object nullableReturn() {",
" // BUG: Diagnostic contains: returning @Nullable",
" return bar();",
" return returnsNullFromModel();",
" }",
" void run() {",
" // just to make sure, flow analysis is also impacted by library models information",
" Object temp = bar();",
" Object temp = returnsNullFromModel();",
" // BUG: Diagnostic contains: assigning @Nullable",
" this.field = temp;",
" }",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@ public class LombokDTO {
private String field;
@Builder.Default private String fieldWithDefault = "Default";
@Nullable private String nullableField;
@Nullable @Builder.Default private String fieldWithNullDefault = null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public ImmutableSetMultimap<MethodRef, Integer> nullImpliesNullParameters() {
@Override
public ImmutableSet<MethodRef> nullableReturns() {
return ImmutableSet.of(
methodRef("com.uber.Foo", "bar()"),
methodRef("com.uber.AnnotatedWithModels", "returnsNullFromModel()"),
methodRef("com.uber.lib.unannotated.UnannotatedWithModels", "returnsNullUnannotated()"),
methodRef("com.uber.lib.unannotated.UnannotatedWithModels", "returnsNullUnannotated2()"));
}
Expand Down

0 comments on commit 2a57c54

Please sign in to comment.