From 07466798de713d8dc69a6ba0bafb7f4e4eebc527 Mon Sep 17 00:00:00 2001
From: Roman Shevchenko <roman.shevchenko@jetbrains.com>
Date: Wed, 4 Sep 2019 20:47:54 +0300
Subject: [PATCH] Post-review cleanup (IDEA-CR-52079)

GitOrigin-RevId: 95a6971c1dbd86de45f3807c24f2544d3796293f
---
 .../JavaTestFrameworkRunnableState.java       | 43 ++++++++---------
 .../impl/analysis/JavaModuleGraphUtil.java    | 46 ++++++++-----------
 2 files changed, 38 insertions(+), 51 deletions(-)

diff --git a/java/execution/impl/src/com/intellij/execution/JavaTestFrameworkRunnableState.java b/java/execution/impl/src/com/intellij/execution/JavaTestFrameworkRunnableState.java
index e96f4912cfc6..42c1ff61bd41 100644
--- a/java/execution/impl/src/com/intellij/execution/JavaTestFrameworkRunnableState.java
+++ b/java/execution/impl/src/com/intellij/execution/JavaTestFrameworkRunnableState.java
@@ -361,15 +361,13 @@ public abstract class JavaTestFrameworkRunnableState<T extends
   }
 
   private static void configureModulePath(JavaParameters javaParameters, @NotNull Module module) {
-    DumbService dumbService = DumbService.getInstance(module.getProject());
-    PsiJavaModule currentModule =
-      dumbService.computeWithAlternativeResolveEnabled(() -> JavaModuleGraphUtil.findDescriptorByModule(module, true));
-    if (currentModule != null) {
-      //add current module explicitly as it's not reachable from `idea.rt` auto modules
+    DumbService dumb = DumbService.getInstance(module.getProject());
+    PsiJavaModule testModule = dumb.computeWithAlternativeResolveEnabled(() -> JavaModuleGraphUtil.findDescriptorByModule(module, true));
+    if (testModule != null) {
+      //adding the test module explicitly as it is unreachable from `idea.rt`
       ParametersList vmParametersList = javaParameters.getVMParametersList();
       vmParametersList.add("--add-modules");
-      vmParametersList.add(currentModule.getName());
-
+      vmParametersList.add(testModule.getName());
       //setup module path
       PathsList classPath = javaParameters.getClassPath();
       PathsList modulePath = javaParameters.getModulePath();
@@ -377,7 +375,10 @@ public abstract class JavaTestFrameworkRunnableState<T extends
       classPath.clear();
     }
     else {
-      splitDepsBetweenModuleAndClasspath(module, javaParameters);
+      PsiJavaModule prodModule = dumb.computeWithAlternativeResolveEnabled(() -> JavaModuleGraphUtil.findDescriptorByModule(module, false));
+      if (prodModule != null) {
+        splitDepsBetweenModuleAndClasspath(javaParameters, module, prodModule);
+      }
     }
   }
 
@@ -385,25 +386,18 @@ public abstract class JavaTestFrameworkRunnableState<T extends
    * Put dependencies reachable from module-info located in production sources on the module path
    * leave all other dependencies on the class path as is
    */
-  private static void splitDepsBetweenModuleAndClasspath(Module module, JavaParameters javaParameters) {
-    PsiJavaModule prodModule =
-      DumbService.getInstance(module.getProject())
-        .computeWithAlternativeResolveEnabled(() -> JavaModuleGraphUtil.findDescriptorByModule(module, false));
-    if (prodModule == null) {
-      return;
-    }
+  private static void splitDepsBetweenModuleAndClasspath(JavaParameters javaParameters, Module module, PsiJavaModule prodModule) {
     CompilerModuleExtension compilerExt = CompilerModuleExtension.getInstance(module);
-    if (compilerExt == null) {
-      return;
-    }
+    if (compilerExt == null) return;
+
     PathsList modulePath = javaParameters.getModulePath();
     PathsList classPath = javaParameters.getClassPath();
 
     //put all transitive required modules on the module path
-    Set<PsiJavaModule> allRequires = JavaModuleGraphUtil.getAllRequires(prodModule);
-    JarFileSystem instance = JarFileSystem.getInstance();
+    Set<PsiJavaModule> allRequires = JavaModuleGraphUtil.getAllDependencies(prodModule);
+    JarFileSystem jarFS = JarFileSystem.getInstance();
     for (PsiJavaModule javaModule : allRequires) {
-      VirtualFile virtualFile = instance.getLocalVirtualFileFor(PsiImplUtil.getModuleVirtualFile(javaModule));
+      VirtualFile virtualFile = jarFS.getLocalVirtualFileFor(PsiImplUtil.getModuleVirtualFile(javaModule));
       if (virtualFile != null) {
         classPath.remove(virtualFile.getPath());
         modulePath.add(virtualFile.getPath());
@@ -417,7 +411,6 @@ public abstract class JavaTestFrameworkRunnableState<T extends
       classPath.remove(out.getPath());
       modulePath.add(out.getPath());
     }
-
     //ensure test output is merged to the production module
     VirtualFile testOutput = compilerExt.getCompilerOutputPathForTests();
     if (testOutput != null) {
@@ -425,12 +418,12 @@ public abstract class JavaTestFrameworkRunnableState<T extends
       vmParametersList.add(prodModule.getName() + "=" + testOutput.getPath());
     }
 
-    //ensure test dependencies which are missed from production module info are available in tests
+    //ensure test dependencies missing from production module descriptor are available in tests
     //todo enumerate all test dependencies explicitly
     vmParametersList.add("--add-reads");
     vmParametersList.add(prodModule.getName() + "=ALL-UNNAMED");
 
-    //ensure production module is explicitly added as test starter doesn't depend on it explicitly
+    //ensure production module is explicitly added as test starter in `idea-rt` doesn't depend on it
     vmParametersList.add("--add-modules");
     vmParametersList.add(prodModule.getName());
   }
@@ -470,7 +463,7 @@ public abstract class JavaTestFrameworkRunnableState<T extends
   }
 
   /**
-   * Configuration based on package which spans multiple modules
+   * Configuration based on a package spanning multiple modules.
    */
   protected boolean forkPerModule() {
     return getScope() != TestSearchScope.SINGLE_MODULE &&
diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java
index 2334cd2bf334..3258e0497a50 100644
--- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java
+++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java
@@ -112,8 +112,8 @@ public class JavaModuleGraphUtil {
     return getRequiresGraph(source).reads(source, destination);
   }
 
-  public static Set<PsiJavaModule> getAllRequires(PsiJavaModule source) {
-    return getRequiresGraph(source).getAllRequires(source);
+  public static Set<PsiJavaModule> getAllDependencies(PsiJavaModule source) {
+    return getRequiresGraph(source).getAllDependencies(source);
   }
 
   @Nullable
@@ -133,7 +133,7 @@ public class JavaModuleGraphUtil {
 
   /*
    * Looks for cycles between Java modules in the project sources.
-   * Library/JDK modules are excluded - in assumption there can't be any lib -> src dependencies.
+   * Library/JDK modules are excluded - in an assumption there can't be any lib -> src dependencies.
    * Module references are resolved "globally" (i.e., without taking project dependencies into account).
    */
   private static List<Set<PsiJavaModule>> findCycles(Project project) {
@@ -213,7 +213,7 @@ public class JavaModuleGraphUtil {
     return new RequiresGraph(graph, transitiveEdges);
   }
 
-  private static void visit(PsiJavaModule module, MultiMap<PsiJavaModule, PsiJavaModule> relations, Set<? super String> transitiveEdges) {
+  private static void visit(PsiJavaModule module, MultiMap<PsiJavaModule, PsiJavaModule> relations, Set<String> transitiveEdges) {
     if (!(module instanceof LightJavaModule) && !relations.containsKey(module)) {
       relations.putValues(module, Collections.emptyList());
       boolean explicitJavaBase = false;
@@ -272,15 +272,11 @@ public class JavaModuleGraphUtil {
       return processExports(module, (pkg, m) -> packageName.equals(pkg) ? m : null);
     }
 
-    private <T> T processExports(PsiJavaModule start, BiFunction<? super String, ? super PsiJavaModule, ? extends T> processor) {
+    private <T> T processExports(PsiJavaModule start, BiFunction<String, PsiJavaModule, T> processor) {
       return myGraph.getNodes().contains(start) ? processExports(start.getName(), start, 0, new HashSet<>(), processor) : null;
     }
 
-    private <T> T processExports(String name,
-                                 PsiJavaModule module,
-                                 int layer,
-                                 Set<? super PsiJavaModule> visited,
-                                 BiFunction<? super String, ? super PsiJavaModule, ? extends T> processor) {
+    private <T> T processExports(String name, PsiJavaModule module, int layer, Set<PsiJavaModule> visited, BiFunction<String, PsiJavaModule, T> processor) {
       if (visited.add(module)) {
         if (layer == 1) {
           for (PsiPackageAccessibilityStatement statement : module.getExports()) {
@@ -292,8 +288,7 @@ public class JavaModuleGraphUtil {
           }
         }
         if (layer < 2) {
-          Iterator<PsiJavaModule> iterator = myGraph.getIn(module);
-          while (iterator.hasNext()) {
+          for (Iterator<PsiJavaModule> iterator = myGraph.getIn(module); iterator.hasNext();) {
             PsiJavaModule dependency = iterator.next();
             if (layer == 0 || myTransitiveEdges.contains(key(dependency, module))) {
               T result = processExports(name, dependency, 1, visited, processor);
@@ -310,22 +305,21 @@ public class JavaModuleGraphUtil {
       return module.getName() + '/' + exporter.getName();
     }
 
-    public Set<PsiJavaModule> getAllRequires(PsiJavaModule module) {
-      HashSet<PsiJavaModule> requires = new HashSet<>();
-      new Object() {
-        void traverse(PsiJavaModule m) {
-          for (Iterator<PsiJavaModule> iterator = myGraph.getIn(m); iterator.hasNext();) {
-            final PsiJavaModule dep = iterator.next();
-            if (!requires.contains(dep)) {
-              requires.add(dep);
-              traverse(dep);
-            }
-          }
-        }
-      }.traverse(module);
-
+    public Set<PsiJavaModule> getAllDependencies(PsiJavaModule module) {
+      Set<PsiJavaModule> requires = new HashSet<>();
+      collectDependencies(module, requires);
       return requires;
     }
+
+    private void collectDependencies(PsiJavaModule module, Set<PsiJavaModule> dependencies) {
+      for (Iterator<PsiJavaModule> iterator = myGraph.getIn(module); iterator.hasNext();) {
+        PsiJavaModule dependency = iterator.next();
+        if (!dependencies.contains(dependency)) {
+          dependencies.add(dependency);
+          collectDependencies(dependency, dependencies);
+        }
+      }
+    }
   }
 
   private static class ChameleonGraph<N> implements Graph<N> {