diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 8fbd5b41f9..7f75bbd68b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -169,8 +169,12 @@ private static void reportMismatchedTypeForTernaryOperator( } /** - * This method returns type of the tree considering that the parameterized typed tree annotations - * are not preserved if obtained directly using ASTHelpers. + * This method returns the type of the given tree, including any type use annotations. + * + *

This method is required because in some cases, the type returned by {@link + * com.google.errorprone.util.ASTHelpers#getType(Tree)} fails to preserve type use annotations, + * particularly when dealing with {@link com.sun.source.tree.NewClassTree} (e.g., {@code new + * Foo<@Nullable A>}). * * @param tree A tree for which we need the type with preserved annotations. * @return Type of the tree with preserved annotations. @@ -239,18 +243,20 @@ public void checkTypeParameterNullnessForFunctionReturnType( return; } - Type methodType = methodSymbol.getReturnType(); + Type formalReturnType = methodSymbol.getReturnType(); // check nullability of parameters only for generics - if (methodType.getTypeArguments().isEmpty()) { + if (formalReturnType.getTypeArguments().isEmpty()) { return; } Type returnExpressionType = getTreeType(retExpr); - if (methodType instanceof Type.ClassType && returnExpressionType instanceof Type.ClassType) { + if (formalReturnType instanceof Type.ClassType + && returnExpressionType instanceof Type.ClassType) { boolean isReturnTypeValid = compareNullabilityAnnotations( - (Type.ClassType) methodType, (Type.ClassType) returnExpressionType); + (Type.ClassType) formalReturnType, (Type.ClassType) returnExpressionType); if (!isReturnTypeValid) { - reportInvalidReturnTypeError(retExpr, methodType, returnExpressionType, state, analysis); + reportInvalidReturnTypeError( + retExpr, formalReturnType, returnExpressionType, state, analysis); } } } diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java index bff571ee01..db6d8c7250 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java @@ -22,6 +22,7 @@ package com.uber.nullaway.fixserialization; +import com.sun.tools.javac.code.Symbol; import com.uber.nullaway.ErrorMessage; import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; import com.uber.nullaway.fixserialization.out.ErrorInfo; @@ -76,7 +77,9 @@ public void serializeSuggestedFixInfo( if (enclosing) { suggestedNullableFixInfo.initEnclosing(); } - appendToFile(suggestedNullableFixInfo.tabSeparatedToString(), suggestedFixesOutputPath); + appendToFile( + suggestedNullableFixInfo.tabSeparatedToString(serializationAdapter), + suggestedFixesOutputPath); } /** @@ -90,7 +93,7 @@ public void serializeErrorInfo(ErrorInfo errorInfo) { } public void serializeFieldInitializationInfo(FieldInitializationInfo info) { - appendToFile(info.tabSeparatedToString(), fieldInitializationOutputPath); + appendToFile(info.tabSeparatedToString(serializationAdapter), fieldInitializationOutputPath); } /** Cleared the content of the file if exists and writes the header in the first line. */ @@ -195,4 +198,27 @@ public static Path pathToSourceFileFromURI(@Nullable URI uri) { return path; } } + + /** + * Serializes the given {@link Symbol} to a string. + * + * @param symbol The symbol to serialize. + * @param adapter adapter used to serialize symbols. + * @return The serialized symbol. + */ + public static String serializeSymbol(@Nullable Symbol symbol, SerializationAdapter adapter) { + if (symbol == null) { + return "null"; + } + switch (symbol.getKind()) { + case FIELD: + case PARAMETER: + return symbol.name.toString(); + case METHOD: + case CONSTRUCTOR: + return adapter.serializeMethodSignature((Symbol.MethodSymbol) symbol); + default: + return symbol.flatName().toString(); + } + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationAdapter.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationAdapter.java index 0120ef785f..c7ef01bc0b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationAdapter.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationAdapter.java @@ -22,6 +22,7 @@ package com.uber.nullaway.fixserialization.adapters; +import com.sun.tools.javac.code.Symbol; import com.uber.nullaway.fixserialization.out.ErrorInfo; /** @@ -62,4 +63,12 @@ public interface SerializationAdapter { * @return Supporting serialization version number. */ int getSerializationVersion(); + + /** + * Serializes the signature of the given {@link Symbol.MethodSymbol} to a string. + * + * @param methodSymbol The method symbol to serialize. + * @return The serialized method symbol. + */ + String serializeMethodSignature(Symbol.MethodSymbol methodSymbol); } diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV1Adapter.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV1Adapter.java index 243c63b4fb..b0ad0cc9c6 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV1Adapter.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV1Adapter.java @@ -24,7 +24,9 @@ import static com.uber.nullaway.fixserialization.out.ErrorInfo.EMPTY_NONNULL_TARGET_LOCATION_STRING; +import com.sun.tools.javac.code.Symbol; import com.uber.nullaway.fixserialization.SerializationService; +import com.uber.nullaway.fixserialization.Serializer; import com.uber.nullaway.fixserialization.location.SymbolLocation; import com.uber.nullaway.fixserialization.out.ErrorInfo; @@ -53,13 +55,11 @@ public String serializeError(ErrorInfo errorInfo) { "\t", errorInfo.getErrorMessage().getMessageType().toString(), SerializationService.escapeSpecialCharacters(errorInfo.getErrorMessage().getMessage()), - (errorInfo.getRegionClass() != null - ? errorInfo.getRegionClass().flatName().toString() - : "null"), - (errorInfo.getRegionMember() != null ? errorInfo.getRegionMember().toString() : "null"), + Serializer.serializeSymbol(errorInfo.getRegionClass(), this), + Serializer.serializeSymbol(errorInfo.getRegionMember(), this), (errorInfo.getNonnullTarget() != null ? SymbolLocation.createLocationFromSymbol(errorInfo.getNonnullTarget()) - .tabSeparatedToString() + .tabSeparatedToString(this) : EMPTY_NONNULL_TARGET_LOCATION_STRING)); } @@ -67,4 +67,9 @@ public String serializeError(ErrorInfo errorInfo) { public int getSerializationVersion() { return 1; } + + @Override + public String serializeMethodSignature(Symbol.MethodSymbol methodSymbol) { + return methodSymbol.toString(); + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV2Adapter.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV2Adapter.java index aa12587080..6b72441852 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV2Adapter.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV2Adapter.java @@ -24,7 +24,9 @@ import static com.uber.nullaway.fixserialization.out.ErrorInfo.EMPTY_NONNULL_TARGET_LOCATION_STRING; +import com.sun.tools.javac.code.Symbol; import com.uber.nullaway.fixserialization.SerializationService; +import com.uber.nullaway.fixserialization.Serializer; import com.uber.nullaway.fixserialization.location.SymbolLocation; import com.uber.nullaway.fixserialization.out.ErrorInfo; @@ -66,15 +68,13 @@ public String serializeError(ErrorInfo errorInfo) { "\t", errorInfo.getErrorMessage().getMessageType().toString(), SerializationService.escapeSpecialCharacters(errorInfo.getErrorMessage().getMessage()), - (errorInfo.getRegionClass() != null - ? errorInfo.getRegionClass().flatName().toString() - : "null"), - (errorInfo.getRegionMember() != null ? errorInfo.getRegionMember().toString() : "null"), + Serializer.serializeSymbol(errorInfo.getRegionClass(), this), + Serializer.serializeSymbol(errorInfo.getRegionMember(), this), String.valueOf(errorInfo.getOffset()), errorInfo.getPath() != null ? errorInfo.getPath().toString() : "null", (errorInfo.getNonnullTarget() != null ? SymbolLocation.createLocationFromSymbol(errorInfo.getNonnullTarget()) - .tabSeparatedToString() + .tabSeparatedToString(this) : EMPTY_NONNULL_TARGET_LOCATION_STRING)); } @@ -82,4 +82,9 @@ public String serializeError(ErrorInfo errorInfo) { public int getSerializationVersion() { return 2; } + + @Override + public String serializeMethodSignature(Symbol.MethodSymbol methodSymbol) { + return methodSymbol.toString(); + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/FieldLocation.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/FieldLocation.java index 533b921492..19ca23dae1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/FieldLocation.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/FieldLocation.java @@ -23,6 +23,8 @@ package com.uber.nullaway.fixserialization.location; import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.fixserialization.Serializer; +import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; import javax.lang.model.element.ElementKind; /** subtype of {@link AbstractSymbolLocation} targeting class fields. */ @@ -37,13 +39,13 @@ public FieldLocation(Symbol target) { } @Override - public String tabSeparatedToString() { + public String tabSeparatedToString(SerializationAdapter adapter) { return String.join( "\t", type.toString(), - enclosingClass.flatName(), + Serializer.serializeSymbol(enclosingClass, adapter), "null", - variableSymbol.toString(), + Serializer.serializeSymbol(variableSymbol, adapter), "null", path != null ? path.toString() : "null"); } diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodLocation.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodLocation.java index 1b23fc2d58..f3c3022842 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodLocation.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodLocation.java @@ -23,6 +23,8 @@ package com.uber.nullaway.fixserialization.location; import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.fixserialization.Serializer; +import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; import javax.lang.model.element.ElementKind; /** subtype of {@link AbstractSymbolLocation} targeting methods. */ @@ -37,12 +39,12 @@ public MethodLocation(Symbol target) { } @Override - public String tabSeparatedToString() { + public String tabSeparatedToString(SerializationAdapter adapter) { return String.join( "\t", type.toString(), - enclosingClass.flatName(), - enclosingMethod.toString(), + Serializer.serializeSymbol(enclosingClass, adapter), + Serializer.serializeSymbol(enclosingMethod, adapter), "null", "null", path != null ? path.toString() : "null"); diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodParameterLocation.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodParameterLocation.java index 95dea975ff..ff4f0a542e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodParameterLocation.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodParameterLocation.java @@ -24,6 +24,8 @@ import com.google.common.base.Preconditions; import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.fixserialization.Serializer; +import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; import javax.lang.model.element.ElementKind; /** subtype of {@link AbstractSymbolLocation} targeting a method parameter. */ @@ -58,13 +60,13 @@ public MethodParameterLocation(Symbol target) { } @Override - public String tabSeparatedToString() { + public String tabSeparatedToString(SerializationAdapter adapter) { return String.join( "\t", type.toString(), - enclosingClass.flatName(), - enclosingMethod.toString(), - paramSymbol.toString(), + Serializer.serializeSymbol(enclosingClass, adapter), + Serializer.serializeSymbol(enclosingMethod, adapter), + Serializer.serializeSymbol(paramSymbol, adapter), String.valueOf(index), path != null ? path.toString() : "null"); } diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/SymbolLocation.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/SymbolLocation.java index 5824866b2e..262df711e1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/SymbolLocation.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/SymbolLocation.java @@ -23,6 +23,7 @@ package com.uber.nullaway.fixserialization.location; import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; /** Provides method for symbol locations. */ public interface SymbolLocation { @@ -32,9 +33,10 @@ public interface SymbolLocation { * of the element, symbol of the containing class, symbol of the enclosing method, symbol of the * variable, index of the element and uri to containing file. * + * @param adapter adapter used to serialize symbols. * @return string representation of contents in a line seperated by tabs. */ - String tabSeparatedToString(); + String tabSeparatedToString(SerializationAdapter adapter); /** * Creates header of an output file containing all {@link SymbolLocation} written in string which diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/FieldInitializationInfo.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/FieldInitializationInfo.java index 369c244938..dac0c6d117 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/FieldInitializationInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/FieldInitializationInfo.java @@ -23,6 +23,8 @@ package com.uber.nullaway.fixserialization.out; import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.fixserialization.Serializer; +import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; import com.uber.nullaway.fixserialization.location.SymbolLocation; /** @@ -44,12 +46,13 @@ public FieldInitializationInfo(Symbol.MethodSymbol initializerMethod, Symbol fie /** * Returns string representation of content of an object. * + * @param adapter adapter used to serialize symbols. * @return string representation of contents of an object in a line seperated by tabs. */ - public String tabSeparatedToString() { - return initializerMethodLocation.tabSeparatedToString() + public String tabSeparatedToString(SerializationAdapter adapter) { + return initializerMethodLocation.tabSeparatedToString(adapter) + '\t' - + field.getSimpleName().toString(); + + Serializer.serializeSymbol(field, adapter); } /** diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java index 284433980e..fb271cd644 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java @@ -24,6 +24,8 @@ import com.sun.source.util.TreePath; import com.uber.nullaway.ErrorMessage; +import com.uber.nullaway.fixserialization.Serializer; +import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; import com.uber.nullaway.fixserialization.location.SymbolLocation; import java.util.Objects; @@ -67,18 +69,17 @@ public int hashCode() { /** * returns string representation of content of an object. * + * @param adapter adapter used to serialize symbols. * @return string representation of contents of an object in a line separated by tabs. */ - public String tabSeparatedToString() { + public String tabSeparatedToString(SerializationAdapter adapter) { return String.join( "\t", - symbolLocation.tabSeparatedToString(), + symbolLocation.tabSeparatedToString(adapter), errorMessage.getMessageType().toString(), "nullable", - (classAndMemberInfo.getClazz() == null ? "null" : classAndMemberInfo.getClazz().flatName()), - (classAndMemberInfo.getMember() == null - ? "null" - : classAndMemberInfo.getMember().toString())); + Serializer.serializeSymbol(classAndMemberInfo.getClazz(), adapter), + Serializer.serializeSymbol(classAndMemberInfo.getMember(), adapter)); } /** Finds the class and member of program point where triggered this type change. */ diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 64a5c59b99..bea967a8c5 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -499,11 +499,11 @@ public void genericFunctionReturnTypeNewClassTree() { "class Test {", " static class A { }", " static A testPositive1() {", - " // BUG: Diagnostic contains: Cannot return", + " // BUG: Diagnostic contains: mismatched nullability of type parameters", " return new A<@Nullable String>();", " }", " static A<@Nullable String> testPositive2() {", - " // BUG: Diagnostic contains: Cannot return", + " // BUG: Diagnostic contains: mismatched nullability of type parameters", " return new A();", " }", " static A<@Nullable String> testNegative() {", @@ -523,7 +523,7 @@ public void genericFunctionReturnTypeNormalTree() { "class Test {", " static class A { }", " static A testPositive(A<@Nullable String> a) {", - " // BUG: Diagnostic contains: Cannot return", + " // BUG: Diagnostic contains: mismatched nullability of type parameters", " return a;", " }", " static A<@Nullable String> testNegative(A<@Nullable String> a) {", @@ -544,7 +544,7 @@ public void genericFunctionReturnTypeMultipleReturnStatementsIfElseBlock() { " static class A { }", " static A testPositive(A<@Nullable String> a, int num) {", " if (num % 2 == 0) {", - " // BUG: Diagnostic contains: Cannot return", + " // BUG: Diagnostic contains: mismatched nullability of type parameters", " return a;", " } else {", " return new A();",