Add support for indexing of impacts of changes on fields on downstrea…

…m dependencies (#221)

This PR adds support for indexing impact of changes on fields on downstream dependencies. Prior to this PR, Annotator might make a field `@Nullable` that can trigger errors on downstream dependencies, with this PR, Annotator is aware of these impacts and if changes on fields, create an unresolvable error on downstream dependencies, the containing fix tree is **rejected**.

See example below:

#### On Target
```java
public class Foo {

  public Object f0;

  public Object f1;

  public Object f2 = new Object();
}
```
#### On Dependency (Dep)
```java
package test.depa;

import test.target.Foo;

public class Dep {

    public void directAccess(Foo foo){
         // making f0 will create an unresolvable error here.
         foo.f0.toString();
    }

    public void flowF1BackToF2(Foo foo) {
      // If f1 is annotated as @nullable, the corresponding error on f2 can be resolved on target.
      foo.f2 = foo.f1;
    }
}
```


Prior to this PR, Annotator would have made `f0` `@Nullable` even thought it causes an unresolvable error in `Dep` module. With this PR, 
 - Annotator is aware of this impact and will avoid making `f0` `@Nullable`
 - Annotator is aware that the triggered error by making `f1` `@Nullable` is resolvable by making `f2` `@Nullable`, therefore it includes the corresponding fix (`f2`) in any tree containing `f1`.

annotator-core/src/main/java/edu/ucr/cs/riple/core/AnalysisMode.java

@@ -61,7 +61,7 @@ public void tag(DownstreamImpactCache downstreamImpactCache, Collection<Report>
       reports.forEach(
           report -> {
             // Check for destructive methods.
-            if (report.containsDestructiveMethod(downstreamImpactCache)) {
+            if (report.containsDestructiveChange(downstreamImpactCache)) {
               report.tag(REJECT);
               return;
             }

annotator-core/src/main/java/edu/ucr/cs/riple/core/Report.java

@@ -107,14 +107,14 @@ public void reflectAnnotationProcessorChangesOnSourceCode(ModuleInfo moduleInfo)
   }
 
   /**
-   * Checks if any of the fix in tree, will trigger an unresolvable error in downstream
+   * Checks if any of the fix in the tree, will trigger an unresolvable error in downstream
    * dependencies.
    *
    * @param cache Downstream cache to check impact of method on downstream dependencies.
    * @return true, if report contains a fix which will trigger an unresolvable error in downstream
    *     dependency.
    */
-  public boolean containsDestructiveMethod(DownstreamImpactCache cache) {
+  public boolean containsDestructiveChange(DownstreamImpactCache cache) {
     return this.tree.stream().anyMatch(cache::triggersUnresolvableErrorsOnDownstream);
   }
 

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

@@ -39,8 +39,8 @@ public DownstreamImpact(Report report) {
     super(report.root);
     // 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");
+        fix.isOnMethod() || fix.isOnField(),
+        "Unexpected Fix instance. Only impacts of fixes on methods / fields should be tracked for downstream dependencies");
     this.triggeredErrors = ImmutableSet.copyOf(report.triggeredErrors);
   }
 }

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

@@ -33,6 +33,7 @@
 import edu.ucr.cs.riple.core.registries.index.Error;
 import edu.ucr.cs.riple.core.registries.index.Fix;
 import edu.ucr.cs.riple.core.registries.method.MethodRecord;
+import edu.ucr.cs.riple.core.registries.region.FieldRegionRegistry;
 import edu.ucr.cs.riple.core.registries.region.MethodRegionRegistry;
 import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation;
 import edu.ucr.cs.riple.injector.location.Location;
@@ -41,6 +42,7 @@
 import java.util.Map;
 import java.util.OptionalInt;
 import java.util.Set;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
 
@@ -80,22 +82,34 @@ public DownstreamImpactCacheImpl(Context context) {
    */
   private ImmutableSet<Location> retrieveLocationsToCacheImpactsOnDownstreamDependencies(
       Context context, ModuleInfo moduleInfo) {
-    // Collect callers of public APIs in module.
+    ImmutableSet.Builder<Location> locationsToCache = ImmutableSet.builder();
+    // Used to collect callers of each method.
     MethodRegionRegistry methodRegionRegistry = new MethodRegionRegistry(moduleInfo);
-    return context
-        .targetModuleInfo
-        .getMethodRegistry()
-        .getPublicMethodsWithNonPrimitivesReturn()
-        .stream()
-        .map(MethodRecord::toLocation)
-        .filter(
-            input ->
-                !methodRegionRegistry
-                    .getImpactedRegionsByUse(input)
-                    // skip methods that are not called anywhere. This has a significant impact
-                    // on performance.
-                    .isEmpty())
-        .collect(ImmutableSet.toImmutableSet());
+    FieldRegionRegistry fieldRegionRegistry = new FieldRegionRegistry(moduleInfo);
+    // Collect public methods with non-primitive return types.
+    locationsToCache.addAll(
+        context
+            .targetModuleInfo
+            .getMethodRegistry()
+            .getPublicMethodsWithNonPrimitivesReturn()
+            .stream()
+            .map(MethodRecord::toLocation)
+            .filter(
+                input ->
+                    !methodRegionRegistry
+                        .getImpactedRegionsByUse(input.toMethod())
+                        // skip methods that are not called anywhere. This has a significant impact
+                        // on performance.
+                        .isEmpty())
+            .collect(Collectors.toSet()));
+    // Collect public fields with non-primitive types.
+    locationsToCache.addAll(
+        context.targetModuleInfo.getFieldRegistry().getPublicFieldWithNonPrimitiveType().stream()
+            // skip fields that are not accessed anywhere. This has a significant impact
+            // on performance.
+            .filter(onField -> !fieldRegionRegistry.getImpactedRegionsByUse(onField).isEmpty())
+            .collect(Collectors.toSet()));
+    return locationsToCache.build();
   }
 
   @Override
@@ -107,10 +121,9 @@ public void analyzeDownstreamDependencies() {
         retrieveLocationsToCacheImpactsOnDownstreamDependencies(context, supplier.getModuleInfo())
             .stream()
             .map(
-                downstreamImpact ->
+                location ->
                     new Fix(
-                        new AddMarkerAnnotation(
-                            downstreamImpact.toMethod(), context.config.nullableAnnot),
+                        new AddMarkerAnnotation(location, context.config.nullableAnnot),
                         "null",
                         false))
             .collect(ImmutableSet.toImmutableSet());
@@ -126,16 +139,17 @@ public void analyzeDownstreamDependencies() {
   }
 
   /**
-   * Retrieves the corresponding {@link DownstreamImpact} to a fix.
+   * Retrieves the corresponding {@link DownstreamImpact} to a fix. The fix should be targeting a
+   * method or a field.
    *
    * @param fix Target fix.
    * @return Corresponding {@link DownstreamImpact}, null if not located.
    */
   @Nullable
   @Override
   public DownstreamImpact fetchImpact(Fix fix) {
-    if (!fix.isOnMethod()) {
-      // we currently store only impacts of fixes for methods on downstream dependencies.
+    if (!(fix.isOnMethod() || fix.isOnField())) {
+      // we currently store only impacts of fixes for methods / fields on downstream dependencies.
       return null;
     }
     return super.fetchImpact(fix);
@@ -184,7 +198,6 @@ public boolean triggersUnresolvableErrorsOnDownstream(Fix fix) {
   @Override
   public ImmutableSet<Error> getTriggeredErrorsForCollection(Collection<Fix> fixTree) {
     return fixTree.stream()
-        .filter(Fix::isOnMethod)
         .flatMap(
             fix -> {
               DownstreamImpact impact = fetchImpact(fix);
@@ -195,8 +208,8 @@ public ImmutableSet<Error> getTriggeredErrorsForCollection(Collection<Fix> fixTr
 
   @Override
   public ImmutableSet<Error> getTriggeredErrors(Fix fix) {
-    // We currently only store impact of methods on downstream dependencies.
-    if (!fix.isOnMethod()) {
+    // We currently only store impact of methods / fields on downstream dependencies.
+    if (!(fix.isOnMethod() || fix.isOnField())) {
       return ImmutableSet.of();
     }
     DownstreamImpact impact = fetchImpact(fix);

annotator-core/src/main/java/edu/ucr/cs/riple/core/module/ModuleInfo.java

@@ -159,6 +159,15 @@ public ImmutableSet<ModuleConfiguration> getModuleConfigurations() {
    * @return True if the passed location is declared in this moduleInfo's modules, false otherwise.
    */
   public boolean declaredInModule(Location location) {
+    if (location.isOnParameter()) {
+      location = location.toMethod();
+    }
+    if (location.isOnMethod()) {
+      return methodRegistry.declaredInModule(location);
+    }
+    if (location.isOnField()) {
+      return fieldRegistry.declaredInModule(location);
+    }
     return methodRegistry.declaredInModule(location);
   }
 

annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/field/ClassFieldRecord.java

@@ -96,6 +96,25 @@ public void addFieldDeclaration(FieldDeclaration fieldDeclaration) {
     this.fields.add(new FieldDeclarationRecord(fieldDeclaration));
   }
 
+  /**
+   * Checks if the class has a field declaration with exactly the specified variable names. For
+   * instance, for a class:
+   *
+   * <pre>{@code
+   * class Example {
+   *     Object a, b, c;
+   * }
+   * }</pre>
+   *
+   * The call with ["a", "b", "c"] will return true, but for ["a", "b"] or ["a"] will return false.
+   *
+   * @param names The set of names to be checked for a matching field declaration.
+   * @return true if there exists a field declaration with exactly the given names; false otherwise.
+   */
+  public boolean hasExactFieldDeclarationWithNames(Set<String> names) {
+    return this.fields.stream().anyMatch(decl -> decl.names.equals(names));
+  }
+
   /** Field declaration record. Used to store information regarding multiple field declaration. */
   public static class FieldDeclarationRecord {
 
@@ -115,5 +134,14 @@ public FieldDeclarationRecord(FieldDeclaration fieldDeclaration) {
       this.type = fieldDeclaration.getVariables().getFirst().get().getType();
       this.fieldDeclaration = fieldDeclaration;
     }
+
+    /**
+     * Checks if the field declaration is public and has non-primitive type.
+     *
+     * @return true, if the field declaration is public and has non-primitive type.
+     */
+    public boolean isPublicFieldWithNonPrimitiveType() {
+      return fieldDeclaration.isPublic() && !type.isPrimitiveType();
+    }
   }
 }

annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/field/FieldRegistry.java

@@ -37,6 +37,7 @@
 import edu.ucr.cs.riple.core.registries.Registry;
 import edu.ucr.cs.riple.injector.Helper;
 import edu.ucr.cs.riple.injector.exceptions.TargetClassNotFound;
+import edu.ucr.cs.riple.injector.location.Location;
 import edu.ucr.cs.riple.injector.location.OnClass;
 import edu.ucr.cs.riple.injector.location.OnField;
 import edu.ucr.cs.riple.scanner.Serializer;
@@ -46,6 +47,7 @@
 import java.util.Collections;
 import java.util.Optional;
 import java.util.Set;
+import javax.annotation.Nullable;
 
 /**
  * Stores field declaration data on classes. It can Detect multiple inline field declarations. An
@@ -214,4 +216,52 @@ public OnClass getLocationOnClass(String clazz) {
     }
     return new OnClass(candidate.pathToSourceFile, candidate.clazz);
   }
+
+  /**
+   * Returns fields with public visibility and a non-primitive return type.
+   *
+   * @return ImmutableSet of fields location.
+   */
+  public ImmutableSet<OnField> getPublicFieldWithNonPrimitiveType() {
+    ImmutableSet.Builder<OnField> builder = ImmutableSet.builder();
+    contents
+        .values()
+        .forEach(
+            record ->
+                record.fields.forEach(
+                    fieldDeclarationRecord -> {
+                      if (fieldDeclarationRecord.isPublicFieldWithNonPrimitiveType()) {
+                        builder.add(
+                            new OnField(
+                                record.pathToSourceFile,
+                                record.clazz,
+                                fieldDeclarationRecord.names));
+                      }
+                    }));
+    return builder.build();
+  }
+
+  /**
+   * Checks if the given location is a field and declared in the module this registry is created
+   * for.
+   *
+   * @param location Location of check.
+   * @return True if the given location is a field and declared in the module this registry is
+   *     created for.
+   */
+  public boolean declaredInModule(@Nullable Location location) {
+    if (location == null || location.clazz.equals("null")) {
+      return false;
+    }
+    if (!location.isOnField()) {
+      return false;
+    }
+    OnField onField = location.toField();
+    return findRecordWithHashHint(
+            node ->
+                node.clazz.equals(location.clazz)
+                    && node.hasExactFieldDeclarationWithNames(onField.variables),
+            ClassFieldRecord.hash(location.clazz))
+        != null;
+  }
 }

annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/index/Error.java

@@ -170,7 +170,7 @@ public boolean equals(Object o) {
    * @return true, if error is resolvable via fixes on target module.
    */
   public boolean isFixableOnTarget(Context context) {
-    return resolvingFixes.size() > 0
+    return !resolvingFixes.isEmpty()
         && this.resolvingFixes.stream()
             .allMatch(fix -> context.targetModuleInfo.declaredInModule(fix.toLocation()));
   }

annotator-core/src/test/java/edu/ucr/cs/riple/core/DownstreamAnalysisTest.java

@@ -24,8 +24,14 @@
 
 package edu.ucr.cs.riple.core;
 
+import static edu.ucr.cs.riple.core.AnalysisMode.STRICT;
+import static edu.ucr.cs.riple.core.Report.Tag.APPROVE;
+import static edu.ucr.cs.riple.core.Report.Tag.REJECT;
+
 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 java.util.Set;
 import org.junit.Test;
 
 public class DownstreamAnalysisTest extends AnnotatorBaseCoreTest {
@@ -89,6 +95,52 @@ public void publicMethodWithDownstreamDependencyDisabled() {
         .start();
   }
 
+  @Test
+  public void publicFieldWithDownstreamDependencyEnabled() {
+    coreTestHelper
+        .onTarget()
+        .withSourceFile("Foo.java", "downstreamDependencyFieldCheck/Foo.java")
+        .withDependency("DepA")
+        .withSourceFile("DepA.java", "downstreamDependencyFieldCheck/DepA.java")
+        .withDependency("DepB")
+        .withSourceFile("DepB.java", "downstreamDependencyFieldCheck/DepB.java")
+        .withDependency("DepC")
+        .withSourceFile("DepC.java", "downstreamDependencyFieldCheck/DepC.java")
+        .withExpectedReports(
+            // Effect on target is -1, Effect on DepA is 0 and on DepB and DepC is 1 ->
+            // Lower bound is 2. And overall effect is -1 + 2 = 1. Effect is greater than 0 and
+            // triggers unresolved errors on downstream dependencies, hence the tree should be
+            // tagged as REJECT.
+            new TReport(new OnField("Foo.java", "test.target.Foo", Set.of("f")), 1, REJECT),
+            // Effect on target is -2. Root is on f1, but it triggers making f @Nullable as well.
+            // Fix tree containing both fixes resolves two errors leaving no remaining error, effect
+            // on target is -2. But f creates 2 errors on downstream dependencies, hence the lower
+            // bound is 2. Overall effect is -2 + 2 = 0. Since f creates unresolved errors on
+            // downstream dependencies, the tree should be tagged as REJECT even though the overall
+            // effect is not greater than 0.
+            new TReport(new OnField("Foo.java", "test.target.Foo", Set.of("f1")), 0, REJECT),
+            // Effect on target is -1. Root is on f2, but it triggers making f3 @Nullable through an
+            // assignment in DepA and the tree is extended to include the corresponding fix. Since,
+            // f2 creates a resolvable error in downstream dependencies that the corresponding fix
+            // is present in the fix tree, the lower bound effect is 0. Overall effect is -1 + 0 =
+            // -1. Since the overall effect is less than 0, with no error in downstream
+            // dependencies, the tree should be tagged as APPROVE.
+            new TReport(new OnField("Foo.java", "test.target.Foo", Set.of("f2")), -1, APPROVE))
+        .setPredicate(
+            (expected, found) ->
+                // check for root equality
+                expected.root.equals(found.root)
+                    && expected.getOverallEffect(coreTestHelper.getConfig())
+                        // check for overall effect equality
+                        == found.getOverallEffect(coreTestHelper.getConfig())
+                    // check for tag equality
+                    && expected.getTag().equals(found.getTag()))
+        .toDepth(5)
+        .disableBailOut()
+        .enableDownstreamDependencyAnalysis(STRICT)
+        .start();
+  }
+
   @Test
   public void lowerBoundComputationTest() {
     coreTestHelper

annotator-core/src/test/resources/downstreamDependencyFieldCheck/DepA.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2022 University of California, Riverside.
+ *
+ * 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 test.depa;
+
+import test.target.Foo;
+
+public class DepA {
+
+    public void flowF2BackToF3(Foo foo) {
+      // If f2 is annotated as @Nullable, the corresponding error on f3 can be resolved on target.
+      foo.f3 = foo.f2;
+    }
+}