reuse more precise cyclic dependencies computation from 'Project Structure' dialog in compiler checks: particularly don't report cycles consisting of module tests if corresponding modules don't have test source roots

java/compiler/impl/testSrc/com/intellij/compiler/ModuleCompilerUtilTest.java

@@ -0,0 +1,84 @@
+/*
+ * Copyright 2000-2016 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.intellij.compiler;
+
+import com.intellij.openapi.module.Module;
+import com.intellij.openapi.roots.DependencyScope;
+import com.intellij.openapi.roots.ModuleRootModificationUtil;
+import com.intellij.testFramework.ModuleTestCase;
+import com.intellij.testFramework.PsiTestUtil;
+import com.intellij.testFramework.fixtures.TempDirTestFixture;
+import com.intellij.testFramework.fixtures.impl.TempDirTestFixtureImpl;
+import com.intellij.util.Chunk;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * @author nik
+ */
+public class ModuleCompilerUtilTest extends ModuleTestCase {
+  private TempDirTestFixture myTempDirTestFixture;
+
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+    myTempDirTestFixture = new TempDirTestFixtureImpl();
+    myTempDirTestFixture.setUp();
+  }
+
+  @Override
+  protected void tearDown() throws Exception {
+    myTempDirTestFixture.tearDown();
+    super.tearDown();
+  }
+
+  public void testNoCyclicDependencies() throws IOException {
+    Module a = createModule("a");
+    Module b = createModule("b");
+    PsiTestUtil.addSourceRoot(a, myTempDirTestFixture.findOrCreateDir("a-main"));
+    PsiTestUtil.addSourceRoot(a, myTempDirTestFixture.findOrCreateDir("a-tests"), true);
+    PsiTestUtil.addSourceRoot(b, myTempDirTestFixture.findOrCreateDir("b-main"));
+    PsiTestUtil.addSourceRoot(b, myTempDirTestFixture.findOrCreateDir("b-tests"), true);
+    ModuleRootModificationUtil.addDependency(a, b);
+    assertEmpty(ModuleCompilerUtil.getCyclicDependencies(myProject, Arrays.asList(a, b)));
+  }
+
+  public void testDoNotReportTestsCyclesIncludedIntoProductionCycles() throws IOException {
+    Module a = createModule("a");
+    Module b = createModule("b");
+    PsiTestUtil.addSourceRoot(a, myTempDirTestFixture.findOrCreateDir("a-main"));
+    PsiTestUtil.addSourceRoot(a, myTempDirTestFixture.findOrCreateDir("a-tests"), true);
+    PsiTestUtil.addSourceRoot(b, myTempDirTestFixture.findOrCreateDir("b-main"));
+    PsiTestUtil.addSourceRoot(b, myTempDirTestFixture.findOrCreateDir("b-tests"), true);
+    ModuleRootModificationUtil.addDependency(a, b);
+    ModuleRootModificationUtil.addDependency(b, a);
+    List<Chunk<ModuleSourceSet>> cycles = ModuleCompilerUtil.getCyclicDependencies(myProject, Arrays.asList(a, b));
+    assertEquals(1, cycles.size());
+  }
+
+  public void testIgnoreEmptySourceSets() throws IOException {
+    Module a = createModule("a");
+    Module b = createModule("b");
+    PsiTestUtil.addSourceRoot(a, myTempDirTestFixture.findOrCreateDir("a-main"));
+    PsiTestUtil.addSourceRoot(b, myTempDirTestFixture.findOrCreateDir("b-main"));
+    PsiTestUtil.addSourceRoot(b, myTempDirTestFixture.findOrCreateDir("b-tests"), true);
+    ModuleRootModificationUtil.addDependency(a, b);
+    ModuleRootModificationUtil.addDependency(b, a, DependencyScope.TEST, false);
+    assertEmpty(ModuleCompilerUtil.getCyclicDependencies(myProject, Arrays.asList(a, b)));
+  }
+}

java/compiler/openapi/src/com/intellij/compiler/ModuleCompilerUtil.java

@@ -24,6 +24,7 @@ import com.intellij.openapi.module.ModuleManager;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.roots.*;
 import com.intellij.openapi.roots.ui.configuration.DefaultModulesProvider;
+import com.intellij.openapi.roots.ui.configuration.ModulesProvider;
 import com.intellij.openapi.util.Condition;
 import com.intellij.openapi.util.Couple;
 import com.intellij.util.Chunk;
@@ -32,6 +33,7 @@ import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.graph.*;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.jps.model.java.JavaSourceRootType;
 
 import java.util.*;
 
@@ -177,8 +179,7 @@ public final class ModuleCompilerUtil {
   }
 
   public static List<Chunk<ModuleSourceSet>> getCyclicDependencies(@NotNull Project project, @NotNull List<Module> modules) {
-    Graph<ModuleSourceSet> graph = createModuleSourceDependenciesGraph(new DefaultModulesProvider(project));
-    Collection<Chunk<ModuleSourceSet>> chunks = GraphAlgorithms.getInstance().computeStronglyConnectedComponents(graph);
+    Collection<Chunk<ModuleSourceSet>> chunks = computeSourceSetCycles(new DefaultModulesProvider(project));
     final Set<Module> modulesSet = new HashSet<Module>(modules);
     return ContainerUtil.filter(chunks, new Condition<Chunk<ModuleSourceSet>>() {
       @Override
@@ -193,7 +194,7 @@ public final class ModuleCompilerUtil {
     });
   }
 
-  public static Graph<ModuleSourceSet> createModuleSourceDependenciesGraph(final RootModelProvider provider) {
+  private static Graph<ModuleSourceSet> createModuleSourceDependenciesGraph(final RootModelProvider provider) {
     return GraphGenerator.create(new CachingSemiGraph<ModuleSourceSet>(new GraphGenerator.SemiGraph<ModuleSourceSet>() {
       @Override
       public Collection<ModuleSourceSet> getNodes() {
@@ -228,4 +229,85 @@ public final class ModuleCompilerUtil {
       }
     }));
   }
+
+  @NotNull
+  public static List<Chunk<ModuleSourceSet>> computeSourceSetCycles(@NotNull ModulesProvider provider) {
+    Graph<ModuleSourceSet> graph = createModuleSourceDependenciesGraph(provider);
+    Collection<Chunk<ModuleSourceSet>> chunks = GraphAlgorithms.getInstance().computeStronglyConnectedComponents(graph);
+    return removeSingleElementChunks(removeDummyNodes(filterDuplicates(removeSingleElementChunks(chunks)), provider));
+  }
+
+  private static List<Chunk<ModuleSourceSet>> removeDummyNodes(List<Chunk<ModuleSourceSet>> chunks, ModulesProvider modulesProvider) {
+    List<Chunk<ModuleSourceSet>> result = new ArrayList<Chunk<ModuleSourceSet>>(chunks.size());
+    for (Chunk<ModuleSourceSet> chunk : chunks) {
+      Set<ModuleSourceSet> nodes = new LinkedHashSet<ModuleSourceSet>();
+      for (ModuleSourceSet sourceSet : chunk.getNodes()) {
+        if (!isDummy(sourceSet, modulesProvider)) {
+          nodes.add(sourceSet);
+        }
+      }
+      result.add(new Chunk<ModuleSourceSet>(nodes));
+    }
+    return result;
+  }
+
+  private static boolean isDummy(ModuleSourceSet set, ModulesProvider modulesProvider) {
+    JavaSourceRootType type = set.getType() == ModuleSourceSet.Type.PRODUCTION ? JavaSourceRootType.SOURCE : JavaSourceRootType.TEST_SOURCE;
+    ModuleRootModel rootModel = modulesProvider.getRootModel(set.getModule());
+    for (ContentEntry entry : rootModel.getContentEntries()) {
+      if (!entry.getSourceFolders(type).isEmpty()) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  private static List<Chunk<ModuleSourceSet>> removeSingleElementChunks(Collection<Chunk<ModuleSourceSet>> chunks) {
+    return ContainerUtil.filter(chunks, new Condition<Chunk<ModuleSourceSet>>() {
+      @Override
+      public boolean value(Chunk<ModuleSourceSet> chunk) {
+        return chunk.getNodes().size() > 1;
+      }
+    });
+  }
+
+  /**
+   * Remove cycles in tests included in cycles between production parts
+   */
+  @NotNull
+  private static List<Chunk<ModuleSourceSet>> filterDuplicates(@NotNull Collection<Chunk<ModuleSourceSet>> sourceSetCycles) {
+    final List<Set<Module>> productionCycles = new ArrayList<Set<Module>>();
+
+    for (Chunk<ModuleSourceSet> cycle : sourceSetCycles) {
+      ModuleSourceSet.Type type = getCommonType(cycle);
+      if (type == ModuleSourceSet.Type.PRODUCTION) {
+        productionCycles.add(ModuleSourceSet.getModules(cycle.getNodes()));
+      }
+    }
+
+    return ContainerUtil.filter(sourceSetCycles, new Condition<Chunk<ModuleSourceSet>>() {
+      @Override
+      public boolean value(Chunk<ModuleSourceSet> chunk) {
+        if (getCommonType(chunk) != ModuleSourceSet.Type.TEST) return true;
+        for (Set<Module> productionCycle : productionCycles) {
+          if (productionCycle.containsAll(ModuleSourceSet.getModules(chunk.getNodes()))) return false;
+        }
+        return true;
+      }
+    });
+  }
+
+  @Nullable
+  private static ModuleSourceSet.Type getCommonType(@NotNull Chunk<ModuleSourceSet> cycle) {
+    ModuleSourceSet.Type type = null;
+    for (ModuleSourceSet set : cycle.getNodes()) {
+      if (type == null) {
+        type = set.getType();
+      }
+      else if (type != set.getType()) {
+        return null;
+      }
+    }
+    return type;
+  }
 }

java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/GeneralProjectSettingsElement.java

@@ -21,21 +21,14 @@ import com.intellij.openapi.module.Module;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.project.ProjectBundle;
 import com.intellij.openapi.projectRoots.Sdk;
-import com.intellij.openapi.roots.ContentEntry;
 import com.intellij.openapi.roots.ModuleRootModel;
 import com.intellij.openapi.roots.ui.configuration.projectRoot.ProjectSdksModel;
 import com.intellij.openapi.roots.ui.configuration.projectRoot.StructureConfigurableContext;
 import com.intellij.openapi.roots.ui.configuration.projectRoot.daemon.*;
-import com.intellij.openapi.util.Condition;
 import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.util.Chunk;
-import com.intellij.util.containers.ContainerUtil;
-import com.intellij.util.graph.Graph;
-import com.intellij.util.graph.GraphAlgorithms;
 import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
-import org.jetbrains.jps.model.java.JavaSourceRootType;
 
 import java.util.*;
 
@@ -77,10 +70,7 @@ public class GeneralProjectSettingsElement extends ProjectStructureElement {
     }
 
 
-    Graph<ModuleSourceSet> graph = ModuleCompilerUtil.createModuleSourceDependenciesGraph(myContext.getModulesConfigurator());
-    Collection<Chunk<ModuleSourceSet>> chunks = GraphAlgorithms.getInstance().computeStronglyConnectedComponents(graph);
-    List<Chunk<ModuleSourceSet>> sourceSetCycles =
-      removeSingleElementChunks(removeDummyNodes(filterDuplicates(removeSingleElementChunks(chunks))));
+    List<Chunk<ModuleSourceSet>> sourceSetCycles = ModuleCompilerUtil.computeSourceSetCycles(myContext.getModulesConfigurator());
 
     List<String> cycles = new ArrayList<String>();
 
@@ -118,31 +108,6 @@ public class GeneralProjectSettingsElement extends ProjectStructureElement {
     }
   }
 
-  private List<Chunk<ModuleSourceSet>> removeDummyNodes(List<Chunk<ModuleSourceSet>> chunks) {
-    List<Chunk<ModuleSourceSet>> result = new ArrayList<Chunk<ModuleSourceSet>>(chunks.size());
-    for (Chunk<ModuleSourceSet> chunk : chunks) {
-      Set<ModuleSourceSet> nodes = new LinkedHashSet<ModuleSourceSet>();
-      for (ModuleSourceSet sourceSet : chunk.getNodes()) {
-        if (!isDummy(sourceSet)) {
-          nodes.add(sourceSet);
-        }
-      }
-      result.add(new Chunk<ModuleSourceSet>(nodes));
-    }
-    return result;
-  }
-
-  private boolean isDummy(ModuleSourceSet set) {
-    JavaSourceRootType type = set.getType() == ModuleSourceSet.Type.PRODUCTION ? JavaSourceRootType.SOURCE : JavaSourceRootType.TEST_SOURCE;
-    ModuleRootModel rootModel = myContext.getModulesConfigurator().getRootModel(set.getModule());
-    for (ContentEntry entry : rootModel.getContentEntries()) {
-      if (!entry.getSourceFolders(type).isEmpty()) {
-        return false;
-      }
-    }
-    return true;
-  }
-
   private boolean containsModuleWithInheritedSdk() {
     for (Module module : myContext.getModules()) {
       ModuleRootModel rootModel = myContext.getModulesConfigurator().getRootModel(module);
@@ -153,55 +118,6 @@ public class GeneralProjectSettingsElement extends ProjectStructureElement {
     return false;
   }
 
-  private static List<Chunk<ModuleSourceSet>> removeSingleElementChunks(Collection<Chunk<ModuleSourceSet>> chunks) {
-    return ContainerUtil.filter(chunks, new Condition<Chunk<ModuleSourceSet>>() {
-      @Override
-      public boolean value(Chunk<ModuleSourceSet> chunk) {
-        return chunk.getNodes().size() > 1;
-      }
-    });
-  }
-
-  /**
-   * Remove cycles in tests included in cycles between production parts
-   */
-  @NotNull
-  private static List<Chunk<ModuleSourceSet>> filterDuplicates(@NotNull Collection<Chunk<ModuleSourceSet>> sourceSetCycles) {
-    final List<Set<Module>> productionCycles = new ArrayList<Set<Module>>();
-
-    for (Chunk<ModuleSourceSet> cycle : sourceSetCycles) {
-      ModuleSourceSet.Type type = getCommonType(cycle);
-      if (type == ModuleSourceSet.Type.PRODUCTION) {
-        productionCycles.add(ModuleSourceSet.getModules(cycle.getNodes()));
-      }
-    }
-
-    return ContainerUtil.filter(sourceSetCycles, new Condition<Chunk<ModuleSourceSet>>() {
-      @Override
-      public boolean value(Chunk<ModuleSourceSet> chunk) {
-        if (getCommonType(chunk) != ModuleSourceSet.Type.TEST) return true;
-        for (Set<Module> productionCycle : productionCycles) {
-          if (productionCycle.containsAll(ModuleSourceSet.getModules(chunk.getNodes()))) return false;
-        }
-        return true;
-      }
-    });
-  }
-
-  @Nullable
-  private static ModuleSourceSet.Type getCommonType(@NotNull Chunk<ModuleSourceSet> cycle) {
-    ModuleSourceSet.Type type = null;
-    for (ModuleSourceSet set : cycle.getNodes()) {
-      if (type == null) {
-        type = set.getType();
-      }
-      else if (type != set.getType()) {
-        return null;
-      }
-    }
-    return type;
-  }
-
   @Override
   public List<ProjectStructureElementUsage> getUsagesInElement() {
     return Collections.emptyList();