Add Java projects to classpath container org.eclipse.pde.core.externaJavaSearch by trancexpress · Pull Request #2270 · eclipse-pde/eclipse.pde

trancexpress · 2026-03-23T20:27:58Z

The classpath container org.eclipse.pde.core.externalJavaSearch skips adding plug-ins if a respective project is found in the workspace. This can result in unexpected behavior when browsing code from the container, such as wrong syntax highlighting, navigation and search results.

This change adjusts the classpath container to contain also projects found in the workspace.

Fixes: #2269

trancexpress · 2026-03-23T20:28:19Z

Are there tests for this container? I'm not sure what to search for, I'm not finding any.

github-actions · 2026-03-23T21:00:04Z

Test Results

147 files ±0 147 suites ±0 33m 1s ⏱️ - 2m 24s
3 498 tests +1 3 444 ✅ +1 54 💤 ±0 0 ❌ ±0
9 315 runs +3 9 185 ✅ +3 130 💤 ±0 0 ❌ ±0

Results for commit ba914fa. ± Comparison against base commit 4808f71.

♻️ This comment has been updated with latest results.

laeubi · 2026-03-24T04:14:26Z

The classpath container org.eclipse.pde.core.externalJavaSearch skips adding plug-ins if a respective project is found in the workspace.

From the naming and from the code it looks like it is intentional... So I think you should do some research what was the reason for this. At a minimum one need to revise the javadoc of the class (and possible other documentations as well).

trancexpress · 2026-03-24T05:39:34Z

From the naming and from the code it looks like it is intentional... So I think you should do some research what was the reason for this.

The project is used to be able to browse target platform code, search in it and so on. I'm sure its intentional, otherwise you woyld e.g. get double search results - one for the project in the workspace, one for the External Plug-in Libraries project.

I'll see if I can find something in the git history, maybe there is a Bugzilla ticket.

trancexpress · 2026-03-24T09:12:00Z

The source plug-in was omitted for: https://bugs.eclipse.org/bugs/show_bug.cgi?id=197817

I20070724-0800

Has already been reported as fixed in bug 56501, but apparently, no regression test caught the reappearance of this problem.

new workspace

Plug-ins view

Select All

Add to Java Search

import org.eclipse.core.expressions as source

Navigate > Open Type, enter "AndExpression"
=> two matches for same class (from source and from External Plug-in Libraries)

With this PR, I don't see multiple matches for: org.eclipse.core.expressions.AndExpression

Unfortunately no test was added for the problem: https://bugs.eclipse.org/bugs/attachment.cgi?id=75349&action=diff

I'm not sure how complicated a test is... I'll check if there are tests for the PDE plug-in container, maybe we can mimic those.

trancexpress · 2026-03-24T13:19:46Z

I added a test, unfortunately its quite slow... ~18 seconds on my laptop. Most of the time is spent indexing the container.

trancexpress · 2026-04-10T05:54:52Z

@iloveeclipse can you take a look here? Whenever I try to use the external plug-ins project, half the time it doesn't work:

All the non-highlighted text is useless, cannot use call hierarchy, navigation and so on.

The only way to work with Eclipse code seems to be to import sources...

…lJavaSearch The classpath container org.eclipse.pde.core.externalJavaSearch skips adding plug-ins if a respective project is found in the workspace. This can result in unexpected behavior when browsing code from the container, such as wrong syntax highlighting, navigation and search results. This change adjusts the classpath container to contain also projects found in the workspace. A test is also added for the initial bug fix, due to which workspace projects are skipped: https://bugs.eclipse.org/bugs/show_bug.cgi?id=197817 Fixes: eclipse-pde#2269

iloveeclipse · 2026-04-10T15:41:09Z

This change adjusts the classpath container to contain also projects found in the workspace.

Just from the name alone external java search means, the original idea was to reference only sources that are not in the workspace. So the proposal to add the projects in the workspace to that container doesn't make sense for me, it just does the opposite of what it should be done. See the javadoc of the SearchablePluginsManager class.

If this PR fixes the problem, it is likely it fixes some bug in a different place, like indexer not properly triggered after dependencies update etc.

Copilot

Pull request overview

Adjusts the PDE “External Plug-in Libraries” (externalJavaSearch) classpath container so that when a plug-in exists as a Java project in the workspace, that project is added to the container (instead of skipping the plug-in entirely), addressing IDE navigation/highlighting/search inconsistencies (Fixes #2269).

Changes:

Update SearchablePluginsManager to add workspace Java projects as CPE_PROJECT entries when available.
Add a regression UI test validating Java type search returns a single, correct match before and after importing a plug-in project.
Register the new test in the PDE UI test suite.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/SearchablePluginsManager.java	Adds workspace Java projects to the externalJavaSearch container entries when present.
ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/classpath/ExternalJavaSearchClasspathContainerTests.java	New regression test for Java search behavior with imported workspace plug-in projects.
ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/AllPDETests.java	Includes the new test in the overall PDE test suite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-10T15:48:07Z

+		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);
+


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 · 2026-04-10T15:48:08Z

+		List<String> matches = performSearch(javaProject, fqn);
+		assertSingleMatch(expected, matches);
+
+		IPluginModelBase plugin = PluginRegistry.findModel(pluginId);


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 · 2026-04-10T15:48:08Z

+		// 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";
+


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 · 2026-04-10T15:48:08Z

+
+		// 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";


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";

iloveeclipse

I believe this is the wrong place to fix the issue. I would propose to close the PR.

trancexpress · 2026-04-10T16:00:04Z

Just from the name alone external java search means, the original idea was to reference only sources that are not in the workspace. So the proposal to add the projects in the workspace to that container doesn't make sense for me, it just does the opposite of what it should be done. See the javadoc of the SearchablePluginsManager class.

If this PR fixes the problem, it is likely it fixes some bug in a different place, like indexer not properly triggered after dependencies update etc.

The problem is explained here: #2269 (comment)

For anything not found in the container, there is a 2nd search with relaxed criteria. But for things found in the container (and they are searched by string-search), that type is used for whichever functionality. So navigation stops working.

trancexpress · 2026-04-10T16:00:14Z

I believe this is the wrong place to fix the issue. I would propose to close the PR.

Sure.

trancexpress force-pushed the gh2269 branch from c88e96a to 2b3ddd1 Compare March 24, 2026 13:19

trancexpress force-pushed the gh2269 branch from 2b3ddd1 to f19201c Compare April 10, 2026 05:53

trancexpress force-pushed the gh2269 branch from f19201c to ba914fa Compare April 10, 2026 12:03

iloveeclipse requested a review from Copilot April 10, 2026 15:41

Copilot started reviewing on behalf of iloveeclipse April 10, 2026 15:41 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

iloveeclipse requested changes Apr 10, 2026

View reviewed changes

trancexpress closed this Apr 10, 2026

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

Conversation

trancexpress commented Mar 23, 2026

Uh oh!

trancexpress commented Mar 23, 2026

Uh oh!

github-actions bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

laeubi commented Mar 24, 2026

Uh oh!

trancexpress commented Mar 24, 2026

Uh oh!

trancexpress commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trancexpress commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trancexpress commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iloveeclipse commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

iloveeclipse left a comment

Choose a reason for hiding this comment

Uh oh!

trancexpress commented Apr 10, 2026

Uh oh!

trancexpress commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Mar 23, 2026 •

edited

Loading

trancexpress commented Mar 24, 2026 •

edited

Loading

trancexpress commented Mar 24, 2026 •

edited

Loading

trancexpress commented Apr 10, 2026 •

edited

Loading