Add informative comment on @NullUnmarked injections

.github/workflows/continuous-integration.yml

@@ -56,4 +56,15 @@ jobs:
         uses: actions/upload-artifact@v2
         with:
           name: Test report
-          path: build/reports/tests/test
+          path: build/reports/tests/test
+  build-with-nullaway-serialization-0:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Set up JDK 11
+        uses: actions/setup-java@v2
+        with:
+          java-version: 11
+          distribution: 'temurin'
+      - name: Build with Gradle
+        run: ./gradlew :core:test --scan -Pnullaway-serialization-format-version=0

core/build.gradle

@@ -48,12 +48,15 @@ dependencies {
     testImplementation deps.build.commonsio
 }
 
+// Should be the latest supporting version of NullAway.
+def NULLAWAY_TEST = "0.10.5"
+
 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.10.0"
+    environment "NULLAWAY_TEST_VERSION", NULLAWAY_TEST
     environment "ANNOTATOR_VERSION", project.version
 }
 
@@ -83,3 +86,24 @@ jar {
 shadowJar {
     archiveClassifier = null
 }
+
+// Configure test environment
+def nullawayVersionMap = [0:"0.10.4", 1:"0.10.5"]
+tasks.create("configureNullAwayVersion"){
+    if(!project.hasProperty("nullaway-serialization-format-version")){
+        return
+    }
+    NULLAWAY_TEST = nullawayVersionMap.get((project.getProperty("nullaway-serialization-format-version")) as int)
+    println "NullAway Test version changed to: " + NULLAWAY_TEST
+
+    switch (NULLAWAY_TEST){
+        case "0.10.4":
+            test {
+                filter {
+                    excludeTest "edu.ucr.cs.riple.core.CoreTest", "errorInFieldDeclarationForceResolveTest"
+                }
+            }
+            break
+    }
+}
+tasks.test.dependsOn("configureNullAwayVersion")

core/src/main/java/edu/ucr/cs/riple/core/Annotator.java

@@ -38,9 +38,11 @@
 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.Bank;
 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.method.MethodNode;
 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;
@@ -49,8 +51,10 @@
 import edu.ucr.cs.riple.injector.changes.AddSingleElementAnnotation;
 import edu.ucr.cs.riple.injector.location.OnField;
 import edu.ucr.cs.riple.injector.location.OnMethod;
-import edu.ucr.cs.riple.scanner.Serializer;
+import edu.ucr.cs.riple.injector.location.OnParameter;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -100,13 +104,13 @@ private void preprocess() {
     Utility.setScannerCheckerActivation(config.target, true);
     System.out.println("Making the first build...");
     Utility.buildTarget(config, true);
+    config.initializeAdapter();
     Set<OnField> uninitializedFields =
         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());
-    FieldInitializationAnalysis analysis =
-        new FieldInitializationAnalysis(config.target.dir.resolve("field_init.tsv"));
+    FieldInitializationAnalysis analysis = new FieldInitializationAnalysis(config);
     Set<AddAnnotation> initializers =
         analysis
             .findInitializers(uninitializedFields)
@@ -120,9 +124,9 @@ private void annotate() {
     Utility.setScannerCheckerActivation(config.target, true);
     Utility.buildTarget(config);
     Utility.setScannerCheckerActivation(config.target, false);
-    FieldDeclarationAnalysis fieldDeclarationAnalysis = new FieldDeclarationAnalysis(config.target);
-    MethodDeclarationTree tree =
-        new MethodDeclarationTree(config.target.dir.resolve(Serializer.METHOD_INFO_FILE_NAME));
+    FieldDeclarationAnalysis fieldDeclarationAnalysis =
+        new FieldDeclarationAnalysis(config, config.target);
+    MethodDeclarationTree tree = new MethodDeclarationTree(config);
     // globalAnalyzer analyzes effects of all public APIs on downstream dependencies.
     // Through iterations, since the source code for downstream dependencies does not change and the
     // computation does not depend on the changes in the target module, it will compute the same
@@ -152,7 +156,7 @@ private void annotate() {
     }
 
     if (config.forceResolveActivated) {
-      forceResolveRemainingErrors(fieldDeclarationAnalysis, tree);
+      forceResolveRemainingErrors(fieldDeclarationAnalysis, tree, globalAnalyzer);
     }
 
     System.out.println("\nFinished annotating.");
@@ -220,9 +224,8 @@ private ImmutableSet<Report> processTriggeredFixes(
             .collect(ImmutableSet.toImmutableSet());
 
     // Initializing required explorer instances.
-    MethodDeclarationTree tree =
-        new MethodDeclarationTree(config.target.dir.resolve(Serializer.METHOD_INFO_FILE_NAME));
-    RegionTracker tracker = new CompoundTracker(config.target, tree);
+    MethodDeclarationTree tree = new MethodDeclarationTree(config);
+    RegionTracker tracker = new CompoundTracker(config, config.target, tree);
     TargetModuleSupplier supplier = new TargetModuleSupplier(config, tree);
     Explorer explorer =
         config.exhaustiveSearch
@@ -248,63 +251,122 @@ private ImmutableSet<Report> processTriggeredFixes(
    *
    * @param fieldDeclarationAnalysis Field declaration analysis.
    * @param tree Method Declaration analysis.
+   * @param analyzer Global analyzer instance, used to compute the effect of {@code @NullUnmarked}
+   *     injections globally
    */
   private void forceResolveRemainingErrors(
-      FieldDeclarationAnalysis fieldDeclarationAnalysis, MethodDeclarationTree tree) {
+      FieldDeclarationAnalysis fieldDeclarationAnalysis,
+      MethodDeclarationTree tree,
+      GlobalAnalyzer analyzer) {
     // Collect regions with remaining errors.
     Utility.buildTarget(config);
-    List<Error> remainingErrors = Utility.readErrorsFromOutputDirectory(config.target);
+    List<Error> remainingErrors = Utility.readErrorsFromOutputDirectory(config, config.target);
     Set<Fix> remainingFixes =
         Utility.readFixesFromOutputDirectory(
             config.target, Fix.factory(config, fieldDeclarationAnalysis));
 
+    Bank<Error> errorBank =
+        new Bank<>(config.target.dir.resolve("errors.tsv"), Error.factory(config));
+
     // 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 =
+    Map<OnMethod, String> methodCommentMap = new HashMap<>();
+    Set<OnMethod> nullUnmarkedMethods =
         remainingErrors.stream()
             // find the corresponding method nodes.
             .map(
                 error -> {
-                  if (!error.encMethod().equals("null")) {
-                    return tree.findNode(error.encMethod(), error.encClass());
-                  }
-                  if (error.nonnullTarget == null) {
-                    // Just a sanity check.
-                    return null;
+                  if (error.getRegion().isOnMethod()) {
+                    return tree.findNode(error.encMember(), error.encClass());
                   }
                   // For methods invoked in an initialization region, where the error is that
                   // `@Nullable` is being passed as an argument, we add a `@NullUnmarked` annotation
                   // to the called method.
-                  if (error.messageType.equals("PASS_NULLABLE")) {
-                    OnMethod calledMethod = error.nonnullTarget.toMethod();
+                  if (error.messageType.equals("PASS_NULLABLE")
+                      && error.nonnullTarget != null
+                      && error.nonnullTarget.isOnParameter()) {
+                    OnParameter calledMethod = error.nonnullTarget.toParameter();
                     return tree.findNode(calledMethod.method, calledMethod.clazz);
                   }
                   return null;
                 })
+            // Filter null values from map above.
             .filter(Objects::nonNull)
-            .map(node -> new AddMarkerAnnotation(node.location, config.nullUnMarkedAnnotation))
+            .map(
+                node -> {
+                  methodCommentMap.put(
+                      node.location,
+                      "//local "
+                          + errorBank
+                              .getEntriesByRegion(node.location.clazz, node.location.method)
+                              .size());
+                  return node.location;
+                })
+            .collect(Collectors.toSet());
+
+    if (config.commentGenerationEnabled) {
+      // Analyze impacts of the annotation locally and globally for comment generation.
+      remainingFixes.forEach(
+          fix -> {
+            if (fix.isOnMethod()) {
+              MethodNode node = tree.findNode(fix.toMethod().method, fix.toMethod().clazz);
+              if (node != null && !methodCommentMap.containsKey(node.location)) {
+                String comment = "";
+                if (config.downStreamDependenciesAnalysisActivated
+                    && node.isPublicMethodWithNonPrimitiveReturnType()) {
+                  comment += " //dependent: " + analyzer.getTriggeredErrors(fix).size();
+                }
+                Report report = cache.getReportForFix(fix);
+                if (report != null) {
+                  comment += " //local: " + report.localEffect;
+                }
+                methodCommentMap.put(node.location, methodCommentMap.get(node.location) + comment);
+              }
+            }
+          });
+    }
+
+    Set<AddAnnotation> nullUnmarkedInjections =
+        nullUnmarkedMethods.stream()
+            .map(
+                onMethod ->
+                    new AddMarkerAnnotation(
+                        onMethod,
+                        config.nullUnMarkedAnnotation,
+                        config.commentGenerationEnabled
+                            ? config.commentPrefix + ":" + methodCommentMap.get(onMethod)
+                            : null))
             .collect(Collectors.toSet());
-    injector.injectAnnotations(nullUnMarkedAnnotations);
 
+    injector.injectAnnotations(nullUnmarkedInjections);
     // Update log.
-    config.log.updateInjectedAnnotations(nullUnMarkedAnnotations);
+    config.log.updateInjectedAnnotations(nullUnmarkedInjections);
 
-    // Collect explicit Nullable initialization to fields
-    Set<AddAnnotation> suppressWarningsAnnotations =
-        remainingFixes.stream()
+    // Collect suppress warnings, errors on field declaration regions.
+    Set<OnField> fieldsWithSuppressWarnings =
+        remainingErrors.stream()
             .filter(
-                fix -> {
-                  if (!(fix.isOnField() && fix.reasons.contains("ASSIGN_FIELD_NULLABLE"))) {
+                error -> {
+                  if (!error.getRegion().isOnField()) {
                     return false;
                   }
-                  OnField onField = fix.toField();
-                  return onField.clazz.equals(fix.encClass()) && fix.encMethod().equals("");
+                  // We can silence them by SuppressWarnings("NullAway.Init")
+                  return !error.messageType.equals("METHOD_NO_INIT")
+                      && !error.messageType.equals("FIELD_NO_INIT");
                 })
             .map(
-                fix ->
-                    new AddSingleElementAnnotation(
-                        fix.toField(), "SuppressWarnings", "NullAway", false))
+                error ->
+                    fieldDeclarationAnalysis.getLocationOnField(
+                        error.getRegion().clazz, error.getRegion().member))
+            .filter(Objects::nonNull)
+            .collect(Collectors.toSet());
+
+    Set<AddAnnotation> suppressWarningsAnnotations =
+        fieldsWithSuppressWarnings.stream()
+            .map(
+                onField ->
+                    new AddSingleElementAnnotation(onField, "SuppressWarnings", "NullAway", false))
             .collect(Collectors.toSet());
     injector.injectAnnotations(suppressWarningsAnnotations);
     // Update log.
@@ -318,12 +380,12 @@ private void forceResolveRemainingErrors(
                     fix.isOnField()
                         && (fix.reasons.contains("METHOD_NO_INIT")
                             || fix.reasons.contains("FIELD_NO_INIT")))
+            // Filter nodes annotated with SuppressWarnings("NullAway")
+            .filter(fix -> !fieldsWithSuppressWarnings.contains(fix.toField()))
             .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);
     // Update log.