Serialization of Type Change Suggestions for Type Violations.

nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java

@@ -24,10 +24,12 @@
 
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.tools.javac.code.Symbol;
+import com.uber.nullaway.fixserialization.FixSerializationConfig;
 import java.util.LinkedHashSet;
 import java.util.Set;
 import java.util.regex.Pattern;
@@ -115,6 +117,27 @@ public abstract class AbstractConfig implements Config {
 
   protected Set<String> customNullableAnnotations;
 
+  /**
+   * If active, NullAway will write all reporting errors in output directory. The output directory
+   * along with the activation status of other serialization features are stored in {@link
+   * FixSerializationConfig}.
+   */
+  protected boolean serializationActivationFlag;
+
+  protected FixSerializationConfig fixSerializationConfig;
+
+  @Override
+  public boolean serializationIsActive() {
+    return serializationActivationFlag;
+  }
+
+  @Override
+  public FixSerializationConfig getSerializationConfig() {
+    Preconditions.checkArgument(
+        serializationActivationFlag, "Fix Serialization is not active, cannot access it's config.");
+    return fixSerializationConfig;
+  }
+
   protected static Pattern getPackagePattern(Set<String> packagePrefixes) {
     // noinspection ConstantConditions
     String choiceRegexp =

nullaway/src/main/java/com/uber/nullaway/Config.java

@@ -24,12 +24,30 @@
 
 import com.google.common.collect.ImmutableSet;
 import com.sun.tools.javac.code.Symbol;
+import com.uber.nullaway.fixserialization.FixSerializationConfig;
 import java.util.Set;
 import javax.annotation.Nullable;
 
 /** Provides configuration parameters for the nullability checker. */
 public interface Config {
 
+  /**
+   * Checks if Serialization feature is active.
+   *
+   * @return true, if Fix Serialization feature is active.
+   */
+  boolean serializationIsActive();
+
+  /**
+   * Getter method for {@link FixSerializationConfig}.
+   *
+   * <p>Fix Serialization feature must be activated, otherwise calling this method will fail the
+   * execution.
+   *
+   * @return {@link FixSerializationConfig} instance in Config.
+   */
+  FixSerializationConfig getSerializationConfig();
+
   /**
    * Checks if a symbol comes from an annotated package.
    *

nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java

@@ -27,6 +27,7 @@
 
 import com.google.common.collect.ImmutableSet;
 import com.sun.tools.javac.code.Symbol;
+import com.uber.nullaway.fixserialization.FixSerializationConfig;
 import java.util.Set;
 import javax.annotation.Nullable;
 
@@ -53,6 +54,16 @@ public class DummyOptionsConfig implements Config {
 
   public DummyOptionsConfig() {}
 
+  @Override
+  public boolean serializationIsActive() {
+    throw new IllegalStateException(ERROR_MESSAGE);
+  }
+
+  @Override
+  public FixSerializationConfig getSerializationConfig() {
+    throw new IllegalStateException(ERROR_MESSAGE);
+  }
+
   @Override
   public boolean fromAnnotatedPackage(Symbol.ClassSymbol symbol) {
     throw new IllegalStateException(ERROR_MESSAGE);

nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java

@@ -32,6 +32,7 @@
 import static com.uber.nullaway.NullAway.getTreesInstance;
 
 import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
@@ -51,6 +52,7 @@
 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 java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -82,12 +84,18 @@ public class ErrorBuilder {
    * @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
    */
   Description createErrorDescription(
-      ErrorMessage errorMessage, Description.Builder descriptionBuilder, VisitorState state) {
+      ErrorMessage errorMessage,
+      Description.Builder descriptionBuilder,
+      VisitorState state,
+      @Nullable Symbol nonNullTarget) {
     Tree enclosingSuppressTree = suppressibleNode(state.getPath());
-    return createErrorDescription(errorMessage, enclosingSuppressTree, descriptionBuilder, state);
+    return createErrorDescription(
+        errorMessage, enclosingSuppressTree, descriptionBuilder, state, nonNullTarget);
   }
 
   /**
@@ -97,13 +105,16 @@ Description createErrorDescription(
    * @param suggestTree the location at which a fix suggestion should be made
    * @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 createErrorDescription(
       ErrorMessage errorMessage,
       @Nullable Tree suggestTree,
       Description.Builder descriptionBuilder,
-      VisitorState state) {
+      VisitorState state,
+      @Nullable Symbol nonNullTarget) {
     Description.Builder builder = descriptionBuilder.setMessage(errorMessage.message);
     String checkName = CORE_CHECK_NAME;
     if (errorMessage.messageType.equals(GET_ON_EMPTY_OPTIONAL)) {
@@ -123,6 +134,14 @@ public Description createErrorDescription(
     if (config.suggestSuppressions() && suggestTree != null) {
       builder = addSuggestedSuppression(errorMessage, suggestTree, builder);
     }
+
+    if (config.serializationIsActive()) {
+      if (nonNullTarget != null) {
+        SerializationService.serializeFixSuggestion(config, state, nonNullTarget, errorMessage);
+      }
+      SerializationService.serializeReportingError(config, state, errorMessage);
+    }
+
     // #letbuildersbuild
     return builder.build();
   }
@@ -201,19 +220,26 @@ private Description.Builder addSuggestedSuppression(
    * @param descriptionBuilder the description builder for the error.
    * @param state the visitor state for the location which triggered the error (i.e. for 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.
    */
   Description createErrorDescriptionForNullAssignment(
       ErrorMessage errorMessage,
       @Nullable Tree suggestTreeIfCastToNonNull,
       Description.Builder descriptionBuilder,
-      VisitorState state) {
+      VisitorState state,
+      Symbol nonNullTarget) {
     if (config.getCastToNonNullMethod() != null) {
       return createErrorDescription(
-          errorMessage, suggestTreeIfCastToNonNull, descriptionBuilder, state);
+          errorMessage, suggestTreeIfCastToNonNull, descriptionBuilder, state, nonNullTarget);
     } else {
       return createErrorDescription(
-          errorMessage, suppressibleNode(state.getPath()), descriptionBuilder, state);
+          errorMessage,
+          suppressibleNode(state.getPath()),
+          descriptionBuilder,
+          state,
+          nonNullTarget);
     }
   }
 
@@ -311,21 +337,40 @@ private Description.Builder removeCastToNonNullFix(
     return builder.addFix(fix);
   }
 
+  /**
+   * Reports initialization errors where a constructor fails to guarantee non-fields are initialized
+   * along all paths at exit points.
+   *
+   * @param methodSymbol Constructor symbol.
+   * @param message Error message.
+   * @param state The VisitorState object.
+   * @param descriptionBuilder the description builder for the error.
+   * @param nonNullFields list of @Nonnull fields that are not guaranteed to be initialized along
+   *     all paths at exit points of the constructor.
+   */
   void reportInitializerError(
       Symbol.MethodSymbol methodSymbol,
       String message,
       VisitorState state,
-      Description.Builder descriptionBuilder) {
+      Description.Builder descriptionBuilder,
+      ImmutableList<Symbol> nonNullFields) {
     // Check needed here, despite check in hasPathSuppression because initialization
     // checking happens at the class-level (meaning state.getPath() might not include the
     // method itself).
     if (symbolHasSuppressWarningsAnnotation(methodSymbol, INITIALIZATION_CHECK_NAME)) {
       return;
     }
     Tree methodTree = getTreesInstance(state).getTree(methodSymbol);
+    ErrorMessage errorMessage = new ErrorMessage(METHOD_NO_INIT, message);
     state.reportMatch(
-        createErrorDescription(
-            new ErrorMessage(METHOD_NO_INIT, message), methodTree, descriptionBuilder, state));
+        createErrorDescription(errorMessage, methodTree, descriptionBuilder, state, null));
+    if (config.serializationIsActive()) {
+      // For now, we serialize each fix suggestion separately and measure their effectiveness
+      // separately
+      nonNullFields.forEach(
+          symbol ->
+              SerializationService.serializeFixSuggestion(config, state, symbol, errorMessage));
+    }
   }
 
   boolean symbolHasSuppressWarningsAnnotation(Symbol symbol, String suppression) {
@@ -429,14 +474,16 @@ void reportInitErrorOnField(Symbol symbol, VisitorState state, Description.Build
                   FIELD_NO_INIT, "@NonNull static field " + fieldName + " not initialized"),
               tree,
               builder,
-              state));
+              state,
+              symbol));
     } else {
       state.reportMatch(
           createErrorDescription(
               new ErrorMessage(FIELD_NO_INIT, "@NonNull field " + fieldName + " not initialized"),
               tree,
               builder,
-              state));
+              state,
+              symbol));
     }
   }
 }

nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java

@@ -53,4 +53,12 @@ public enum MessageTypes {
     WRONG_OVERRIDE_POSTCONDITION,
     WRONG_OVERRIDE_PRECONDITION,
   }
+
+  public String getMessage() {
+    return message;
+  }
+
+  public MessageTypes getMessageType() {
+    return messageType;
+  }
 }

nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java

@@ -24,6 +24,7 @@
 
 import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.ErrorProneFlags;
+import com.uber.nullaway.fixserialization.FixSerializationConfig;
 import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.Optional;
@@ -74,6 +75,11 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig {
   static final String FL_JI_REGEX_MODEL_PATH = EP_FL_NAMESPACE + ":JarInferRegexStripModelJar";
   static final String FL_JI_REGEX_CODE_PATH = EP_FL_NAMESPACE + ":JarInferRegexStripCodeJar";
   static final String FL_ERROR_URL = EP_FL_NAMESPACE + ":ErrorURL";
+  /** --- Serialization configs --- */
+  static final String FL_FIX_SERIALIZATION = EP_FL_NAMESPACE + ":SerializeFixMetadata";
+
+  static final String FL_FIX_SERIALIZATION_CONFIG_PATH =
+      EP_FL_NAMESPACE + ":FixSerializationConfigPath";
 
   private static final String DELIMITER = ",";
 
@@ -200,6 +206,35 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig {
               + FL_ACKNOWLEDGE_RESTRICTIVE
               + " is also set");
     }
+    serializationActivationFlag = flags.getBoolean(FL_FIX_SERIALIZATION).orElse(false);
+    Optional<String> fixSerializationConfigPath = flags.get(FL_FIX_SERIALIZATION_CONFIG_PATH);
+    if (serializationActivationFlag && !fixSerializationConfigPath.isPresent()) {
+      throw new IllegalStateException(
+          "DO NOT report an issue to Error Prone for this crash!  NullAway Fix Serialization configuration is "
+              + "incorrect.  "
+              + "Must specify AutoFixer Output Directory, using the "
+              + "-XepOpt:"
+              + FL_FIX_SERIALIZATION_CONFIG_PATH
+              + " flag.  If you feel you have gotten this message in error report an issue"
+              + " at https://github.com/uber/NullAway/issues.");
+    }
+    /*
+     * if fixSerializationActivationFlag is false, the default constructor is invoked for
+     * creating FixSerializationConfig which all features are deactivated.  This lets the
+     * field be @Nonnull, allowing us to avoid null checks in various places.
+     */
+    fixSerializationConfig =
+        serializationActivationFlag
+            ? new FixSerializationConfig(fixSerializationConfigPath.get())
+            : new FixSerializationConfig();
+    if (serializationActivationFlag && isSuggestSuppressions) {
+      throw new IllegalStateException(
+          "In order to activate Fix Serialization mode ("
+              + FL_FIX_SERIALIZATION
+              + "), Suggest Suppressions mode must be deactivated ("
+              + FL_SUGGEST_SUPPRESSIONS
+              + ")");
+    }
   }
 
   private static ImmutableSet<String> getFlagStringSet(ErrorProneFlags flags, String flagName) {