From c1340c48ad171aa53d00b66fb8aaa09853a7792d Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Thu, 19 Jul 2018 14:51:34 +0200 Subject: [PATCH] [java] upgrade module path: resolve (IDEA-189693) Uses relative location of files on module dependency list to disambiguate upgradeable modules. --- .../impl/file/impl/JavaFileManagerImpl.java | 58 ++++++++++++++++-- .../codeInsight/jigsaw/lib-xml-ws.jar | Bin 0 -> 1170 bytes .../completion/ModuleCompletionTest.kt | 4 +- .../daemon/ModuleHighlightingTest.kt | 9 +++ .../MultiModuleJava9ProjectDescriptor.kt | 8 +++ java/mockJDK-1.9/jre/lib/java.xml.ws.jar | Bin 0 -> 1249 bytes .../scopes/ModuleWithDependenciesScope.java | 33 ++-------- 7 files changed, 78 insertions(+), 34 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/jigsaw/lib-xml-ws.jar create mode 100644 java/mockJDK-1.9/jre/lib/java.xml.ws.jar diff --git a/java/java-indexing-impl/src/com/intellij/psi/impl/file/impl/JavaFileManagerImpl.java b/java/java-indexing-impl/src/com/intellij/psi/impl/file/impl/JavaFileManagerImpl.java index b005fa82a265..a22aadcd0680 100644 --- a/java/java-indexing-impl/src/com/intellij/psi/impl/file/impl/JavaFileManagerImpl.java +++ b/java/java-indexing-impl/src/com/intellij/psi/impl/file/impl/JavaFileManagerImpl.java @@ -5,6 +5,8 @@ import com.intellij.ProjectTopics; import com.intellij.ide.highlighter.JavaClassFileType; import com.intellij.openapi.Disposable; import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.module.Module; +import com.intellij.openapi.module.impl.scopes.ModuleWithDependenciesScope; import com.intellij.openapi.project.Project; import com.intellij.openapi.roots.*; import com.intellij.openapi.util.Comparing; @@ -28,6 +30,7 @@ import org.jetbrains.jps.model.java.JavaModuleSourceRootTypes; import java.util.*; import java.util.stream.Collectors; +import java.util.stream.Stream; import static java.util.Objects.requireNonNull; @@ -172,18 +175,18 @@ public class JavaFileManagerImpl implements JavaFileManager, Disposable { @NotNull @Override public Collection findModules(@NotNull String moduleName, @NotNull GlobalSearchScope scope) { - scope = new LibSrcExcludingScope(scope); + GlobalSearchScope excludingScope = new LibSrcExcludingScope(scope); - Collection named = JavaModuleNameIndex.getInstance().get(moduleName, myManager.getProject(), scope); + Collection named = JavaModuleNameIndex.getInstance().get(moduleName, myManager.getProject(), excludingScope); if (!named.isEmpty()) { - return named; + return upgradeModules(sortModules(named, scope), moduleName, scope); } - Collection jars = JavaAutoModuleNameIndex.getFilesByKey(moduleName, scope); + Collection jars = JavaAutoModuleNameIndex.getFilesByKey(moduleName, excludingScope); if (!jars.isEmpty()) { List automatic = jars.stream().map(f -> LightJavaModule.getModule(myManager, f)).collect(Collectors.toList()); if (!automatic.isEmpty()) { - return automatic; + return sortModules(automatic, scope); } } @@ -203,4 +206,49 @@ public class JavaFileManagerImpl implements JavaFileManager, Disposable { return super.contains(file) && !myIndex.isInLibrarySource(file); } } + + private static Collection sortModules(Collection modules, GlobalSearchScope scope) { + if (modules.size() > 1) { + List list = new ArrayList<>(modules); + list.sort((m1, m2) -> scope.compare(virtualFile(m2), virtualFile(m1))); + modules = list; + } + return modules; + } + + private static VirtualFile virtualFile(PsiJavaModule m) { + return m instanceof LightJavaModule ? ((LightJavaModule)m).getRootVirtualFile() : m.getContainingFile().getVirtualFile(); + } + + private static Collection upgradeModules(Collection modules, String moduleName, GlobalSearchScope scope) { + if (modules.size() > 1 && PsiJavaModule.UPGRADEABLE.contains(moduleName) && scope instanceof ModuleWithDependenciesScope) { + Module module = ((ModuleWithDependenciesScope)scope).getModule(); + boolean isModular = Stream.of(ModuleRootManager.getInstance(module).getSourceRoots(true)) + .filter(scope::contains) + .anyMatch(root -> root.findChild(PsiJavaModule.MODULE_INFO_FILE) != null); + if (isModular) { + List list = new ArrayList<>(modules); + + ModuleFileIndex index = ModuleRootManager.getInstance(module).getFileIndex(); + for (ListIterator i = list.listIterator(); i.hasNext(); ) { + PsiJavaModule candidate = i.next(); + if (index.getOrderEntryForFile(candidate.getContainingFile().getVirtualFile()) instanceof JdkOrderEntry) { + if (i.previousIndex() > 0) { + i.remove(); // not at the top -> is upgraded + } + else { + list = Collections.singletonList(candidate); // shadows subsequent modules + break; + } + } + } + + if (list.size() != modules.size()) { + modules = list; + } + } + } + + return modules; + } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/jigsaw/lib-xml-ws.jar b/java/java-tests/testData/codeInsight/jigsaw/lib-xml-ws.jar new file mode 100644 index 0000000000000000000000000000000000000000..03ffc3e087b768441c1b08756922c375eba75e6c GIT binary patch literal 1170 zcmWIWW@Zs#;Nak3&$Ck#XSH_lC#wO>+W;XlGo-;z$Zft41XARug#Ge0|65_^IH~-9@ko}kLEV;61 z!jUC13pm0H>M{e1Rz<6I9DcTD`qYSNCk`Jtbl}v91Kdfk!=F5RF4dS-I&6cQ~e{u}{iLa7u zgasogX6p@-ZqEn0eh&~MViw(hf*QeOS7tV!l%&AF zBxb#mx%>J(GbT0)pYit6@oPQxf;sie>27HW2?@!B22Kt~6=P=Z3DXi2Si70lvaCvC zza&#Il?R+I{!1nJwE|tZ73>E_CJ_eI#0^Wipu~*|;Hedq)C0UxwIU}sP|`*KTObpz z6)B;EOyFX`oh%V190oF>i4?aMP%=e;XFw)c3nbCv)&@$p2*AsTqz#;i5xxXPGjaj| uC1V6IN7D&S)X2dEG5|UBLFohm#DM{ik!k|GS=m5J*nw~%(1zRK*Z}~8g%-E~ literal 0 HcmV?d00001 diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/ModuleCompletionTest.kt b/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/ModuleCompletionTest.kt index 952856610c76..93a07cd3ff94 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/ModuleCompletionTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/ModuleCompletionTest.kt @@ -33,7 +33,7 @@ class ModuleCompletionTest : LightJava9ModulesCodeInsightFixtureTestCase() { fun testRequiresBare() = variants("module M { requires ", - "transitive", "static", "M2", "java.base", "java.se", "java.xml.bind", "javax.doomed", + "transitive", "static", "M2", "java.base", "java.se", "java.xml.bind", "java.xml.ws", "javax.doomed", "lib.multi.release", "lib.named", "lib.auto", "lib.claimed") fun testRequiresTransitive() = complete("module M { requires tr }", "module M { requires transitive }") fun testRequiresSimpleName() = complete("module M { requires M }", "module M { requires M2; }") @@ -46,7 +46,7 @@ class ModuleCompletionTest : LightJava9ModulesCodeInsightFixtureTestCase() { fun testExportsTo() = complete("module M { exports pkg.other }", "module M { exports pkg.other to }") fun testExportsToList() = variants("module M { exports pkg.other to }", - "M2", "java.base", "java.se", "java.xml.bind", "javax.doomed", "lib.multi.release", "lib.named") + "M2", "java.base", "java.se", "java.xml.bind", "java.xml.ws", "javax.doomed", "lib.multi.release", "lib.named") fun testExportsToUnambiguous() = complete("module M { exports pkg.other to M }", "module M { exports pkg.other to M2 }") fun testUsesPrefixed() = complete("module M { uses p }", "module M { uses pkg. }") diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/ModuleHighlightingTest.kt b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/ModuleHighlightingTest.kt index acb0f99d876a..5619c0963256 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/ModuleHighlightingTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/ModuleHighlightingTest.kt @@ -321,6 +321,15 @@ class ModuleHighlightingTest : LightJava9ModulesCodeInsightFixtureTestCase() { """.trimIndent()) } + fun testUpgradeableModuleOnModulePath() { + myFixture.enableInspections(DeprecationInspection(), MarkedForRemovalInspection()) + highlight(""" + module M { + requires java.xml.bind; + requires java.xml.ws; + }""".trimIndent()) + } + fun testLinearModuleGraphBug() { addFile("module-info.java", "module M6 { requires M7; }", M6) addFile("module-info.java", "module M7 { }", M7) diff --git a/java/java-tests/testSrc/com/intellij/java/testFramework/fixtures/MultiModuleJava9ProjectDescriptor.kt b/java/java-tests/testSrc/com/intellij/java/testFramework/fixtures/MultiModuleJava9ProjectDescriptor.kt index d037b9981a9c..7fdf715b8f77 100644 --- a/java/java-tests/testSrc/com/intellij/java/testFramework/fixtures/MultiModuleJava9ProjectDescriptor.kt +++ b/java/java-tests/testSrc/com/intellij/java/testFramework/fixtures/MultiModuleJava9ProjectDescriptor.kt @@ -74,6 +74,14 @@ object MultiModuleJava9ProjectDescriptor : DefaultLightProjectDescriptor() { ModuleRootModificationUtil.addModuleLibrary(main, "${libDir}/lib-multi-release.jar!/") ModuleRootModificationUtil.addModuleLibrary(main, "${libDir}/lib_invalid_1_2.jar!/") ModuleRootModificationUtil.addModuleLibrary(main, "${libDir}/lib-xml-bind.jar!/") + + ModuleRootModificationUtil.addModuleLibrary(main, "${libDir}/lib-xml-ws.jar!/") + ModuleRootModificationUtil.updateModel(main) { + val entries = it.orderEntries.toMutableList() + entries.add(0, entries.last()) // places an upgrade module before the JDK + entries.removeAt(entries.size - 1) + it.rearrangeOrderEntries(entries.toTypedArray()) + } } } diff --git a/java/mockJDK-1.9/jre/lib/java.xml.ws.jar b/java/mockJDK-1.9/jre/lib/java.xml.ws.jar new file mode 100644 index 0000000000000000000000000000000000000000..9e4e323e8b92ac815dc259cafd1f9ab33acc3190 GIT binary patch literal 1249 zcmWIWW@Zs#;Nak3kPiOr!+-=h8CV#6T|*poJ^kGD|D9rBU}gyLX6FE@V1g$Ck#XSH_lC#wO>+W;XlGo-;z$Zft41XARug#Ge0|65_^IH~-9@ko}kLEV;61 z!jUC13pm0H>M{e1Rz<6I9DcTD`qYSNCk`Jtbl}v91Kdfk!=F5RF4dS-I&6cQ~e{u}{iLa7u zgasogX6p@-ZqEn0eh&~MViw(hf}|uH5-EwbSy9!)^&l$s08X)wqp6 za2kIwHrf#NO7g|b4g(GYqXOldoKG5kuB<+B_M$d>)B(7p$tV!E6*vi=W7BuiMt4By)2;~*Xl-m);22N%lK8L)&0`xF1 zBPd81nM4>+^9n2@fbt3|fG2)X&Oz6ToO(ey1p#bCpivf2kN0@LJ$b_bO z+*&}X9s!;KnP4rDJb+spC>J0AC{W myModules; @@ -56,11 +40,9 @@ public class ModuleWithDependenciesScope extends GlobalSearchScope { super(module.getProject()); myModule = module; myOptions = options; - myProjectFileIndex = (ProjectFileIndexImpl)ProjectRootManager.getInstance(module.getProject()).getFileIndex(); - final LinkedHashSet roots = ContainerUtil.newLinkedHashSet(); - + Set roots = ContainerUtil.newLinkedHashSet(); if (hasOption(CONTENT)) { Set modules = calcModules(); myModules = ContainerUtil.newTroveSet(modules); @@ -87,10 +69,7 @@ public class ModuleWithDependenciesScope extends GlobalSearchScope { private OrderEnumerator getOrderEnumeratorForOptions() { OrderEnumerator en = ModuleRootManager.getInstance(myModule).orderEntries(); en.recursively(); - - if (hasOption(COMPILE_ONLY)) { - en.exportedOnly().compileOnly(); - } + if (hasOption(COMPILE_ONLY)) en.exportedOnly().compileOnly(); if (!hasOption(LIBRARIES)) en.withoutLibraries().withoutSdk(); if (!hasOption(MODULES)) en.withoutDepModules(); if (!hasOption(TESTS)) en.productionOnly(); @@ -101,7 +80,7 @@ public class ModuleWithDependenciesScope extends GlobalSearchScope { private Set calcModules() { // In the case that hasOption(CONTENT), the order of the modules set matters for // ordering the content roots, so use a LinkedHashSet - final Set modules = ContainerUtil.newLinkedHashSet(); + Set modules = ContainerUtil.newLinkedHashSet(); OrderEnumerator en = getOrderEnumeratorForOptions(); en.forEach(each -> { if (each instanceof ModuleOrderEntry) { @@ -215,4 +194,4 @@ public class ModuleWithDependenciesScope extends GlobalSearchScope { " include other modules:" + hasOption(MODULES) + " include tests:" + hasOption(TESTS); } -} +} \ No newline at end of file