Skip to content

Add Java projects to classpath container org.eclipse.pde.core.externaJavaSearch #2270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
trancexpress wants to merge 1 commit into from
Closed

Add Java projects to classpath container org.eclipse.pde.core.externaJavaSearch #2270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/SearchablePluginsManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,16 @@ public IClasspathEntry[] computeContainerClasspathEntries() {
for (String id : plugins) {
ModelEntry entry = PluginRegistry.findEntry(id);
if (entry != null) {
boolean addModel = Arrays.stream(entry.getWorkspaceModels())
List<IProject> javaProjects = Arrays.stream(entry.getWorkspaceModels())
.map(IPluginModelBase::getUnderlyingResource).map(IResource::getProject)
.noneMatch(PluginProject::isJavaProject);
.filter(PluginProject::isJavaProject).collect(Collectors.toList());
boolean addModel = javaProjects.isEmpty();
if (!addModel) {
for (IProject project : javaProjects) {
IPath path = project.getFullPath();
IClasspathEntry projectEntry = JavaCore.newProjectEntry(path);
result.add(projectEntry);
}
continue;
}
IPluginModelBase[] models = entry.getExternalModels();
Expand Down
138 changes: 138 additions & 0 deletions .../eclipse/pde/core/tests/internal/classpath/ExternalJavaSearchClasspathContainerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/*******************************************************************************
* Copyright (c) 2026 Simeon Andreev and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Simeon Andreev - initial API and implementation
*******************************************************************************/
package org.eclipse.pde.core.tests.internal.classpath;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import java.util.ArrayList;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IWorkspaceRoot;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.search.IJavaSearchConstants;
import org.eclipse.jdt.core.search.SearchEngine;
import org.eclipse.jdt.core.search.SearchPattern;
import org.eclipse.jdt.core.search.TypeNameRequestor;
import org.eclipse.jdt.internal.core.JavaModelManager;
import org.eclipse.pde.core.plugin.IPluginModelBase;
import org.eclipse.pde.core.plugin.PluginRegistry;
import org.eclipse.pde.internal.core.SearchablePluginsManager;
import org.eclipse.pde.internal.ui.wizards.imports.PluginImportOperation;
import org.eclipse.pde.internal.ui.wizards.imports.PluginImportWizard;
import org.eclipse.pde.ui.tests.runtime.TestUtils;
import org.eclipse.pde.ui.tests.util.ProjectUtils;
import org.eclipse.ui.IWorkbenchWindow;
import org.eclipse.ui.PlatformUI;
import org.eclipse.ui.handlers.IHandlerService;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestName;
import org.junit.rules.TestRule;

/**
* Test that the External Plug-in Libraries project doesn't find duplicated
* types, when a project is imported into the workspace.
*/
public class ExternalJavaSearchClasspathContainerTests {

public static final String ADD_PLUGINS_TO_SEARCH_COMMAND_ID = "org.eclipse.pde.ui.addAllPluginsToJavaSearch";
@ClassRule
public static final TestRule CLEAR_WORKSPACE = ProjectUtils.DELETE_ALL_WORKSPACE_PROJECTS_BEFORE_AND_AFTER;
@Rule
public final TestRule deleteCreatedTestProjectsAfter = ProjectUtils.DELETE_CREATED_WORKSPACE_PROJECTS_AFTER;
@Rule
public final TestName name = new TestName();

@Test
public void testSearchWithImportedProject() throws Exception {
IWorkbenchWindow window = PlatformUI.getWorkbench().getActiveWorkbenchWindow();
IHandlerService handlerService = window.getService(IHandlerService.class);
handlerService.executeCommand(ADD_PLUGINS_TO_SEARCH_COMMAND_ID, null);
TestUtils.waitForJobs(name.getMethodName(), 100, 10000);
JavaModelManager.getIndexManager().waitForIndex(false, null);
IWorkspaceRoot root = ResourcesPlugin.getWorkspace().getRoot();
IProject proxyProject = root.getProject(SearchablePluginsManager.PROXY_PROJECT_NAME);
assertNotNull("Adding " + SearchablePluginsManager.PROXY_PROJECT_NAME + " failed", proxyProject);
IJavaProject javaProject = JavaCore.create(proxyProject);

Comment on lines +75 to +79
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IWorkspaceRoot#getProject(String) never returns null (it returns a project handle even if the project doesn’t exist), so assertNotNull here won’t actually verify that the command created/opened the proxy project. Consider asserting proxyProject.exists()/proxyProject.isOpen() (and optionally PluginProject.isJavaProject(proxyProject) or javaProject.exists()) to make test failures clearer and more accurate.

Copilot uses AI. Check for mistakes.
String pluginId = "org.eclipse.core.expressions";
String fqn = "org.eclipse.core.expressions.AndExpression";
// expect a match like this:
// .../plugins/org.eclipse.equinox.common_3.20.300.v20251111-0312.jar|org/eclipse/core/runtime/IProgressMonitor.class
String expected = ".*.jar\\|" + fqn.replace('.', '/') + ".class";

Comment on lines +82 to +85
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expected-path regex is very permissive because . isn’t escaped in .*.jar and .class. This can lead to accidental matches and flaky tests. Consider using .*\\.jar\\| and \\.class, or build the fixed parts using Pattern.quote(...).

Copilot uses AI. Check for mistakes.
List<String> matches = performSearch(javaProject, fqn);
assertSingleMatch(expected, matches);

IPluginModelBase plugin = PluginRegistry.findModel(pluginId);
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PluginRegistry.findModel(pluginId) can return null (e.g., if the bundle isn’t present in the running target for the test), which would cause a confusing NPE later in the import. Add an assertNotNull for plugin (with a helpful message) before building the plugins array.

Suggested change
IPluginModelBase plugin = PluginRegistry.findModel(pluginId);
IPluginModelBase plugin = PluginRegistry.findModel(pluginId);
assertNotNull("Required plug-in model not found for '" + pluginId
+ "'. The test target may be missing this bundle.", plugin);

Copilot uses AI. Check for mistakes.
IPluginModelBase[] plugins = { plugin };
// import as binary so we don't have to compile, compiling will likely fail
PluginImportWizard.doImportOperation(PluginImportOperation.IMPORT_BINARY, plugins, true, false, null, null);
TestUtils.waitForJobs(name.getMethodName(), 100, 10000);
JavaModelManager.getIndexManager().waitForIndex(false, null);
IProject pluginProject = root.getProject(pluginId);
assertNotNull("Importing " + pluginId + " failed", pluginProject);

// expect a match like this:
// /org.eclipse.core.expressions/org.eclipse.core.expressions_3.9.500.v20250608-0434.jar|org/eclipse/core/expressions/AndExpression.class
expected = pluginProject.getFullPath() + ".*.jar\\|" + fqn.replace('.', '/') + ".class";
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pluginProject.getFullPath() contains dots which are regex metacharacters; concatenating it directly into the regexp can match unintended paths. Use Pattern.quote(pluginProject.getFullPath().toString()) (and escape \\.jar/\\.class) to make the assertion precise and resilient.

Suggested change
expected = pluginProject.getFullPath() + ".*.jar\\|" + fqn.replace('.', '/') + ".class";
expected = Pattern.quote(pluginProject.getFullPath().toString()) + ".*\\.jar\\|" + fqn.replace('.', '/') + "\\.class";

Copilot uses AI. Check for mistakes.
matches = performSearch(javaProject, fqn);
assertSingleMatch(expected, matches);
}

private static void assertSingleMatch(String regexp, List<String> matches) {
assertEquals("Expected only one search match, but found: " + matches, 1, matches.size());
Pattern pattern = Pattern.compile(regexp);
Matcher matcher = pattern.matcher(matches.get(0));
assertTrue("Unexpected search matches: " + matches + ", should match regexp: " + regexp, matcher.matches());
}

private static List<String> performSearch(IJavaProject javaProject, String fqn) throws JavaModelException {
TestRequestor requestor = new TestRequestor();
SearchEngine searchEngine = new SearchEngine();
searchEngine.searchAllTypeNames(
fqn.substring(0, fqn.lastIndexOf('.')).toCharArray(),
SearchPattern.R_EXACT_MATCH | SearchPattern.R_CASE_SENSITIVE,
fqn.substring(fqn.lastIndexOf('.') + 1).toCharArray(),
SearchPattern.R_EXACT_MATCH | SearchPattern.R_CASE_SENSITIVE,
IJavaSearchConstants.TYPE,
SearchEngine.createJavaSearchScope(new IJavaElement[] { javaProject }),
requestor,
IJavaSearchConstants.WAIT_UNTIL_READY_TO_SEARCH,
new NullProgressMonitor());
List<String> matches = requestor.matches;
return matches;
}

private static class TestRequestor extends TypeNameRequestor {

private final List<String> matches = new ArrayList<>();

@Override
public void acceptType(int modifiers, char[] packageName, char[] simpleTypeName, char[][] enclosingTypeNames, String path) {
matches.add(path);
}
}
}
2 changes: 2 additions & 0 deletions ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/AllPDETests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.eclipse.pde.core.tests.internal.classpath.ChainedReexportPerformanceTest;
import org.eclipse.pde.core.tests.internal.classpath.ClasspathResolutionTest;
import org.eclipse.pde.core.tests.internal.classpath.ClasspathResolutionTest2;
import org.eclipse.pde.core.tests.internal.classpath.ExternalJavaSearchClasspathContainerTests;
import org.eclipse.pde.core.tests.internal.classpath.RequiredPluginsClasspathContainerPerformanceTest;
import org.eclipse.pde.core.tests.internal.core.builders.BundleErrorReporterTest;
import org.eclipse.pde.core.tests.internal.util.PDESchemaHelperTest;
Expand Down Expand Up @@ -69,6 +70,7 @@
DynamicPluginProjectReferencesTest.class, //
ClasspathResolutionTest.class, //
ClasspathResolutionTest2.class, //
ExternalJavaSearchClasspathContainerTests.class, //
BundleErrorReporterTest.class, //
AllPDECoreTests.class, //
ProjectSmartImportTest.class, //
Expand Down
Loading