From b469d5fbd22a855ee3d1af58a15b589de5c45974 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Wed, 24 Aug 2016 19:33:35 +0300 Subject: [PATCH] [java] module cycles highlighting --- .../impl/analysis/HighlightVisitorImpl.java | 3 +- .../impl/analysis/JavaModuleGraphBuilder.java | 136 ++++++++++++++++++ .../impl/analysis/ModuleHighlightUtil.java | 27 +++- .../src/messages/JavaErrorMessages.properties | 3 +- .../daemon/ModuleHighlightingTest.kt | 21 ++- 5 files changed, 181 insertions(+), 9 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphBuilder.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java index d95ce00d89b9..c3ac0b1996bd 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java @@ -1628,8 +1628,9 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh public void visitRequiresStatement(PsiRequiresStatement statement) { super.visitRequiresStatement(statement); if (PsiUtil.isLanguageLevel9OrHigher(myFile)) { + PsiJavaModule container = (PsiJavaModule)statement.getParent(); PsiJavaModuleReferenceElement ref = statement.getReferenceElement(); - if (!myHolder.hasErrorResults()) myHolder.add(ModuleHighlightUtil.checkModuleReference(ref)); + if (!myHolder.hasErrorResults()) myHolder.add(ModuleHighlightUtil.checkModuleReference(ref, container)); } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphBuilder.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphBuilder.java new file mode 100644 index 000000000000..53915d2d097b --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphBuilder.java @@ -0,0 +1,136 @@ +/* + * 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.codeInsight.daemon.impl.analysis; + +import com.intellij.openapi.application.ReadAction; +import com.intellij.openapi.module.Module; +import com.intellij.openapi.module.ModuleManager; +import com.intellij.openapi.module.impl.scopes.ModulesScope; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.*; +import com.intellij.psi.search.FilenameIndex; +import com.intellij.psi.util.CachedValueProvider.Result; +import com.intellij.psi.util.CachedValuesManager; +import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.MultiMap; +import com.intellij.util.graph.DFSTBuilder; +import com.intellij.util.graph.Graph; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +import static com.intellij.psi.PsiJavaModule.MODULE_INFO_FILE; +import static com.intellij.psi.util.PsiModificationTracker.OUT_OF_CODE_BLOCK_MODIFICATION_COUNT; + +public class JavaModuleGraphBuilder { + private JavaModuleGraphBuilder() { } + + @Nullable + public static Graph getOrBuild(@NotNull Project project) { + return CachedValuesManager.getManager(project).getCachedValue(project, () -> { + Graph graph = ReadAction.compute(() -> build(project)); + return Result.create(graph, OUT_OF_CODE_BLOCK_MODIFICATION_COUNT); + }); + } + + @Nullable + public static Collection findCycle(@NotNull Graph graph, @NotNull PsiJavaModule module) { + return ((JavaModuleGraph)graph).myCycles.stream().filter(set -> set.contains(module)).findFirst().orElse(null); + } + + private static Graph build(Project project) { + Set projectModules = ContainerUtil.newHashSet(); + for (Module module : ModuleManager.getInstance(project).getModules()) { + Collection files = FilenameIndex.getVirtualFilesByName(project, MODULE_INFO_FILE, new ModulesScope(module)); + if (files.size() > 1) { + return null; + } + VirtualFile vFile = ContainerUtil.getFirstItem(files); + if (vFile != null) { + PsiFile psiFile = PsiManager.getInstance(project).findFile(vFile); + if (psiFile instanceof PsiJavaFile) { + PsiJavaModule moduleDeclaration = ((PsiJavaFile)psiFile).getModuleDeclaration(); + if (moduleDeclaration != null) { + projectModules.add(moduleDeclaration); + } + } + } + } + + MultiMap relations = MultiMap.create(); + for (PsiJavaModule moduleDeclaration : projectModules) { + for (PsiElement child = moduleDeclaration.getFirstChild(); child != null; child = child.getNextSibling()) { + if (child instanceof PsiRequiresStatement) { + PsiJavaModule dependency = resolveDependency((PsiRequiresStatement)child); + if (dependency != null && projectModules.contains(dependency)) { + relations.putValue(moduleDeclaration, dependency); + } + } + } + } + return new JavaModuleGraph(relations); + } + + private static PsiJavaModule resolveDependency(PsiRequiresStatement statement) { + PsiJavaModuleReferenceElement refElement = statement.getReferenceElement(); + if (refElement != null) { + PsiPolyVariantReference ref = refElement.getReference(); + if (ref != null) { + PsiElement target = ref.resolve(); + if (target instanceof PsiJavaModule) { + return (PsiJavaModule)target; + } + } + } + + return null; + } + + private static class JavaModuleGraph implements Graph { + private final MultiMap myMap; + private final List> myCycles; + + public JavaModuleGraph(MultiMap map) { + myMap = map; + + DFSTBuilder builder = new DFSTBuilder<>(this); + myCycles = builder.getComponents().stream() + .map(ContainerUtil::newLinkedHashSet) + .collect(Collectors.toList()); + } + + @Override + public Collection getNodes() { + return myMap.keySet(); + } + + @Override + public Iterator getIn(PsiJavaModule n) { + return ContainerUtil.emptyIterator(); + } + + @Override + public Iterator getOut(PsiJavaModule n) { + return myMap.get(n).iterator(); + } + } +} \ No newline at end of file diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/ModuleHighlightUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/ModuleHighlightUtil.java index 82358fa9c96f..c1d69fb4948b 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/ModuleHighlightUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/ModuleHighlightUtil.java @@ -24,6 +24,7 @@ import com.intellij.codeInsight.daemon.impl.quickfix.GoToSymbolFix; import com.intellij.codeInsight.daemon.impl.quickfix.MoveFileFix; import com.intellij.codeInsight.daemon.impl.quickfix.QuickFixAction; import com.intellij.codeInsight.intention.QuickFixFactory; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.module.Module; import com.intellij.openapi.module.ModuleUtilCore; import com.intellij.openapi.module.impl.scopes.ModulesScope; @@ -34,12 +35,15 @@ import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.*; import com.intellij.psi.search.FilenameIndex; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.graph.Graph; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; import static com.intellij.psi.PsiJavaModule.MODULE_INFO_FILE; @@ -118,14 +122,31 @@ public class ModuleHighlightUtil { } @Nullable - static HighlightInfo checkModuleReference(@Nullable PsiJavaModuleReferenceElement refElement) { + static HighlightInfo checkModuleReference(@Nullable PsiJavaModuleReferenceElement refElement, @NotNull PsiJavaModule container) { if (refElement != null) { PsiPolyVariantReference ref = refElement.getReference(); assert ref != null : refElement.getParent(); - if (ref.multiResolve(false).length == 0) { - String message = JavaErrorMessages.message("module.ref.unknown", refElement.getReferenceText()); + PsiElement target = ref.resolve(); + if (!(target instanceof PsiJavaModule)) { + String message = JavaErrorMessages.message("module.not.found", refElement.getReferenceText()); return HighlightInfo.newHighlightInfo(HighlightInfoType.WRONG_REF).range(refElement).description(message).create(); } + else if (target == container) { + String message = JavaErrorMessages.message("module.cyclic.dependence", container.getModuleName()); + return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(refElement).description(message).create(); + } + else { + Graph graph = JavaModuleGraphBuilder.getOrBuild(target.getProject()); + if (graph != null) { + Collection cycle = JavaModuleGraphBuilder.findCycle(graph, (PsiJavaModule)target); + if (cycle != null && cycle.contains(container)) { + Stream stream = cycle.stream().map(PsiJavaModule::getModuleName); + if (ApplicationManager.getApplication().isUnitTestMode()) stream = stream.sorted(); + String message = JavaErrorMessages.message("module.cyclic.dependence", stream.collect(Collectors.joining(", "))); + return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(refElement).description(message).create(); + } + } + } } return null; diff --git a/java/java-psi-impl/src/messages/JavaErrorMessages.properties b/java/java-psi-impl/src/messages/JavaErrorMessages.properties index 4ad337b56ab3..07c796e2f5b0 100644 --- a/java/java-psi-impl/src/messages/JavaErrorMessages.properties +++ b/java/java-psi-impl/src/messages/JavaErrorMessages.properties @@ -396,7 +396,8 @@ module.file.duplicate='module-info.java' already exists in the module module.duplicate.requires=Duplicate requires: {0} module.file.wrong.location=Module declaration should be located in a module's source root module.open.duplicate.text=Go to duplicate -module.ref.unknown=Cannot resolve module ''{0}'' +module.not.found=Module not found: {0} +module.cyclic.dependence=Cyclic dependence: {0} feature.generics=Generics feature.annotations=Annotations diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ModuleHighlightingTest.kt b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ModuleHighlightingTest.kt index 2ac083cf3e71..4044be749da8 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ModuleHighlightingTest.kt +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ModuleHighlightingTest.kt @@ -24,11 +24,13 @@ import com.intellij.openapi.roots.ModifiableRootModel import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.vfs.VfsUtil import com.intellij.pom.java.LanguageLevel +import com.intellij.psi.PsiJavaModule import com.intellij.testFramework.IdeaTestUtil import com.intellij.testFramework.LightPlatformTestCase import com.intellij.testFramework.LightProjectDescriptor import com.intellij.testFramework.fixtures.DefaultLightProjectDescriptor import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase +import com.intellij.testFramework.fixtures.impl.CodeInsightTestFixtureImpl class ModuleHighlightingTest : LightCodeInsightFixtureTestCase() { override fun getProjectDescriptor(): LightProjectDescriptor = DESCRIPTOR @@ -49,11 +51,19 @@ class ModuleHighlightingTest : LightCodeInsightFixtureTestCase() { } fun testDuplicateRequires() { - doTest("""module M { requires M2; requires M2; }""") + doTest("""module M { requires M2; requires M2; }""", true) } fun testUnresolvedModule() { - doTest("""module M { requires M.missing; }""") + doTest("""module M { requires M.missing; }""") + } + + fun testSelfDependence() { + doTest("""module M { requires M; }""") + } + + fun testCyclicDependence() { + doTest("""module M1 { requires M2; }""", true) } // @@ -68,7 +78,7 @@ class ModuleHighlightingTest : LightCodeInsightFixtureTestCase() { val src2 = createSourceRoot(m2, "src2") createContentEntry(m2, src2) - VfsUtil.saveText(src2.createChildData(this, "module-info.java"), "module M2 { }") + VfsUtil.saveText(src2.createChildData(this, "module-info.java"), "module M2 { requires M1; }") } } @@ -84,8 +94,11 @@ class ModuleHighlightingTest : LightCodeInsightFixtureTestCase() { file }) - private fun doTest(text: String) { + private fun doTest(text: String, filter: Boolean = false) { myFixture.configureByText("module-info.java", text) + if (filter) { + (myFixture as CodeInsightTestFixtureImpl).setVirtualFileFilter { it.name != PsiJavaModule.MODULE_INFO_FILE } + } myFixture.checkHighlighting() } //