Skip to content

Commit

Permalink
Merge branch 'master' into generics-ternary-operator-assignment-changes
Browse files Browse the repository at this point in the history
  • Loading branch information
msridhar committed Feb 22, 2023
2 parents 6287e96 + ff6b090 commit 4820854
Show file tree
Hide file tree
Showing 12 changed files with 106 additions and 43 deletions.
20 changes: 13 additions & 7 deletions nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>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.
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -76,7 +77,9 @@ public void serializeSuggestedFixInfo(
if (enclosing) {
suggestedNullableFixInfo.initEnclosing();
}
appendToFile(suggestedNullableFixInfo.tabSeparatedToString(), suggestedFixesOutputPath);
appendToFile(
suggestedNullableFixInfo.tabSeparatedToString(serializationAdapter),
suggestedFixesOutputPath);
}

/**
Expand All @@ -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. */
Expand Down Expand Up @@ -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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

package com.uber.nullaway.fixserialization.adapters;

import com.sun.tools.javac.code.Symbol;
import com.uber.nullaway.fixserialization.out.ErrorInfo;

/**
Expand Down Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -53,18 +55,21 @@ 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));
}

@Override
public int getSerializationVersion() {
return 1;
}

@Override
public String serializeMethodSignature(Symbol.MethodSymbol methodSymbol) {
return methodSymbol.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -66,20 +68,23 @@ 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));
}

@Override
public int getSerializationVersion() {
return 2;
}

@Override
public String serializeMethodSignature(Symbol.MethodSymbol methodSymbol) {
return methodSymbol.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,11 +499,11 @@ public void genericFunctionReturnTypeNewClassTree() {
"class Test {",
" static class A<T extends @Nullable Object> { }",
" static A<String> 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<String>();",
" }",
" static A<@Nullable String> testNegative() {",
Expand All @@ -523,7 +523,7 @@ public void genericFunctionReturnTypeNormalTree() {
"class Test {",
" static class A<T extends @Nullable Object> { }",
" static A<String> 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) {",
Expand All @@ -544,7 +544,7 @@ public void genericFunctionReturnTypeMultipleReturnStatementsIfElseBlock() {
" static class A<T extends @Nullable Object> { }",
" static A<String> 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<String>();",
Expand Down

0 comments on commit 4820854

Please sign in to comment.