Treat impacted parameters from downstream dependencies as triggered f…

…ixes. (#65) Resolves #64 by treating impacted parameters by downstream dependencies as triggered fixes while processing public methods. It resolves the issue with race condition in unit tests as they are using a shared `library model loader`. This is resolved in the unit test infrastructure within this PR. It also resolves issue with updating the stored impacts in downstream dependencies after each injection to prevent double counting. It also contains huge refactoring over creation of `Explorers`.
ucr-riple · Sep 7, 2022 · c824d26 · c824d26
1 parent f77349c
commit c824d26
Show file tree

Hide file tree

Showing 31 changed files with 1,377 additions and 568 deletions.
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
@@ -26,16 +26,18 @@
 
 import com.google.common.collect.ImmutableSet;
 import edu.ucr.cs.riple.core.explorers.BasicExplorer;
-import edu.ucr.cs.riple.core.explorers.DownStreamDependencyExplorer;
 import edu.ucr.cs.riple.core.explorers.ExhaustiveExplorer;
 import edu.ucr.cs.riple.core.explorers.Explorer;
 import edu.ucr.cs.riple.core.explorers.OptimizedExplorer;
+import edu.ucr.cs.riple.core.explorers.suppliers.ExhaustiveSupplier;
+import edu.ucr.cs.riple.core.explorers.suppliers.TargetModuleSupplier;
+import edu.ucr.cs.riple.core.global.GlobalAnalyzer;
+import edu.ucr.cs.riple.core.global.GlobalAnalyzerImpl;
+import edu.ucr.cs.riple.core.global.NoOpGlobalAnalyzer;
 import edu.ucr.cs.riple.core.injectors.AnnotationInjector;
 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.trackers.CompoundTracker;
@@ -45,24 +47,34 @@
 import edu.ucr.cs.riple.injector.location.OnField;
 import edu.ucr.cs.riple.scanner.Serializer;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+/**
+ * The main class of the core module. Responsible for analyzing the target module and injecting the
+ * corresponding annotations.
+ */
 public class Annotator {
 
+  /** Injector instance. */
   private final AnnotationInjector injector;
+  /** Annotator config. */
   private final Config config;
-  // Set does not have get method, here we use map which retrieves elements efficiently.
-  public final HashMap<Fix, Report> reports;
+  /**
+   * Map of fix to their corresponding reports, used to detect processed fixes in a new batch of fix
+   * suggestion. Note: Set does not have get method, here we use map which retrieves elements
+   * efficiently.
+   */
+  public final HashMap<Fix, Report> cachedReports;
 
   public Annotator(Config config) {
     this.config = config;
-    this.reports = new HashMap<>();
+    this.cachedReports = new HashMap<>();
     this.injector = new PhysicalInjector(config);
   }
 
+  /** Starts the annotating process consist of preprocess followed by explore phase. */
   public void start() {
     preprocess();
     long timer = config.log.startTimer();
@@ -71,10 +83,20 @@ public void start() {
     Utility.writeLog(config);
   }
 
+  /**
+   * Performs all the preprocessing tasks.
+   *
+   * <ul>
+   *   <li>Performs the first build of the target module.
+   *   <li>Detects uninitialized fields.
+   *   <li>Detects initializer method candidates.
+   *   <li>Marks selected initializer methods with {@code @Initializer} annotation.
+   * </ul>
+   */
   private void preprocess() {
     System.out.println("Preprocessing...");
     Utility.setScannerCheckerActivation(config.target, true);
-    this.reports.clear();
+    this.cachedReports.clear();
     System.out.println("Making the first build...");
     Utility.buildTarget(config, true);
     Set<OnField> uninitializedFields =
@@ -92,98 +114,95 @@ private void preprocess() {
     this.injector.injectAnnotations(initializers);
   }
 
+  /** Performs iterations of inference/injection until no unseen fix is suggested. */
   private void explore() {
     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));
-    // downStreamDependencyExplorer analyzes effects of all public APIs on downstream dependencies.
+    // 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
     // result in each iteration, therefore we perform the analysis only once and reuse it in each
     // iteration.
-    DownStreamDependencyExplorer downStreamDependencyExplorer =
-        new DownStreamDependencyExplorer(config, tree);
-    if (config.downStreamDependenciesAnalysisActivated) {
-      downStreamDependencyExplorer.explore();
-    }
-    // Set of fixes collected from downstream dependencies that are triggered due to changes in the
-    // upstream module (target) public API.
-    Set<Fix> triggeredFixesFromDownstreamDependencies = new HashSet<>();
-
-    while (true) {
-      Utility.buildTarget(config);
-      ImmutableSet<Fix> fixes =
-          Stream.concat(
-                  triggeredFixesFromDownstreamDependencies.stream(),
-                  Utility.readFixesFromOutputDirectory(
-                      config.target, Fix.factory(config, fieldDeclarationAnalysis)))
-              .filter(fix -> !config.useCache || !reports.containsKey(fix))
-              .collect(ImmutableSet.toImmutableSet());
-      Bank<Error> errorBank = new Bank<>(config.target.dir.resolve("errors.tsv"), Error::new);
-      Bank<Fix> fixBank =
-          new Bank<>(
-              config.target.dir.resolve("fixes.tsv"),
-              Fix.factory(config, fieldDeclarationAnalysis));
-      tree = new MethodDeclarationTree(config.target.dir.resolve(Serializer.METHOD_INFO_FILE_NAME));
-      RegionTracker tracker = new CompoundTracker(config.target, tree);
-      Explorer explorer =
-          config.exhaustiveSearch
-              ? new ExhaustiveExplorer(injector, errorBank, fixBank, fixes, tree, config)
-              : config.optimized
-                  ? new OptimizedExplorer(
-                      injector, errorBank, fixBank, tracker, fixes, tree, config.depth, config)
-                  : new BasicExplorer(injector, errorBank, fixBank, fixes, tree, config);
-      ImmutableSet<Report> latestReports = explorer.explore();
-      int sizeBefore = reports.size();
+    GlobalAnalyzer globalAnalyzer =
+        config.downStreamDependenciesAnalysisActivated
+            ? new GlobalAnalyzerImpl(config, tree)
+            : new NoOpGlobalAnalyzer();
+    globalAnalyzer.analyzeDownstreamDependencies();
+    boolean noNewFixTriggered = false;
+    while (!noNewFixTriggered) {
+      ImmutableSet<Report> latestReports =
+          executeNextIteration(globalAnalyzer, fieldDeclarationAnalysis);
+      int sizeBefore = cachedReports.size();
+      // Update cached reports store.
       latestReports.forEach(
           report -> {
             if (config.downStreamDependenciesAnalysisActivated) {
-              report.computeBoundariesOfEffectivenessOnDownstreamDependencies(
-                  downStreamDependencyExplorer);
+              report.computeBoundariesOfEffectivenessOnDownstreamDependencies(globalAnalyzer);
             }
-            reports.putIfAbsent(report.root, report);
-            reports.get(report.root).localEffect = report.localEffect;
-            reports.get(report.root).finished = report.finished;
-            reports.get(report.root).tree = report.tree;
-            reports.get(report.root).triggered = report.triggered;
+            cachedReports.putIfAbsent(report.root, report);
+            Report cachedReport = cachedReports.get(report.root);
+            cachedReport.localEffect = report.localEffect;
+            cachedReport.finished = report.finished;
+            cachedReport.tree = report.tree;
+            cachedReport.triggeredFixes = report.triggeredFixes;
+            cachedReport.triggeredErrors = report.triggeredErrors;
           });
-      injector.injectFixes(
+
+      // Inject approved fixes.
+      Set<Fix> selectedFixes =
           latestReports.stream()
               .filter(report -> report.getOverallEffect(config) < 1)
               .flatMap(report -> config.chain ? report.tree.stream() : Stream.of(report.root))
-              .collect(Collectors.toSet()));
-      // Collect impacted parameters from changes in target module due to usages in downstream
-      // dependencies.
-      if (config.downStreamDependenciesAnalysisActivated) {
-        triggeredFixesFromDownstreamDependencies =
-            latestReports.stream()
-                .flatMap(
-                    report ->
-                        downStreamDependencyExplorer.getImpactedParameters(report.tree).stream()
-                            .map(
-                                onParameter ->
-                                    new Fix(
-                                        new AddAnnotation(onParameter, config.nullableAnnot),
-                                        "PASSING_NULLABLE",
-                                        onParameter.clazz,
-                                        onParameter.method)))
-                // Remove already processed fixes
-                .filter(input -> !reports.containsKey(input))
-                .collect(Collectors.toSet());
-      }
-      if (sizeBefore == this.reports.size()
-          && triggeredFixesFromDownstreamDependencies.size() == 0) {
-        System.out.println("\nFinished annotating.");
-        Utility.writeReports(
-            config, reports.values().stream().collect(ImmutableSet.toImmutableSet()));
-        return;
-      }
+              .collect(Collectors.toSet());
+      injector.injectFixes(selectedFixes);
+
+      // Update impact saved state.
+      globalAnalyzer.updateImpactsAfterInjection(selectedFixes);
+
       if (config.disableOuterLoop) {
-        return;
+        break;
       }
+      noNewFixTriggered = sizeBefore == this.cachedReports.size();
     }
+    System.out.println("\nFinished annotating.");
+    Utility.writeReports(
+        config, cachedReports.values().stream().collect(ImmutableSet.toImmutableSet()));
+  }
+
+  /**
+   * Executes the next iteration of exploration.
+   *
+   * @param globalAnalyzer Global Analyzer instance.
+   * @param fieldDeclarationAnalysis Field Declaration analysis to detect fixes on multiple inline
+   *     field declaration statements.
+   * @return Immutable set of reports from the executed iteration.
+   */
+  private ImmutableSet<Report> executeNextIteration(
+      GlobalAnalyzer globalAnalyzer, FieldDeclarationAnalysis fieldDeclarationAnalysis) {
+    Utility.buildTarget(config);
+    // Suggested fixes of target at the current state.
+    ImmutableSet<Fix> fixes =
+        Utility.readFixesFromOutputDirectory(
+                config.target, Fix.factory(config, fieldDeclarationAnalysis))
+            .filter(fix -> !config.useCache || !cachedReports.containsKey(fix))
+            .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);
+    TargetModuleSupplier supplier = new TargetModuleSupplier(config, tree);
+    Explorer explorer =
+        config.exhaustiveSearch
+            ? new ExhaustiveExplorer(fixes, new ExhaustiveSupplier(config, tree))
+            : config.optimized
+                ? new OptimizedExplorer(fixes, supplier, globalAnalyzer, tracker)
+                : new BasicExplorer(fixes, supplier, globalAnalyzer);
+    // Result of the iteration analysis.
+    return explorer.explore();
   }
 }
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
@@ -263,7 +263,7 @@ public Config(String[] args) {
 
     HelpFormatter formatter = new HelpFormatter();
     CommandLineParser parser = new DefaultParser();
-    CommandLine cmd = null;
+    CommandLine cmd;
 
     if (args.length == 1 && (args[0].equals("-h") || args[0].equals("--help"))) {
       showHelp(formatter, options);

diff --git a/core/src/main/java/edu/ucr/cs/riple/core/Main.java b/core/src/main/java/edu/ucr/cs/riple/core/Main.java
@@ -26,8 +26,15 @@
 
 import java.nio.file.Paths;
 
+/** Starting point. */
 public class Main {
 
+  /**
+   * Starting point.
+   *
+   * @param args if flag '--path' is found, all configurations will be set up based on the given
+   *     json file, otherwise they will be set up according to the set of received cli arguments.
+   */
   public static void main(String[] args) {
     Config config;
     if (args.length == 2 && args[0].equals("--path")) {

diff --git a/core/src/main/java/edu/ucr/cs/riple/core/Report.java b/core/src/main/java/edu/ucr/cs/riple/core/Report.java
@@ -24,11 +24,13 @@
 
 package edu.ucr.cs.riple.core;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
-import edu.ucr.cs.riple.core.explorers.DownStreamDependencyExplorer;
+import edu.ucr.cs.riple.core.global.GlobalAnalyzer;
+import edu.ucr.cs.riple.core.metadata.index.Error;
 import edu.ucr.cs.riple.core.metadata.index.Fix;
 import edu.ucr.cs.riple.injector.location.Location;
-import java.util.HashSet;
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -48,7 +50,11 @@ public class Report {
   /**
    * Set of fixes that will be triggered in target module if fix tree is applied to the source code.
    */
-  public Set<Fix> triggered;
+  public ImmutableSet<Fix> triggeredFixes;
+  /**
+   * Set of fixes that will be triggered in target module if fix tree is applied to the source code.
+   */
+  public ImmutableList<Error> triggeredErrors;
   /** If true, all leaves of fix tree are not resolvable by any {@code @Nullable} annotation. */
   public boolean finished;
   /**
@@ -67,7 +73,8 @@ public Report(Fix root, int localEffect) {
     this.root = root;
     this.tree = Sets.newHashSet(root);
     this.finished = false;
-    this.triggered = new HashSet<>();
+    this.triggeredFixes = ImmutableSet.of();
+    this.triggeredErrors = ImmutableList.of();
     this.lowerBoundEffectOnDownstreamDependencies = 0;
     this.upperBoundEffectOnDownstreamDependencies = 0;
   }
@@ -107,17 +114,15 @@ public boolean testEquals(Config config, Report other) {
     }
     this.tree.add(this.root);
     other.tree.add(other.root);
-    Set<Location> thisTree =
-        this.tree.stream().map(fix -> fix.change.location).collect(Collectors.toSet());
-    Set<Location> otherTree =
-        other.tree.stream().map(fix -> fix.change.location).collect(Collectors.toSet());
+    Set<Location> thisTree = this.tree.stream().map(Fix::toLocation).collect(Collectors.toSet());
+    Set<Location> otherTree = other.tree.stream().map(Fix::toLocation).collect(Collectors.toSet());
     if (!thisTree.equals(otherTree)) {
       return false;
     }
     Set<Location> thisTriggered =
-        this.triggered.stream().map(fix -> fix.change.location).collect(Collectors.toSet());
+        this.triggeredFixes.stream().map(Fix::toLocation).collect(Collectors.toSet());
     Set<Location> otherTriggered =
-        other.triggered.stream().map(fix -> fix.change.location).collect(Collectors.toSet());
+        other.triggeredFixes.stream().map(Fix::toLocation).collect(Collectors.toSet());
     return otherTriggered.equals(thisTriggered);
   }
 
@@ -128,7 +133,7 @@ public String toString() {
         + ", "
         + root
         + ", "
-        + tree.stream().map(fix -> fix.change.location).collect(Collectors.toSet());
+        + tree.stream().map(Fix::toLocation).collect(Collectors.toSet());
   }
 
   /**
@@ -137,8 +142,7 @@ public String toString() {
    *
    * @param explorer Downstream dependency instance.
    */
-  public void computeBoundariesOfEffectivenessOnDownstreamDependencies(
-      DownStreamDependencyExplorer explorer) {
+  public void computeBoundariesOfEffectivenessOnDownstreamDependencies(GlobalAnalyzer explorer) {
     this.lowerBoundEffectOnDownstreamDependencies =
         explorer.computeLowerBoundOfNumberOfErrors(tree);
     this.upperBoundEffectOnDownstreamDependencies =
@@ -150,12 +154,14 @@ public void computeBoundariesOfEffectivenessOnDownstreamDependencies(
    * dependency analysis is activated, overall effect will be sum of local effect and lower bound of
    * number of errors on downstream dependencies.
    *
+   * @param config Annotator config.
    * @return Overall effect ot applying the fix tree.
    */
   public int getOverallEffect(Config config) {
-    return config.downStreamDependenciesAnalysisActivated
-        ? this.localEffect + this.lowerBoundEffectOnDownstreamDependencies
-        : this.localEffect;
+    if (config.downStreamDependenciesAnalysisActivated) {
+      return this.localEffect + this.lowerBoundEffectOnDownstreamDependencies;
+    }
+    return this.localEffect;
   }
 
   /**
@@ -175,4 +181,16 @@ public int getLowerBoundEffectOnDownstreamDependencies() {
   public int getUpperBoundEffectOnDownstreamDependencies() {
     return upperBoundEffectOnDownstreamDependencies;
   }
+
+  /**
+   * Checks if the report needs further investigation. If a fix is suggested from downstream
+   * dependencies, it should still be included the next cycle.
+   *
+   * @param config Annotator config instance.
+   * @return true, if report needs further investigation.
+   */
+  public boolean isInProgress(Config config) {
+    return (!finished && (!config.bailout || localEffect > 0))
+        || triggeredFixes.stream().anyMatch(input -> !input.fixSourceIsInTarget);
+  }
 }