diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java b/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java index af269cb648..e9bfcbe63f 100755 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java @@ -45,6 +45,7 @@ import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.ClassTree; +import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.ModifiersTree; @@ -52,12 +53,16 @@ import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.JCTree.JCCompilationUnit; import com.sun.tools.javac.util.DiagnosticSource; import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition; import com.uber.nullaway.fixserialization.SerializationService; +import com.uber.nullaway.fixserialization.scanners.OriginScanner; +import com.uber.nullaway.fixserialization.scanners.OriginTrace; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.StreamSupport; @@ -97,10 +102,49 @@ public Description createErrorDescription( ErrorMessage errorMessage, Description.Builder descriptionBuilder, VisitorState state, - @Nullable Symbol nonNullTarget) { + NullAway.MayBeNullableInquiry inquiry, + @Nullable Symbol nonNullTarget, + @Nullable ExpressionTree nullableExpression) { Tree enclosingSuppressTree = suppressibleNode(state.getPath()); - return createErrorDescription( - errorMessage, enclosingSuppressTree, descriptionBuilder, state, nonNullTarget); + return createErrorDescriptionWithInfo( + errorMessage, + enclosingSuppressTree, + descriptionBuilder, + state, + inquiry, + nonNullTarget, + nullableExpression, + Map.of()); + } + + /** + * create an error description for a nullability warning + * + * @param errorMessage the error message object. + * @param descriptionBuilder the description builder for the error. + * @param state the visitor state (used for e.g. suppression finding). + * @param nonNullTarget if non-null, this error involved a pseudo-assignment of a @Nullable + * expression into a @NonNull target, and this parameter is the Symbol for that target. + * @return the error description + */ + public Description createErrorDescriptionWithInfo( + ErrorMessage errorMessage, + Description.Builder descriptionBuilder, + VisitorState state, + NullAway.MayBeNullableInquiry inquiry, + @Nullable Symbol nonNullTarget, + @Nullable ExpressionTree nullableExpression, + Map args) { + Tree enclosingSuppressTree = suppressibleNode(state.getPath()); + return createErrorDescriptionWithInfo( + errorMessage, + enclosingSuppressTree, + descriptionBuilder, + state, + inquiry, + nonNullTarget, + nullableExpression, + args); } /** @@ -119,7 +163,29 @@ public Description createErrorDescription( @Nullable Tree suggestTree, Description.Builder descriptionBuilder, VisitorState state, - @Nullable Symbol nonNullTarget) { + NullAway.MayBeNullableInquiry inquiry, + @Nullable Symbol nonNullTarget, + @Nullable ExpressionTree nullableExpression) { + return createErrorDescriptionWithInfo( + errorMessage, + suggestTree, + descriptionBuilder, + state, + inquiry, + nonNullTarget, + nullableExpression, + Map.of()); + } + + public Description createErrorDescriptionWithInfo( + ErrorMessage errorMessage, + @Nullable Tree suggestTree, + Description.Builder descriptionBuilder, + VisitorState state, + NullAway.MayBeNullableInquiry inquiry, + @Nullable Symbol nonNullTarget, + @Nullable ExpressionTree nullableExpression, + Map args) { Description.Builder builder = descriptionBuilder.setMessage(errorMessage.message); String checkName = CORE_CHECK_NAME; if (errorMessage.messageType.equals(GET_ON_EMPTY_OPTIONAL)) { @@ -140,7 +206,25 @@ public Description createErrorDescription( builder = addSuggestedSuppression(errorMessage, suggestTree, builder, state); } + Set origins = Set.of(); if (config.serializationIsActive()) { + if (nullableExpression != null) { + Symbol nullableExpressionSymbol = ASTHelpers.getSymbol(nullableExpression); + if (nullableExpressionSymbol != null + && nullableExpressionSymbol.getKind() == ElementKind.LOCAL_VARIABLE) { + // locate assignments to this local variable. Use the nullable expression's start + // position as the bound: the enclosing leaf can start earlier (e.g. an outer + // MethodInvocationTree or enhanced-for), which would cause OriginScanner to drop + // assignments that sit between the leaf start and the actual use. + int diagPos = ((JCTree) nullableExpression).getStartPosition(); + if (diagPos < 0) { + diagPos = ((JCTree) state.getPath().getLeaf()).getStartPosition(); + } + origins = + new OriginScanner(inquiry, state, diagPos) + .retrieveOrigins(state.findEnclosing(MethodTree.class), nullableExpressionSymbol); + } + } // For the case of initializer errors, the leaf of state.getPath() may not be the field / // method on which the error is being reported (since we do a class-wide analysis to find such // errors). In such cases, the suggestTree is the appropriate field / method tree, so use @@ -152,7 +236,7 @@ public Description createErrorDescription( ? suggestTree : state.getPath().getLeaf(); SerializationService.serializeReportingError( - config, state, errorTree, nonNullTarget, errorMessage); + config, state, errorTree, nonNullTarget, errorMessage, origins, args); } // #letbuildersbuild @@ -240,25 +324,57 @@ private Description.Builder addSuggestedSuppression( * expression into a @NonNull target, and this parameter is the Symbol for that target. * @return the error description. */ - Description createErrorDescriptionForNullAssignment( + Description createErrorDescriptionForNullAssignmentWithInfo( ErrorMessage errorMessage, @Nullable Tree suggestTreeIfCastToNonNull, Description.Builder descriptionBuilder, VisitorState state, - @Nullable Symbol nonNullTarget) { + NullAway.MayBeNullableInquiry inquiry, + @Nullable Symbol nonNullTarget, + @Nullable ExpressionTree nullableExpression, + Map args) { if (config.getCastToNonNullMethod() != null) { - return createErrorDescription( - errorMessage, suggestTreeIfCastToNonNull, descriptionBuilder, state, nonNullTarget); + return createErrorDescriptionWithInfo( + errorMessage, + suggestTreeIfCastToNonNull, + descriptionBuilder, + state, + inquiry, + nonNullTarget, + nullableExpression, + args); } else { - return createErrorDescription( + return createErrorDescriptionWithInfo( errorMessage, suppressibleNode(state.getPath()), descriptionBuilder, state, - nonNullTarget); + inquiry, + nonNullTarget, + nullableExpression, + args); } } + Description createErrorDescriptionForNullAssignment( + ErrorMessage errorMessage, + @Nullable Tree suggestTreeIfCastToNonNull, + Description.Builder descriptionBuilder, + VisitorState state, + NullAway.MayBeNullableInquiry inquiry, + @Nullable Symbol nonNullTarget, + @Nullable ExpressionTree nullableExpression) { + return createErrorDescriptionForNullAssignmentWithInfo( + errorMessage, + suggestTreeIfCastToNonNull, + descriptionBuilder, + state, + inquiry, + nonNullTarget, + nullableExpression, + Map.of()); + } + Description.Builder addSuppressWarningsFix( Tree suggestTree, Description.Builder builder, String suppressionName) { SuppressWarnings extantSuppressWarnings = null; @@ -403,6 +519,7 @@ void reportInitializerError( Symbol.MethodSymbol methodSymbol, String message, VisitorState state, + NullAway.MayBeNullableInquiry inquiry, Description.Builder descriptionBuilder) { // Check needed here, despite check in hasPathSuppression because initialization // checking happens at the class-level (meaning state.getPath() might not include the @@ -415,7 +532,8 @@ void reportInitializerError( Tree methodTree = getTreesInstance(state).getTree(methodSymbol); ErrorMessage errorMessage = new ErrorMessage(METHOD_NO_INIT, message); state.reportMatch( - createErrorDescription(errorMessage, methodTree, descriptionBuilder, state, null)); + createErrorDescription( + errorMessage, methodTree, descriptionBuilder, state, inquiry, null, null)); } boolean symbolHasSuppressWarningsAnnotation(Symbol symbol, String suppression) { @@ -493,7 +611,11 @@ static String errMsgForInitializer(Set uninitFields, VisitorState state return message.toString(); } - void reportInitErrorOnField(Symbol symbol, VisitorState state, Description.Builder builder) { + void reportInitErrorOnField( + Symbol symbol, + VisitorState state, + NullAway.MayBeNullableInquiry inquiry, + Description.Builder builder) { // Check needed here, despite check in hasPathSuppression because initialization // checking happens at the class-level (meaning state.getPath() might not include the // field itself). @@ -518,7 +640,9 @@ void reportInitErrorOnField(Symbol symbol, VisitorState state, Description.Build tree, builder, state, - symbol)); + inquiry, + symbol, + null)); } else { state.reportMatch( createErrorDescription( @@ -526,7 +650,9 @@ void reportInitErrorOnField(Symbol symbol, VisitorState state, Description.Build tree, builder, state, - symbol)); + inquiry, + symbol, + null)); } } } diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 6773c68808..bfddd5022e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -96,6 +96,8 @@ import com.uber.nullaway.ErrorMessage.MessageTypes; import com.uber.nullaway.dataflow.AccessPathNullnessAnalysis; import com.uber.nullaway.dataflow.EnclosingEnvironmentNullness; +import com.uber.nullaway.fixserialization.Serializer; +import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; import com.uber.nullaway.generics.GenericsChecks; import com.uber.nullaway.generics.JSpecifyJavacConfig; import com.uber.nullaway.handlers.Handler; @@ -107,6 +109,7 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -197,6 +200,9 @@ public class NullAway extends BugChecker private static final ImmutableSet TYPE_USE_OR_TYPE_PARAMETER = ImmutableSet.of(TYPE_USE, TYPE_PARAMETER); + /** Default inquiry for whether an expression may be null. */ + public final MayBeNullableInquiry mayBeNullInquiry = (tree, state) -> mayBeNullExpr(state, tree); + /** * Possible levels of null-marking / annotatedness for a class. This may be set to FULLY_MARKED or * FULLY_UNMARKED optimistically but then adjusted to PARTIALLY_MARKED later based on annotations @@ -288,6 +294,8 @@ public GenericsChecks getGenericsChecks() { return genericsChecks; } + private final SerializationAdapter adapter; + /** * Error Prone requires us to have an empty constructor for each Plugin, in addition to the * constructor taking an ErrorProneFlags object. This constructor should not be used anywhere @@ -300,6 +308,7 @@ public NullAway() { errorBuilder = new ErrorBuilder(config, "", ImmutableSet.of()); // annoying to leak `this` here; we assign the field last to make it as safe as possible genericsChecks = new GenericsChecks(this, config, handler); + this.adapter = SerializationAdapter.getAdapterForVersion(SerializationAdapter.LATEST_VERSION); } @Inject // For future Error Prone versions in which checkers are loaded using Guice @@ -316,6 +325,7 @@ public NullAway(ErrorProneFlags flags) { errorBuilder = new ErrorBuilder(config, canonicalName(), allSuppressionNames); // annoying to leak `this` here; we assign the field last to make it as safe as possible genericsChecks = new GenericsChecks(this, config, handler); + this.adapter = SerializationAdapter.getAdapterForVersion(SerializationAdapter.LATEST_VERSION); } public boolean isMethodUnannotated(@Nullable MethodInvocationNode invocationNode) { @@ -532,7 +542,12 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { ErrorMessage errorMessage = new ErrorMessage(MessageTypes.ASSIGN_NULLABLE_TO_NONNULL_ARRAY, message); return errorBuilder.createErrorDescription( - errorMessage, buildDescription(tree), state, arraySymbol); + errorMessage, + buildDescription(tree), + state, + mayBeNullInquiry, + arraySymbol, + expression); } } } @@ -555,7 +570,9 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { expression, buildDescription(tree), state, - ASTHelpers.getSymbol(tree.getVariable())); + mayBeNullInquiry, + ASTHelpers.getSymbol(tree.getVariable()), + expression); } handler.onNonNullFieldAssignment(assigned, getNullnessAnalysis(state), state); return Description.NO_MATCH; @@ -743,7 +760,9 @@ private Description checkSwitchSelectorExpression( switchSelectorExpression, buildDescription(switchSelectorExpression), state, - null); + mayBeNullInquiry, + null, + switchSelectorExpression); } return Description.NO_MATCH; @@ -890,6 +909,8 @@ private Description checkParamOverriding( new ErrorMessage(MessageTypes.WRONG_OVERRIDE_PARAM, message), buildDescription(memberReferenceTree), state, + mayBeNullInquiry, + null, null); } @@ -957,7 +978,9 @@ private Description checkParamOverriding( new ErrorMessage(MessageTypes.WRONG_OVERRIDE_PARAM, message), buildDescription(errorTree), paramState, - paramSymbol); + mayBeNullInquiry, + paramSymbol, + null); } } return Description.NO_MATCH; @@ -1139,7 +1162,9 @@ private Description checkReturnExpression( retExpr, buildDescription(errorTree), state, - methodSymbol); + mayBeNullInquiry, + methodSymbol, + retExpr); } return Description.NO_MATCH; } @@ -1255,7 +1280,9 @@ && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL) new ErrorMessage(MessageTypes.WRONG_OVERRIDE_RETURN, message), buildDescription(errorTree), state, - overriddenMethod); + mayBeNullInquiry, + overriddenMethod, + null); } // if any parameter in the super method is annotated @Nullable, // overriding method cannot assume @Nonnull @@ -1435,7 +1462,8 @@ private Description checkPossibleUninitFieldRead( new ErrorMessage( MessageTypes.NONNULL_FIELD_READ_BEFORE_INIT, "read of @NonNull field " + symbol + " before initialization"); - return errorBuilder.createErrorDescription(errorMessage, buildDescription(tree), state, null); + return errorBuilder.createErrorDescription( + errorMessage, buildDescription(tree), state, mayBeNullInquiry, null, null); } else { return Description.NO_MATCH; } @@ -1681,7 +1709,13 @@ public Description matchVariable(VariableTree tree, VisitorState state) { MessageTypes.ASSIGN_FIELD_NULLABLE, "assigning @Nullable expression to @NonNull field"); return errorBuilder.createErrorDescriptionForNullAssignment( - errorMessage, initializer, buildDescription(tree), state, symbol); + errorMessage, + initializer, + buildDescription(tree), + state, + mayBeNullInquiry, + symbol, + initializer); } } } @@ -1762,7 +1796,8 @@ private void checkNullableAnnotationPositionInType( "Type-use nullability annotations should be applied on inner class"); state.reportMatch( - errorBuilder.createErrorDescription(errorMessage, buildDescription(type), state, null)); + errorBuilder.createErrorDescription( + errorMessage, buildDescription(type), state, mayBeNullInquiry, null, null)); } } } @@ -1944,12 +1979,24 @@ public Description matchEnhancedForLoop(EnhancedForLoopTree tree, VisitorState s return Description.NO_MATCH; } ExpressionTree expr = tree.getExpression(); - ErrorMessage errorMessage = - new ErrorMessage( - MessageTypes.DEREFERENCE_NULLABLE, - "enhanced-for expression " + state.getSourceForNode(expr) + " is @Nullable"); + Symbol derefedSymbol = ASTHelpers.getSymbol(expr); + Map args = new LinkedHashMap<>(); + if (derefedSymbol != null) { + args.put("expression", state.getSourceForNode(expr)); + args.put("kind", derefedSymbol.getKind().toString().toLowerCase(Locale.ROOT)); + args.put("class", Serializer.serializeSymbol(derefedSymbol.enclClass(), adapter)); + args.put( + "isAnnotated", + Boolean.toString( + !codeAnnotationInfo.isSymbolUnannotated(derefedSymbol, config, handler))); + args.put("symbol", Serializer.serializeSymbol(derefedSymbol, adapter)); + args.put("position", Integer.toString(((JCTree) expr).pos().getStartPosition())); + } + String message = "enhanced-for expression " + state.getSourceForNode(expr) + " is @Nullable"; + ErrorMessage errorMessage = new ErrorMessage(MessageTypes.DEREFERENCE_NULLABLE, message); if (mayBeNullExpr(state, expr)) { - return errorBuilder.createErrorDescription(errorMessage, buildDescription(expr), state, null); + return errorBuilder.createErrorDescriptionWithInfo( + errorMessage, buildDescription(expr), state, mayBeNullInquiry, null, expr, args); } // auto-unboxing check in JSpecify mode if (!config.isJSpecifyMode()) { @@ -1971,7 +2018,12 @@ public Description matchEnhancedForLoop(EnhancedForLoopTree tree, VisitorState s new ErrorMessage(MessageTypes.UNBOX_NULLABLE, "unboxing of a @Nullable value"); state.reportMatch( errorBuilder.createErrorDescription( - errorMessageUnbox, buildDescription(loopVariable), state, null)); + errorMessageUnbox, + buildDescription(loopVariable), + state, + mayBeNullInquiry, + null, + null)); } } @@ -1997,7 +2049,7 @@ public Description matchSynchronized(SynchronizedTree tree, VisitorState state) + state.getSourceForNode(lockExpr) + "\" is @Nullable"); return errorBuilder.createErrorDescription( - errorMessage, buildDescription(lockExpr), state, null); + errorMessage, buildDescription(lockExpr), state, mayBeNullInquiry, null, lockExpr); } return Description.NO_MATCH; } @@ -2019,10 +2071,12 @@ private void doUnboxingCheck(VisitorState state, ExpressionTree... expressions) if (!type.isPrimitive()) { if (mayBeNullExpr(state, tree)) { ErrorMessage errorMessage = - new ErrorMessage(MessageTypes.UNBOX_NULLABLE, "unboxing of a @Nullable value"); + new ErrorMessage( + MessageTypes.UNBOX_NULLABLE, + "unboxing of a @Nullable value - " + state.getSourceForNode(tree)); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, tree, buildDescription(tree), state, null)); + errorMessage, tree, buildDescription(tree), state, mayBeNullInquiry, null, tree)); } } } @@ -2141,7 +2195,9 @@ private Description handleInvocation( actual, buildDescription(actual), state, - formalParams.get(argPos))); + mayBeNullInquiry, + formalParams.get(argPos), + actual)); } }); // Check for @NonNull being passed to castToNonNull (if configured) @@ -2198,7 +2254,9 @@ private Description checkCastToNonNullTakesNullable( actual, buildDescription(tree), state, - null); + mayBeNullInquiry, + null, + actual); } } return Description.NO_MATCH; @@ -2254,7 +2312,10 @@ private void checkFieldInitialization(ClassTree tree, VisitorState state) { if (!(symbolHasExternalInitAnnotation(classSymbol) && entities.instanceInitializerMethods().isEmpty())) { errorBuilder.reportInitErrorOnField( - uninitField, state, buildDescription(getTreesInstance(state).getTree(uninitField))); + uninitField, + state, + mayBeNullInquiry, + buildDescription(getTreesInstance(state).getTree(uninitField))); } } else { // report it on each constructor that does not initialize it @@ -2271,6 +2332,7 @@ private void checkFieldInitialization(ClassTree tree, VisitorState state) { (Symbol.MethodSymbol) constructorElement, errMsgForInitializer(errorFieldsForInitializer.get(constructorElement), state), state, + mayBeNullInquiry, buildDescription(getTreesInstance(state).getTree(constructorElement))); } // For static fields @@ -2280,7 +2342,10 @@ private void checkFieldInitialization(ClassTree tree, VisitorState state) { // initialization block // anyways). errorBuilder.reportInitErrorOnField( - uninitSField, state, buildDescription(getTreesInstance(state).getTree(uninitSField))); + uninitSField, + state, + mayBeNullInquiry, + buildDescription(getTreesInstance(state).getTree(uninitSField))); } } @@ -2843,12 +2908,32 @@ private Description matchDereference( } } if (mayBeNullExpr(state, baseExpression)) { + ExpressionTree stripped = NullabilityUtil.stripParensAndCasts(baseExpression); + Symbol derefedSymbol = ASTHelpers.getSymbol(stripped); + Map args = new LinkedHashMap<>(); + if (derefedSymbol != null) { + args.put("expression", state.getSourceForNode(baseExpression)); + args.put("kind", derefedSymbol.getKind().toString().toLowerCase(Locale.ROOT)); + args.put("class", Serializer.serializeSymbol(derefedSymbol.enclClass(), adapter)); + args.put( + "isAnnotated", + Boolean.toString( + !codeAnnotationInfo.isSymbolUnannotated(derefedSymbol, config, handler))); + args.put("symbol", Serializer.serializeSymbol(derefedSymbol, adapter)); + args.put("position", Integer.toString(((JCTree) baseExpression).pos().getStartPosition())); + } String message = "dereferenced expression " + state.getSourceForNode(baseExpression) + " is @Nullable"; ErrorMessage errorMessage = new ErrorMessage(MessageTypes.DEREFERENCE_NULLABLE, message); - - return errorBuilder.createErrorDescriptionForNullAssignment( - errorMessage, baseExpression, buildDescription(derefExpression), state, null); + return errorBuilder.createErrorDescriptionForNullAssignmentWithInfo( + errorMessage, + baseExpression, + buildDescription(derefExpression), + state, + mayBeNullInquiry, + null, + baseExpression, + args); } Optional handlerErrorMessage = @@ -2859,7 +2944,9 @@ private Description matchDereference( derefExpression, buildDescription(derefExpression), state, - null); + mayBeNullInquiry, + null, + baseExpression); } return Description.NO_MATCH; @@ -2897,6 +2984,10 @@ public Nullness getComputedNullness(ExpressionTree e) { return computedNullnessMap.getOrDefault(e, Nullness.NULLABLE); } + public interface MayBeNullableInquiry { + boolean maybeNullable(ExpressionTree expr, VisitorState state); + } + /** * Add computed nullness information to an expression. * diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/SerializationService.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/SerializationService.java index 50d42126ae..2f549eaba4 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/SerializationService.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/SerializationService.java @@ -30,7 +30,9 @@ import com.uber.nullaway.Config; import com.uber.nullaway.ErrorMessage; import com.uber.nullaway.fixserialization.out.ErrorInfo; +import com.uber.nullaway.fixserialization.scanners.OriginTrace; import java.util.Map; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.jspecify.annotations.Nullable; @@ -78,16 +80,21 @@ public static String escapeSpecialCharacters(String str) { * @param state Visitor state. * @param errorTree Tree of the element involved in the reporting error. * @param errorMessage Error caused by the target. + * @param origins Symbol of the elements contributing to the nullability of the expression. + * @param args Arguments to be passed to the error message to automatically generate a fix. */ public static void serializeReportingError( Config config, VisitorState state, Tree errorTree, @Nullable Symbol target, - ErrorMessage errorMessage) { + ErrorMessage errorMessage, + Set origins, + Map args) { Serializer serializer = config.getSerializationConfig().getSerializer(); Preconditions.checkNotNull( serializer, "Serializer shouldn't be null at this point, error in configuration setting!"); - serializer.serializeErrorInfo(new ErrorInfo(state.getPath(), errorTree, errorMessage, target)); + serializer.serializeErrorInfo( + new ErrorInfo(state.getPath(), errorTree, errorMessage, target, origins, args)); } } 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 8ccd70f417..64d9a4a22e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java @@ -30,12 +30,16 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.io.StringWriter; import java.io.Writer; import java.net.URI; import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import javax.xml.stream.XMLOutputFactory; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamWriter; import org.jspecify.annotations.Nullable; /** @@ -43,9 +47,14 @@ * of this class. */ public class Serializer { + + private static final XMLOutputFactory XML_OUTPUT_FACTORY = XMLOutputFactory.newInstance(); + /** Path to write errors. */ private final Path errorOutputPath; + private final Path errorOutputXmlPath; + /** Path to write suggested fix metadata. */ private final Path fieldInitializationOutputPath; @@ -59,6 +68,7 @@ public class Serializer { public Serializer(FixSerializationConfig config, SerializationAdapter serializationAdapter) { String outputDirectory = config.outputDirectory; this.errorOutputPath = Paths.get(outputDirectory, "errors.tsv"); + this.errorOutputXmlPath = Paths.get(outputDirectory, "errors.xml"); this.fieldInitializationOutputPath = Paths.get(outputDirectory, "field_init.tsv"); this.serializationAdapter = serializationAdapter; serializeVersion(outputDirectory); @@ -72,7 +82,35 @@ public Serializer(FixSerializationConfig config, SerializationAdapter serializat */ public void serializeErrorInfo(ErrorInfo errorInfo) { errorInfo.initEnclosing(); - appendToFile(serializationAdapter.serializeError(errorInfo), errorOutputPath); + if (isXmlMode()) { + appendToFile(buildErrorXml(errorInfo), errorOutputXmlPath); + } else { + appendToFile(serializationAdapter.serializeError(errorInfo), errorOutputPath); + } + } + + private String buildErrorXml(ErrorInfo errorInfo) { + StringWriter sw = new StringWriter(); + try { + XMLStreamWriter writer = XML_OUTPUT_FACTORY.createXMLStreamWriter(sw); + errorInfo.writeXml(writer, serializationAdapter); + writer.flush(); + writer.close(); + } catch (XMLStreamException e) { + throw new RuntimeException("Failed to serialize error to XML", e); + } + return sw.toString(); + } + + /** + * Whether the active serialization adapter emits errors as XML rather than TSV. V4 switched the + * error log to XML to carry the structured Annotator auto-fix metadata; earlier versions remain + * TSV. Each {@code } record is appended as a standalone XML fragment (there is no + * post-analysis hook to emit a closing root element), so downstream consumers should treat the + * file as a fragment stream rather than a single well-formed XML document. + */ + private boolean isXmlMode() { + return serializationAdapter.getSerializationVersion() >= 4; } public void serializeFieldInitializationInfo(FieldInitializationInfo info) { @@ -127,7 +165,11 @@ private void initializeOutputFiles(FixSerializationConfig config) { if (config.fieldInitInfoEnabled) { initializeFile(fieldInitializationOutputPath, FieldInitializationInfo.header()); } - initializeFile(errorOutputPath, serializationAdapter.getErrorsOutputFileHeader()); + if (isXmlMode()) { + initializeFile(errorOutputXmlPath, ""); + } else { + initializeFile(errorOutputPath, serializationAdapter.getErrorsOutputFileHeader()); + } } catch (IOException e) { throw new RuntimeException("Could not finish resetting serializer", e); } @@ -195,4 +237,12 @@ public static String serializeSymbol(@Nullable Symbol symbol, SerializationAdapt default -> symbol.flatName().toString(); }; } + + /** Writes a leaf element {@code value} to {@code writer}. */ + public static void writeTextElement(XMLStreamWriter writer, String name, String value) + throws XMLStreamException { + writer.writeStartElement(name); + writer.writeCharacters(value); + writer.writeEndElement(); + } } 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 a458557a0c..584d916118 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 @@ -37,7 +37,7 @@ public interface SerializationAdapter { * Latest version number. If version is not defined by the user, NullAway will use the * corresponding adapter to this version in its serialization. */ - int LATEST_VERSION = 3; + int LATEST_VERSION = 4; /** * Returns header of "errors.tsv" which contains all serialized {@link ErrorInfo} reported by @@ -85,6 +85,7 @@ static SerializationAdapter getAdapterForVersion(int version) { throw new RuntimeException( "Serialization version v2 is skipped and was used for an alpha version of the auto-annotator tool. Please use version 3 instead."); case 3 -> new SerializationV3Adapter(); + case 4 -> new SerializationV4Adapter(); default -> throw new RuntimeException( "Unrecognized NullAway serialization version: " diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV4Adapter.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV4Adapter.java new file mode 100644 index 0000000000..fbef509f1d --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV4Adapter.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2025 Uber Technologies, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.nullaway.fixserialization.adapters; + +/** + * Adapter for serialization version 4. + * + *

Switches the error log from TSV ({@code errors.tsv}) to XML ({@code errors.xml}) to carry the + * structured Annotator auto-fix metadata. The TSV-related methods inherited from {@link + * SerializationV3Adapter} are not used at this version: {@code Serializer} routes V4+ writes + * through {@code ErrorInfo#appendXml} instead. + */ +public class SerializationV4Adapter extends SerializationV3Adapter { + + @Override + public int getSerializationVersion() { + return 4; + } +} diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/AbstractSymbolLocation.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/AbstractSymbolLocation.java index e6fa0f822e..11f6e56313 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/AbstractSymbolLocation.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/AbstractSymbolLocation.java @@ -28,9 +28,12 @@ import com.google.errorprone.util.ASTHelpers; import com.sun.tools.javac.code.Symbol; import com.uber.nullaway.fixserialization.Serializer; +import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; import java.net.URI; import java.nio.file.Path; import javax.lang.model.element.ElementKind; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamWriter; import org.jspecify.annotations.Nullable; /** abstract base class for {@link SymbolLocation}. */ @@ -56,4 +59,16 @@ public AbstractSymbolLocation(ElementKind type, Symbol target) { URI pathInURI = enclosingClass.sourcefile != null ? enclosingClass.sourcefile.toUri() : null; this.path = Serializer.pathToSourceFileFromURI(pathInURI); } + + @Override + public void writeXmlFields(XMLStreamWriter writer, SerializationAdapter adapter) + throws XMLStreamException { + String[] infos = tabSeparatedToString(adapter).split("\t", -1); + Serializer.writeTextElement(writer, "target_kind", infos[0]); + Serializer.writeTextElement(writer, "target_class", infos[1]); + Serializer.writeTextElement(writer, "target_method", infos[2]); + Serializer.writeTextElement(writer, "target_param", infos[3]); + Serializer.writeTextElement(writer, "target_index", infos[4]); + Serializer.writeTextElement(writer, "target_path", infos[5]); + } } 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 e6a0f71ced..fff469afe2 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 @@ -24,6 +24,8 @@ import com.sun.tools.javac.code.Symbol; import com.uber.nullaway.fixserialization.adapters.SerializationAdapter; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamWriter; /** Provides method for symbol locations. */ public interface SymbolLocation { @@ -38,6 +40,16 @@ public interface SymbolLocation { */ String tabSeparatedToString(SerializationAdapter adapter); + /** + * Writes an XML representation of this location to {@code writer}. Emits a sequence of child + * elements (no wrapping tag); the caller is responsible for the enclosing element. + * + * @param writer the writer to emit to. + * @param adapter adapter used to serialize symbols. + */ + void writeXmlFields(XMLStreamWriter writer, SerializationAdapter adapter) + throws XMLStreamException; + /** * Creates header of an output file containing all {@link SymbolLocation} written in string which * values are separated tabs. diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java index 5dda09d43c..bb28917bd9 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java @@ -24,14 +24,26 @@ import static com.uber.nullaway.ErrorMessage.MessageTypes.FIELD_NO_INIT; import static com.uber.nullaway.ErrorMessage.MessageTypes.METHOD_NO_INIT; +import static com.uber.nullaway.NullabilityUtil.castToNonNull; +import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.Tree; import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.util.JCDiagnostic; 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 com.uber.nullaway.fixserialization.scanners.OriginTrace; +import java.net.URI; import java.nio.file.Path; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamWriter; import org.jspecify.annotations.Nullable; /** Stores information regarding an error which will be reported by NullAway. */ @@ -59,8 +71,18 @@ public class ErrorInfo { /** Path to the containing source file where this error is reported. */ private final @Nullable Path path; + /** Extra argument regarding the error required to generate a fix automatically. */ + private final Map infos; + + private final Set origins; + public ErrorInfo( - TreePath path, Tree errorTree, ErrorMessage errorMessage, @Nullable Symbol nonnullTarget) { + TreePath path, + Tree errorTree, + ErrorMessage errorMessage, + @Nullable Symbol nonnullTarget, + Set origins, + Map args) { this.classAndMemberInfo = (errorMessage.getMessageType().equals(FIELD_NO_INIT) || errorMessage.getMessageType().equals(METHOD_NO_INIT)) @@ -72,6 +94,8 @@ public ErrorInfo( this.offset = treePosition.getStartPosition(); this.path = Serializer.pathToSourceFileFromURI(path.getCompilationUnit().getSourceFile().toUri()); + this.origins = origins; + this.infos = args; } /** @@ -130,8 +154,84 @@ public int getOffset() { return path; } + /** + * Returns extra information regarding the error required to generate a fix automatically. + * + * @return Map from info keys to their values. + */ + public Map getInfos() { + return infos; + } + /** Finds the class and member of program point where the error is reported. */ public void initEnclosing() { classAndMemberInfo.findValues(); } + + /** + * Writes an XML representation of the error information to {@code writer}. + * + * @param writer the writer to emit to. + * @param adapter adapter used to serialize symbols. + */ + public void writeXml(XMLStreamWriter writer, SerializationAdapter adapter) + throws XMLStreamException { + writer.writeStartElement("error"); + Serializer.writeTextElement(writer, "message_type", errorMessage.getMessageType().toString()); + Serializer.writeTextElement(writer, "message", errorMessage.getMessage()); + Serializer.writeTextElement( + writer, "enc_class", Serializer.serializeSymbol(getRegionClass(), adapter)); + Serializer.writeTextElement( + writer, "enc_member", Serializer.serializeSymbol(getRegionMember(), adapter)); + Serializer.writeTextElement(writer, "offset", Integer.toString(offset)); + Serializer.writeTextElement(writer, "path", path != null ? path.toString() : "null"); + if (nonnullTarget != null) { + writer.writeStartElement("nonnull_target"); + SymbolLocation.createLocationFromSymbol(nonnullTarget).writeXmlFields(writer, adapter); + writer.writeEndElement(); + } + if (!origins.isEmpty()) { + writer.writeStartElement("origins"); + for (OriginTrace trace : origins) { + Symbol sym = trace.origin(); + writer.writeStartElement("origin"); + writer.writeStartElement("location"); + SymbolLocation.createLocationFromSymbol(sym).writeXmlFields(writer, adapter); + writer.writeEndElement(); + Serializer.writeTextElement( + writer, "kind", sym.getKind().toString().toLowerCase(Locale.ROOT)); + Serializer.writeTextElement( + writer, "class", Serializer.serializeSymbol(sym.enclClass(), adapter)); + Serializer.writeTextElement(writer, "isAnnotated", Boolean.toString(isAnnotated(sym))); + Serializer.writeTextElement(writer, "expression", trace.trace().toString()); + Serializer.writeTextElement( + writer, + "position", + Integer.toString(((JCTree) trace.trace()).pos().getStartPosition())); + Serializer.writeTextElement(writer, "symbol", Serializer.serializeSymbol(sym, adapter)); + writer.writeEndElement(); + } + writer.writeEndElement(); + } + writer.writeStartElement("infos"); + for (Map.Entry entry : infos.entrySet()) { + Serializer.writeTextElement(writer, entry.getKey(), entry.getValue()); + } + writer.writeEndElement(); + writer.writeEndElement(); + } + + /** + * Checks if the symbol is from annotated code. + * + * @param symbol The symbol to check. + * @return True if the symbol is from annotated code, false otherwise. + */ + private static boolean isAnnotated(Symbol symbol) { + // TODO for now we follow a very simple heuristic to determine if a symbol is annotated and + // check if the path to the symbol exists + Symbol.ClassSymbol enclosingClass = castToNonNull(ASTHelpers.enclosingClass(symbol)); + URI pathInURI = enclosingClass.sourcefile != null ? enclosingClass.sourcefile.toUri() : null; + return Serializer.pathToSourceFileFromURI(pathInURI) != null; + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/scanners/ExpressionToSymbolScanner.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/scanners/ExpressionToSymbolScanner.java new file mode 100644 index 0000000000..4ec22a1117 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/scanners/ExpressionToSymbolScanner.java @@ -0,0 +1,141 @@ +/* + * MIT License + * + * Copyright (c) 2025 Nima Karimipour + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.nullaway.fixserialization.scanners; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ConditionalExpressionTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.LiteralTree; +import com.sun.source.tree.MemberReferenceTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.util.TreeScanner; +import com.sun.tools.javac.code.Symbol; +import com.uber.nullaway.NullAway; +import java.util.HashSet; +import java.util.Set; +import org.jspecify.annotations.Nullable; + +/** Scanner that finds the symbols of all identifiers in expressions. */ +public class ExpressionToSymbolScanner + extends TreeScanner, NullAway.MayBeNullableInquiry> { + + private final VisitorState state; + + public ExpressionToSymbolScanner(VisitorState state) { + this.state = state; + } + + @Override + public Set reduce(Set r1, Set r2) { + if (r2 == null && r1 == null) { + return Set.of(); + } + Set combined = new HashSet<>(); + if (r1 != null) { + combined.addAll(r1); + } + if (r2 != null) { + combined.addAll(r2); + } + return combined; + } + + private @Nullable Symbol defaultResult(ExpressionTree node) { + Symbol symbol = ASTHelpers.getSymbol(node); + if (symbol != null) { + return switch (symbol.getKind()) { + case FIELD, PARAMETER, LOCAL_VARIABLE, METHOD, RESOURCE_VARIABLE -> symbol; + default -> null; + }; + } + return null; + } + + @Override + public Set visitLiteral(LiteralTree node, NullAway.MayBeNullableInquiry inquiry) { + return Set.of(); + } + + @Override + public Set visitIdentifier(IdentifierTree node, NullAway.MayBeNullableInquiry inquiry) { + boolean isNullable = inquiry.maybeNullable(node, state); + if (!isNullable) { + return Set.of(); + } + Symbol symbol = defaultResult(node); + return symbol == null ? Set.of() : Set.of(symbol); + } + + @Override + public Set visitMemberReference( + MemberReferenceTree node, NullAway.MayBeNullableInquiry inquiry) { + boolean isNullable = inquiry.maybeNullable(node, state); + if (!isNullable) { + return Set.of(); + } + Symbol symbol = defaultResult(node); + return symbol == null ? Set.of() : Set.of(symbol); + } + + @Override + public Set visitMemberSelect( + MemberSelectTree node, NullAway.MayBeNullableInquiry inquiry) { + boolean isNullable = inquiry.maybeNullable(node, state); + if (!isNullable) { + return Set.of(); + } + Symbol symbol = defaultResult(node); + return symbol == null ? Set.of() : Set.of(symbol); + } + + @Override + public Set visitMethodInvocation( + MethodInvocationTree node, NullAway.MayBeNullableInquiry inquiry) { + if (!inquiry.maybeNullable(node, state)) { + return Set.of(); + } + Symbol symbol = defaultResult(node); + return symbol == null ? Set.of() : Set.of(symbol); + } + + @Override + public Set visitConditionalExpression( + ConditionalExpressionTree node, NullAway.MayBeNullableInquiry inquiry) { + if (!inquiry.maybeNullable(node, state)) { + return Set.of(); + } + Set symbols = new HashSet<>(); + if (inquiry.maybeNullable(node.getTrueExpression(), state)) { + symbols.addAll(node.getTrueExpression().accept(this, inquiry)); + } + if (inquiry.maybeNullable(node.getFalseExpression(), state)) { + symbols.addAll(node.getFalseExpression().accept(this, inquiry)); + } + return symbols; + } +} diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/scanners/OriginScanner.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/scanners/OriginScanner.java new file mode 100644 index 0000000000..ebc1c832de --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/scanners/OriginScanner.java @@ -0,0 +1,206 @@ +/* + * MIT License + * + * Copyright (c) 2025 Nima Karimipour + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.nullaway.fixserialization.scanners; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.EnhancedForLoopTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.TreeScanner; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.tree.JCTree; +import com.uber.nullaway.NullAway; +import java.util.ArrayDeque; +import java.util.HashSet; +import java.util.Queue; +import java.util.Set; +import java.util.stream.Collectors; +import javax.lang.model.element.ElementKind; +import org.jspecify.annotations.Nullable; + +/** + * Scanner that finds the origin of a variable. An origin is a variable that is either a field, a + * parameter or a return result of a method invocation that contributes to the value of the + * variable. + */ +public class OriginScanner extends TreeScanner, Symbol> { + + private final NullAway.MayBeNullableInquiry inquiry; + private final VisitorState state; + + /** + * Source offset of the diagnostic expression. Assignments, variable declarations, and for-each + * loops that start at or after this offset cannot reach the error site and are skipped. Use + * {@link #NO_BOUND} to disable the filter. + */ + private final int diagnosticStartPosition; + + public static final int NO_BOUND = Integer.MAX_VALUE; + + public OriginScanner(NullAway.MayBeNullableInquiry inquiry, VisitorState state) { + this(inquiry, state, NO_BOUND); + } + + public OriginScanner( + NullAway.MayBeNullableInquiry inquiry, VisitorState state, int diagnosticStartPosition) { + this.inquiry = inquiry; + this.state = state; + this.diagnosticStartPosition = diagnosticStartPosition; + } + + private boolean startsAfterDiagnostic(Tree node) { + return ((JCTree) node).getStartPosition() >= diagnosticStartPosition; + } + + @Override + public Set reduce(Set r1, Set r2) { + if (r2 == null && r1 == null) { + return Set.of(); + } + Set combined = new HashSet<>(); + if (r1 != null) { + combined.addAll(r1); + } + if (r2 != null) { + combined.addAll(r2); + } + return combined; + } + + @Override + public Set visitAssignment(AssignmentTree node, Symbol target) { + Tree variable = node.getVariable(); + Symbol symbol = ASTHelpers.getSymbol(variable); + if (symbol != null && symbol.equals(target)) { + if (startsAfterDiagnostic(node)) { + return Set.of(); + } + ExpressionTree expr = node.getExpression(); + if (!inquiry.maybeNullable(expr, state)) { + return Set.of(); + } + return expr.accept(new ExpressionToSymbolScanner(state), inquiry).stream() + .map(input -> new OriginTrace(input, node)) + .collect(Collectors.toSet()); + } + return super.visitAssignment(node, target); + } + + @Override + public Set visitVariable(VariableTree node, Symbol target) { + Symbol symbol = ASTHelpers.getSymbol(node); + if (symbol != null && symbol.equals(target)) { + if (startsAfterDiagnostic(node)) { + return Set.of(); + } + if (node.getInitializer() == null) { + return Set.of(); + } + ExpressionTree initializer = node.getInitializer(); + if (initializer == null || !inquiry.maybeNullable(initializer, state)) { + return Set.of(); + } + return initializer.accept(new ExpressionToSymbolScanner(state), inquiry).stream() + .map(input -> new OriginTrace(input, node)) + .collect(Collectors.toSet()); + } + return super.visitVariable(node, target); + } + + @Override + public Set visitEnhancedForLoop(EnhancedForLoopTree node, Symbol target) { + Symbol variable = ASTHelpers.getSymbol(node.getVariable()); + if (variable != null && variable.equals(target)) { + if (startsAfterDiagnostic(node)) { + return Set.of(); + } + ExpressionTree expr = node.getExpression(); + if (!inquiry.maybeNullable(expr, state)) { + return Set.of(); + } + return expr.accept(new ExpressionToSymbolScanner(state), inquiry).stream() + .map(input -> new OriginTrace(input, node)) + .collect(Collectors.toSet()); + } + return super.visitEnhancedForLoop(node, target); + } + + /** + * Retrieve the origins of a variable in a method. + * + * @param tree the enclosing method. + * @param target the variable to find the origins of. + * @return a set of symbols that are the origins of the variable. + */ + public Set retrieveOrigins(@Nullable MethodTree tree, Symbol target) { + if (tree == null) { + return Set.of(); + } + if (isOriginal(target)) { + return Set.of(); + } + Set result = new HashSet<>(); + Queue queue = new ArrayDeque<>(); + queue.add(target); + Set visited = new HashSet<>(); + while (!queue.isEmpty()) { + Symbol current = queue.poll(); + if (visited.contains(current)) { + continue; + } + visited.add(current); + Set involvedSymbols = tree.accept(this, current); + involvedSymbols.forEach( + trace -> { + Symbol origin = trace.origin(); + if (isOriginal(origin)) { + result.add(trace); + } else { + if (!visited.contains(origin)) { + queue.add(origin); + } + } + }); + } + return result; + } + + /** + * Check if the symbol is an original symbol. An original symbol is a field, a parameter or a + * method return call. + * + * @param symbol the symbol to check + * @return true if the symbol is an original symbol, false otherwise + */ + private static boolean isOriginal(Symbol symbol) { + // An exception parameter is not an original symbol, but we are not interested in their origins. + return !symbol.getKind().equals(ElementKind.LOCAL_VARIABLE) + && !symbol.getKind().equals(ElementKind.RESOURCE_VARIABLE); + } +} diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/scanners/OriginTrace.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/scanners/OriginTrace.java new file mode 100644 index 0000000000..fab3d10001 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/scanners/OriginTrace.java @@ -0,0 +1,36 @@ +/* + * MIT License + * + * Copyright (c) 2025 Nima Karimipour + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.nullaway.fixserialization.scanners; + +import com.sun.source.tree.Tree; +import com.sun.tools.javac.code.Symbol; + +/** + * A provenance symbol together with the AST tree where it contributes to a local variable's value. + * + * @param origin The origin symbol. + * @param trace The tree where the origin contributes to the value of the local variable. + */ +public record OriginTrace(Symbol origin, Tree trace) {} diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index c2ce7d050c..7fcd469663 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -287,7 +287,12 @@ private void reportInvalidTypeArgumentError( methodSymbol, typeVariable.tsym.toString())); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(tree), state, null)); + errorMessage, + analysis.buildDescription(tree), + state, + analysis.mayBeNullInquiry, + null, + null)); } private void reportInvalidInstantiationError( @@ -301,7 +306,12 @@ private void reportInvalidInstantiationError( baseTypeVariable.tsym.toString(), baseType.tsym.toString())); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(tree), state, null)); + errorMessage, + analysis.buildDescription(tree), + state, + analysis.mayBeNullInquiry, + null, + null)); } private void reportInvalidAssignmentInstantiationError( @@ -312,7 +322,12 @@ private void reportInvalidAssignmentInstantiationError( new ErrorMessage(ErrorMessage.MessageTypes.ASSIGN_GENERIC_NULLABLE, msg); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(tree), state, null)); + errorMessage, + analysis.buildDescription(tree), + state, + analysis.mayBeNullInquiry, + null, + null)); } private String errorMessageForIncompatibleTypesAtPseudoAssignment( @@ -348,7 +363,12 @@ private void reportInvalidReturnTypeError( errorMessageForIncompatibleTypesAtPseudoAssignment(methodType, returnType, state)); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(tree), state, null)); + errorMessage, + analysis.buildDescription(tree), + state, + analysis.mayBeNullInquiry, + null, + null)); } private void reportMismatchedTypeForTernaryOperator( @@ -365,7 +385,12 @@ private void reportMismatchedTypeForTernaryOperator( + ", which has mismatched nullability of type parameters")); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(tree), state, null)); + errorMessage, + analysis.buildDescription(tree), + state, + analysis.mayBeNullInquiry, + null, + null)); } private void reportInvalidParametersNullabilityError( @@ -381,7 +406,12 @@ private void reportInvalidParametersNullabilityError( formalParameterType, actualParameterType, state)); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(paramExpression), state, null)); + errorMessage, + analysis.buildDescription(paramExpression), + state, + analysis.mayBeNullInquiry, + null, + null)); } private void reportInvalidMethodReferenceReturnTypeError( @@ -400,7 +430,12 @@ private void reportInvalidMethodReferenceReturnTypeError( prettyTypeForError(functionalInterfaceReturnType, state))); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(memberReferenceTree), state, null)); + errorMessage, + analysis.buildDescription(memberReferenceTree), + state, + analysis.mayBeNullInquiry, + null, + null)); } private void reportInvalidMethodReferenceParameterTypeError( @@ -419,7 +454,12 @@ private void reportInvalidMethodReferenceParameterTypeError( prettyTypeForError(functionalInterfaceParameterType, state))); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(memberReferenceTree), state, null)); + errorMessage, + analysis.buildDescription(memberReferenceTree), + state, + analysis.mayBeNullInquiry, + null, + null)); } private void reportInvalidOverridingMethodReturnTypeError( @@ -438,7 +478,12 @@ private void reportInvalidOverridingMethodReturnTypeError( + ", which has mismatched type parameter nullability"); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(methodTree), state, null)); + errorMessage, + analysis.buildDescription(methodTree), + state, + analysis.mayBeNullInquiry, + null, + null)); } private void reportInvalidOverridingMethodParamTypeError( @@ -454,7 +499,12 @@ private void reportInvalidOverridingMethodParamTypeError( + ", which has mismatched type parameter nullability"); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(formalParameterTree), state, null)); + errorMessage, + analysis.buildDescription(formalParameterTree), + state, + analysis.mayBeNullInquiry, + null, + null)); } /** @@ -949,7 +999,12 @@ private MethodInferenceResult runInferenceForCall( state.getSourceForNode(invocationTree), e.getMessage())); state.reportMatch( errorBuilder.createErrorDescription( - errorMessage, analysis.buildDescription(invocationTree), state, null)); + errorMessage, + analysis.buildDescription(invocationTree), + state, + analysis.mayBeNullInquiry, + null, + null)); } InferenceFailure failureResult = new InferenceFailure(e.getMessage()); // don't cache result if we were called from dataflow, since the result may rely on dataflow diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java index 89c1dd1f6e..6c0c1de81f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java @@ -160,6 +160,8 @@ protected boolean validateAnnotationSyntax( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); return false; } else { @@ -180,6 +182,8 @@ protected boolean validateAnnotationSyntax( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); return false; } @@ -206,6 +210,8 @@ protected boolean validateAnnotationSyntax( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); return false; } else { @@ -230,6 +236,8 @@ protected boolean validateAnnotationSyntax( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); return false; } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java index 54b62538fd..b5381dde33 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java @@ -127,6 +127,8 @@ public void onMatchMethod(MethodTree tree, MethodAnalysisContext methodAnalysisC tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); checkMethodBody = false; } else if (!checkableValueConstraints.contains(valueConstraint)) { @@ -199,6 +201,8 @@ public void onMatchMethod(MethodTree tree, MethodAnalysisContext methodAnalysisC returnTree, analysis.buildDescription(returnTree), state, + analysis.mayBeNullInquiry, + null, null)); } return super.visitReturn(returnTree, null); diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java index 08c5fb4e2b..2ffa6374b9 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java @@ -61,6 +61,8 @@ static String getConsequent( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); } return ""; @@ -111,6 +113,8 @@ static String[] getAntecedent( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); } return antecedent; diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullHandler.java index 425a6c6bd8..f5c8e134eb 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullHandler.java @@ -119,6 +119,8 @@ protected boolean validateAnnotationSemantics( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); return false; } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullIfHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullIfHandler.java index 4af97e6e64..fa34f5ab33 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullIfHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullIfHandler.java @@ -249,6 +249,8 @@ private void raiseError(Tree returnTree, VisitorState state, String message) { returnTree, analysis.buildDescription(returnTree), state, + analysis.mayBeNullInquiry, + null, null)); } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/FieldContractUtils.java b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/FieldContractUtils.java index ea25480aca..d408a21852 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/FieldContractUtils.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/FieldContractUtils.java @@ -90,6 +90,8 @@ public static void ensureStrictPostConditionInheritance( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/RequiresNonNullHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/RequiresNonNullHandler.java index 8b7896bae8..f3e2ad4b2b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/RequiresNonNullHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/RequiresNonNullHandler.java @@ -124,6 +124,8 @@ protected void validateOverridingRules( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); } @@ -173,6 +175,8 @@ public void onMatchMethodInvocation( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); } continue; @@ -197,6 +201,8 @@ public void onMatchMethodInvocation( tree, analysis.buildDescription(tree), state, + analysis.mayBeNullInquiry, + null, null)); } } diff --git a/nullaway/src/test/java/com/uber/nullaway/SerializationTest.java b/nullaway/src/test/java/com/uber/nullaway/SerializationTest.java index 389dfdf5b7..400b75ea5a 100644 --- a/nullaway/src/test/java/com/uber/nullaway/SerializationTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/SerializationTest.java @@ -65,6 +65,7 @@ public class SerializationTest extends NullAwayTestsBase { private static final String ERROR_FILE_NAME = "errors.tsv"; private static final String ERROR_FILE_HEADER = new SerializationV3Adapter().getErrorsOutputFileHeader(); + private static final String ERROR_XML_FILE_NAME = "errors.xml"; private static final String FIELD_INIT_FILE_NAME = "field_init.tsv"; private static final String FIELD_INIT_HEADER = FieldInitializationInfo.header(); @@ -133,6 +134,7 @@ public void suggestNullableReturnSimpleTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/SubClass.java", @@ -177,6 +179,7 @@ public void suggestNullableReturnSuperClassTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/android/Super.java", @@ -235,6 +238,7 @@ public void suggestNullableParamSimpleTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/android/Test.java", @@ -281,6 +285,7 @@ public void suggestNullableParamSubclassTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/android/Super.java", @@ -339,6 +344,7 @@ public void suggestNullableParamThisConstructorTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/test/Test.java", @@ -386,6 +392,7 @@ public void suggestNullableParamGenericsTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/Super.java", @@ -440,6 +447,7 @@ public void suggestNullableFieldSimpleTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/android/Super.java", @@ -484,6 +492,7 @@ public void suggestNullableFieldInitializationTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/android/Super.java", @@ -530,6 +539,7 @@ public void suggestNullableFieldControlFlowTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/android/Test.java", @@ -572,26 +582,14 @@ public Test(boolean b, boolean a) { "com.uber.Test", "Test(boolean)", 184, - "com/uber/android/Test.java", - "null", - "null", - "null", - "null", - "null", - "null"), + "com/uber/android/Test.java"), new ErrorDisplay( "METHOD_NO_INIT", "initializer method does not guarantee @NonNull fields g (line 5), i (line 5) are initialized along all control-flow paths", "com.uber.Test", "Test(boolean,boolean)", 425, - "com/uber/android/Test.java", - "null", - "null", - "null", - "null", - "null", - "null")) + "com/uber/android/Test.java")) .setFactory(errorDisplayFactory) .setOutputFileNameAndHeader(ERROR_FILE_NAME, ERROR_FILE_HEADER) .doTest(); @@ -607,6 +605,7 @@ public void suggestNullableNoInitializationFieldTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/android/Test.java", @@ -646,6 +645,7 @@ public void errorSerializationTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/Super.java", @@ -788,6 +788,7 @@ public void errorSerializationEscapeSpecialCharactersTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/Test.java", @@ -846,6 +847,7 @@ public void fieldInitializationSerializationTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/Test.java", @@ -896,6 +898,7 @@ public void errorSerializationTestAnonymousInnerClass() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/TestWithAnonymousRunnable.java", @@ -963,6 +966,7 @@ public void errorSerializationTestLocalTypes() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/TestWithLocalType.java", @@ -1011,6 +1015,7 @@ public void errorSerializationTestIdenticalLocalTypes() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/TestWithLocalTypes.java", @@ -1105,6 +1110,7 @@ public void errorSerializationTestLocalTypesNested() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/TestWithLocalType.java", @@ -1160,6 +1166,7 @@ public void errorSerializationTestLocalTypesInitializers() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/TestWithLocalTypes.java", @@ -1242,6 +1249,7 @@ public void errorSerializationTestInheritanceViolationForParameter() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/Foo.java", @@ -1296,6 +1304,7 @@ public void errorSerializationTestInheritanceViolationForMethod() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/Foo.java", @@ -1352,6 +1361,7 @@ public void errorSerializationTestAnonymousClassField() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines("com/uber/Foo.java", "package com.uber;", "public interface Foo { }") .addSourceLines( @@ -1397,6 +1407,7 @@ public void errorSerializationTestLocalClassField() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/Main.java", @@ -1450,6 +1461,7 @@ public void errorSerializationTestEnclosedByFieldDeclaration() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/Main.java", @@ -1565,6 +1577,7 @@ public void suggestNullableArgumentOnBytecode() { // Explicitly avoid excluding com.uber.nullaway.testdata.unannotated, // so we can suggest fixes there "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/UsesUnannotated.java", @@ -1629,6 +1642,7 @@ public void suggestNullableArgumentOnBytecodeNoFileInfo() { // Explicitly avoid excluding com.uber.nullaway.testdata.unannotated, // so we can suggest fixes there "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/UsesUnannotated.java", @@ -1672,6 +1686,7 @@ public void fieldRegionComputationWithMemberSelectTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/A.java", @@ -1768,6 +1783,7 @@ public void constructorSerializationTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/A.java", @@ -1822,6 +1838,7 @@ public void fieldRegionComputationWithMemberSelectForInnerClassTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( "com/uber/A.java", @@ -2053,48 +2070,6 @@ ERROR_FILE_NAME, new SerializationV1Adapter().getErrorsOutputFileHeader()) .doTest(); } - /** - * Helper method to verify the correct serialization version number is written in - * "serialization_version.txt". Version number can be configured via Error Prone flags by the - * user, and {@link com.uber.nullaway.fixserialization.Serializer} should write the exact number - * in "serialization_version.txt". - * - * @param version Version number to pass to NullAway via Error Prone flags and the expected number - * to be read from "serialization_version.txt". - */ - public void checkVersionSerialization(int version) { - SerializationTestHelper tester = new SerializationTestHelper<>(root); - SerializationAdapter adapter = SerializationAdapter.getAdapterForVersion(version); - tester - .setArgs( - Arrays.asList( - "-d", - temporaryFolder.getRoot().getAbsolutePath(), - "-XepOpt:NullAway:AnnotatedPackages=com.uber", - "-XepOpt:NullAway:SerializeFixMetadata=true", - "-XepOpt:NullAway:SerializeFixMetadataVersion=" + version, - "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) - // Just to run serialization features, the serialized fixes are not point of interest in - // this test. - .addSourceLines("com/uber/Test.java", "package com.uber;", "public class Test { }") - .expectNoOutput() - .setFactory(errorDisplayFactory) - .setOutputFileNameAndHeader(ERROR_FILE_NAME, adapter.getErrorsOutputFileHeader()) - .doTest(); - - Path serializationVersionPath = root.resolve("serialization_version.txt"); - try { - List lines = Files.readAllLines(serializationVersionPath); - // Check if it contains only one line. - assertEquals(lines.size(), 1); - // Check the serialized version. - assertEquals(Integer.parseInt(lines.get(0)), version); - } catch (IOException e) { - throw new RuntimeException( - "Could not read serialization version at path: " + serializationVersionPath, e); - } - } - @Test public void varArgsWithTypeUseAnnotationMethodSerializationTest() { SerializationTestHelper tester = new SerializationTestHelper<>(root); @@ -2105,6 +2080,7 @@ public void varArgsWithTypeUseAnnotationMethodSerializationTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( // toString() call on method symbol will serialize the annotation below which should not @@ -2174,6 +2150,7 @@ public void anonymousSubClassConstructorSerializationTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) .addSourceLines( // correct serialization of names for constructors invoked while creating anonymous @@ -2218,6 +2195,7 @@ public void errorSerializationTestArrayComponentNull() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath))) .addSourceLines( "com/uber/A.java", @@ -2262,6 +2240,7 @@ public void errorSerializationTestArrayComponentNullLocalVariable() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath))) .addSourceLines( "com/uber/A.java", @@ -2306,6 +2285,7 @@ public void errorSerializationTestArrayComponentNullLocalVariableLambda() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=3", "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath))) .addSourceLines( "com/uber/A.java", @@ -2423,4 +2403,193 @@ void exec(Runnable runnable) { .setOutputFileNameAndHeader(ERROR_FILE_NAME, ERROR_FILE_HEADER) .doTest(); } + + @Test + public void xmlOutputForDereferencedNullableField() { + Path tempRoot = Paths.get(temporaryFolder.getRoot().getAbsolutePath(), "v4_field_deref"); + String output = tempRoot.toString(); + try { + Files.createDirectories(tempRoot); + FixSerializationConfig.Builder builder = + new FixSerializationConfig.Builder().setFieldInitInfo(true).setOutputDirectory(output); + Path config = tempRoot.resolve("serializer.xml"); + Files.createFile(config); + configPath = config.toString(); + builder.writeAsXML(configPath); + } catch (IOException ex) { + throw new UncheckedIOException(ex); + } + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) + .addSourceLines( + "com/uber/Foo.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Foo {", + " @Nullable Object f;", + " public void init() {", + " this.f = new Object();", + " }", + " public String bar() {", + " // BUG: Diagnostic contains: dereferenced expression", + " return f.toString();", + " }", + "}") + .doTest(); + assertXmlContains( + tempRoot, + "DEREFERENCE_NULLABLE", + "dereferenced expression f is @Nullable", + "com.uber.Foo", + "bar()", + "246"); + } + + @Test + public void xmlOutputForLocalVariableOriginTrace() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) + .addSourceLines( + "com/uber/Foo.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Foo {", + " @Nullable Object f;", + " public void foo(Object p) { }", + " public void bar(boolean b, Object p) {", + " Object l = b ? f : p;", + " // BUG: Diagnostic contains: dereferenced expression l is @Nullable", + " l.toString();", + " }", + "}") + .doTest(); + assertXmlContains( + root, + "DEREFERENCE_NULLABLE", + "dereferenced expression l is @Nullable", + "com.uber.Foo", + "bar(boolean,java.lang.Object)", + "274", + "", + "f"); + } + + @Test + public void xmlOutputForLocalVariableOriginTraceInEnhancedFor() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) + .addSourceLines( + "com/uber/Bar.java", + "package com.uber;", + "import java.util.Map;", + "import javax.annotation.Nullable;", + "public class Bar {", + " @Nullable public Map baz() {", + " return null;", + " }", + "}") + .addSourceLines( + "com/uber/Foo.java", + "package com.uber;", + "import java.util.Map;", + "import java.util.List;", + "import javax.annotation.Nullable;", + "public class Foo {", + " List list = List.of();", + " public void bar() {", + " for(Bar b : list){", + " Map l = b.baz();", + " // BUG: Diagnostic contains: dereferenced expression l is @Nullable", + " for(String s : l.values()) {}", + " }", + " }", + "}") + .doTest(); + assertXmlContains( + root, + "DEREFERENCE_NULLABLE", + "dereferenced expression l is @Nullable", + "com.uber.Foo", + "bar()", + "331", + ""); + } + + private void assertXmlContains(Path outputDir, String... fragments) { + Path xmlPath = outputDir.resolve(ERROR_XML_FILE_NAME); + String xml; + try { + xml = Files.readString(xmlPath); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + for (String fragment : fragments) { + if (!xml.contains(fragment)) { + throw new AssertionError( + "Expected fragment not found in " + + xmlPath + + ":\n " + + fragment + + "\nActual file contents:\n" + + xml); + } + } + } + + /** + * Helper method to verify the correct serialization version number is written in + * "serialization_version.txt". Version number can be configured via Error Prone flags by the + * user, and {@link com.uber.nullaway.fixserialization.Serializer} should write the exact number + * in "serialization_version.txt". + * + * @param version Version number to pass to NullAway via Error Prone flags and the expected number + * to be read from "serialization_version.txt". + */ + private void checkVersionSerialization(int version) { + SerializationTestHelper tester = new SerializationTestHelper<>(root); + SerializationAdapter adapter = SerializationAdapter.getAdapterForVersion(version); + tester + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:SerializeFixMetadata=true", + "-XepOpt:NullAway:SerializeFixMetadataVersion=" + version, + "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) + // Just to run serialization features, the serialized fixes are not point of interest in + // this test. + .addSourceLines("com/uber/Test.java", "package com.uber;", "public class Test { }") + .expectNoOutput() + .setFactory(errorDisplayFactory) + .setOutputFileNameAndHeader(ERROR_FILE_NAME, adapter.getErrorsOutputFileHeader()) + .doTest(); + + Path serializationVersionPath = root.resolve("serialization_version.txt"); + try { + List lines = Files.readAllLines(serializationVersionPath); + // Check if it contains only one line. + assertEquals(1, lines.size()); + // Check the serialized version. + assertEquals(Integer.parseInt(lines.get(0)), version); + } catch (IOException e) { + throw new RuntimeException( + "Could not read serialization version at path: " + serializationVersionPath, e); + } + } }