Remove fixes.tsv serialization from NullAway serialization service

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

@@ -36,7 +36,6 @@
 
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
@@ -141,9 +140,6 @@ public Description createErrorDescription(
     }
 
     if (config.serializationIsActive()) {
-      if (nonNullTarget != null) {
-        SerializationService.serializeFixSuggestion(config, state, nonNullTarget, errorMessage);
-      }
       // 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
@@ -403,15 +399,12 @@ private Description.Builder removeCastToNonNullFix(
    * @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,
-      ImmutableList<Symbol> nonNullFields) {
+      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
     // method itself).
@@ -424,13 +417,6 @@ void reportInitializerError(
     ErrorMessage errorMessage = new ErrorMessage(METHOD_NO_INIT, message);
     state.reportMatch(
         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) {

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

@@ -2062,17 +2062,11 @@ private void checkFieldInitialization(ClassTree tree, VisitorState state) {
       }
     }
     for (Element constructorElement : errorFieldsForInitializer.keySet()) {
-      ImmutableList<Symbol> fieldSymbols =
-          errorFieldsForInitializer.get(constructorElement).stream()
-              .map(element -> ASTHelpers.getSymbol(getTreesInstance(state).getTree(element)))
-              .collect(ImmutableList.toImmutableList());
-
       errorBuilder.reportInitializerError(
           (Symbol.MethodSymbol) constructorElement,
           errMsgForInitializer(errorFieldsForInitializer.get(constructorElement), state),
           state,
-          buildDescription(getTreesInstance(state).getTree(constructorElement)),
-          fieldSymbols);
+          buildDescription(getTreesInstance(state).getTree(constructorElement)));
     }
     // For static fields
     Set<Symbol> notInitializedStaticFields = notInitializedStatic(entities, state);

nullaway/src/main/java/com/uber/nullaway/fixserialization/FixSerializationConfig.java

@@ -26,7 +26,7 @@
 import com.uber.nullaway.fixserialization.adapters.SerializationAdapter;
 import com.uber.nullaway.fixserialization.adapters.SerializationV1Adapter;
 import com.uber.nullaway.fixserialization.adapters.SerializationV3Adapter;
-import com.uber.nullaway.fixserialization.out.SuggestedNullableFixInfo;
+import com.uber.nullaway.fixserialization.adapters.SerializationV4Adapter;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
@@ -40,23 +40,6 @@
 /** Config class for Fix Serialization package. */
 public class FixSerializationConfig {
 
-  /**
-   * If enabled, the corresponding output file will be cleared and for all reported errors, NullAway
-   * will serialize information and suggest type changes to resolve them, in case these errors could
-   * be fixed by adding a {@code @Nullable} annotation. These type change suggestions are in form of
-   * {@link SuggestedNullableFixInfo} instances and will be serialized at output directory. If
-   * deactivated, no {@code SuggestedFixInfo} will be created and the output file will remain
-   * untouched.
-   */
-  public final boolean suggestEnabled;
-
-  /**
-   * If enabled, serialized information of a fix suggest will also include the enclosing method and
-   * class of the element involved in error. Finding enclosing elements is costly and will only be
-   * computed at request.
-   */
-  public final boolean suggestEnclosing;
-
   /**
    * If enabled, NullAway will serialize information about methods that initialize a field and leave
    * it {@code @NonNull} at exit point.
@@ -70,20 +53,12 @@ public class FixSerializationConfig {
 
   /** Default Constructor, all features are disabled with this config. */
   public FixSerializationConfig() {
-    suggestEnabled = false;
-    suggestEnclosing = false;
     fieldInitInfoEnabled = false;
     outputDirectory = null;
     serializer = null;
   }
 
-  public FixSerializationConfig(
-      boolean suggestEnabled,
-      boolean suggestEnclosing,
-      boolean fieldInitInfoEnabled,
-      @Nullable String outputDirectory) {
-    this.suggestEnabled = suggestEnabled;
-    this.suggestEnclosing = suggestEnclosing;
+  public FixSerializationConfig(boolean fieldInitInfoEnabled, @Nullable String outputDirectory) {
     this.fieldInitInfoEnabled = fieldInitInfoEnabled;
     this.outputDirectory = outputDirectory;
     serializer = new Serializer(this, initializeAdapter(SerializationAdapter.LATEST_VERSION));
@@ -111,17 +86,6 @@ public FixSerializationConfig(String configFilePath, int serializationVersion) {
         XMLUtil.getValueFromTag(document, "/serialization/path", String.class).orElse(null);
     Preconditions.checkNotNull(
         this.outputDirectory, "Error in FixSerialization Config: Output path cannot be null");
-    suggestEnabled =
-        XMLUtil.getValueFromAttribute(document, "/serialization/suggest", "active", Boolean.class)
-            .orElse(false);
-    suggestEnclosing =
-        XMLUtil.getValueFromAttribute(
-                document, "/serialization/suggest", "enclosing", Boolean.class)
-            .orElse(false);
-    if (suggestEnclosing && !suggestEnabled) {
-      throw new IllegalStateException(
-          "Error in the fix serialization configuration, suggest flag must be enabled to activate enclosing method and class serialization.");
-    }
     fieldInitInfoEnabled =
         XMLUtil.getValueFromAttribute(
                 document, "/serialization/fieldInitInfo", "active", Boolean.class)
@@ -142,6 +106,8 @@ private SerializationAdapter initializeAdapter(int version) {
             "Serialization version v2 is skipped and was used for an alpha version of the auto-annotator tool. Please use version 3 instead.");
       case 3:
         return new SerializationV3Adapter();
+      case 4:
+        return new SerializationV4Adapter();
       default:
         throw new RuntimeException(
             "Unrecognized NullAway serialization version: "
@@ -159,23 +125,13 @@ private SerializationAdapter initializeAdapter(int version) {
   /** Builder class for Serialization Config */
   public static class Builder {
 
-    private boolean suggestEnabled;
-    private boolean suggestEnclosing;
     private boolean fieldInitInfo;
     private @Nullable String outputDir;
 
     public Builder() {
-      suggestEnabled = false;
-      suggestEnclosing = false;
       fieldInitInfo = false;
     }
 
-    public Builder setSuggest(boolean value, boolean withEnclosing) {
-      this.suggestEnabled = value;
-      this.suggestEnclosing = withEnclosing && suggestEnabled;
-      return this;
-    }
-
     public Builder setFieldInitInfo(boolean enabled) {
       this.fieldInitInfo = enabled;
       return this;
@@ -200,7 +156,7 @@ public FixSerializationConfig build() {
       if (outputDir == null) {
         throw new IllegalStateException("did not set mandatory output directory");
       }
-      return new FixSerializationConfig(suggestEnabled, suggestEnclosing, fieldInitInfo, outputDir);
+      return new FixSerializationConfig(fieldInitInfo, outputDir);
     }
   }
 }

nullaway/src/main/java/com/uber/nullaway/fixserialization/SerializationService.java

@@ -26,16 +26,10 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.errorprone.VisitorState;
 import com.sun.source.tree.Tree;
-import com.sun.source.util.TreePath;
-import com.sun.source.util.Trees;
 import com.sun.tools.javac.code.Symbol;
-import com.sun.tools.javac.processing.JavacProcessingEnvironment;
 import com.uber.nullaway.Config;
 import com.uber.nullaway.ErrorMessage;
-import com.uber.nullaway.Nullness;
-import com.uber.nullaway.fixserialization.location.SymbolLocation;
 import com.uber.nullaway.fixserialization.out.ErrorInfo;
-import com.uber.nullaway.fixserialization.out.SuggestedNullableFixInfo;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -77,42 +71,6 @@ public static String escapeSpecialCharacters(String str) {
     return str;
   }
 
-  /**
-   * Serializes the suggested type change of an element in the source code that can resolve the
-   * error. We do not want suggested fix changes to override explicit annotations in the code,
-   * therefore, if the target element has an explicit {@code @Nonnull} annotation, no type change is
-   * suggested.
-   *
-   * @param config NullAway config.
-   * @param state Visitor state.
-   * @param target Target element to alternate it's type.
-   * @param errorMessage Error caused by the target.
-   */
-  public static void serializeFixSuggestion(
-      Config config, VisitorState state, Symbol target, ErrorMessage errorMessage) {
-    FixSerializationConfig serializationConfig = config.getSerializationConfig();
-    if (!serializationConfig.suggestEnabled) {
-      return;
-    }
-    // Skip if the element has an explicit @Nonnull annotation.
-    if (Nullness.hasNonNullAnnotation(target, config)) {
-      return;
-    }
-    Trees trees = Trees.instance(JavacProcessingEnvironment.instance(state.context));
-    // Skip if the element is received as bytecode.
-    if (trees.getPath(target) == null) {
-      return;
-    }
-    SymbolLocation location = SymbolLocation.createLocationFromSymbol(target);
-    SuggestedNullableFixInfo suggestedNullableFixInfo =
-        buildFixMetadata(state.getPath(), errorMessage, location);
-    Serializer serializer = serializationConfig.getSerializer();
-    Preconditions.checkNotNull(
-        serializer, "Serializer shouldn't be null at this point, error in configuration setting!");
-    serializer.serializeSuggestedFixInfo(
-        suggestedNullableFixInfo, serializationConfig.suggestEnclosing);
-  }
-
   /**
    * Serializes the reporting error.
    *
@@ -132,27 +90,4 @@ public static void serializeReportingError(
         serializer, "Serializer shouldn't be null at this point, error in configuration setting!");
     serializer.serializeErrorInfo(new ErrorInfo(state.getPath(), errorTree, errorMessage, target));
   }
-
-  /**
-   * Builds the {@link SuggestedNullableFixInfo} instance based on the {@link ErrorMessage} type.
-   */
-  private static SuggestedNullableFixInfo buildFixMetadata(
-      TreePath path, ErrorMessage errorMessage, SymbolLocation location) {
-    SuggestedNullableFixInfo suggestedNullableFixInfo;
-    switch (errorMessage.getMessageType()) {
-      case RETURN_NULLABLE:
-      case WRONG_OVERRIDE_RETURN:
-      case WRONG_OVERRIDE_PARAM:
-      case PASS_NULLABLE:
-      case FIELD_NO_INIT:
-      case ASSIGN_FIELD_NULLABLE:
-      case METHOD_NO_INIT:
-        suggestedNullableFixInfo = new SuggestedNullableFixInfo(path, location, errorMessage);
-        break;
-      default:
-        throw new IllegalStateException(
-            "Cannot suggest a type to resolve error of type: " + errorMessage.getMessageType());
-    }
-    return suggestedNullableFixInfo;
-  }
 }

nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java

@@ -27,7 +27,6 @@
 import com.uber.nullaway.fixserialization.adapters.SerializationAdapter;
 import com.uber.nullaway.fixserialization.out.ErrorInfo;
 import com.uber.nullaway.fixserialization.out.FieldInitializationInfo;
-import com.uber.nullaway.fixserialization.out.SuggestedNullableFixInfo;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -47,9 +46,6 @@ public class Serializer {
   /** Path to write errors. */
   private final Path errorOutputPath;
 
-  /** Path to write suggested fix metadata. */
-  private final Path suggestedFixesOutputPath;
-
   /** Path to write suggested fix metadata. */
   private final Path fieldInitializationOutputPath;
 
@@ -63,29 +59,12 @@ public class Serializer {
   public Serializer(FixSerializationConfig config, SerializationAdapter serializationAdapter) {
     String outputDirectory = config.outputDirectory;
     this.errorOutputPath = Paths.get(outputDirectory, "errors.tsv");
-    this.suggestedFixesOutputPath = Paths.get(outputDirectory, "fixes.tsv");
     this.fieldInitializationOutputPath = Paths.get(outputDirectory, "field_init.tsv");
     this.serializationAdapter = serializationAdapter;
     serializeVersion(outputDirectory);
     initializeOutputFiles(config);
   }
 
-  /**
-   * Appends the string representation of the {@link SuggestedNullableFixInfo}.
-   *
-   * @param suggestedNullableFixInfo SuggestedFixInfo object.
-   * @param enclosing Flag to control if enclosing method and class should be included.
-   */
-  public void serializeSuggestedFixInfo(
-      SuggestedNullableFixInfo suggestedNullableFixInfo, boolean enclosing) {
-    if (enclosing) {
-      suggestedNullableFixInfo.initEnclosing();
-    }
-    appendToFile(
-        suggestedNullableFixInfo.tabSeparatedToString(serializationAdapter),
-        suggestedFixesOutputPath);
-  }
-
   /**
    * Appends the string representation of the {@link ErrorMessage}.
    *
@@ -145,9 +124,6 @@ private void serializeVersion(@Nullable String outputDirectory) {
   private void initializeOutputFiles(FixSerializationConfig config) {
     try {
       Files.createDirectories(Paths.get(config.outputDirectory));
-      if (config.suggestEnabled) {
-        initializeFile(suggestedFixesOutputPath, SuggestedNullableFixInfo.header());
-      }
       if (config.fieldInitInfoEnabled) {
         initializeFile(fieldInitializationOutputPath, FieldInitializationInfo.header());
       }

nullaway/src/main/java/com/uber/nullaway/fixserialization/XMLUtil.java

@@ -133,12 +133,6 @@ public static void writeInXMLFormat(FixSerializationConfig config, String path)
       Element rootElement = doc.createElement("serialization");
       doc.appendChild(rootElement);
 
-      // Suggest
-      Element suggestElement = doc.createElement("suggest");
-      suggestElement.setAttribute("active", String.valueOf(config.suggestEnabled));
-      suggestElement.setAttribute("enclosing", String.valueOf(config.suggestEnclosing));
-      rootElement.appendChild(suggestElement);
-
       // Field Initialization
       Element fieldInitInfoEnabled = doc.createElement("fieldInitInfo");
       fieldInitInfoEnabled.setAttribute("active", String.valueOf(config.fieldInitInfoEnabled));

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

nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV4Adapter.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2024 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.
+ *
+ * <p>Updates to previous version (version 3):
+ *
+ * <ul>
+ *   <li>Fixes are not serialized anymore in a separate file. Since the current version of error
+ *       structure contains all the necessary information to recreate the fix, there is no need to
+ *       serialize the fix separately.
+ * </ul>
+ */
+public class SerializationV4Adapter extends SerializationV3Adapter {
+
+  @Override
+  public int getSerializationVersion() {
+    return 4;
+  }
+}