From ebad2bae2e7a7e020dd23b7bd95d6012ac5c59d7 Mon Sep 17 00:00:00 2001 From: nik Date: Wed, 3 Feb 2016 12:44:51 +0300 Subject: [PATCH] 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 --- .../compiler/ModuleCompilerUtilTest.java | 84 ++++++++++++++++++ .../intellij/compiler/ModuleCompilerUtil.java | 88 ++++++++++++++++++- .../GeneralProjectSettingsElement.java | 86 +----------------- 3 files changed, 170 insertions(+), 88 deletions(-) create mode 100644 java/compiler/impl/testSrc/com/intellij/compiler/ModuleCompilerUtilTest.java diff --git a/java/compiler/impl/testSrc/com/intellij/compiler/ModuleCompilerUtilTest.java b/java/compiler/impl/testSrc/com/intellij/compiler/ModuleCompilerUtilTest.java new file mode 100644 index 000000000000..cda75c17a88c --- /dev/null +++ b/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> 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))); + } +} \ No newline at end of file diff --git a/java/compiler/openapi/src/com/intellij/compiler/ModuleCompilerUtil.java b/java/compiler/openapi/src/com/intellij/compiler/ModuleCompilerUtil.java index a6cb5ca90454..37b7d8ab7e52 100644 --- a/java/compiler/openapi/src/com/intellij/compiler/ModuleCompilerUtil.java +++ 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> getCyclicDependencies(@NotNull Project project, @NotNull List modules) { - Graph graph = createModuleSourceDependenciesGraph(new DefaultModulesProvider(project)); - Collection> chunks = GraphAlgorithms.getInstance().computeStronglyConnectedComponents(graph); + Collection> chunks = computeSourceSetCycles(new DefaultModulesProvider(project)); final Set modulesSet = new HashSet(modules); return ContainerUtil.filter(chunks, new Condition>() { @Override @@ -193,7 +194,7 @@ public final class ModuleCompilerUtil { }); } - public static Graph createModuleSourceDependenciesGraph(final RootModelProvider provider) { + private static Graph createModuleSourceDependenciesGraph(final RootModelProvider provider) { return GraphGenerator.create(new CachingSemiGraph(new GraphGenerator.SemiGraph() { @Override public Collection getNodes() { @@ -228,4 +229,85 @@ public final class ModuleCompilerUtil { } })); } + + @NotNull + public static List> computeSourceSetCycles(@NotNull ModulesProvider provider) { + Graph graph = createModuleSourceDependenciesGraph(provider); + Collection> chunks = GraphAlgorithms.getInstance().computeStronglyConnectedComponents(graph); + return removeSingleElementChunks(removeDummyNodes(filterDuplicates(removeSingleElementChunks(chunks)), provider)); + } + + private static List> removeDummyNodes(List> chunks, ModulesProvider modulesProvider) { + List> result = new ArrayList>(chunks.size()); + for (Chunk chunk : chunks) { + Set nodes = new LinkedHashSet(); + for (ModuleSourceSet sourceSet : chunk.getNodes()) { + if (!isDummy(sourceSet, modulesProvider)) { + nodes.add(sourceSet); + } + } + result.add(new Chunk(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> removeSingleElementChunks(Collection> chunks) { + return ContainerUtil.filter(chunks, new Condition>() { + @Override + public boolean value(Chunk chunk) { + return chunk.getNodes().size() > 1; + } + }); + } + + /** + * Remove cycles in tests included in cycles between production parts + */ + @NotNull + private static List> filterDuplicates(@NotNull Collection> sourceSetCycles) { + final List> productionCycles = new ArrayList>(); + + for (Chunk cycle : sourceSetCycles) { + ModuleSourceSet.Type type = getCommonType(cycle); + if (type == ModuleSourceSet.Type.PRODUCTION) { + productionCycles.add(ModuleSourceSet.getModules(cycle.getNodes())); + } + } + + return ContainerUtil.filter(sourceSetCycles, new Condition>() { + @Override + public boolean value(Chunk chunk) { + if (getCommonType(chunk) != ModuleSourceSet.Type.TEST) return true; + for (Set productionCycle : productionCycles) { + if (productionCycle.containsAll(ModuleSourceSet.getModules(chunk.getNodes()))) return false; + } + return true; + } + }); + } + + @Nullable + private static ModuleSourceSet.Type getCommonType(@NotNull Chunk 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; + } } diff --git a/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/GeneralProjectSettingsElement.java b/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/GeneralProjectSettingsElement.java index d42789f1e194..2c6f8b0551c8 100644 --- a/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/GeneralProjectSettingsElement.java +++ b/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 graph = ModuleCompilerUtil.createModuleSourceDependenciesGraph(myContext.getModulesConfigurator()); - Collection> chunks = GraphAlgorithms.getInstance().computeStronglyConnectedComponents(graph); - List> sourceSetCycles = - removeSingleElementChunks(removeDummyNodes(filterDuplicates(removeSingleElementChunks(chunks)))); + List> sourceSetCycles = ModuleCompilerUtil.computeSourceSetCycles(myContext.getModulesConfigurator()); List cycles = new ArrayList(); @@ -118,31 +108,6 @@ public class GeneralProjectSettingsElement extends ProjectStructureElement { } } - private List> removeDummyNodes(List> chunks) { - List> result = new ArrayList>(chunks.size()); - for (Chunk chunk : chunks) { - Set nodes = new LinkedHashSet(); - for (ModuleSourceSet sourceSet : chunk.getNodes()) { - if (!isDummy(sourceSet)) { - nodes.add(sourceSet); - } - } - result.add(new Chunk(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> removeSingleElementChunks(Collection> chunks) { - return ContainerUtil.filter(chunks, new Condition>() { - @Override - public boolean value(Chunk chunk) { - return chunk.getNodes().size() > 1; - } - }); - } - - /** - * Remove cycles in tests included in cycles between production parts - */ - @NotNull - private static List> filterDuplicates(@NotNull Collection> sourceSetCycles) { - final List> productionCycles = new ArrayList>(); - - for (Chunk cycle : sourceSetCycles) { - ModuleSourceSet.Type type = getCommonType(cycle); - if (type == ModuleSourceSet.Type.PRODUCTION) { - productionCycles.add(ModuleSourceSet.getModules(cycle.getNodes())); - } - } - - return ContainerUtil.filter(sourceSetCycles, new Condition>() { - @Override - public boolean value(Chunk chunk) { - if (getCommonType(chunk) != ModuleSourceSet.Type.TEST) return true; - for (Set productionCycle : productionCycles) { - if (productionCycle.containsAll(ModuleSourceSet.getModules(chunk.getNodes()))) return false; - } - return true; - } - }); - } - - @Nullable - private static ModuleSourceSet.Type getCommonType(@NotNull Chunk 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 getUsagesInElement() { return Collections.emptyList();