Fix #55 by supporting @NullUnmarked (#69)

This PR resolves #55 by adding a configuration mode which if activated, all remaining errors will be resolved with following the rules below: 1. Enclosing method of triggered errors will be marked as `@NullUnmarked`. 2. Uninitialized fields (inline or in constructor) will be annotated as `@SuppressWarnings("NullAway.Init")`. 3. Explicit `@Nullable` assignments to fields will be annotated as `@SuppressWarnings("NullAway")`. With this PR, the target module will enroll into NullAway with no triggered errors. To enable this mode, `cli` flag below must be passed to Annotator: ``` -fr or --force-resolve [followed by the "@NullUnmarked" fully qualified name] ```
ucr-riple · Sep 14, 2022 · 5f57920 · 5f57920
1 parent cbc7bea
commit 5f57920
Show file tree

Hide file tree

Showing 47 changed files with 1,274 additions and 159 deletions.
diff --git a/core/build.gradle b/core/build.gradle
@@ -46,11 +46,12 @@ dependencies {
 }
 
 tasks.test.dependsOn(':type-annotator-scanner:publishToMavenLocal')
+tasks.test.dependsOn(':qual:publishToMavenLocal')
 
 // Set up environment variables for test configuration tu run with the latest development version.
 tasks.test.doFirst {
-    environment "NULLAWAY_TEST_VERSION", "0.9.10"
-    environment "TYPE_ANNOTATOR_SCANNER_VERSION", project.version
+    environment "NULLAWAY_TEST_VERSION", "0.10.0"
+    environment "ANNOTATOR_VERSION", project.version
 }
 
 publishing {

diff --git a/core/src/main/java/edu/ucr/cs/riple/core/Annotator.java b/core/src/main/java/edu/ucr/cs/riple/core/Annotator.java
@@ -38,14 +38,19 @@
 import edu.ucr.cs.riple.core.injectors.PhysicalInjector;
 import edu.ucr.cs.riple.core.metadata.field.FieldDeclarationAnalysis;
 import edu.ucr.cs.riple.core.metadata.field.FieldInitializationAnalysis;
+import edu.ucr.cs.riple.core.metadata.index.Error;
 import edu.ucr.cs.riple.core.metadata.index.Fix;
 import edu.ucr.cs.riple.core.metadata.method.MethodDeclarationTree;
 import edu.ucr.cs.riple.core.metadata.trackers.CompoundTracker;
 import edu.ucr.cs.riple.core.metadata.trackers.RegionTracker;
 import edu.ucr.cs.riple.core.util.Utility;
 import edu.ucr.cs.riple.injector.changes.AddAnnotation;
+import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation;
+import edu.ucr.cs.riple.injector.changes.AddSingleElementAnnotation;
 import edu.ucr.cs.riple.injector.location.OnField;
 import edu.ucr.cs.riple.scanner.Serializer;
+import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -70,11 +75,11 @@ public Annotator(Config config) {
     this.injector = new PhysicalInjector(config);
   }
 
-  /** Starts the annotating process consist of preprocess followed by explore phase. */
+  /** Starts the annotating process consist of preprocess followed by the "annotate" phase. */
   public void start() {
     preprocess();
     long timer = config.log.startTimer();
-    explore();
+    annotate();
     config.log.stopTimerAndCapture(timer);
     Utility.writeLog(config);
   }
@@ -95,7 +100,7 @@ private void preprocess() {
     System.out.println("Making the first build...");
     Utility.buildTarget(config, true);
     Set<OnField> uninitializedFields =
-        Utility.readFixesFromOutputDirectory(config.target, Fix.factory(config, null))
+        Utility.readFixesFromOutputDirectory(config.target, Fix.factory(config, null)).stream()
             .filter(fix -> fix.isOnField() && fix.reasons.contains("FIELD_NO_INIT"))
             .map(Fix::toField)
             .collect(Collectors.toSet());
@@ -104,13 +109,13 @@ private void preprocess() {
     Set<AddAnnotation> initializers =
         analysis
             .findInitializers(uninitializedFields)
-            .map(onMethod -> new AddAnnotation(onMethod, config.initializerAnnot))
+            .map(onMethod -> new AddMarkerAnnotation(onMethod, config.initializerAnnot))
             .collect(Collectors.toSet());
     this.injector.injectAnnotations(initializers);
   }
 
   /** Performs iterations of inference/injection until no unseen fix is suggested. */
-  private void explore() {
+  private void annotate() {
     Utility.setScannerCheckerActivation(config.target, true);
     Utility.buildTarget(config);
     Utility.setScannerCheckerActivation(config.target, false);
@@ -143,6 +148,10 @@ private void explore() {
       cache.enable();
     }
 
+    if (config.forceResolveActivated) {
+      forceResolveRemainingErrors(fieldDeclarationAnalysis, tree);
+    }
+
     System.out.println("\nFinished annotating.");
     Utility.writeReports(config, cache.reports().stream().collect(ImmutableSet.toImmutableSet()));
   }
@@ -199,6 +208,7 @@ private ImmutableSet<Report> processTriggeredFixes(
     ImmutableSet<Fix> fixes =
         Utility.readFixesFromOutputDirectory(
                 config.target, Fix.factory(config, fieldDeclarationAnalysis))
+            .stream()
             .filter(fix -> !cache.processedFix(fix))
             .collect(ImmutableSet.toImmutableSet());
 
@@ -216,4 +226,79 @@ private ImmutableSet<Report> processTriggeredFixes(
     // Result of the iteration analysis.
     return explorer.explore();
   }
+
+  /**
+   * Resolves all remaining errors in target module by following steps below:
+   *
+   * <ul>
+   *   <li>Enclosing method of triggered errors will be marked with {@code @NullUnmarked}
+   *       annotation.
+   *   <li>Uninitialized fields (inline or by constructor) will be annotated as
+   *       {@code @SuppressWarnings("NullAway.Init")}.
+   *   <li>Explicit {@code Nullable} assignments to fields will be annotated as
+   *       {@code @SuppressWarnings("NullAway")}.
+   * </ul>
+   *
+   * @param fieldDeclarationAnalysis Field declaration analysis.
+   * @param tree Method Declaration analysis.
+   */
+  private void forceResolveRemainingErrors(
+      FieldDeclarationAnalysis fieldDeclarationAnalysis, MethodDeclarationTree tree) {
+    // Collect regions with remaining errors.
+    Utility.buildTarget(config);
+    List<Error> remainingErrors = Utility.readErrorsFromOutputDirectory(config.target);
+    Set<Fix> remainingFixes =
+        Utility.readFixesFromOutputDirectory(
+            config.target, Fix.factory(config, fieldDeclarationAnalysis));
+
+    // Collect all regions for NullUnmarked.
+    // For all errors in regions which correspond to a method's body, we can add @NullUnmarked at
+    // the method level.
+    Set<AddAnnotation> nullUnMarkedAnnotations =
+        remainingErrors.stream()
+            // filter non-method regions.
+            .filter(error -> !error.encMethod().equals("null"))
+            // find the corresponding method nodes.
+            .map(error -> tree.findNode(error.encMethod(), error.encClass()))
+            // impossible, just sanity check or future nullness checker hints
+            .filter(Objects::nonNull)
+            .map(node -> new AddMarkerAnnotation(node.location, config.nullUnMarkedAnnotation))
+            .collect(Collectors.toSet());
+    injector.injectAnnotations(nullUnMarkedAnnotations);
+
+    // Collect explicit Nullable initialization to fields
+    Set<AddAnnotation> suppressWarningsAnnotations =
+        remainingFixes.stream()
+            .filter(
+                fix -> {
+                  if (!(fix.isOnField() && fix.reasons.contains("ASSIGN_FIELD_NULLABLE"))) {
+                    return false;
+                  }
+                  OnField onField = fix.toField();
+                  return onField.clazz.equals(fix.encClass()) && fix.encMethod().equals("");
+                })
+            .map(
+                fix ->
+                    new AddSingleElementAnnotation(
+                        fix.toField(), "SuppressWarnings", "NullAway", false))
+            .collect(Collectors.toSet());
+    injector.injectAnnotations(suppressWarningsAnnotations);
+
+    // Collect NullAway.Init SuppressWarnings
+    Set<AddAnnotation> initializationSuppressWarningsAnnotations =
+        remainingFixes.stream()
+            .filter(
+                fix ->
+                    fix.isOnField()
+                        && (fix.reasons.contains("METHOD_NO_INIT")
+                            || fix.reasons.contains("FIELD_NO_INIT")))
+            .map(
+                fix ->
+                    new AddSingleElementAnnotation(
+                        fix.toField(), "SuppressWarnings", "NullAway.Init", false))
+            // Exclude already annotated fields with a general NullAway suppress warning.
+            .filter(f -> !suppressWarningsAnnotations.contains(f))
+            .collect(Collectors.toSet());
+    injector.injectAnnotations(initializationSuppressWarningsAnnotations);
+  }
 }
diff --git a/core/src/main/java/edu/ucr/cs/riple/core/Config.java b/core/src/main/java/edu/ucr/cs/riple/core/Config.java
@@ -118,6 +118,16 @@ public class Config {
   /** Global counter for assigning unique id for each instance. */
   public int moduleCounterID;
 
+  /**
+   * If activated, AutoAnnotator will try to resolve all remaining errors by marking the enclosing
+   * method as {@code NullUnMarked}. It will also mark uninitialized fields with
+   * {@code @SuppressWarning("NullAway.Init")}
+   */
+  public final boolean forceResolveActivated;
+
+  /** Fully qualified NullUnmarked annotation. */
+  public final String nullUnMarkedAnnotation;
+
   public final Log log;
   public final int depth;
 
@@ -275,6 +285,12 @@ public Config(String[] args) {
     analysisMode.setRequired(false);
     options.addOption(analysisMode);
 
+    // Force resolve activation
+    Option activateForceResolveOption =
+        new Option("fr", "force-resolve", true, "Activates force resolve mode.");
+    activateForceResolveOption.setRequired(false);
+    options.addOption(activateForceResolveOption);
+
     HelpFormatter formatter = new HelpFormatter();
     CommandLineParser parser = new DefaultParser();
     CommandLine cmd;
@@ -361,6 +377,11 @@ public Config(String[] args) {
       this.downstreamInfo = ImmutableSet.of();
       this.downstreamDependenciesBuildCommand = null;
     }
+    this.forceResolveActivated = cmd.hasOption(activateForceResolveOption);
+    this.nullUnMarkedAnnotation =
+        this.forceResolveActivated
+            ? cmd.getOptionValue(activateForceResolveOption)
+            : "org.jspecify.nullness.NullUnmarked";
     this.moduleCounterID = 0;
     this.log = new Log();
     this.log.reset();
@@ -436,6 +457,11 @@ public Config(Path configPath) {
 
     this.downstreamInfo = ImmutableSet.copyOf(moduleInfoList);
     this.moduleCounterID = 0;
+    this.forceResolveActivated =
+        getValueFromKey(jsonObject, "FORCE_RESOLVE", Boolean.class).orElse(false);
+    this.nullUnMarkedAnnotation =
+        getValueFromKey(jsonObject, "ANNOTATION:NULL_UNMARKED", String.class)
+            .orElse("org.jspecify.nullness.NullUnmarked");
     this.log = new Log();
     log.reset();
   }
@@ -550,6 +576,9 @@ public static class Builder {
     public Path nullawayLibraryModelLoaderPath;
     public AnalysisMode mode = AnalysisMode.LOCAL;
     public String downstreamBuildCommand;
+
+    public boolean forceResolveActivation = false;
+    public String nullUnmarkedAnnotation = "org.jspecify.nullness.NullUnmarked";
     public int depth = 1;
 
     @SuppressWarnings("unchecked")
@@ -568,6 +597,7 @@ public void write(Path path) {
       JSONObject annotation = new JSONObject();
       annotation.put("INITIALIZER", initializerAnnotation);
       annotation.put("NULLABLE", nullableAnnotation);
+      annotation.put("NULL_UNMARKED", nullUnmarkedAnnotation);
       json.put("ANNOTATION", annotation);
       json.put("LEXICAL_PRESERVATION", lexicalPreservationActivation);
       json.put("OUTER_LOOP", outerLoopActivation);
@@ -579,6 +609,7 @@ public void write(Path path) {
       json.put("DEPTH", depth);
       json.put("EXHAUSTIVE_SEARCH", exhaustiveSearch);
       json.put("REDIRECT_BUILD_OUTPUT_TO_STDERR", redirectBuildOutputToStdErr);
+      json.put("FORCE_RESOLVE", forceResolveActivation);
       JSONArray configPathsJson = new JSONArray();
       configPathsJson.addAll(
           configPaths.stream()
@@ -607,6 +638,7 @@ public void write(Path path) {
         downstreamDependency.put("ANALYSIS_MODE", mode.name());
       }
       json.put("DOWNSTREAM_DEPENDENCY_ANALYSIS", downstreamDependency);
+
       try (BufferedWriter file =
           Files.newBufferedWriter(path.toFile().toPath(), Charset.defaultCharset())) {
         file.write(json.toJSONString());

diff --git a/core/src/main/java/edu/ucr/cs/riple/core/explorers/BasicExplorer.java b/core/src/main/java/edu/ucr/cs/riple/core/explorers/BasicExplorer.java
@@ -32,7 +32,7 @@
 import edu.ucr.cs.riple.core.metadata.index.Fix;
 import edu.ucr.cs.riple.core.metadata.index.Result;
 import edu.ucr.cs.riple.core.util.Utility;
-import edu.ucr.cs.riple.injector.changes.AddAnnotation;
+import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation;
 import edu.ucr.cs.riple.injector.location.Location;
 import java.util.Collection;
 import java.util.Set;
@@ -90,7 +90,7 @@ public void addTriggeredFixesFromDownstream(Node node, Collection<Fix> localTrig
             .map(
                 onParameter ->
                     new Fix(
-                        new AddAnnotation(onParameter, config.nullableAnnot),
+                        new AddMarkerAnnotation(onParameter, config.nullableAnnot),
                         "PASSING_NULLABLE",
                         onParameter.clazz,
                         onParameter.method,

diff --git a/core/src/main/java/edu/ucr/cs/riple/core/global/GlobalAnalyzerImpl.java b/core/src/main/java/edu/ucr/cs/riple/core/global/GlobalAnalyzerImpl.java
@@ -37,7 +37,7 @@
 import edu.ucr.cs.riple.core.metadata.method.MethodNode;
 import edu.ucr.cs.riple.core.metadata.trackers.MethodRegionTracker;
 import edu.ucr.cs.riple.core.util.Utility;
-import edu.ucr.cs.riple.injector.changes.AddAnnotation;
+import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation;
 import edu.ucr.cs.riple.injector.location.Location;
 import edu.ucr.cs.riple.injector.location.OnMethod;
 import edu.ucr.cs.riple.injector.location.OnParameter;
@@ -93,7 +93,7 @@ public void analyzeDownstreamDependencies() {
             .map(
                 methodImpact ->
                     new Fix(
-                        new AddAnnotation(
+                        new AddMarkerAnnotation(
                             new OnMethod(
                                 "null",
                                 methodImpact.node.location.clazz,

diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Error.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Error.java
@@ -87,4 +87,9 @@ public boolean equals(Object o) {
   public int hashCode() {
     return Objects.hash(messageType, message, nonnullTarget);
   }
+
+  @Override
+  public String toString() {
+    return "Type='" + messageType + '\'' + ", message='" + message + '\'';
+  }
 }
diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Fix.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Fix.java
@@ -30,6 +30,7 @@
 import edu.ucr.cs.riple.core.metadata.field.FieldDeclarationAnalysis;
 import edu.ucr.cs.riple.injector.Helper;
 import edu.ucr.cs.riple.injector.changes.AddAnnotation;
+import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation;
 import edu.ucr.cs.riple.injector.location.Location;
 import edu.ucr.cs.riple.injector.location.OnField;
 import edu.ucr.cs.riple.injector.location.OnMethod;
@@ -93,7 +94,7 @@ public static Factory<Fix> factory(Config config, FieldDeclarationAnalysis analy
       }
       Preconditions.checkArgument(info[7].equals("nullable"), "unsupported annotation: " + info[7]);
       return new Fix(
-          new AddAnnotation(location, config.nullableAnnot), info[6], info[8], info[9], true);
+          new AddMarkerAnnotation(location, config.nullableAnnot), info[6], info[8], info[9], true);
     };
   }