From 952919e6dd542004d85b42f19eae608e9fe8d08f Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Mon, 15 Apr 2024 17:33:37 +0200 Subject: [PATCH] StackOverflowError on autocomplete imports on modular projects fixes #2169 + Stop traversal when encountering duplicate references + Fix indentation Also-by: Stephan Herrmann --- .../core/tests/model/CompletionTests9.java | 80 +++++++++++++++++++ .../internal/core/ModuleDescriptionInfo.java | 58 ++++++++++++++ .../internal/core/SearchableEnvironment.java | 80 ++++++++++--------- 3 files changed, 180 insertions(+), 38 deletions(-) diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTests9.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTests9.java index 8f36c05e043..564e5a2789b 100644 --- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTests9.java +++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTests9.java @@ -1680,4 +1680,84 @@ public void testBug573632d() throws Exception { deleteProject(project1); } } +public void testGH2169() throws CoreException { + IJavaProject project1 = createJava9Project("Completion9_1"); + IJavaProject project2 = createJava9Project("Completion9_2"); + IJavaProject project3 = createJava9Project("Completion9_3"); + try { + project1.open(null); + createType("/Completion9_1/src/", "pack11", "X11"); + createFile("/Completion9_1/src/module-info.java", + "module first {\n" + + " requires transitive second;\n" + + " requires transitive third;\n" + + " requires transitive java.base;\n" + + "}\n"); + String fileContent = + "package pack0;\n" + + "import java.util\n" + + "public class Main {\n" + + "}\n"; + String completeBehind = "import java.util"; + createFolder("/Completion9_1/src/pack0"); + String filePath = "/Completion9_1/src/pack0/Main.java"; + createFile(filePath, fileContent); + addClasspathEntry(project1, JavaCore.newContainerEntry(new Path("org.eclipse.jdt.MODULE_PATH"))); + + project2.open(null); + createType("/Completion9_2/src/", "java.utilities", "X21"); + + createFile("/Completion9_2/src/module-info.java", + "module second { \n" + + " requires transitive java.base;\n" + + " requires transitive third;\n" + + " exports java.utilities;\n" + + "}\n"); + addClasspathEntry(project2, JavaCore.newContainerEntry(new Path("org.eclipse.jdt.MODULE_PATH"))); + + project3.open(null); + createType("/Completion9_3/src/", "java/utils/", "X31"); + + createFile("/Completion9_3/src/module-info.java", + "module third {\n" + + " requires transitive first;\n" + // yep, cycle here + " exports java.utils;\n" + + "}\n"); + addClasspathEntry(project3, JavaCore.newContainerEntry(new Path("org.eclipse.jdt.MODULE_PATH"))); + + project1.close(); // sync + project2.close(); + project3.close(); + project3.open(null); + project2.open(null); + project1.open(null); + + int cursorLocation = fileContent.lastIndexOf(completeBehind) + completeBehind.length(); + CompletionTestsRequestor2 requestor = new CompletionTestsRequestor2(); + + ICompilationUnit unit = getCompilationUnit(filePath); + unit.codeComplete(cursorLocation, requestor); + + String expected = "java.util.concurrent[PACKAGE_REF]{java.util.concurrent.*;, java.util.concurrent, null, null, 49}\n" + + "java.util.concurrent.atomic[PACKAGE_REF]{java.util.concurrent.atomic.*;, java.util.concurrent.atomic, null, null, 49}\n" + + "java.util.concurrent.locks[PACKAGE_REF]{java.util.concurrent.locks.*;, java.util.concurrent.locks, null, null, 49}\n" + + "java.util.function[PACKAGE_REF]{java.util.function.*;, java.util.function, null, null, 49}\n" + + "java.util.jar[PACKAGE_REF]{java.util.jar.*;, java.util.jar, null, null, 49}\n" + + "java.util.logging[PACKAGE_REF]{java.util.logging.*;, java.util.logging, null, null, 49}\n" + + "java.util.prefs[PACKAGE_REF]{java.util.prefs.*;, java.util.prefs, null, null, 49}\n" + + "java.util.random[PACKAGE_REF]{java.util.random.*;, java.util.random, null, null, 49}\n" + + "java.util.regex[PACKAGE_REF]{java.util.regex.*;, java.util.regex, null, null, 49}\n" + + "java.util.spi[PACKAGE_REF]{java.util.spi.*;, java.util.spi, null, null, 49}\n" + + "java.util.stream[PACKAGE_REF]{java.util.stream.*;, java.util.stream, null, null, 49}\n" + + "java.util.zip[PACKAGE_REF]{java.util.zip.*;, java.util.zip, null, null, 49}\n" + + "java.utilities[PACKAGE_REF]{java.utilities.*;, java.utilities, null, null, 49}\n" + + "java.utils[PACKAGE_REF]{java.utils.*;, java.utils, null, null, 49}\n" + + "java.util[PACKAGE_REF]{java.util.*;, java.util, null, null, 53}"; + assertResults(expected, requestor.getResults()); + } finally { + deleteProject(project1); + deleteProject(project2); + deleteProject(project3); + } +} } \ No newline at end of file diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ModuleDescriptionInfo.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ModuleDescriptionInfo.java index bc06f17e4ad..ddcf387bd17 100644 --- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ModuleDescriptionInfo.java +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ModuleDescriptionInfo.java @@ -13,8 +13,10 @@ *******************************************************************************/ package org.eclipse.jdt.internal.core; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import org.eclipse.jdt.core.IJavaElement; import org.eclipse.jdt.core.IModuleDescription; @@ -63,7 +65,41 @@ public char[] name() { public int getModifiers() { return this.modifiers; } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + Arrays.hashCode(this.name); + result = prime * result + Objects.hash(this.modifiers); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof ModuleReferenceInfo)) { + return false; + } + ModuleReferenceInfo other = (ModuleReferenceInfo) obj; + return this.modifiers == other.modifiers && Arrays.equals(this.name, other.name); + } + + @Override + public String toString() { + StringBuilder builder = new StringBuilder(); + builder.append("ModuleReferenceInfo ["); //$NON-NLS-1$ + if (this.name() != null) { + builder.append("name="); //$NON-NLS-1$ + builder.append(String.valueOf(this.name())); + } + builder.append("]"); //$NON-NLS-1$ + return builder.toString(); + } } + static class PackageExportInfo extends MemberElementInfo implements IModule.IPackageExport { char[] pack; char[][] target; @@ -116,6 +152,28 @@ public String toString() { buffer.append(';'); return buffer.toString(); } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + Arrays.deepHashCode(this.implNames); + result = prime * result + Arrays.hashCode(this.serviceName); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof ServiceInfo)) { + return false; + } + ServiceInfo other = (ServiceInfo) obj; + return Arrays.deepEquals(this.implNames, other.implNames) + && Arrays.equals(this.serviceName, other.serviceName); + } } public static ModuleDescriptionInfo createModule(ModuleDeclaration module) { diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java index 6ec21958b27..18053fee470 100644 --- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java @@ -326,54 +326,58 @@ public void findPackages(char[] prefix, ISearchRequestor requestor, IPackageFrag new String(prefix), true, new SearchableEnvironmentRequestor(requestor), moduleContext); - if (followRequires && this.knownModuleLocations != null) { - try { - boolean isMatchAllPrefix = CharOperation.equals(CharOperation.ALL_PREFIX, prefix); - Set modDescs = new HashSet<>(); - for (IPackageFragmentRoot root : moduleContext) { - IModuleDescription desc = root.getJavaProject().getModuleDescription(); - if (desc instanceof AbstractModule) - modDescs.add(desc); - } - for (IModuleDescription md : modDescs) { - IModuleReference[] reqModules = ((AbstractModule) md).getRequiredModules(); - char[] modName = md.getElementName().toCharArray(); - for (IModuleReference moduleReference : reqModules) { - findPackagesFromRequires(prefix, isMatchAllPrefix, requestor, moduleReference, modName); + if (followRequires && this.knownModuleLocations != null) { + try { + boolean isMatchAllPrefix = CharOperation.equals(CharOperation.ALL_PREFIX, prefix); + Set modDescs = new HashSet<>(); + for (IPackageFragmentRoot root : moduleContext) { + IModuleDescription desc = root.getJavaProject().getModuleDescription(); + if (desc instanceof AbstractModule) + modDescs.add(desc); + } + for (IModuleDescription md : modDescs) { + IModuleReference[] reqModules = ((AbstractModule) md).getRequiredModules(); + char[] modName = md.getElementName().toCharArray(); + Set visited = new HashSet<>(); + for (IModuleReference moduleReference : reqModules) { + findPackagesFromRequires(prefix, isMatchAllPrefix, requestor, moduleReference, modName, visited); + } } + } catch (JavaModelException e) { + // silent } - } catch (JavaModelException e) { - // silent } } -} -private void findPackagesFromRequires(char[] prefix, boolean isMatchAllPrefix, ISearchRequestor requestor, IModuleReference moduleReference, char[] clientModuleName) { - IPackageFragmentRoot[] fragmentRoots = findModuleContext(moduleReference.name()); - if (fragmentRoots == null) return; - for (IPackageFragmentRoot root : fragmentRoots) { - IJavaProject requiredProject = root.getJavaProject(); - try { - IModuleDescription module = requiredProject.getModuleDescription(); - if (module instanceof AbstractModule) { - AbstractModule requiredModule = (AbstractModule) module; - for (IPackageExport packageExport : requiredModule.getExportedPackages()) { - if (!packageExport.isQualified() || CharOperation.containsEqual(packageExport.targets(), clientModuleName)) { - char[] exportName = packageExport.name(); - if (isMatchAllPrefix || CharOperation.prefixEquals(prefix, exportName)) - requestor.acceptPackage(exportName); + private void findPackagesFromRequires(char[] prefix, boolean isMatchAllPrefix, ISearchRequestor requestor, IModuleReference moduleReference, char[] clientModuleName, Set visited) { + if (!visited.add(moduleReference)) { + return; + } + IPackageFragmentRoot[] fragmentRoots = findModuleContext(moduleReference.name()); + if (fragmentRoots == null) return; + for (IPackageFragmentRoot root : fragmentRoots) { + IJavaProject requiredProject = root.getJavaProject(); + try { + IModuleDescription module = requiredProject.getModuleDescription(); + if (module instanceof AbstractModule) { + AbstractModule requiredModule = (AbstractModule) module; + for (IPackageExport packageExport : requiredModule.getExportedPackages()) { + if (!packageExport.isQualified() || CharOperation.containsEqual(packageExport.targets(), clientModuleName)) { + char[] exportName = packageExport.name(); + if (isMatchAllPrefix || CharOperation.prefixEquals(prefix, exportName)) + requestor.acceptPackage(exportName); + } + } + for (IModuleReference ref : requiredModule.getRequiredModules()) { + if (ref.isTransitive()) + findPackagesFromRequires(prefix, isMatchAllPrefix, requestor, ref, clientModuleName, visited); } } - for (IModuleReference ref : requiredModule.getRequiredModules()) { - if (ref.isTransitive()) - findPackagesFromRequires(prefix, isMatchAllPrefix, requestor, ref, clientModuleName); - } + } catch (JavaModelException e) { + // silent } - } catch (JavaModelException e) { - // silent } } -} /** * Find the top-level types that are defined * in the current environment and whose simple name matches the given name.