From 457a129f41941435118ff1be09c4ce50d5ed356b Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Mon, 20 Feb 2023 12:35:58 -0800 Subject: [PATCH 1/3] Refactoring in symbol serialization (#736) This PR refactors the serialization logic to call a `static` method in `Serializer` to convert a `Symbol` to its corresponding `String` representation. This PR is in preparation for the update on serialization service in #735. --- .../nullaway/fixserialization/Serializer.java | 23 +++++++++++++++++++ .../adapters/SerializationV1Adapter.java | 7 +++--- .../adapters/SerializationV2Adapter.java | 7 +++--- .../location/FieldLocation.java | 5 ++-- .../location/MethodLocation.java | 5 ++-- .../location/MethodParameterLocation.java | 7 +++--- .../out/FieldInitializationInfo.java | 3 ++- .../out/SuggestedNullableFixInfo.java | 7 +++--- 8 files changed, 44 insertions(+), 20 deletions(-) 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..87154d0a39 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; @@ -195,4 +196,26 @@ public static Path pathToSourceFileFromURI(@Nullable URI uri) { return path; } } + + /** + * Serializes the given {@link Symbol} to a string. + * + * @param symbol The symbol to serialize. + * @return The serialized symbol. + */ + public static String serializeSymbol(@Nullable Symbol symbol) { + if (symbol == null) { + return "null"; + } + switch (symbol.getKind()) { + case FIELD: + case PARAMETER: + return symbol.name.toString(); + case METHOD: + case CONSTRUCTOR: + return symbol.toString(); + default: + return symbol.flatName().toString(); + } + } } 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..d622cec71e 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 @@ -25,6 +25,7 @@ import static com.uber.nullaway.fixserialization.out.ErrorInfo.EMPTY_NONNULL_TARGET_LOCATION_STRING; 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,10 +54,8 @@ 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()), + Serializer.serializeSymbol(errorInfo.getRegionMember()), (errorInfo.getNonnullTarget() != null ? SymbolLocation.createLocationFromSymbol(errorInfo.getNonnullTarget()) .tabSeparatedToString() 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..eda75d6d03 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 @@ -25,6 +25,7 @@ import static com.uber.nullaway.fixserialization.out.ErrorInfo.EMPTY_NONNULL_TARGET_LOCATION_STRING; 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,10 +67,8 @@ 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()), + Serializer.serializeSymbol(errorInfo.getRegionMember()), String.valueOf(errorInfo.getOffset()), errorInfo.getPath() != null ? errorInfo.getPath().toString() : "null", (errorInfo.getNonnullTarget() != null 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..71c8989f99 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,7 @@ package com.uber.nullaway.fixserialization.location; import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.fixserialization.Serializer; import javax.lang.model.element.ElementKind; /** subtype of {@link AbstractSymbolLocation} targeting class fields. */ @@ -41,9 +42,9 @@ public String tabSeparatedToString() { return String.join( "\t", type.toString(), - enclosingClass.flatName(), + Serializer.serializeSymbol(enclosingClass), "null", - variableSymbol.toString(), + Serializer.serializeSymbol(variableSymbol), "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..c53075d904 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,7 @@ package com.uber.nullaway.fixserialization.location; import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.fixserialization.Serializer; import javax.lang.model.element.ElementKind; /** subtype of {@link AbstractSymbolLocation} targeting methods. */ @@ -41,8 +42,8 @@ public String tabSeparatedToString() { return String.join( "\t", type.toString(), - enclosingClass.flatName(), - enclosingMethod.toString(), + Serializer.serializeSymbol(enclosingClass), + Serializer.serializeSymbol(enclosingMethod), "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..622d3109b2 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,7 @@ import com.google.common.base.Preconditions; import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.fixserialization.Serializer; import javax.lang.model.element.ElementKind; /** subtype of {@link AbstractSymbolLocation} targeting a method parameter. */ @@ -62,9 +63,9 @@ public String tabSeparatedToString() { return String.join( "\t", type.toString(), - enclosingClass.flatName(), - enclosingMethod.toString(), - paramSymbol.toString(), + Serializer.serializeSymbol(enclosingClass), + Serializer.serializeSymbol(enclosingMethod), + Serializer.serializeSymbol(paramSymbol), String.valueOf(index), path != null ? path.toString() : "null"); } 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..b5caca2697 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,7 @@ package com.uber.nullaway.fixserialization.out; import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.fixserialization.Serializer; import com.uber.nullaway.fixserialization.location.SymbolLocation; /** @@ -49,7 +50,7 @@ public FieldInitializationInfo(Symbol.MethodSymbol initializerMethod, Symbol fie public String tabSeparatedToString() { return initializerMethodLocation.tabSeparatedToString() + '\t' - + field.getSimpleName().toString(); + + Serializer.serializeSymbol(field); } /** 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..68377a818d 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,7 @@ import com.sun.source.util.TreePath; import com.uber.nullaway.ErrorMessage; +import com.uber.nullaway.fixserialization.Serializer; import com.uber.nullaway.fixserialization.location.SymbolLocation; import java.util.Objects; @@ -75,10 +76,8 @@ public String tabSeparatedToString() { symbolLocation.tabSeparatedToString(), errorMessage.getMessageType().toString(), "nullable", - (classAndMemberInfo.getClazz() == null ? "null" : classAndMemberInfo.getClazz().flatName()), - (classAndMemberInfo.getMember() == null - ? "null" - : classAndMemberInfo.getMember().toString())); + Serializer.serializeSymbol(classAndMemberInfo.getClazz()), + Serializer.serializeSymbol(classAndMemberInfo.getMember())); } /** Finds the class and member of program point where triggered this type change. */ From 1548c69a27e9e3dd1cb185d04b2e870f3b11a771 Mon Sep 17 00:00:00 2001 From: NikitaAware Date: Wed, 22 Feb 2023 10:50:26 -0800 Subject: [PATCH 2/3] Add JSpecify checking for return statements (#734) For JSpecify, this pull request adds a check that the type parameter nullability of the return type of the method matches the type of any returned expression. For example: ``` static A<@Nullable String> test() { return new A(); } ``` For the method return type, the type parameter is `@Nullable String`, but the type of the type parameter of the returned expression is `@NonNull String`. NullAway will now report an error here. --- .../java/com/uber/nullaway/ErrorMessage.java | 1 + .../com/uber/nullaway/GenericsChecks.java | 108 ++++++++++++++---- .../main/java/com/uber/nullaway/NullAway.java | 2 + .../NullAwayJSpecifyGenericsTests.java | 71 +++++++++++- 4 files changed, 159 insertions(+), 23 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java index bb1395a6bf..f451a921a8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java @@ -54,6 +54,7 @@ public enum MessageTypes { WRONG_OVERRIDE_PRECONDITION, TYPE_PARAMETER_CANNOT_BE_NULLABLE, ASSIGN_GENERIC_NULLABLE, + RETURN_NULLABLE_GENERIC, } public String getMessage() { diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 4fdfa4b9d1..0cb338df44 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -10,11 +10,13 @@ import com.sun.source.tree.AnnotatedTypeTree; import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.NewClassTree; import com.sun.source.tree.ParameterizedTypeTree; import com.sun.source.tree.Tree; import com.sun.source.tree.VariableTree; import com.sun.tools.javac.code.Attribute; +import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.TypeMetadata; import com.sun.tools.javac.code.Types; @@ -23,6 +25,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import javax.annotation.Nullable; /** Methods for performing checks related to generic types and nullability. */ public final class GenericsChecks { @@ -130,6 +133,51 @@ private static void reportInvalidAssignmentInstantiationError( errorMessage, analysis.buildDescription(tree), state, null)); } + private static void reportInvalidReturnTypeError( + Tree tree, Type methodType, Type returnType, VisitorState state, NullAway analysis) { + ErrorBuilder errorBuilder = analysis.getErrorBuilder(); + ErrorMessage errorMessage = + new ErrorMessage( + ErrorMessage.MessageTypes.RETURN_NULLABLE_GENERIC, + String.format( + "Cannot return expression of type " + + returnType + + " from method with return type " + + methodType + + " due to mismatched nullability of type parameters")); + state.reportMatch( + errorBuilder.createErrorDescription( + errorMessage, analysis.buildDescription(tree), state, null)); + } + + /** + * 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. + */ + @Nullable + private Type getTreeType(Tree tree) { + if (tree instanceof NewClassTree + && ((NewClassTree) tree).getIdentifier() instanceof ParameterizedTypeTree) { + ParameterizedTypeTree paramTypedTree = + (ParameterizedTypeTree) ((NewClassTree) tree).getIdentifier(); + if (paramTypedTree.getTypeArguments().isEmpty()) { + // diamond operator, which we do not yet support; for now, return null + // TODO: support diamond operators + return null; + } + return typeWithPreservedAnnotations(paramTypedTree); + } else { + return ASTHelpers.getType(tree); + } + } + /** * For a tree representing an assignment, ensures that from the perspective of type parameter * nullability, the type of the right-hand side is assignable to (a subtype of) the type of the @@ -159,23 +207,39 @@ public void checkTypeParameterNullnessForAssignability(Tree tree) { if (rhsTree == null || rhsTree.getKind().equals(Tree.Kind.NULL_LITERAL)) { return; } - Type lhsType = ASTHelpers.getType(lhsTree); - Type rhsType = ASTHelpers.getType(rhsTree); - // For NewClassTrees with annotated type parameters, javac does not preserve the annotations in - // its computed type for the expression. As a workaround, we construct a replacement Type - // object with the appropriate annotations. - if (rhsTree instanceof NewClassTree - && ((NewClassTree) rhsTree).getIdentifier() instanceof ParameterizedTypeTree) { - ParameterizedTypeTree paramTypedTree = - (ParameterizedTypeTree) ((NewClassTree) rhsTree).getIdentifier(); - if (paramTypedTree.getTypeArguments().isEmpty()) { - // no explicit type parameters - return; + Type lhsType = getTreeType(lhsTree); + Type rhsType = getTreeType(rhsTree); + + if (lhsType instanceof Type.ClassType && rhsType instanceof Type.ClassType) { + boolean isAssignmentValid = + compareNullabilityAnnotations((Type.ClassType) lhsType, (Type.ClassType) rhsType); + if (!isAssignmentValid) { + reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis); } - rhsType = typeWithPreservedAnnotations(paramTypedTree); } - if (lhsType instanceof Type.ClassType && rhsType instanceof Type.ClassType) { - compareNullabilityAnnotations((Type.ClassType) lhsType, (Type.ClassType) rhsType, tree); + } + + public void checkTypeParameterNullnessForFunctionReturnType( + ExpressionTree retExpr, Symbol.MethodSymbol methodSymbol) { + if (!config.isJSpecifyMode()) { + return; + } + + Type formalReturnType = methodSymbol.getReturnType(); + // check nullability of parameters only for generics + if (formalReturnType.getTypeArguments().isEmpty()) { + return; + } + Type returnExpressionType = getTreeType(retExpr); + if (formalReturnType instanceof Type.ClassType + && returnExpressionType instanceof Type.ClassType) { + boolean isReturnTypeValid = + compareNullabilityAnnotations( + (Type.ClassType) formalReturnType, (Type.ClassType) returnExpressionType); + if (!isReturnTypeValid) { + reportInvalidReturnTypeError( + retExpr, formalReturnType, returnExpressionType, state, analysis); + } } } @@ -189,10 +253,8 @@ public void checkTypeParameterNullnessForAssignability(Tree tree) { * * @param lhsType type for the lhs of the assignment * @param rhsType type for the rhs of the assignment - * @param tree tree representing the assignment */ - private void compareNullabilityAnnotations( - Type.ClassType lhsType, Type.ClassType rhsType, Tree tree) { + private boolean compareNullabilityAnnotations(Type.ClassType lhsType, Type.ClassType rhsType) { Types types = state.getTypes(); // The base type of rhsType may be a subtype of lhsType's base type. In such cases, we must // compare lhsType against the supertype of rhsType with a matching base type. @@ -232,15 +294,17 @@ private void compareNullabilityAnnotations( } } if (isLHSNullableAnnotated != isRHSNullableAnnotated) { - reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis); - return; + return false; } // nested generics if (lhsTypeArgument.getTypeArguments().length() > 0) { - compareNullabilityAnnotations( - (Type.ClassType) lhsTypeArgument, (Type.ClassType) rhsTypeArgument, tree); + if (!compareNullabilityAnnotations( + (Type.ClassType) lhsTypeArgument, (Type.ClassType) rhsTypeArgument)) { + return false; + } } } + return true; } /** diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 7fe7fdbdb1..6e6fd570ba 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -834,6 +834,8 @@ private Description checkReturnExpression( state, methodSymbol); } + new GenericsChecks(state, config, this) + .checkTypeParameterNullnessForFunctionReturnType(retExpr, methodSymbol); return Description.NO_MATCH; } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 18f2e69b84..d78304c18f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -155,7 +155,8 @@ public void returnTypeParamInstantiation() { " static class NullableTypeParam {}", " // BUG: Diagnostic contains: Generic type parameter", " static NonNullTypeParam<@Nullable String> testBadNonNull() {", - " return new NonNullTypeParam();", + " // BUG: Diagnostic contains: Generic type parameter", + " return new NonNullTypeParam<@Nullable String>();", " }", " static NullableTypeParam<@Nullable String> testOKNull() {", " return new NullableTypeParam<@Nullable String>();", @@ -488,6 +489,74 @@ public void testForDiamondInAnAssignment() { .doTest(); } + @Test + public void genericFunctionReturnTypeNewClassTree() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static class A { }", + " static A testPositive1() {", + " // BUG: Diagnostic contains: mismatched nullability of type parameters", + " return new A<@Nullable String>();", + " }", + " static A<@Nullable String> testPositive2() {", + " // BUG: Diagnostic contains: mismatched nullability of type parameters", + " return new A();", + " }", + " static A<@Nullable String> testNegative() {", + " return new A<@Nullable String>();", + " }", + "}") + .doTest(); + } + + @Test + public void genericFunctionReturnTypeNormalTree() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static class A { }", + " static A testPositive(A<@Nullable String> a) {", + " // BUG: Diagnostic contains: mismatched nullability of type parameters", + " return a;", + " }", + " static A<@Nullable String> testNegative(A<@Nullable String> a) {", + " return a;", + " }", + "}") + .doTest(); + } + + @Test + public void genericFunctionReturnTypeMultipleReturnStatementsIfElseBlock() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static class A { }", + " static A testPositive(A<@Nullable String> a, int num) {", + " if (num % 2 == 0) {", + " // BUG: Diagnostic contains: mismatched nullability of type parameters", + " return a;", + " } else {", + " return new A();", + " }", + " }", + " static A testNegative(A a, int num) {", + " return a;", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From ff6b0907601156d42ae88c77ddc6c4331c8e5744 Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Wed, 22 Feb 2023 11:01:18 -0800 Subject: [PATCH 3/3] Refactoring tabSeparatedToString logic to prepare for serialization version 3. (#738) This PR is merely refactoring which prepares serialization logic for version `v3` implemented in #735. Changes in this PR - Uses adapter to serialize method signatures. - Adds `SerializationAdapter` to `tabSeparatedToString` methods for symbol serialization. - Adds `SerializationAdapter` to `serializeSymbol` static method for symbol serialization. --- .../uber/nullaway/fixserialization/Serializer.java | 11 +++++++---- .../adapters/SerializationAdapter.java | 9 +++++++++ .../adapters/SerializationV1Adapter.java | 12 +++++++++--- .../adapters/SerializationV2Adapter.java | 12 +++++++++--- .../fixserialization/location/FieldLocation.java | 7 ++++--- .../fixserialization/location/MethodLocation.java | 7 ++++--- .../location/MethodParameterLocation.java | 9 +++++---- .../fixserialization/location/SymbolLocation.java | 4 +++- .../out/FieldInitializationInfo.java | 8 +++++--- .../out/SuggestedNullableFixInfo.java | 10 ++++++---- 10 files changed, 61 insertions(+), 28 deletions(-) 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 87154d0a39..db6d8c7250 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java @@ -77,7 +77,9 @@ public void serializeSuggestedFixInfo( if (enclosing) { suggestedNullableFixInfo.initEnclosing(); } - appendToFile(suggestedNullableFixInfo.tabSeparatedToString(), suggestedFixesOutputPath); + appendToFile( + suggestedNullableFixInfo.tabSeparatedToString(serializationAdapter), + suggestedFixesOutputPath); } /** @@ -91,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. */ @@ -201,9 +203,10 @@ public static Path pathToSourceFileFromURI(@Nullable URI uri) { * 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) { + public static String serializeSymbol(@Nullable Symbol symbol, SerializationAdapter adapter) { if (symbol == null) { return "null"; } @@ -213,7 +216,7 @@ public static String serializeSymbol(@Nullable Symbol symbol) { return symbol.name.toString(); case METHOD: case CONSTRUCTOR: - return symbol.toString(); + 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 d622cec71e..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,6 +24,7 @@ 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; @@ -54,11 +55,11 @@ public String serializeError(ErrorInfo errorInfo) { "\t", errorInfo.getErrorMessage().getMessageType().toString(), SerializationService.escapeSpecialCharacters(errorInfo.getErrorMessage().getMessage()), - Serializer.serializeSymbol(errorInfo.getRegionClass()), - Serializer.serializeSymbol(errorInfo.getRegionMember()), + 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)); } @@ -66,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 eda75d6d03..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,6 +24,7 @@ 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; @@ -67,13 +68,13 @@ public String serializeError(ErrorInfo errorInfo) { "\t", errorInfo.getErrorMessage().getMessageType().toString(), SerializationService.escapeSpecialCharacters(errorInfo.getErrorMessage().getMessage()), - Serializer.serializeSymbol(errorInfo.getRegionClass()), - Serializer.serializeSymbol(errorInfo.getRegionMember()), + 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)); } @@ -81,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 71c8989f99..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 @@ -24,6 +24,7 @@ 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. */ @@ -38,13 +39,13 @@ public FieldLocation(Symbol target) { } @Override - public String tabSeparatedToString() { + public String tabSeparatedToString(SerializationAdapter adapter) { return String.join( "\t", type.toString(), - Serializer.serializeSymbol(enclosingClass), + Serializer.serializeSymbol(enclosingClass, adapter), "null", - Serializer.serializeSymbol(variableSymbol), + 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 c53075d904..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 @@ -24,6 +24,7 @@ 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. */ @@ -38,12 +39,12 @@ public MethodLocation(Symbol target) { } @Override - public String tabSeparatedToString() { + public String tabSeparatedToString(SerializationAdapter adapter) { return String.join( "\t", type.toString(), - Serializer.serializeSymbol(enclosingClass), - Serializer.serializeSymbol(enclosingMethod), + 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 622d3109b2..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 @@ -25,6 +25,7 @@ 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. */ @@ -59,13 +60,13 @@ public MethodParameterLocation(Symbol target) { } @Override - public String tabSeparatedToString() { + public String tabSeparatedToString(SerializationAdapter adapter) { return String.join( "\t", type.toString(), - Serializer.serializeSymbol(enclosingClass), - Serializer.serializeSymbol(enclosingMethod), - Serializer.serializeSymbol(paramSymbol), + 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 b5caca2697..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 @@ -24,6 +24,7 @@ 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; /** @@ -45,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' - + Serializer.serializeSymbol(field); + + 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 68377a818d..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 @@ -25,6 +25,7 @@ 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; @@ -68,16 +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", - Serializer.serializeSymbol(classAndMemberInfo.getClazz()), - Serializer.serializeSymbol(classAndMemberInfo.getMember())); + Serializer.serializeSymbol(classAndMemberInfo.getClazz(), adapter), + Serializer.serializeSymbol(classAndMemberInfo.getMember(), adapter)); } /** Finds the class and member of program point where triggered this type change. */