Index flow of nullable back to upstream through a field write (#211)

This PR is built on (~#210~ landed)

Resolves #209 by indexing flow of nullable back to upstream through field writes. Previously we only indexed flow of nullable back to upstream via parameter passing, e.g. code below:
```java
class Target{
    Object getFoo()
    void bar(Object param);
}
class Dependency {
    Target t;
    void baz(){
       t.bar(t.getFoo());
    }
}
```
We know that making `getFoo()` will trigger a fix on `bar()` parameter. This PR updates Annotator to index all flows back to upstream including field writes. Hence, in the code below:
```java
class Target{
    Object bar;
    Object getFoo();
}
class Dependency {
    Target t;
    void baz(){
       t.bar = t.getFoo();
    }
}
```
Annotator now indexes the flow of nullable back to target through write to a field and includes the corresponding fix in the processing fix tree.

Please note this PR only updates indexing flow of nullable back to upstream. Annotator still does **not** index the impact of making public fields `@Nullable` on downstream dependencies.

annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpact.java

@@ -26,39 +26,24 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
 import edu.ucr.cs.riple.core.Report;
 import edu.ucr.cs.riple.core.cache.Impact;
-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.MethodRecord;
 import edu.ucr.cs.riple.injector.location.OnMethod;
-import edu.ucr.cs.riple.injector.location.OnParameter;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.stream.Collectors;
 
 /**
  * Container class for storing overall impact of a fix applied in target module on downstream
  * dependencies. At this moment, only impact of public methods with non-primitive return are stored.
  */
 public class DownstreamImpact extends Impact {
 
-  /**
-   * Map of parameters in target module that will receive {@code Nullable} value if targeted method
-   * in node is annotated as {@code @Nullable} with their corresponding triggered errors.
-   */
-  private final HashMap<OnParameter, Set<Error>> impactedParametersMap;
-
   public DownstreamImpact(Fix fix) {
     super(fix);
     // Only store impacts of fixes targeting methods.
     Preconditions.checkArgument(
         fix.isOnMethod(),
         "Unexpected Fix instance. Only impacts of fixes on methods should be tracked for downstream dependencies");
-    this.impactedParametersMap = new HashMap<>();
     this.triggeredErrors = ImmutableSet.of();
   }
 
@@ -72,21 +57,9 @@ public int hashCode() {
    *
    * @param report Result of applying making method in node {@code @Nullable} in downstream
    *     dependencies.
-   * @param impactedParameters Set of impacted paramaters.
    */
-  public void setStatus(Report report, Set<OnParameter> impactedParameters) {
+  public void setStatus(Report report) {
     this.triggeredErrors = ImmutableSet.copyOf(report.triggeredErrors);
-    // Count the number of times each parameter received a @Nullable.
-    impactedParameters.forEach(
-        onParameter -> {
-          Set<Error> triggered =
-              triggeredErrors.stream()
-                  .filter(
-                      error ->
-                          error.isSingleFix() && error.toResolvingLocation().equals(onParameter))
-                  .collect(Collectors.toSet());
-          impactedParametersMap.put(onParameter, triggered);
-        });
   }
 
   /**
@@ -101,42 +74,6 @@ public static int hash(String method, String clazz) {
     return MethodRecord.hash(method, clazz);
   }
 
-  /**
-   * Returns set of triggered errors if method is {@code @Nullable} on downstream dependencies.
-   *
-   * @return Set of errors.
-   */
-  @Override
-  public ImmutableSet<Error> getTriggeredErrors() {
-    return triggeredErrors;
-  }
-
-  /**
-   * Updates the status of method's impact after injection of fixes in target module. Potentially
-   * part of stored impact result is invalid due to injection of fixes. (e.g. some impacted
-   * parameters may already be annotated as {@code @Nullable} and will no longer trigger errors on
-   * downstream dependencies). This method addresses this issue by updating method's status.
-   *
-   * @param fixes List of injected fixes.
-   */
-  @Override
-  public void updateStatusAfterInjection(Collection<Fix> fixes) {
-    Set<OnParameter> annotatedParameters = new HashSet<>();
-    Set<Error> temp = Sets.newHashSet(triggeredErrors);
-    fixes.forEach(
-        fix ->
-            fix.ifOnParameter(
-                onParameter -> {
-                  if (impactedParametersMap.containsKey(onParameter)) {
-                    Set<Error> errors = impactedParametersMap.get(onParameter);
-                    temp.removeAll(errors);
-                    annotatedParameters.add(onParameter);
-                  }
-                }));
-    this.triggeredErrors = ImmutableSet.copyOf(temp);
-    annotatedParameters.forEach(impactedParametersMap::remove);
-  }
-
   /**
    * Gets the containing method location.
    *

annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpactCacheImpl.java

@@ -38,7 +38,6 @@
 import edu.ucr.cs.riple.core.metadata.region.MethodRegionRegistry;
 import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation;
 import edu.ucr.cs.riple.injector.location.Location;
-import edu.ucr.cs.riple.injector.location.OnParameter;
 import java.util.Collection;
 import java.util.OptionalInt;
 import java.util.Set;
@@ -112,14 +111,11 @@ public void analyzeDownstreamDependencies() {
     this.store
         .values()
         .forEach(
-            method -> {
-              Set<OnParameter> impactedParameters =
-                  evaluator.getImpactedParameters(method.toMethod());
-              reports.stream()
-                  .filter(input -> input.root.toMethod().equals(method.toMethod()))
-                  .findAny()
-                  .ifPresent(report -> method.setStatus(report, impactedParameters));
-            });
+            methodImpact ->
+                reports.stream()
+                    .filter(input -> input.root.toMethod().equals(methodImpact.toMethod()))
+                    .findAny()
+                    .ifPresent(methodImpact::setStatus));
     System.out.println("Analyzing downstream dependencies completed!");
   }
 

annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpactEvaluator.java

@@ -29,10 +29,7 @@
 import edu.ucr.cs.riple.core.evaluators.BasicEvaluator;
 import edu.ucr.cs.riple.core.evaluators.suppliers.DownstreamDependencySupplier;
 import edu.ucr.cs.riple.core.metadata.index.Error;
-import edu.ucr.cs.riple.injector.location.OnMethod;
-import edu.ucr.cs.riple.injector.location.OnParameter;
-import java.util.Collections;
-import java.util.HashMap;
+import edu.ucr.cs.riple.injector.location.Location;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -43,16 +40,8 @@
  */
 class DownstreamImpactEvaluator extends BasicEvaluator {
 
-  /**
-   * Map of public methods in target module to parameters in target module, which are source of
-   * nullable flow back to upstream module (target) from downstream dependencies, if annotated as
-   * {@code @Nullable}.
-   */
-  private final HashMap<OnMethod, Set<OnParameter>> nullableFlowMap;
-
   public DownstreamImpactEvaluator(DownstreamDependencySupplier supplier) {
     super(supplier);
-    this.nullableFlowMap = new HashMap<>();
   }
 
   @Override
@@ -65,45 +54,27 @@ protected void collectGraphResults(ImmutableSet<Report> reports) {
             node ->
                 node.root.ifOnMethod(
                     method -> {
-                      // Impacted parameters.
-                      Set<OnParameter> parameters =
+                      // Impacted locations.
+                      Set<Location> locations =
                           node.triggeredErrors.stream()
                               .filter(
                                   error ->
                                       error.isSingleFix()
-                                          && error.toResolvingLocation().isOnParameter()
                                           // Method is declared in the target module.
                                           && context.targetModuleInfo.declaredInModule(
-                                              error.toResolvingParameter()))
-                              .map(Error::toResolvingParameter)
+                                              error.toResolvingLocation()))
+                              .map(Error::toResolvingLocation)
                               .collect(Collectors.toSet());
-                      if (!parameters.isEmpty()) {
-                        // Update path for each parameter. These triggered fixes do not have an
+                      if (!locations.isEmpty()) {
+                        // Update path for each location. These triggered fixes do not have an
                         // actual physical path since they are provided as a jar file in downstream
                         // dependencies.
-                        parameters.forEach(
-                            onParameter ->
-                                onParameter.path =
-                                    context
-                                        .targetModuleInfo
-                                        .getMethodRegistry()
-                                        .findMethodByName(
-                                            onParameter.clazz, onParameter.enclosingMethod.method)
-                                        .location
+                        locations.forEach(
+                            location ->
+                                location.path =
+                                    context.targetModuleInfo.getLocationOnClass(location.clazz)
                                         .path);
-                        nullableFlowMap.put(method, parameters);
                       }
                     }));
   }
-
-  /**
-   * Returns set of parameters that will receive {@code @Nullable} if the passed method is annotated
-   * as {@code @Nullable}.
-   *
-   * @param method Method to be annotated.
-   * @return Set of impacted parameters. If no parameter is impacted, empty set will be returned.
-   */
-  public Set<OnParameter> getImpactedParameters(OnMethod method) {
-    return nullableFlowMap.getOrDefault(method, Collections.emptySet());
-  }
 }

annotator-core/src/main/java/edu/ucr/cs/riple/core/evaluators/graph/processors/AbstractConflictGraphProcessor.java

@@ -74,14 +74,13 @@ protected Set<Fix> getTriggeredFixesFromDownstreamErrors(Node node) {
         .filter(
             error ->
                 error.isSingleFix()
-                    && error.toResolvingLocation().isOnParameter()
                     && error.isFixableOnTarget(context)
                     && !currentLocationsTargetedByTree.contains(error.toResolvingLocation()))
         .map(
             error ->
                 new Fix(
                     new AddMarkerAnnotation(
-                        error.toResolvingLocation().toParameter(), context.config.nullableAnnot),
+                        error.toResolvingLocation(), context.config.nullableAnnot),
                     "PASSING_NULLABLE",
                     false))
         .collect(Collectors.toSet());

annotator-core/src/test/java/edu/ucr/cs/riple/core/ArgumentNullableFlowTest.java → annotator-core/src/test/java/edu/ucr/cs/riple/core/NullableFlowToUpstreamTest.java

@@ -27,14 +27,16 @@
 import static com.google.common.collect.Sets.newHashSet;
 
 import edu.ucr.cs.riple.core.tools.TReport;
+import edu.ucr.cs.riple.injector.location.OnField;
 import edu.ucr.cs.riple.injector.location.OnMethod;
 import edu.ucr.cs.riple.injector.location.OnParameter;
 import java.util.Collections;
+import java.util.Set;
 import org.junit.Test;
 
-public class ArgumentNullableFlowTest extends AnnotatorBaseCoreTest {
+public class NullableFlowToUpstreamTest extends AnnotatorBaseCoreTest {
 
-  public ArgumentNullableFlowTest() {
+  public NullableFlowToUpstreamTest() {
     super("nullable-multi-modular");
   }
 
@@ -109,4 +111,46 @@ public void nullableFlowDetectionDisabledTest() {
         .disableBailOut()
         .start();
   }
+
+  @Test
+  public void nullableFlowToUpstreamThroughFieldWriteTest() {
+    coreTestHelper
+        .onTarget()
+        .withSourceLines(
+            "Bar.java",
+            "package test;",
+            "public class Bar {",
+            "    public String foo;",
+            "    public String foo2 = \"\";",
+            "    public String getFoo() {",
+            "        return foo;",
+            "    }",
+            "}")
+        .withDependency("Dep")
+        .withSourceLines(
+            "Dep.java",
+            "package test.dep;",
+            "import test.Bar;",
+            "public class Dep {",
+            "    public Bar bar = new Bar();",
+            "    public void exec() {",
+            "        bar.foo2 = bar.getFoo();",
+            "    }",
+            "}")
+        .withExpectedReports(
+            new TReport(
+                new OnField("Bar.java", "test.Bar", Set.of("foo")),
+                -1,
+                // fixes in tree:
+                Set.of(
+                    new OnMethod("Bar.java", "test.Bar", "getFoo(String)"),
+                    // coming from flow of nullable back to target through a field write.
+                    new OnField("Bar.java", "test.Bar", Set.of("foo2"))),
+                Set.of()))
+        .disableBailOut()
+        .enableDownstreamDependencyAnalysis(AnalysisMode.STRICT)
+        .toDepth(5)
+        .checkExpectedOutput("nullableflowfieldwrite/expected")
+        .start();
+  }
 }

annotator-core/src/test/resources/nullableflowfieldwrite/expected/Dep/src/main/java/test/Dep.java

@@ -0,0 +1,8 @@
+package test.dep;
+import test.Bar;
+public class Dep {
+    public Bar bar = new Bar();
+    public void exec() {
+        bar.foo2 = bar.getFoo();
+    }
+}

annotator-core/src/test/resources/nullableflowfieldwrite/expected/Target/src/main/java/test/Bar.java

@@ -0,0 +1,9 @@
+package test;
+import javax.annotation.Nullable;
+public class Bar {
+    @Nullable public String foo;
+    @Nullable public String foo2 = "";
+    @Nullable public String getFoo() {
+        return foo;
+    }
+}